From a9f4dc38c6794e77b854653ca2f358fef125c280 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Marc=20V=C3=A9ron?= Date: Sun, 21 May 2017 18:28:48 +0200 Subject: [PATCH] Bug 16711: OPAC Password recovery: Handling if multiple accounts have the same mail address To reproduce: - Create 3 Accounts, login names are test01, test02, test03, Email is the same for all. - Go to OPAC -> Password recovery and indicate E-Mail only - You will get an email for only one of the accounts above. To test: - Apply patch, restart memcached and plack - Go to db, delete from borrower_password_recovery; - Try steps above to reproduce. You will get an error message: Account identification with this email address only is ambiguous. Please use the field 'Login' as well. - Verify that other cases work as before (provide valid / invalid login only, provide valid email for an existing account, provide unknown email, provide both login and email with all combinations of valid / invalid) Signed-off-by: Josef Moravec Signed-off-by: Nick Clemens Signed-off-by: Jonathan Druart Bug 16711: (QA-followup) Use count directly See comment # 13 Signed-off-by: Nick Clemens Signed-off-by: Jonathan Druart (cherry picked from commit 3829020c2664ec531354f78f0c04d00f5dd6795d) Signed-off-by: Fridolin Somers (cherry picked from commit 739a0f4b9b6bfba236d297608001180f4cb38af3) Signed-off-by: Katrin Fischer --- .../en/modules/opac-password-recovery.tt | 5 ++++- opac/opac-password-recovery.pl | 21 ++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt index f5a0e30961..4f3f8571d5 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt @@ -54,6 +54,9 @@
Please try again later. [% ELSIF (errNoBorrowerFound) %] No account was found with the provided information. + [% ELSIF (errMultipleAccountsForEmail) %] + Account identification with this email address only is ambiguous. +
Please use the field 'Login' as well. [% ELSIF (errAlreadyStartRecovery) %] The process of password recovery has already been started for this account ("[% username %]")
You should have received an email with a link to reset your password. @@ -78,7 +81,7 @@
-

To reset your password, enter your login and email address. +

To reset your password, enter your login and your email address. diff --git a/opac/opac-password-recovery.pl b/opac/opac-password-recovery.pl index 7b8a02b7a0..f99595b629 100755 --- a/opac/opac-password-recovery.pl +++ b/opac/opac-password-recovery.pl @@ -40,6 +40,7 @@ my $hasError; #email form error my $errNoBorrowerFound; my $errNoBorrowerEmail; +my $errMultipleAccountsForEmail; my $errAlreadyStartRecovery; my $errTooManyEmailFound; my $errBadEmail; @@ -54,20 +55,29 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { #try with the main email $email ||= ''; # avoid undef my $borrower; - my $search_results = []; + my $search_results; # Find the borrower by his userid or email if ($username) { - $search_results = [ Koha::Patrons->search( { userid => $username } ) ]; + $search_results = Koha::Patrons->search( { userid => $username } ); } elsif ($email) { - $search_results = [ Koha::Patrons->search( { -or => { email => $email, emailpro => $email, B_email => $email } } ) ]; + $search_results = Koha::Patrons->search( { -or => { email => $email, emailpro => $email, B_email => $email } } ); } - if ( not $search_results || scalar @$search_results > 1 ) { + + if ( not $search_results || $search_results->count < 1) { + $hasError = 1; + $errNoBorrowerFound = 1; + } + elsif ( $username && $search_results->count > 1) { # Multiple accounts for username $hasError = 1; $errNoBorrowerFound = 1; } - elsif ( $borrower = shift @$search_results ) { # One matching borrower + elsif ( $email && $search_results->count > 1) { # Muliple accounts for E-Mail + $hasError = 1; + $errMultipleAccountsForEmail = 1; + } + elsif ( $borrower = $search_results->next() ) { # One matching borrower $username ||= $borrower->userid; my @emails = ( $borrower->email, $borrower->emailpro, $borrower->B_email ); @@ -112,6 +122,7 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { errAlreadyStartRecovery => $errAlreadyStartRecovery, errBadEmail => $errBadEmail, errNoBorrowerEmail => $errNoBorrowerEmail, + errMultipleAccountsForEmail => $errMultipleAccountsForEmail, password_recovery => 1, email => HTML::Entities::encode($email), username => $username -- 2.39.5