Bug 20287: Add plain_text_password (& Remove AddMember_Opac)

But actually we could remove it if it does not make sense for other use.
Callers could deal with it since the password is not generated here

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

diff --git a/C4/Members.pm b/C4/Members.pm
index 7fc67fb..080146e 100644 (file)
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -76,11 +76,6 @@ BEGIN {
         &changepassword
     );
 
-    #Insert data
-    push @EXPORT, qw(
-        &AddMember_Opac
-    );
-
     #Check data
     push @EXPORT, qw(
         &checkuserpassword
@@ -738,27 +733,6 @@ sub IssueSlip {
     );
 }
 
-=head2 AddMember_Opac
-
-=cut
-
-sub AddMember_Opac {
-    my ( %borrower ) = @_;
-
-    $borrower{'categorycode'} //= C4::Context->preference('PatronSelfRegistrationDefaultCategory');
-    my $password = $borrower{password};
-    if (not defined $password){
-        my $sr = new String::Random;
-        $sr->{'A'} = [ 'A'..'Z', 'a'..'z' ];
-        $password = $sr->randpattern("AAAAAAAAAA");
-        $borrower{'password'} = $password;
-    }
-
-    my $patron = Koha::Patron->new(\%borrower)->store;
-
-    return ( $patron->borrowernumber, $password );
-}
-
 =head2 DeleteExpiredOpacRegistrations
 
     Delete accounts that haven't been upgraded from the 'temporary' category

diff --git a/Koha/Patron.pm b/Koha/Patron.pm
index 1c24526..fa0b53a 100644 (file)
--- a/Koha/Patron.pm
+++ b/Koha/Patron.pm
@@ -127,6 +127,18 @@ sub trim_whitespaces {
     return $self;
 }
 
+sub plain_text_password {
+    my ( $self, $password ) = @_;
+    if ( $password ) {
+        $self->{_plain_text_password} = $password;
+        return $self;
+    }
+    return $self->{_plain_text_password}
+        if $self->{_plain_text_password};
+
+    return;
+}
+
 sub store {
     my ($self) = @_;
 
@@ -182,7 +194,7 @@ sub store {
                 }
 
                 # Make a copy of the plain text password for later use
-                my $plain_text_password = $self->password;
+                $self->plain_text_password( $self->password );
 
                 # Create a disabled account if no password provided
                 $self->password( $self->password
@@ -214,8 +226,7 @@ sub store {
                             'sync'           => 1,
                             'syncstatus'     => 'new',
                             'hashed_pin' =>
-                              Koha::NorwegianPatronDB::NLEncryptPIN(
-                                $plain_text_password),
+                              Koha::NorwegianPatronDB::NLEncryptPIN($self->plain_text_password),
                         }
                       );
                 }

diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl
index 9cfcb4d..4eeacf0 100755 (executable)
--- a/opac/opac-memberentry.pl
+++ b/opac/opac-memberentry.pl
@@ -169,7 +169,7 @@ if ( $action eq 'create' ) {
                 $verification_token = md5_hex( time().{}.rand().{}.$$ );
             }
 
-            $borrower{password}           = Koha::AuthUtils::generate_password unless $borrower{password};
+            $borrower{password}          = Koha::AuthUtils::generate_password unless $borrower{password};
             $borrower{verification_token} = $verification_token;
 
             Koha::Patron::Modification->new( \%borrower )->store();
