From 286344dd19d4f2c54c9e70707151c3860e84c2d6 Mon Sep 17 00:00:00 2001
From: Guillaume Rousse <guillaume.rousse@renater.fr>
Date: Fri, 24 Nov 2017 17:11:46 +0100
Subject: [PATCH] let the caller compute relevant data

Passing data at saving time is ugly, and computing default values in the
constructor results in object being reinitialised before loading their
attributes from the database...
---
 bin/account-manager.pl.in            | 24 +++++++++++++++---------
 lib/IdPAccountManager/TestAccount.pm | 20 --------------------
 lib/IdPAccountManager/WebRequest.pm  | 16 +++++++++++-----
 3 files changed, 26 insertions(+), 34 deletions(-)

diff --git a/bin/account-manager.pl.in b/bin/account-manager.pl.in
index 1364137..4356bf1 100755
--- a/bin/account-manager.pl.in
+++ b/bin/account-manager.pl.in
@@ -9,6 +9,7 @@ no warnings 'experimental::smartmatch';
 
 use Config::Tiny;
 use Data::Dumper;
+use DateTime;
 use English qw(-no_match_vars);
 use Getopt::Long qw(:config auto_help);
 use Log::Any::Adapter;
@@ -86,23 +87,28 @@ sub add_account {
         -verbose => 0
     ) unless $options{sp_entityid};
 
-    my $test_account = IdPAccountManager::TestAccount->new(
-        db          => $db,
-        profile     => $options{profile},
-        sp_entityid => $options{sp_entityid},
-        scope       => $configuration->{idp}->{scope},
-    );
-
     my $entity = $options{sp_entityid};
     my $validity_period =
         $configuration->{$entity}->{account_validity_period} ||
         $configuration->{service}->{account_validity_period};
+    my $password = IdPAccountManager::Tools::generate_password();
+
+    my $account = IdPAccountManager::TestAccount->new(
+        db              => $db,
+        profile         => $options{profile},
+        sp_entityid     => $options{sp_entityid},
+        scope           => $configuration->{idp}->{scope},
+        password        => $password,
+        password_hash   => IdPAccountManager::Tools::sha256_hash($password),
+        creation_date   => DateTime->today(),
+        expiration_date => DateTime->today()->add(days => $validity_period)
+    );
 
     die "Failed to save test account\n"
-        unless $test_account->save(accounts_validity_period => $validity_period);
+        unless $account->save();
 
     printf "Account created:\n\tuserid: user%d\n\tpassword: %s\n",
-      $test_account->id(), $test_account->password();
+      $account->id(), $account->password();
 
 }
 
diff --git a/lib/IdPAccountManager/TestAccount.pm b/lib/IdPAccountManager/TestAccount.pm
index f089daf..3f6c9c8 100644
--- a/lib/IdPAccountManager/TestAccount.pm
+++ b/lib/IdPAccountManager/TestAccount.pm
@@ -5,8 +5,6 @@ use warnings;
 
 use base 'IdPAccountManager::DB::Object';
 
-use DateTime;
-
 __PACKAGE__->meta->setup(
     table   => 'testaccounts',
 
@@ -126,24 +124,6 @@ sub print {
       $self->expiration_date()->strftime('%Y:%m:%d');
 }
 
-sub save {
-    my ($self, %args) = @_;
-
-    # If no ID is defined, it is a new account
-    if (! defined $self->id()) {
-        $self->password(
-            IdPAccountManager::Tools::generate_password());
-        $self->password_hash(
-            IdPAccountManager::Tools::sha256_hash($self->password()));
-        $self->creation_date(DateTime->today());
-        $self->expiration_date(
-            DateTime->today()->add(days => $args{accounts_validity_period})
-        );
-    }
-
-    $self->SUPER::save();
-}
-
 sub internal_uid {
     my ($self) = @_;
     return 'user' . $self->id();
diff --git a/lib/IdPAccountManager/WebRequest.pm b/lib/IdPAccountManager/WebRequest.pm
index 3470138..7b40270 100644
--- a/lib/IdPAccountManager/WebRequest.pm
+++ b/lib/IdPAccountManager/WebRequest.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 
 use CGI;
+use DateTime;
 use English qw(-no_match_vars);
 use Template;
 use Log::Any::Adapter;
@@ -463,13 +464,18 @@ sub req_validate_token {
         $self->{configuration}->{service}->{account_validity_period};
 
     foreach my $profile (split(/, */, $profiles)) {
+        my $password = IdPAccountManager::Tools::generate_password();
         my $account = IdPAccountManager::TestAccount->new(
-            db          => $self->{db},
-            profile     => $profile,
-            sp_entityid => $entity,
-            scope       => $self->{configuration}->{idp}->{scope},
+            db              => $self->{db},
+            profile         => $profile,
+            sp_entityid     => $entity,
+            scope           => $self->{configuration}->{idp}->{scope},
+            password        => $password,
+            password_hash   => IdPAccountManager::Tools::sha256_hash($password),
+            creation_date   => DateTime->today(),
+            expiration_date => DateTime->today()->add(days => $validity_period)
         );
-        next unless $account->save(accounts_validity_period => $validity_period);
+        next unless $account->save();
         push @accounts, $account;
     }
 
-- 
GitLab