From 6d6201534dff03400b1e4f99d47f4095d08d0948 Mon Sep 17 00:00:00 2001
From: Guillaume Rousse <guillaume.rousse@renater.fr>
Date: Fri, 3 Aug 2018 11:57:08 +0200
Subject: [PATCH] factorize failure handling code
---
lib/AccountManager/App.pm | 508 ++++++++++++++------------------------
1 file changed, 179 insertions(+), 329 deletions(-)
diff --git a/lib/AccountManager/App.pm b/lib/AccountManager/App.pm
index 1ca0bfe..90028ae 100644
--- a/lib/AccountManager/App.pm
+++ b/lib/AccountManager/App.pm
@@ -169,13 +169,9 @@ sub run {
my $method = $actions{$action};
$self->$method();
} else {
- ## unknown action
- $self->{logger}->error( "Unknown action '$action'");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "Unknown action '$action'" ]
- }
+ $self->abort(
+ logs => "Unknown action '$action'",
+ user => "Unknown action '$action'"
);
}
@@ -224,6 +220,20 @@ sub respond {
exit 0;
}
+sub abort {
+ my $self = shift;
+ my %args = @_;
+
+ $self->{logger}->error($args{log}) if $args{log};
+
+ $self->respond(
+ template => 'errors.tt2.html',
+ data => {
+ errors => [ $args{user} ]
+ }
+ );
+}
+
sub req_start {
my ($self) = @_;
@@ -256,43 +266,27 @@ sub req_select_sp {
my $federation = $self->{in}->{federation};
- if (!$federation) {
- $self->{logger}->error("Missing parameter: federation");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_federation" ]
- }
- );
- }
+ $self->abort(
+ log => "Missing parameter: federation",
+ user => "missing_federation"
+ ) if !$federation;
my $file = $self->{configuration}->{federations}->{$federation};
- if (!$file) {
- $self->{logger}->error("Incorrect parameter: federation");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "invalid_federation" ]
- }
- );
- }
+ $self->abort(
+ log => "Incorrect parameter: federation",
+ user => "invalid_federation"
+ ) if !$file;
my $metadata;
-
eval {
$metadata = AccountManager::Metadata->new(
file => $file
);
};
- if ($EVAL_ERROR) {
- $self->{logger}->error("Failed to load federation metadata: $EVAL_ERROR");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "internal" ]
- }
- );
- }
+ $self->abort(
+ log => "Failed to load federation metadata: $EVAL_ERROR",
+ user => "internal"
+ ) if $EVAL_ERROR;
$self->respond(
template => 'select_sp.tt2.html',
@@ -308,47 +302,25 @@ sub req_select_email {
my ($self) = @_;
my $federation = $self->{in}->{federation};
-
- if (!$federation) {
- $self->{logger}->error("Missing parameter: federation");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_federation" ]
- }
- );
- }
+ $self->abort(
+ log => "Missing parameter: federation",
+ user => "missing_federation"
+ ) if !$federation;
my $file = $self->{configuration}->{federations}->{$federation};
- if (!$file) {
- $self->{logger}->error("Incorrect parameter: federation");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "invalid_federation" ]
- }
- );
- }
-
- if (! $self->{in}->{entityid}) {
- $self->{logger}->error("Missing parameter: entityid");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_entityid" ]
- }
- );
- }
-
- if ($self->{in}->{entityid} !~ $entity_id_pattern) {
- $self->{logger}->error("Incorrect parameter format: entityid");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "format_entityid" ]
- }
- );
- }
+ $self->abort(
+ log => "Incorrect parameter: federation",
+ user => "invalid_federation"
+ ) if !$file;
+
+ $self->abort(
+ log => "Missing parameter: entityid",
+ user => "missing_entityid"
+ ) if !$self->{in}->{entityid};
+ $self->abort(
+ log => "Incorrect parameter format: entityid",
+ user => "format_entityid"
+ ) if $self->{in}->{entityid} !~ $entity_id_pattern;
# Create a persistent service provider object
my $sp = AccountManager::Service->new(
@@ -367,29 +339,17 @@ sub req_select_email {
file => $file
);
};
- if ($EVAL_ERROR) {
- $self->{logger}->error("Failed to load federation metadata: $EVAL_ERROR");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "internal" ]
- }
- );
- }
+ $self->abort(
+ log => "Failed to load federation metadata: $EVAL_ERROR",
+ user => "internal"
+ ) if $EVAL_ERROR;
my $entities = $metadata->parse(id => $self->{in}->{entityid});
- if (!@$entities) {
- $self->{logger}->errorf(
- "No such SP '%s' in metadata", $self->{in}->{entityid}
- );
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "no_such_entity" ]
- }
- );
- }
my $entity = $entities->[0];
+ $self->abort(
+ log => sprintf("No such SP '%s' in metadata", $self->{in}->{entityid}),
+ user => "no_such_entity"
+ ) if !$entity;
# complete persistent object
$sp->displayname($entity->{display_name});
@@ -397,21 +357,15 @@ sub req_select_email {
if $entity->{contacts};
# save in DB
- unless ($sp->save()) {
- $self->{logger}->error("Failed to save service provider object");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "internal" ]
- }
- );
- }
+ $self->abort(
+ log => "Failed to save service provider object",
+ user => "internal"
+ ) if !$sp->save();
}
# override metadata contacts if needed
- my $id = $self->{in}->{entityid};
my $contacts =
- $self->{configuration}->{$id}->{contacts} ||
+ $self->{configuration}->{$self->{in}->{entityid}}->{contacts} ||
$self->{configuration}->{service}->{contacts};
if ($contacts) {
if ($contacts =~ /^\+(.+)/) {
@@ -429,7 +383,7 @@ sub req_select_email {
action => 'select_email',
federation => $federation,
sp => $sp,
- entityid => $sp->entityid(),
+ entityid => $self->{in}->{entityid},
}
);
}
@@ -438,67 +392,43 @@ sub req_complete_challenge {
my ($self) = @_;
my $federation = $self->{in}->{federation};
-
- if (!$federation) {
- $self->{logger}->error("Missing parameter: federation");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_federation" ]
- }
- );
- }
+ $self->abort(
+ log => "Missing parameter: federation",
+ user => "missing_federation"
+ ) if !$federation;
my $file = $self->{configuration}->{federations}->{$federation};
- if (!$file) {
- $self->{logger}->error("Incorrect parameter: federation");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "invalid_federation" ]
- }
- );
- }
-
- unless ($self->{in}->{entityid}) {
- $self->{logger}->error("Missing parameter entityid");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_entityid" ]
- }
- );
- }
-
- unless ($self->{in}->{email}) {
- $self->{logger}->error("Missing parameter email");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_email" ]
- }
- );
- }
+ $self->abort(
+ log => "Incorrect parameter: federation",
+ user => "invalid_federation"
+ ) if !$file;
+
+ $self->abort(
+ log => "Missing parameter: entityid",
+ user => "missing_entityid"
+ ) if !$self->{in}->{entityid};
+ $self->abort(
+ log => "Incorrect parameter format: entityid",
+ user => "format_entityid"
+ ) if $self->{in}->{entityid} !~ $entity_id_pattern;
+
+ $self->abort(
+ log => "Missing parameter: email",
+ user => "missing_email"
+ ) if !$self->{in}->{email};
my $provider = AccountManager::Service->new(
db => $self->{db},
entityid => $self->{in}->{entityid},
);
-
- unless ($provider->load(speculative => 1)) {
- $self->{logger}->errorf("No such SP '%s' in database", $self->{in}->{entityid});
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "no_such_entity" ]
- }
- );
- }
+ $self->abort(
+ log => sprintf("No such SP '%s' in database", $self->{in}->{entityid}),
+ user => "no_such_entity"
+ ) if !$provider->load(speculative => 1);
# override metadata contacts if needed
- my $entity = $self->{in}->{entityid};
my $contacts =
- $self->{configuration}->{$entity}->{contacts} ||
+ $self->{configuration}->{$self->{in}->{entityid}}->{contacts} ||
$self->{configuration}->{service}->{contacts};
if ($contacts) {
if ($contacts =~ /^\+(.+)/) {
@@ -511,41 +441,27 @@ sub req_complete_challenge {
}
## Check that email is a known contact for this SP
- unless ($provider->is_contact($self->{in}->{email}))
- {
- $self->{logger}->errorf(
+ $self->abort(
+ log => sprintf(
"Requested a token for %s for an unautorized address '%s'",
- $self->{in}->{entityid},
+ $self->{in}->entityid,
$self->{in}->{email}
- );
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "internal" ]
- }
- );
- }
+ ),
+ user => "internal",
+ ) if !$provider->is_contact($self->{in}->{email});
# delete any previous token for the same email/service couple
my $old_token = AccountManager::Token->new(
db => $self->{db},
email_address => $self->{in}->{email},
- sp_entityid => $self->{in}->{entityid}
+ sp_entityid => $self->{in}->{entityid},
);
if ($old_token->load(speculative => 1)) {
- unless ($old_token->delete()) {
- $self->{logger}->errorf(
- "Failed to delete previous authentication token with ID %s",
- $old_token->id()
- );
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "internal" ]
- }
- );
- }
+ $self->abort(
+ log => sprintf("Failed to delete previous authentication token with ID %s", $old_token->id()),
+ user => "internal"
+ ) if !$old_token->delete();
}
# compute a new token
@@ -561,15 +477,10 @@ sub req_complete_challenge {
token => AccountManager::Tools::generate_secret(20)
);
- unless ($token->save()) {
- $self->{logger}->error("Failed to save authentication token");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "internal" ]
- }
- );
- }
+ $self->abort(
+ log => "Failed to save service authentication token",
+ user => "internal"
+ ) if !$token->save();
# build content
my $tt2 = Template->new({
@@ -635,15 +546,10 @@ sub req_complete_challenge {
eval {
Email::Sender::Simple->send($email);
};
- if ($EVAL_ERROR) {
- $self->{logger}->errorf("Mail notification error: %s", $EVAL_ERROR);
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "mail_notification_failure" ]
- }
- );
- }
+ $self->abort(
+ log => "Mail notification error: $EVAL_ERROR",
+ user => "mail_notification_failure"
+ ) if $EVAL_ERROR;
$self->{logger}->infof(
"Token send to %s for entityid=%s;token=%s",
@@ -666,68 +572,47 @@ sub req_complete_challenge {
sub req_create_accounts {
my ($self) = @_;
- unless ($self->{in}->{entityid}) {
- $self->{logger}->error("Missing parameter entityid");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_entityid" ]
- }
- );
- }
-
- unless ($self->{in}->{token}) {
- $self->{logger}->error("Missing parameter token");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_token" ]
- }
- );
- }
-
- unless ($self->{in}->{email}) {
- $self->{logger}->error("Missing parameter email");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_email" ]
- }
- );
- }
+ $self->abort(
+ log => "Missing parameter: entityid",
+ user => "missing_entityid"
+ ) if !$self->{in}->{entityid};
+ $self->abort(
+ log => "Incorrect parameter format: entityid",
+ user => "format_entityid"
+ ) if $self->{in}->{entityid} !~ $entity_id_pattern;
+
+ $self->abort(
+ log => "Missing parameter: token",
+ user => "missing_token"
+ ) if !$self->{in}->{token};
+
+ $self->abort(
+ log => "Missing parameter: email",
+ user => "missing_email"
+ ) if !$self->{in}->{email};
my $token = AccountManager::Token->new(
db => $self->{db},
token => $self->{in}->{token}
);
- if (! $token->load(speculative => 1)) {
- $self->{logger}->errorf(
+ $self->abort(
+ log => sprintf(
"Failed to validate authentication token %s for entityid %s",
$self->{in}->{token},
$self->{in}->{entityid}
- );
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "wrong_token" ]
- }
- );
- }
+ ),
+ user => "wrong_token"
+ ) if !$token->load(speculative => 1);
- if (! $token->sp_entityid() eq $self->{in}->{entityid}) {
- $self->{logger}->errorf(
+ $self->abort(
+ log => sprintf(
"Authentication token %s cannot be used for SP with entityid %s",
$self->{in}->{token},
$self->{in}->{entityid}
- );
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "wrong_token_for_sp" ]
- }
- );
- }
+ ),
+ user => "wrong_token_for_sp"
+ ) if $token->sp_entityid() ne $self->{in}->{entityid};
## delete the token
unless ($token->delete()) {
@@ -758,15 +643,10 @@ sub req_create_accounts {
token => AccountManager::Tools::generate_secret(20)
);
- unless ($download_token->save()) {
- $self->{logger}->error("Failed to save authentication token");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "internal" ]
- }
- );
- }
+ $self->abort(
+ log => "Failed to save download authentication token",
+ user => "internal"
+ ) if !$download_token->save();
my $key = AccountManager::Tools::generate_secret(10);
@@ -788,18 +668,13 @@ sub req_create_accounts {
push @accounts, $account;
}
- unless (@accounts) {
- $self->{logger}->errorf(
+ $self->abort(
+ log => sprintf(
"Failed to create test accounts for SP with entityid %s",
$self->{in}->{entityid}
- );
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "accounts_creation_failure" ]
- }
- );
- }
+ ),
+ user => "accounts_creation_failure"
+ ) if !@accounts;
## Update simpleSAMLphp configuration to enable test accounts
my $accounts = AccountManager::Account::Manager->get_accounts(
@@ -813,18 +688,13 @@ sub req_create_accounts {
$accounts
);
};
- if ($EVAL_ERROR) {
- $self->{logger}->errorf(
+ $self->abort(
+ log => sprintf(
"Failed to create simpleSAMLphp configuration file: %s",
$EVAL_ERROR
- );
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "accounts_creation_failed" ]
- }
- );
- }
+ ),
+ user => "accounts_creation_failed"
+ ) if $EVAL_ERROR;
$self->{logger}->infof(
"Token validated for entityid=%s;token=%s",
@@ -849,67 +719,47 @@ sub req_create_accounts {
sub req_download_accounts {
my ($self) = @_;
- unless ($self->{in}->{entityid}) {
- $self->{logger}->error("Missing parameter entityid");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_entityid" ]
- }
- );
- }
-
- unless ($self->{in}->{token}) {
- $self->{logger}->error("Missing parameter token");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_token" ]
- }
- );
- }
-
- unless ($self->{in}->{key}) {
- $self->{logger}->error("Missing parameter key");
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "missing_key" ]
- }
- );
- }
+ $self->abort(
+ log => "Missing parameter: entityid",
+ user => "missing_entityid"
+ ) if !$self->{in}->{entityid};
+ $self->abort(
+ log => "Incorrect parameter format: entityid",
+ user => "format_entityid"
+ ) if $self->{in}->{entityid} !~ $entity_id_pattern;
+
+ $self->abort(
+ log => "Missing parameter: token",
+ user => "missing_token"
+ ) if !$self->{in}->{token};
+
+ $self->abort(
+ log => "Missing parameter: key",
+ user => "missing_key"
+ ) if !$self->{in}->{key};
my $token = AccountManager::Token->new(
db => $self->{db},
token => $self->{in}->{token}
);
- if (! $token->load(speculative => 1)) {
- $self->{logger}->errorf(
- "Non-existing authentication token %s",
+ $self->abort(
+ log => sprintf(
+ "Failed to validate authentication token %s for entityid %s",
$self->{in}->{token},
- );
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "wrong_token" ]
- }
- );
- }
+ $self->{in}->{entityid}
+ ),
+ user => "wrong_token"
+ ) if !$token->load(speculative => 1);
- if (! $token->sp_entityid() eq $self->{in}->{entityid}) {
- $self->{logger}->errorf(
- "Authentication token %s cannot be used for SP %s",
+ $self->abort(
+ log => sprintf(
+ "Authentication token %s cannot be used for SP with entityid %s",
$self->{in}->{token},
$self->{in}->{entityid}
- );
- $self->respond(
- template => 'errors.tt2.html',
- data => {
- errors => [ "wrong_token_for_sp" ]
- }
- );
- }
+ ),
+ user => "wrong_token_for_sp"
+ ) if $token->sp_entityid() ne $self->{in}->{entityid};
# delete the token
unless ($token->delete()) {
--
GitLab