Bugzilla – Attachment 60303 Details for
Bug 18025
Expired password recovery links cause sql crash
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18025 - Expired password recovery links cause sql crash
Bug-18025---Expired-password-recovery-links-cause-.patch (text/plain), 7.54 KB, created by
Jonathan Druart
on 2017-02-16 07:49:45 UTC
(
hide
)
Description:
Bug 18025 - Expired password recovery links cause sql crash
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2017-02-16 07:49:45 UTC
Size:
7.54 KB
patch
obsolete
>From 60bfc9c69ceb688c4029750302518c46ccbd167b Mon Sep 17 00:00:00 2001 >From: Liz Rea <liz@catalyst.net.nz> >Date: Tue, 31 Jan 2017 21:59:01 +0000 >Subject: [PATCH] Bug 18025 - Expired password recovery links cause sql crash >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >When a user gets an email, but doesn't act or visit it within two days, > attempting to create a new one causes a collision. We should just > delete the old one, assuming they still want to reset their > password. > >To test: >create yourself a borrower with a userid and password. >Attempt a password recovery on the OPAC >update the entry in the database for that user to have an expired token >e.g. update borrower_password_recovery set valid_until = '2017-01-25 >03:25:26' where borrowernumber = 12; >Attempt another password recovery operation - should error >apply the patch >Try it again - no error, new token is generated and additional email >with new link is sent. > >Issue reproduced - is resolved by patch >Signed-off-by: Marc Véron <veron@veron.ch> > >Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> >--- > Koha/Patron/Password/Recovery.pm | 21 +++++++++++++++++-- > opac/opac-password-recovery.pl | 7 ++++++- > t/db_dependent/Passwordrecovery.t | 43 +++++++++++++++++++++++++++++++++++++-- > 3 files changed, 66 insertions(+), 5 deletions(-) > >diff --git a/Koha/Patron/Password/Recovery.pm b/Koha/Patron/Password/Recovery.pm >index 04491e6..7c35c5d 100644 >--- a/Koha/Patron/Password/Recovery.pm >+++ b/Koha/Patron/Password/Recovery.pm >@@ -32,6 +32,7 @@ BEGIN { > &SendPasswordRecoveryEmail > &GetValidLinkInfo > &CompletePasswordRecovery >+ &DeleteExpiredPasswordRecovery > ); > } > >@@ -66,11 +67,9 @@ sub ValidateBorrowernumber { > }, > { columns => 'borrowernumber' } > ); >- > if ( $rs->next ) { > return 1; > } >- > return 0; > } > >@@ -181,6 +180,24 @@ sub CompletePasswordRecovery { > return $entry->delete(); > } > >+=head2 DeleteExpiredPasswordRecovery >+ >+ $bool = DeleteExpiredPasswordRecovery($borrowernumber) >+ >+ Deletes an expired password recovery entry. >+ >+=cut >+ >+sub DeleteExpiredPasswordRecovery { >+ my $borrower_number = shift; >+ my $model = >+ Koha::Database->new->schema->resultset('BorrowerPasswordRecovery'); >+ my $entry = $model->search( >+ { borrowernumber => $borrower_number } ); >+ return $entry->delete(); >+} >+ >+ > END { } # module clean-up code here (global destructor) > > 1; >diff --git a/opac/opac-password-recovery.pl b/opac/opac-password-recovery.pl >index 9560c38..2099caf 100755 >--- a/opac/opac-password-recovery.pl >+++ b/opac/opac-password-recovery.pl >@@ -8,7 +8,7 @@ use C4::Koha; > use C4::Output; > use C4::Context; > use Koha::Patron::Password::Recovery >- qw(SendPasswordRecoveryEmail ValidateBorrowernumber GetValidLinkInfo CompletePasswordRecovery); >+ qw(SendPasswordRecoveryEmail ValidateBorrowernumber GetValidLinkInfo CompletePasswordRecovery DeleteExpiredPasswordRecovery); > use Koha::Patrons; > use Koha::AuthUtils qw(hash_password); > use Koha::Patrons; >@@ -96,6 +96,11 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { > $hasError = 1; > $errAlreadyStartRecovery = 1; > } >+ elsif ( !ValidateBorrowernumber($borrower->borrowernumber) >+ && !$query->param('resendEmail') ) >+ { >+ DeleteExpiredPasswordRecovery($borrower->borrowernumber); >+ } > } > else { # 0 matching borrower > $hasError = 1; >diff --git a/t/db_dependent/Passwordrecovery.t b/t/db_dependent/Passwordrecovery.t >index 92cdf4b..9067e38 100755 >--- a/t/db_dependent/Passwordrecovery.t >+++ b/t/db_dependent/Passwordrecovery.t >@@ -22,7 +22,7 @@ use C4::Letters; > use Koha::Database; > use Koha::Patrons; > >-use Test::More tests => 16; >+use Test::More tests => 18; > > use_ok('Koha::Patron::Password::Recovery'); > >@@ -38,12 +38,16 @@ $dbh->{RaiseError} = 1; > > my $borrowernumber1 = '2000000000'; > my $borrowernumber2 = '2000000001'; >+my $borrowernumber3 = '2000000002'; > my $userid1 = "I83MFItzRpGPxD3vW0"; > my $userid2 = "Gh5t43980hfSAOcvne"; >+my $userid3 = "adsfada80hfSAOcvne"; > my $email1 = $userid1 . '@koha-community.org'; > my $email2 = $userid2 . '@koha-community.org'; >+my $email3 = $userid3 . '@koha-community.org'; > my $uuid1 = "ABCD1234"; > my $uuid2 = "WXYZ0987"; >+my $uuid3 = "LMNO4561"; > > my $categorycode = 'S'; # staff > my $branch = $schema->resultset('Branch')->first(); # legit branch from your db >@@ -74,6 +78,18 @@ $schema->resultset('Borrower')->create( > branchcode => $branch, > } > ); >+$schema->resultset('Borrower')->create( >+ { >+ borrowernumber => $borrowernumber3, >+ surname => '', >+ address => '', >+ city => '', >+ userid => $userid3, >+ email => $email3, >+ categorycode => $categorycode, >+ branchcode => $branch, >+ } >+); > > $schema->resultset('BorrowerPasswordRecovery')->create( > { >@@ -89,6 +105,14 @@ $schema->resultset('BorrowerPasswordRecovery')->create( > valid_until => DateTime->now( time_zone => C4::Context->tz() )->subtract( days => 2 )->datetime() > } > ); >+$schema->resultset('BorrowerPasswordRecovery')->create( >+ { >+ borrowernumber => $borrowernumber3, >+ uuid => $uuid3, >+ valid_until => DateTime->now( time_zone => C4::Context->tz() )->subtract( days => 3 )->datetime() >+ } >+); >+ > > can_ok( "Koha::Patron::Password::Recovery", qw(ValidateBorrowernumber GetValidLinkInfo SendPasswordRecoveryEmail CompletePasswordRecovery) ); > >@@ -117,7 +141,7 @@ ok( ! defined($bnum3), "[GetValidLinkInfo] Invalid UUID returns no borrowernumbe > # Koha::Patron::Password::Recovery::CompletePasswordRecovery # > ############################################################## > >-ok( Koha::Patron::Password::Recovery::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 used entry" ); > > $schema->resultset('BorrowerPasswordRecovery')->create( > { >@@ -130,6 +154,21 @@ $schema->resultset('BorrowerPasswordRecovery')->create( > 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" ); > >+################################################################### >+# Koha::Patron::Password::Recovery::DeleteExpiredPasswordRecovery # >+################################################################### >+ >+$schema->resultset('BorrowerPasswordRecovery')->create( >+ { >+ borrowernumber => $borrowernumber3, >+ uuid => $uuid3, >+ valid_until => DateTime->now( time_zone => C4::Context->tz() )->subtract( days => 3 )->datetime() >+ } >+); >+ >+ok( Koha::Patron::Password::Recovery::DeleteExpiredPasswordRecovery($borrowernumber3) == 1, "[DeleteExpiredPasswordRecovery] we can delete the unused entry" ); >+ok( Koha::Patron::Password::Recovery::DeleteExpiredPasswordRecovery($borrowernumber3) == 0, "[DeleteExpiredPasswordRecovery] Returns 0 on a clean table" ); >+ > ############################################################### > # Koha::Patron::Password::Recovery::SendPasswordRecoveryEmail # > ############################################################### >-- >2.9.3
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18025
:
59760
|
60108
|
60223
|
60245
|
60281
|
60282
|
60295
|
60296
|
60303
|
60304
|
60305
|
60311
|
60312
|
60313