From 08dc4956984606d72d983e7313075222ed1f0871 Mon Sep 17 00:00:00 2001
From: Guillaume Rousse <guillaume.rousse@renater.fr>
Date: Fri, 3 Aug 2018 15:35:58 +0200
Subject: [PATCH] code factorization

---
 lib/AccountManager/App.pm | 278 +++++++++++++++-----------------------
 1 file changed, 106 insertions(+), 172 deletions(-)

diff --git a/lib/AccountManager/App.pm b/lib/AccountManager/App.pm
index e502773..4f00dd1 100644
--- a/lib/AccountManager/App.pm
+++ b/lib/AccountManager/App.pm
@@ -18,15 +18,17 @@ use AccountManager::Tools;
 use AccountManager::L10N;
 
 # Format de type URL HTTP ou URN
-my $entity_id_pattern = qr{
-    ^
-    (?:
-        https?://[\w.:/-]+
-    |
-        urn:[\w.:-]+
-    )
-    $
-}x;
+my %patterns = (
+    entityid => qr{
+        ^
+        (?:
+            https?://[\w.:/-]+
+        |
+            urn:[\w.:-]+
+        )
+        $
+    }x
+);
 
 my %actions = (
     home               => 'req_home',
@@ -242,22 +244,15 @@ sub req_select_federation {
 sub req_select_sp {
     my ($self, %args) = @_;
 
-    my $federation = $args{federation} || $self->{cgi}->param('federation');
-    $self->abort(
-        log => "Missing parameter: federation",
-        user => "missing_federation"
-    ) if !$federation;
+    my $federation = $args{federation} ||
+                     $self->get_parameter(name => 'federation');
 
-    my $file = $self->{configuration}->{federations}->{$federation};
-    $self->abort(
-        log  => "Incorrect parameter: federation",
-        user => "invalid_federation"
-    ) if !$file;
+    my $metadata_file = $self->get_metadata_file(federation => $federation);
 
     my $metadata;
     eval {
         $metadata = AccountManager::Metadata->new(
-            file => $file
+            file => $metadata_file
         );
     };
     $self->abort(
@@ -278,27 +273,10 @@ sub req_select_sp {
 sub req_select_email {
     my ($self, %args) = @_;
 
-    my $federation = $self->{cgi}->param('federation');
-    $self->abort(
-        log => "Missing parameter: federation",
-        user => "missing_federation"
-    ) if !$federation;
-
-    my $file = $self->{configuration}->{federations}->{$federation};
-    $self->abort(
-        log  => "Incorrect parameter: federation",
-        user => "invalid_federation"
-    ) if !$file;
+    my $federation = $self->get_parameter(name => 'federation');
+    my $entityid   = $self->get_parameter(name => 'entityid');
 
-    my $entityid = $self->{cgi}->param('entityid');
-    $self->abort(
-        log => "Missing parameter: entityid",
-        user => "missing_entityid"
-    ) if !$entityid;
-    $self->abort(
-        log  => "Incorrect parameter format: entityid",
-        user => "format_entityid"
-    ) if $entityid !~ $entity_id_pattern;
+    my $metadata_file = $self->get_metadata_file(federation => $federation);
 
     # Create a persistent service provider object
     my $sp = AccountManager::ServiceProvider->new(
@@ -314,7 +292,7 @@ sub req_select_email {
 
         eval {
             $metadata = AccountManager::Metadata->new(
-                file => $file
+                file => $metadata_file
             );
         };
         $self->abort(
@@ -369,33 +347,11 @@ sub req_select_email {
 sub req_complete_challenge {
     my ($self, %args) = @_;
 
-    my $federation = $self->{cgi}->param('federation');
-    $self->abort(
-        log => "Missing parameter: federation",
-        user => "missing_federation"
-    ) if !$federation;
+    my $federation = $self->get_parameter(name => 'federation');
+    my $entityid   = $self->get_parameter(name => 'entityid');
+    my $email      = $self->get_parameter(name => 'email');
 
-    my $file = $self->{configuration}->{federations}->{$federation};
-    $self->abort(
-        log  => "Incorrect parameter: federation",
-        user => "invalid_federation"
-    ) if !$file;
-
-    my $entityid = $self->{cgi}->param('entityid');
-    $self->abort(
-        log => "Missing parameter: entityid",
-        user => "missing_entityid"
-    ) if !$entityid;
-    $self->abort(
-        log  => "Incorrect parameter format: entityid",
-        user => "format_entityid"
-    ) if $entityid !~ $entity_id_pattern;
-
-    my $email = $self->{cgi}->param('email');
-    $self->abort(
-        log => "Missing parameter: email",
-        user => "missing_email"
-    ) if !$email;
+    my $metadata_file = $self->get_metadata_file(federation => $federation);
 
     my $provider = AccountManager::ServiceProvider->new(
         db       => $self->{db},
@@ -552,58 +508,11 @@ sub req_complete_challenge {
 sub req_create_accounts {
     my ($self, %args) = @_;
 
-    my $entityid = $self->{cgi}->param('entityid');
-    $self->abort(
-        log => "Missing parameter: entityid",
-        user => "missing_entityid"
-    ) if !$entityid;
-    $self->abort(
-        log  => "Incorrect parameter format: entityid",
-        user => "format_entityid"
-    ) if $entityid !~ $entity_id_pattern;
-
-    my $token_secret = $self->{cgi}->param('token');
-    $self->abort(
-        log => "Missing parameter: token",
-        user => "missing_token"
-    ) if !$token_secret;
+    my $entityid = $self->get_parameter(name => 'entityid');
+    my $token    = $self->get_parameter(name => 'token');
+    my $email    = $self->get_parameter(name => 'email');
 
-    my $email = $self->{cgi}->param('email');
-    $self->abort(
-        log => "Missing parameter: email",
-        user => "missing_email"
-    ) if !$email;
-
-    my $token = AccountManager::Token->new(
-        db     => $self->{db},
-        secret => $token_secret
-    );
-
-    $self->abort(
-        log => sprintf(
-            "Failed to validate authentication token %s for entityid %s",
-            $token_secret,
-            $entityid
-        ),
-        user => "wrong_token"
-    ) if !$token->load(speculative => 1);
-
-    $self->abort(
-        log => sprintf(
-            "Authentication token %s cannot be used for SP with entityid %s",
-            $token_secret,
-            $entityid
-        ),
-        user => "wrong_token_for_sp"
-    ) if $token->sp_entityid() ne $entityid;
-
-    ## delete the token
-    unless ($token->delete()) {
-        $self->{logger}->errorf(
-            "Failed to delete authentication token %s",
-            $token_secret
-        );
-    }
+    $self->check_token(token => $token, entityid => $entityid);
 
     ## create test accounts
     my @accounts;
@@ -677,9 +586,8 @@ sub req_create_accounts {
     ) if $EVAL_ERROR;
 
     $self->{logger}->infof(
-        "Token validated for entityid=%s;token=%s",
+        "Token validated for entityid=%s",
         $entityid,
-        $token_secret
     );
 
     $self->respond(
@@ -699,64 +607,17 @@ sub req_create_accounts {
 sub req_download_accounts {
     my ($self) = @_;
 
-    my $entityid = $self->{cgi}->param('entityid');
-    $self->abort(
-        log => "Missing parameter: entityid",
-        user => "missing_entityid"
-    ) if !$entityid;
-    $self->abort(
-        log  => "Incorrect parameter format: entityid",
-        user => "format_entityid"
-    ) if $entityid !~ $entity_id_pattern;
-
-    my $token_secret = $self->{cgi}->param('token');
-    $self->abort(
-        log => "Missing parameter: token",
-        user => "missing_token"
-    ) if !$token_secret;
-
-    my $key = $self->{cgi}->param('key');
-    $self->abort(
-        log => "Missing parameter: key",
-        user => "missing_key"
-    ) if !$key;
-
-    my $token = AccountManager::Token->new(
-        db     => $self->{db},
-        secret => $token_secret
-    );
-
-    $self->abort(
-        log => sprintf(
-            "Failed to validate authentication token %s for entityid %s",
-            $token_secret,
-            $entityid
-        ),
-        user => "wrong_token"
-    ) if !$token->load(speculative => 1);
+    my $entityid = $self->get_parameter(name => 'entityid');
+    my $token    = $self->get_parameter(name => 'token');
+    my $key      = $self->get_parameter(name => 'key');
 
-    $self->abort(
-        log => sprintf(
-            "Authentication token %s cannot be used for SP with entityid %s",
-            $token_secret,
-            $entityid
-        ),
-        user => "wrong_token_for_sp"
-    ) if $token->sp_entityid() ne $self->{in}->{entityid};
-
-    # delete the token
-    unless ($token->delete()) {
-        $self->{logger}->errorf(
-            "Failed to delete authentication token %s",
-            $token_secret
-        );
-    }
+    $self->check_token(token => $token, entityid => $entityid);
 
     # load accounts from database
     my $accounts = AccountManager::Account->get_accounts(
         db    => $self->{db},
         query => [
-            token => $token_secret
+            token => $token
         ],
     );
 
@@ -818,4 +679,77 @@ sub req_home {
     );
 }
 
+sub get_parameter {
+    my ($self, %args) = @_;
+
+    my $name  = $args{name};
+    my $value = $self->{cgi}->param($name);
+
+    $self->abort(
+        log  => "Missing parameter: $name",
+        user => "missing_$name"
+    ) if !$value;
+
+    if ($patterns{$name}) {
+        $self->abort(
+            log  => "Incorrect parameter format: entityid",
+            user => "format_entityid"
+        ) if $value !~ $patterns{$name};
+    }
+
+    return $value;
+}
+
+sub get_metadata_file {
+    my ($self, %args) = @_;
+
+    my $federation = $args{federation};
+
+    my $file = $self->{configuration}->{federations}->{$federation};
+
+    $self->abort(
+        log  => "Incorrect parameter: federation",
+        user => "invalid_federation"
+    ) if !$file;
+
+    return $file;
+}
+
+sub check_token {
+    my ($self, %args) = @_;
+
+    my $secret = $args{token};
+
+    my $token = AccountManager::Token->new(
+        db     => $self->{db},
+        secret => $secret
+    );
+
+    $self->abort(
+        log => sprintf(
+            "Failed to validate authentication token %s for entityid %s",
+            $secret,
+            $args{entityid}
+        ),
+        user => "wrong_token"
+    ) if !$token->load(speculative => 1);
+
+    $self->abort(
+        log => sprintf(
+            "Authentication token %s cannot be used for SP with entityid %s",
+            $secret,
+            $args{entityid}
+        ),
+        user => "wrong_token_for_sp"
+    ) if $token->sp_entityid() ne $args{entityid};
+
+    ## delete the token
+    unless ($token->delete()) {
+        $self->{logger}->errorf(
+            "Failed to delete authentication token %s",
+            $secret
+        );
+    }
+}
+
 1;
-- 
GitLab