From 72a82b6f0d32360d6679525dc2a3bd25a79a44c0 Mon Sep 17 00:00:00 2001
From: Guillaume Rousse <guillaume.rousse@renater.fr>
Date: Thu, 2 Nov 2017 13:04:15 +0100
Subject: [PATCH] kill direct access to configuration from modules
---
bin/account-manager-client.pl | 22 +++++++++++----
lib/IdPAccountManager/ServiceProvider.pm | 13 +++++++--
lib/IdPAccountManager/TestAccount.pm | 7 ++---
lib/IdPAccountManager/Tools.pm | 29 +++++++++-----------
lib/IdPAccountManager/WebRequest.pm | 35 +++++++++++++++++-------
5 files changed, 68 insertions(+), 38 deletions(-)
diff --git a/bin/account-manager-client.pl b/bin/account-manager-client.pl
index 8d78378..f63d022 100755
--- a/bin/account-manager-client.pl
+++ b/bin/account-manager-client.pl
@@ -67,7 +67,9 @@ if ($action eq 'add_test_account') {
unless $test_account;
die "Failed to save test account\n"
- unless $test_account->save();
+ unless $test_account->save(
+ accounts_validity_period => $Conf::global{'accounts_validity_period'}
+ );
printf "Account created:\n\tuserid: user%d\n\tpassword: %s\n",
$test_account->get('id'), $test_account->get('user_password');
@@ -105,7 +107,10 @@ if ($action eq 'add_test_account') {
printf "%d accounts removed\n", $#{$all} + 1;
die "failed to update simpleSAMLphp configuration file\n"
- unless IdPAccountManager::Tools::update_ssp_authsources();
+ unless IdPAccountManager::Tools::update_ssp_authsources(
+ $Conf::global{'root_manager_dir'},
+ \%Conf::global
+ );
printf "Update simpleSamlPhp configuration file...\n";
}
@@ -295,10 +300,15 @@ if ($action eq 'add_test_account') {
die "Failed to send mail notice to $options{'email_address'}\n"
unless IdPAccountManager::Tools::mail_notice(
- 'template' => 'templates/mail/notification_generic_error.tt2.eml',
- 'data' => {},
- 'to' => $options{'email_address'},
- 'logger' => $logger
+ 'template' => 'templates/mail/notification_generic_error.tt2.eml',
+ 'data' => {},
+ 'to' => $options{'email_address'},
+ 'logger' => $logger,
+ 'conf' => \%Conf::global,
+ 'admin_email' => $Conf::global{'admin_email'},
+ 'dev_no_mail_outside' => $Conf::global{'dev_no_mail_outside'},
+ 'dev_sp_contact' => $Conf::global{'dev_sp_contact'},
+ 'notice_from' => $Conf::global{'notice_from'}
);
printf "Mail notice sent to $options{'email_address'}\n";
diff --git a/lib/IdPAccountManager/ServiceProvider.pm b/lib/IdPAccountManager/ServiceProvider.pm
index f773674..78cdf68 100644
--- a/lib/IdPAccountManager/ServiceProvider.pm
+++ b/lib/IdPAccountManager/ServiceProvider.pm
@@ -13,7 +13,6 @@ use IdPAccountManager::Data::ServiceProvider;
use IdPAccountManager::Data::ServiceProvider::Manager;
use IdPAccountManager::Tools;
-use Conf;
use Carp;
@@ -22,6 +21,16 @@ INIT {
IdPAccountManager::Data::ServiceProvider::Manager->error_mode('return');
}
+sub new {
+ my ($pkg, %args) = @_;
+
+ my $self = SUPER::new->(%args);
+
+ $self->{dev_sp_contact} = $args{dev_sp_contact};
+
+ return $self;
+}
+
## Print the content of a test account
sub print {
my ($self, $fd) = @_;
@@ -42,7 +51,7 @@ sub list_contacts_as_array {
$contact_list{$contact_email}++;
}
- foreach my $contact_email (split /,/, $Conf::global{'dev_sp_contact'}) {
+ foreach my $contact_email (split /,/, $self->{dev_sp_contact}) {
$contact_list{$contact_email}++;
}
diff --git a/lib/IdPAccountManager/TestAccount.pm b/lib/IdPAccountManager/TestAccount.pm
index ffa9633..70dbf10 100644
--- a/lib/IdPAccountManager/TestAccount.pm
+++ b/lib/IdPAccountManager/TestAccount.pm
@@ -11,7 +11,6 @@ use IdPAccountManager::Data::TestAccount;
use IdPAccountManager::Data::TestAccount::Manager;
use IdPAccountManager::Tools;
-use Conf;
use POSIX qw(strftime);
use Carp;
@@ -53,13 +52,13 @@ sub get {
}
sub save {
- my ($self) = @_;
+ my ($self, %args) = @_;
## If no id is defined, it is a new account
unless (defined $self->{'persistent'}->id) {
$self->{'persistent'}->creation_date(time);
$self->{'persistent'}->expiration_date(
- time + ($Conf::global{'accounts_validity_period'} * 3600 * 24));
+ time + ($args{'accounts_validity_period'} * 3600 * 24));
$self->{'user_password'} =
IdPAccountManager::Tools::generate_password();
$self->{'persistent'}->user_password_hash(
@@ -118,7 +117,7 @@ sub create_test_accounts_for_sp {
return undef;
}
- foreach my $profile (@{ $Conf::global{'account_profiles'} }) {
+ foreach my $profile (@{ $args{'account_profiles'} }) {
my $test_account = IdPAccountManager::TestAccount->new(
account_profile => $profile,
sp_entityid => $args{'sp_entityid'}
diff --git a/lib/IdPAccountManager/Tools.pm b/lib/IdPAccountManager/Tools.pm
index dd718ba..8209c23 100644
--- a/lib/IdPAccountManager/Tools.pm
+++ b/lib/IdPAccountManager/Tools.pm
@@ -4,7 +4,6 @@ package IdPAccountManager::Tools;
## This software was developed by RENATER. The research leading to these results has received funding
## from the European Community¹s Seventh Framework Programme (FP7/2007-2013) under grant agreement nº 238875 (GÉANT).
-use Conf;
use Template;
# load Template::Stash to make method tables visible
@@ -85,17 +84,15 @@ sub generate_password {
## Updates simpleSamlPhp authsources.php configuration file
sub update_ssp_authsources {
+ my ($root_manager_dir, $conf) = @_;
- my $tt2 = Template->new(
- {
- 'INCLUDE_PATH' => $Conf::global{'root_manager_dir'} . ':'
- . $Conf::global{'root_manager_dir'}
- . '/templates/accountProfiles'
- }
- );
+ my $tt2 = Template->new({
+ 'INCLUDE_PATH' => $root_manager_dir . ':' .
+ $root_manager_dir . '/templates/accountProfiles'
+ });
my %args = (
'accounts' => IdPAccountManager::TestAccount::list_test_accounts(),
- 'conf' => \%Conf::global
+ 'conf' => $conf,
);
#chdir $Conf::global{'root_manager_dir'};
@@ -153,20 +150,20 @@ sub mail_notice {
my $mail_data = $args{'data'};
my $logger = $args{'logger'};
- $mail_data->{'conf'} ||= \%Conf::global;
+ $mail_data->{'conf'} ||= $args{'conf'};
- my $notice_email = $in{'to'} || $Conf::global{'admin_email'};
+ my $notice_email = $args{'to'} || $args{'admin_email'};
$mail_data->{'to'} = $notice_email;
## Protection to prevent notifications during test dev phases
## Notify only admin_email or dev_sp_contact addresses
- if ($Conf::global{'dev_no_mail_outside'}) {
+ if ($args{'dev_no_mail_outside'}) {
my %rcpt = map { $_ => 1 } split(/,/, $notice_email);
my %authorized_rcpt = map { $_ => 1 } split(
/,/,
join(',',
- $Conf::global{'admin_email'},
- $Conf::global{'dev_sp_contact'})
+ $args{'admin_email'},
+ $args{'dev_sp_contact'})
);
my $change_rcpt = 0;
@@ -185,7 +182,7 @@ sub mail_notice {
$notice_email
)
);
- $notice_email = $Conf::global{'admin_email'};
+ $notice_email = $args{'admin_email'};
}
}
@@ -196,7 +193,7 @@ sub mail_notice {
open SENDMAIL,
"|/usr/sbin/sendmail -f "
- . $Conf::global{'notice_from'}
+ . $args{'notice_from'}
. " $notice_email";
my $tt2 = Template->new(FILTERS => { qencode => [ \qencode, 0 ] });
diff --git a/lib/IdPAccountManager/WebRequest.pm b/lib/IdPAccountManager/WebRequest.pm
index a4261e4..71237b4 100755
--- a/lib/IdPAccountManager/WebRequest.pm
+++ b/lib/IdPAccountManager/WebRequest.pm
@@ -196,9 +196,14 @@ sub respond {
if (@errors_admin) {
$self->{'param_out'}{'subject'} = 'Error notification - web interface';
IdPAccountManager::Tools::mail_notice(
- 'template' => 'templates/mail/notification_generic_error.tt2.eml',
- 'data' => $self->{'param_out'},
- 'logger' => $self->{'logger'}
+ 'template' => 'templates/mail/notification_generic_error.tt2.eml',
+ 'data' => $self->{'param_out'},
+ 'logger' => $self->{'logger'},
+ 'conf' => \%Conf::global,
+ 'admin_email' => $Conf::global{'admin_email'},
+ 'dev_no_mail_outside' => $Conf::global{'dev_no_mail_outside'},
+ 'dev_sp_contact' => $Conf::global{'dev_sp_contact'},
+ 'notice_from' => $Conf::global{'notice_from'}
);
}
}
@@ -290,7 +295,9 @@ sub req_select_sp {
## Create a serviceprovider object to store major parameters for this SP in DB
my $service_provider = IdPAccountManager::ServiceProvider->new(
- entityid => $self->{'param_in'}{'sp_entityid'});
+ entityid => $self->{'param_in'}{'sp_entityid'},
+ dev_sp_contact => $Conf::global{'dev_sp_contact'}
+ );
## Prepare data
my $sp_metadata_as_hashref =
@@ -328,9 +335,10 @@ sub req_select_sp {
} else {
$service_provider = IdPAccountManager::ServiceProvider->new(
- entityid => $self->{'param_in'}{'sp_entityid'},
- contacts => join(',', @contacts),
- displayname => $display_name
+ entityid => $self->{'param_in'}{'sp_entityid'},
+ contacts => join(',', @contacts),
+ displayname => $display_name,
+ dev_sp_contact => $Conf::global{'dev_sp_contact'}
);
unless (defined $service_provider) {
push @{ $self->{'param_out'}{'errors'} }, "internal";
@@ -382,7 +390,9 @@ sub req_generate_token {
## Create a serviceprovider object to load parameters for this SP from DB
my $service_provider = IdPAccountManager::ServiceProvider->new(
- entityid => $self->{'param_in'}{'sp_entityid'});
+ entityid => $self->{'param_in'}{'sp_entityid'},
+ dev_sp_contact => $Conf::global{'dev_sp_contact'}
+ );
# Try loading DB object first
unless ($service_provider->load(speculative => 1)) {
@@ -550,7 +560,9 @@ sub req_validate_token {
## create test accounts
my @test_accounts =
IdPAccountManager::TestAccount::create_test_accounts_for_sp(
- sp_entityid => $self->{'param_in'}{'sp_entityid'});
+ sp_entityid => $self->{'param_in'}{'sp_entityid'},
+ account_profiles => $Conf::global{'account_profiles'}
+ );
unless (@test_accounts) {
push @{ $self->{'param_out'}{'errors'} }, "accounts_creation_failed";
@@ -563,7 +575,10 @@ sub req_validate_token {
}
## Update simpleSAMLphp configuration to enable test accounts
- unless (IdPAccountManager::Tools::update_ssp_authsources()) {
+ unless (IdPAccountManager::Tools::update_ssp_authsources(
+ $Conf::global{'root_manager_dir'},
+ \%Conf::global
+ )) {
push @{ $self->{'param_out'}{'errors'} }, "accounts_creation_failed";
$self->{logger}->log(
level => LOG_ERROR,
--
GitLab