From db0ecc3cc5dbde028db8b083dbe5d75740552445 Mon Sep 17 00:00:00 2001 From: charles Date: Wed, 27 Jan 2016 15:17:32 -0500 Subject: [PATCH] Bug 15585 - Move C4::Passwordrecovery to the new namespace Koha::Patron::Password::Reset As promised, here is the long-awaited sequel to #8753. What has changed : - The Koha::Patron::Password::Reset is now used in place of C4::Passwordrecovery - That ugly shift-grep contraption is no more (goodbye old friend) - The generated unique key won't end in a dot anymore Signed-off-by: Kyle M Hall Signed-off-by: Jonathan Druart Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com --- .../Patron/Password/Recovery.pm | 15 +++--- opac/opac-password-recovery.pl | 15 +++--- t/db_dependent/Passwordrecovery.t | 50 +++++++++---------- 3 files changed, 43 insertions(+), 37 deletions(-) rename C4/Passwordrecovery.pm => Koha/Patron/Password/Recovery.pm (91%) diff --git a/C4/Passwordrecovery.pm b/Koha/Patron/Password/Recovery.pm similarity index 91% rename from C4/Passwordrecovery.pm rename to Koha/Patron/Password/Recovery.pm index a67c4d42b6..6cb1d94595 100644 --- a/C4/Passwordrecovery.pm +++ b/Koha/Patron/Password/Recovery.pm @@ -1,4 +1,4 @@ -package C4::Passwordrecovery; +package Koha::Patron::Password::Recovery; # Copyright 2014 Solutions InLibro inc. # @@ -38,11 +38,11 @@ BEGIN { =head1 NAME -C4::Passwordrecovery - Koha password recovery module +Koha::Patron::Password::Recovery - Koha password recovery module =head1 SYNOPSIS -use C4::Passwordrecovery; +use Koha::Patron::Password::Recovery; =head1 FUNCTIONS @@ -110,8 +110,10 @@ sub SendPasswordRecoveryEmail { my $schema = Koha::Database->new->schema; # generate UUID - my @chars = ( "A" .. "Z", "a" .. "z", "0" .. "9" ); - my $uuid_str = '$2a$08$'.en_base64(Koha::AuthUtils::generate_salt('weak', 16)); + my $uuid_str; + do { + $uuid_str = '$2a$08$'.en_base64(Koha::AuthUtils::generate_salt('weak', 16)); + } while ( substr ( $uuid_str, -1, 1 ) eq '.' ); # insert into database my $expirydate = @@ -134,7 +136,8 @@ sub SendPasswordRecoveryEmail { } # create link - my $uuidLink = C4::Context->preference('OPACBaseURL') + my $opacbase = C4::Context->preference('OPACBaseURL') || ''; + my $uuidLink = $opacbase . "/cgi-bin/koha/opac-password-recovery.pl?uniqueKey=$uuid_str"; # prepare the email diff --git a/opac/opac-password-recovery.pl b/opac/opac-password-recovery.pl index aa98040614..2f7fbfe2ac 100755 --- a/opac/opac-password-recovery.pl +++ b/opac/opac-password-recovery.pl @@ -8,7 +8,7 @@ use C4::Koha; use C4::Members qw(changepassword); use C4::Output; use C4::Context; -use C4::Passwordrecovery +use Koha::Patron::Password::Recovery qw(SendPasswordRecoveryEmail ValidateBorrowernumber GetValidLinkInfo CompletePasswordRecovery); use Koha::AuthUtils qw(hash_password); use Koha::Patrons; @@ -71,6 +71,12 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { $username ||= $borrower->userid; my @emails = ( $borrower->email, $borrower->emailpro, $borrower->B_email ); + my $firstNonEmptyEmail = ''; + foreach my $address ( @emails ) { + $firstNonEmptyEmail = $address if length $address; + last if $firstNonEmptyEmail; + } + # Is the given email one of the borrower's ? if ( $email && !( grep { $_ eq $email } @emails ) ) { $hasError = 1; @@ -78,12 +84,9 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { } # If we dont have an email yet. Get one of the borrower's email or raise an error. -# FIXME: That ugly shift-grep contraption. -# $email = shift [ grep { length() } @emails ] -# It's supposed to get a non-empty string from the @emails array. There's surely a simpler way - elsif ( !$email && !( $email = shift [ grep { length() } @emails ] ) ) { + elsif ( !$email && !( $email = $firstNonEmptyEmail ) ) { $hasError = 1; - $errNoBorrowerFound = 1; + $errNoBorrowerEmail = 1; } # Check if a password reset already issued for this borrower AND we are not asking for a new email diff --git a/t/db_dependent/Passwordrecovery.t b/t/db_dependent/Passwordrecovery.t index a428033bbd..9a3b8b24b8 100755 --- a/t/db_dependent/Passwordrecovery.t +++ b/t/db_dependent/Passwordrecovery.t @@ -24,7 +24,7 @@ use Koha::Patrons; use Test::More tests => 16; -use_ok('C4::Passwordrecovery'); +use_ok('Koha::Patron::Password::Recovery'); my $schema = Koha::Database->new()->schema(); $schema->storage->txn_begin(); @@ -90,34 +90,34 @@ $schema->resultset('BorrowerPasswordRecovery')->create( } ); -can_ok( "C4::Passwordrecovery", qw(ValidateBorrowernumber GetValidLinkInfo SendPasswordRecoveryEmail CompletePasswordRecovery) ); +can_ok( "Koha::Patron::Password::Recovery", qw(ValidateBorrowernumber GetValidLinkInfo SendPasswordRecoveryEmail CompletePasswordRecovery) ); -################################################ -# C4::Passwordrecovery::ValidateBorrowernumber # -################################################ +############################################################ +# Koha::Patron::Password::Recovery::ValidateBorrowernumber # +############################################################ -ok( C4::Passwordrecovery::ValidateBorrowernumber($borrowernumber1), "[ValidateBorrowernumber] Borrower has a password recovery entry" ); -ok( ! C4::Passwordrecovery::ValidateBorrowernumber($borrowernumber2), "[ValidateBorrowernumber] Borrower's number is not found; password recovery entry is expired" ); -ok( ! C4::Passwordrecovery::ValidateBorrowernumber(9999), "[ValidateBorrowernumber] Borrower has no password recovery entry" ); +ok( Koha::Patron::Password::Recovery::ValidateBorrowernumber($borrowernumber1), "[ValidateBorrowernumber] Borrower has a password recovery entry" ); +ok( ! Koha::Patron::Password::Recovery::ValidateBorrowernumber($borrowernumber2), "[ValidateBorrowernumber] Borrower's number is not found; password recovery entry is expired" ); +ok( ! Koha::Patron::Password::Recovery::ValidateBorrowernumber(9999), "[ValidateBorrowernumber] Borrower has no password recovery entry" ); -########################################## -# C4::Passwordrecovery::GetValidLinkInfo # -########################################## +###################################################### +# Koha::Patron::Password::Recovery::GetValidLinkInfo # +###################################################### -my ($bnum1, $uname1) = C4::Passwordrecovery::GetValidLinkInfo($uuid1); -my ($bnum2, $uname2) = C4::Passwordrecovery::GetValidLinkInfo($uuid2); -my ($bnum3, $uname3) = C4::Passwordrecovery::GetValidLinkInfo("THISISANINVALIDUUID"); +my ($bnum1, $uname1) = Koha::Patron::Password::Recovery::GetValidLinkInfo($uuid1); +my ($bnum2, $uname2) = Koha::Patron::Password::Recovery::GetValidLinkInfo($uuid2); +my ($bnum3, $uname3) = Koha::Patron::Password::Recovery::GetValidLinkInfo("THISISANINVALIDUUID"); is( $bnum1, $borrowernumber1, "[GetValidLinkInfo] Borrower has a valid link" ); is( $uname1, $userid1, "[GetValidLinkInfo] Borrower's username is fetched when a valid link is found" ); ok( ! defined($bnum2), "[GetValidLinkInfo] Borrower's link is no longer valid; entry is expired" ); ok( ! defined($bnum3), "[GetValidLinkInfo] Invalid UUID returns no borrowernumber" ); -################################################## -# C4::Passwordrecovery::CompletePasswordRecovery # -################################################## +############################################################## +# Koha::Patron::Password::Recovery::CompletePasswordRecovery # +############################################################## -ok( C4::Passwordrecovery::CompletePasswordRecovery($uuid1) == 2, "[CompletePasswordRecovery] Completing a password recovery deletes the entry and expired entries" ); +ok( Koha::Patron::Password::Recovery::CompletePasswordRecovery($uuid1) == 2, "[CompletePasswordRecovery] Completing a password recovery deletes the entry and expired entries" ); $schema->resultset('BorrowerPasswordRecovery')->create( { @@ -127,22 +127,22 @@ $schema->resultset('BorrowerPasswordRecovery')->create( } ); -ok( C4::Passwordrecovery::CompletePasswordRecovery($uuid2) == 1, "[CompletePasswordRecovery] An expired or invalid UUID purges expired entries" ); -ok( C4::Passwordrecovery::CompletePasswordRecovery($uuid2) == 0, "[CompletePasswordRecovery] Returns 0 on a clean table" ); +ok( Koha::Patron::Password::Recovery::CompletePasswordRecovery($uuid2) == 1, "[CompletePasswordRecovery] An expired or invalid UUID purges expired entries" ); +ok( Koha::Patron::Password::Recovery::CompletePasswordRecovery($uuid2) == 0, "[CompletePasswordRecovery] Returns 0 on a clean table" ); -################################################### -# C4::Passwordrecovery::SendPasswordRecoveryEmail # -################################################### +############################################################### +# Koha::Patron::Password::Recovery::SendPasswordRecoveryEmail # +############################################################### my $borrower = shift [ Koha::Patrons->search( { userid => $userid1 } ) ]; -ok( C4::Passwordrecovery::SendPasswordRecoveryEmail($borrower, $email1, 0) == 1, "[SendPasswordRecoveryEmail] Returns 1 on success" ); +ok( Koha::Patron::Password::Recovery::SendPasswordRecoveryEmail($borrower, $email1, 0) == 1, "[SendPasswordRecoveryEmail] Returns 1 on success" ); my $letters = C4::Letters::GetQueuedMessages( { borrowernumber => $borrowernumber1, limit => 99 } ); ok( scalar @$letters == 1, "[SendPasswordRecoveryEmail] There is a letter in the queue for our borrower"); my $bpr = $schema->resultset('BorrowerPasswordRecovery')->search( { borrowernumber => $borrowernumber1 } ); my $tempuuid1 = $bpr->next->uuid; -C4::Passwordrecovery::SendPasswordRecoveryEmail($borrower, $email1, 1); +Koha::Patron::Password::Recovery::SendPasswordRecoveryEmail($borrower, $email1, 1); $bpr = $schema->resultset('BorrowerPasswordRecovery')->search( { borrowernumber => $borrowernumber1 } ); my $tempuuid2 = $bpr->next->uuid; -- 2.39.5