Bugzilla – Attachment 53103 Details for
Bug 16853
Move changepassword to Koha::Patron->update_password
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 16853: Move changepassword to Koha::Patron->update_password
Bug-16853-Move-changepassword-to-KohaPatron-update.patch (text/plain), 9.64 KB, created by
Marc Véron
on 2016-07-05 15:18:19 UTC
(
hide
)
Description:
Bug 16853: Move changepassword to Koha::Patron->update_password
Filename:
MIME Type:
Creator:
Marc Véron
Created:
2016-07-05 15:18:19 UTC
Size:
9.64 KB
patch
obsolete
>From 6f2ab8861ac36aee81515273043db9b45361bea3 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Mon, 4 Jul 2016 14:00:58 +0100 >Subject: [PATCH] Bug 16853: Move changepassword to > Koha::Patron->update_password >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >This patch moves the code from C4::Members::changepassword to >Koha::Patron->update_password > >Test plan: >Change your password at the OPAC and the staff interface >This should work as before > >Signed-off-by: Marc Véron <veron@veron.ch> >--- > C4/Auth_with_ldap.pm | 5 +++-- > C4/Members.pm | 29 ----------------------------- > Koha/Patron.pm | 21 +++++++++++++++++++++ > members/member-password.pl | 2 +- > opac/opac-password-recovery.pl | 4 ++-- > t/db_dependent/Auth.t | 3 ++- > t/db_dependent/Koha/Patrons.t | 31 ++++++++++++++++++++++++++++++- > 7 files changed, 59 insertions(+), 36 deletions(-) > >diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm >index c50df76..03e81e1 100644 >--- a/C4/Auth_with_ldap.pm >+++ b/C4/Auth_with_ldap.pm >@@ -23,11 +23,12 @@ use Carp; > > use C4::Debug; > use C4::Context; >-use C4::Members qw(AddMember changepassword); >+use C4::Members qw(AddMember); > use C4::Members::Attributes; > use C4::Members::AttributeTypes; > use C4::Members::Messaging; > use C4::Auth qw(checkpw_internal); >+use Koha::Patrons; > use Koha::AuthUtils qw(hash_password); > use List::MoreUtils qw( any ); > use Net::LDAP; >@@ -334,7 +335,7 @@ sub _do_changepassword { > my $digest = hash_password($password); > > $debug and print STDERR "changing local password for borrowernumber=$borrowerid to '$digest'\n"; >- changepassword($userid, $borrowerid, $digest); >+ Koha::Patrons->find($borrowerid)->update_password( $userid, $digest ); > > my ($ok, $cardnum) = checkpw_internal(C4::Context->dbh, $userid, $password); > return $cardnum if $ok; >diff --git a/C4/Members.pm b/C4/Members.pm >index 383799d..730fdd9 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -813,35 +813,6 @@ sub Generate_Userid { > return $newuid; > } > >-sub changepassword { >- my ( $uid, $member, $digest ) = @_; >- my $dbh = C4::Context->dbh; >- >-#Make sure the userid chosen is unique and not theirs if non-empty. If it is not, >-#Then we need to tell the user and have them create a new one. >- my $resultcode; >- my $sth = >- $dbh->prepare( >- "SELECT * FROM borrowers WHERE userid=? AND borrowernumber != ?"); >- $sth->execute( $uid, $member ); >- if ( ( $uid ne '' ) && ( my $row = $sth->fetchrow_hashref ) ) { >- $resultcode=0; >- } >- else { >- #Everything is good so we can update the information. >- $sth = >- $dbh->prepare( >- "update borrowers set userid=?, password=? where borrowernumber=?"); >- $sth->execute( $uid, $digest, $member ); >- $resultcode=1; >- } >- >- logaction("MEMBERS", "CHANGE PASS", $member, "") if C4::Context->preference("BorrowersLog"); >- return $resultcode; >-} >- >- >- > =head2 fixup_cardnumber > > Warning: The caller is responsible for locking the members table in write >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index ac62594..0ca3a52 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -21,6 +21,8 @@ use Modern::Perl; > > use Carp; > >+use C4::Context; >+use C4::Log; > use Koha::Database; > use Koha::Patrons; > use Koha::Patron::Images; >@@ -95,6 +97,25 @@ sub siblings { > ); > } > >+=head2 update_password >+ >+my $updated = $patron->update_password( $userid, $password ); >+ >+Update the userid and the password of a patron. >+If the userid already exists, returns and let DBIx::Class warns >+This will add an entry to action_logs if BorrowersLog is set. >+ >+=cut >+ >+sub update_password { >+ my ( $self, $userid, $password ) = @_; >+ eval { $self->userid($userid)->store; }; >+ return if $@; # Make sure the userid is not already in used by another patron >+ $self->password($password)->store; >+ logaction( "MEMBERS", "CHANGE PASS", $self->borrowernumber, "" ) if C4::Context->preference("BorrowersLog"); >+ return 1; >+} >+ > =head3 type > > =cut >diff --git a/members/member-password.pl b/members/member-password.pl >index 85e4b97..64d6347 100755 >--- a/members/member-password.pl >+++ b/members/member-password.pl >@@ -66,7 +66,7 @@ if ( $newpassword && !scalar(@errors) ) { > my $digest = Koha::AuthUtils::hash_password( $input->param('newpassword') ); > my $uid = $input->param('newuserid'); > my $dbh = C4::Context->dbh; >- if ( changepassword( $uid, $member, $digest ) ) { >+ if ( Koha::Patrons->find( $member )->update_password($uid, $digest) ) { > $template->param( newpassword => $newpassword ); > if ( $destination eq 'circ' ) { > print $input->redirect("/cgi-bin/koha/circ/circulation.pl?findborrower=$cardnumber"); >diff --git a/opac/opac-password-recovery.pl b/opac/opac-password-recovery.pl >index 257ca92..9560c38 100755 >--- a/opac/opac-password-recovery.pl >+++ b/opac/opac-password-recovery.pl >@@ -5,11 +5,11 @@ use CGI; > > use C4::Auth; > use C4::Koha; >-use C4::Members qw(changepassword); > use C4::Output; > use C4::Context; > use Koha::Patron::Password::Recovery > qw(SendPasswordRecoveryEmail ValidateBorrowernumber GetValidLinkInfo CompletePasswordRecovery); >+use Koha::Patrons; > use Koha::AuthUtils qw(hash_password); > use Koha::Patrons; > my $query = new CGI; >@@ -135,7 +135,7 @@ elsif ( $query->param('passwordReset') ) { > && ( $password eq $repeatPassword ) > && ( length($password) >= $minPassLength ) ) > { #apply changes >- changepassword( $username, $borrower_number, hash_password($password) ); >+ Koha::Patrons->find($borrower_number)->update_password( $username, hash_password($password) ); > CompletePasswordRecovery($uniqueKey); > $template->param( > password_reset_done => 1, >diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t >index aa703ec..233b2be 100644 >--- a/t/db_dependent/Auth.t >+++ b/t/db_dependent/Auth.t >@@ -17,6 +17,7 @@ use C4::Auth qw(checkpw); > use C4::Members; > use Koha::AuthUtils qw/hash_password/; > use Koha::Database; >+use Koha::Patrons; > > BEGIN { > use_ok('C4::Auth'); >@@ -32,7 +33,7 @@ my $hash2 = hash_password('password'); > > { # tests no_set_userenv parameter > my $patron = $builder->build( { source => 'Borrower' } ); >- changepassword( $patron->{userid}, $patron->{borrowernumber}, $hash1 ); >+ Koha::Patrons->find( $patron->{borrowernumber} )->update_password( $patron->{userid}, $hash1 ); > my $library = $builder->build( > { > source => 'Branch', >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index fafd829..980bf84 100644 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -19,13 +19,15 @@ > > use Modern::Perl; > >-use Test::More tests => 5; >+use Test::More tests => 6; >+use Test::Warn; > > use Koha::Patron; > use Koha::Patrons; > use Koha::Database; > > use t::lib::TestBuilder; >+use t::lib::Mocks; > > my $schema = Koha::Database->new->schema; > $schema->storage->txn_begin; >@@ -40,6 +42,7 @@ my $new_patron_1 = Koha::Patron->new( > categorycode => $category->{categorycode}, > surname => 'surname for patron1', > firstname => 'firstname for patron1', >+ userid => 'a_nonexistent_userid_1', > } > )->store; > my $new_patron_2 = Koha::Patron->new( >@@ -48,6 +51,7 @@ my $new_patron_2 = Koha::Patron->new( > categorycode => $category->{categorycode}, > surname => 'surname for patron2', > firstname => 'firstname for patron2', >+ userid => 'a_nonexistent_userid_2', > } > )->store; > >@@ -98,6 +102,31 @@ subtest 'siblings' => sub { > $retrieved_guarantee_1->delete; > }; > >+subtest 'update_password' => sub { >+ plan tests => 7; >+ >+ t::lib::Mocks::mock_preference( 'BorrowersLog', 1 ); >+ my $original_userid = $new_patron_1->userid; >+ my $original_password = $new_patron_1->password; >+ warning_like { $retrieved_patron_1->update_password( $new_patron_2->userid, 'another_password' ) } >+ qr{Duplicate entry}, >+ 'Koha::Patron->update_password should warn if the userid is already used by another patron'; >+ is( Koha::Patrons->find( $new_patron_1->borrowernumber )->userid, $original_userid, 'Koha::Patron->update_password should not have updated the userid' ); >+ is( Koha::Patrons->find( $new_patron_1->borrowernumber )->password, $original_password, 'Koha::Patron->update_password should not have updated the userid' ); >+ >+ $retrieved_patron_1->update_password( 'another_nonexistent_userid_1', 'another_password' ); >+ is( Koha::Patrons->find( $new_patron_1->borrowernumber )->userid, 'another_nonexistent_userid_1', 'Koha::Patron->update_password should have updated the userid' ); >+ is( Koha::Patrons->find( $new_patron_1->borrowernumber )->password, 'another_password', 'Koha::Patron->update_password should have updated the password' ); >+ >+ my $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'CHANGE PASS', object => $new_patron_1->borrowernumber } )->count; >+ is( $number_of_logs, 1, 'With BorrowerLogs, Koha::Patron->update_password should have logged' ); >+ >+ t::lib::Mocks::mock_preference( 'BorrowersLog', 0 ); >+ $retrieved_patron_1->update_password( 'yet_another_nonexistent_userid_1', 'another_password' ); >+ $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'MEMBERS', action => 'CHANGE PASS', object => $new_patron_1->borrowernumber } )->count; >+ is( $number_of_logs, 1, 'With BorrowerLogs, Koha::Patron->update_password should not have logged' ); >+}; >+ > $retrieved_patron_1->delete; > is( Koha::Patrons->search->count, $nb_of_patrons + 1, 'Delete should have deleted the patron' ); > >-- >1.7.10.4
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 16853
:
53098
|
53103
|
53239
|
53435