From cfbc53bb22e6f4a851a2a7cd6ede891012951410 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 22 Feb 2018 17:13:22 -0300 Subject: [PATCH] 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 Signed-off-by: Tomas Cohen Arazi Signed-off-by: Martin Renvoize Signed-off-by: Nick Clemens --- C4/Members.pm | 26 -------------------------- Koha/Patron.pm | 17 ++++++++++++++--- opac/opac-memberentry.pl | 6 ++++-- opac/opac-registration-verify.pl | 15 ++++++++------- t/db_dependent/Members.t | 12 +----------- 5 files changed, 27 insertions(+), 49 deletions(-) diff --git a/C4/Members.pm b/C4/Members.pm index 7fc67fba17..080146eb11 100644 --- 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 1c245262ba..fa0b53a55b 100644 --- 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 9cfcb4df61..4eeacf093a 100755 --- 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 2d9b9f72cb..3748f5c330 100755 --- 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 da5d70bbec..bc0d04902d 100755 --- 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 { -- 2.39.5