@@ -207,6 +207,8 @@ if ( $action eq 'create' ) {
             $template->param( OpacPasswordChange =>
                   C4::Context->preference('OpacPasswordChange') );
 
+            $borrower{categorycode}     ||= C4::Context->preference('PatronSelfRegistrationDefaultCategory');
+            $borrower{password}         ||= Koha::AuthUtils::generate_password;
             my $patron = Koha::Patron->new( \%borrower )->store;
             if ( $patron ) {
                 C4::Members::Attributes::SetBorrowerAttributes( $patron->borrowernumber, $attributes );
@@ -220,7 +222,7 @@ if ( $action eq 'create' ) {
                     );
                 }
 
-                $template->param( password_cleartext => $password );
+                $template->param( password_cleartext => $patron->plain_text_password );
                 $template->param( borrower => $patron->unblessed );
             } else {
                 # FIXME Handle possible errors here

diff --git a/opac/opac-registration-verify.pl b/opac/opac-registration-verify.pl
index 2d9b9f7..3748f5c 100755 (executable)
--- a/opac/opac-registration-verify.pl
+++ b/opac/opac-registration-verify.pl
@@ -23,6 +23,7 @@ use C4::Auth;
 use C4::Output;
 use C4::Members;
 use C4::Form::MessagingPreferences;
+use Koha::AuthUtils;
 use Koha::Patrons;
 use Koha::Patron::Modifications;
 
@@ -59,17 +60,17 @@ if (
     $template->param(
         OpacPasswordChange => C4::Context->preference('OpacPasswordChange') );
 
-    my $borrower = $m->unblessed();
+    my $patron_attrs = $m->unblessed;
+    $patron_attrs->{password} ||= Koha::AuthUtils::generate_password;
 
-    my $password;
-    ( $borrowernumber, $password ) = AddMember_Opac(%$borrower);
+    $patron_attrs->{categorycode} ||= C4::Context->preference('PatronSelfRegistrationDefaultCategory');
+    my $patron = Koha::Patron->new( $patron_attrs )->store;
 
-    if ($borrowernumber) {
+    if ($patron) {
         $m->delete();
-        C4::Form::MessagingPreferences::handle_form_action($cgi, { borrowernumber => $borrowernumber }, $template, 1, C4::Context->preference('PatronSelfRegistrationDefaultCategory') ) if C4::Context->preference('EnhancedMessagingPreferences');
+        C4::Form::MessagingPreferences::handle_form_action($cgi, { borrowernumber => $patron->borrowernumber }, $template, 1, C4::Context->preference('PatronSelfRegistrationDefaultCategory') ) if C4::Context->preference('EnhancedMessagingPreferences');
 
-        $template->param( password_cleartext => $password );
-        my $patron = Koha::Patrons->find( $borrowernumber );
+        $template->param( password_cleartext => $patron->plain_text_password );
         $template->param( borrower => $patron->unblessed );
         $template->param(
             PatronSelfRegistrationAdditionalInstructions =>

diff --git a/t/db_dependent/Members.t b/t/db_dependent/Members.t
index da5d70b..bc0d049 100755 (executable)
--- a/t/db_dependent/Members.t
+++ b/t/db_dependent/Members.t
@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 50;
+use Test::More tests => 47;
 use Test::MockModule;
 use Test::Exception;
 
@@ -376,16 +376,6 @@ sub _find_member {
     return $found;
 }
 
-# Regression tests for BZ15343
-my $password="";
-( $borrowernumber, $password ) = AddMember_Opac(surname=>"Dick",firstname=>'Philip',branchcode => $library2->{branchcode});
-is( $password =~ /^[a-zA-Z]{10}$/ , 1, 'Test for autogenerated password if none submitted');
-( $borrowernumber, $password ) = AddMember_Opac(surname=>"Deckard",firstname=>"Rick",password=>"Nexus-6",branchcode => $library2->{branchcode});
-is( $password eq "Nexus-6", 1, 'Test password used if submitted');
-$borrower = Koha::Patrons->find( $borrowernumber )->unblessed;
-my $hashed_up =  Koha::AuthUtils::hash_password("Nexus-6", $borrower->{password});
-is( $borrower->{password} eq $hashed_up, 1, 'Check password hash equals hash of submitted password' );
-
 $schema->storage->txn_rollback;
 
 subtest 'Koha::Patron->store (invalid categorycode) tests' => sub {