Bug 21087: Hash passwords in ->update_password

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: John Doe <you@example.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Jonathan Druart 2018-07-18 19:54:34 -03:00 committed by Tomas Cohen Arazi
parent 3d41d947d2
commit 2e6fb40ef8
10 changed files with 29 additions and 34 deletions

View File

@ -344,10 +344,10 @@ sub _do_changepassword {
$sth->execute($borrowerid);
return $cardnum;
}
my $digest = hash_password($password);
my $digest = hash_password($password);
$debug and print STDERR "changing local password for borrowernumber=$borrowerid to '$digest'\n";
Koha::Patrons->find($borrowerid)->update_password( $userid, $digest );
Koha::Patrons->find($borrowerid)->update_password( $userid, $password );
my ($ok, $cardnum) = checkpw_internal(C4::Context->dbh, $userid, $password);
return $cardnum if $ok;

View File

@ -28,6 +28,7 @@ use Text::Unaccent qw( unac_string );
use C4::Context;
use C4::Log;
use Koha::AuthUtils;
use Koha::Checkouts;
use Koha::Database;
use Koha::DateUtils;
@ -267,20 +268,9 @@ sub store {
if C4::Context->preference("BorrowersLog");
}
else { #ModMember
# test to know if you must update or not the borrower password
if ( defined $self->password ) {
if ( $self->password eq '****' or $self->password eq '' ) {
$self->password(undef);
} else {
if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
# Update the hashed PIN in borrower_sync.hashed_pin, before Koha hashes it
Koha::NorwegianPatronDB::NLUpdateHashedPIN( $self->borrowernumber, $self->password );
}
$self->password(Koha::AuthUtils::hash_password($self->password));
}
}
# We could add a test here to make sure the password is not update (?)
# Come from ModMember, but should not be possible (?)
# Come from ModMember, but should not be possible (?)
$self->dateenrolled(undef) unless $self->dateenrolled;
$self->dateexpiry(undef) unless $self->dateexpiry;
@ -687,14 +677,24 @@ 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
return 0 if $password eq '****' or $password eq ''; # Do we need that?
if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
# Update the hashed PIN in borrower_sync.hashed_pin, before Koha hashes it
Koha::NorwegianPatronDB::NLUpdateHashedPIN( $self->borrowernumber, $password );
}
my $digest = Koha::AuthUtils::hash_password($password);
$self->update(
{
password => $password,
password => $digest,
login_attempts => 0,
}
);
logaction( "MEMBERS", "CHANGE PASS", $self->borrowernumber, "" ) if C4::Context->preference("BorrowersLog");
return 1;
return $digest;
}
=head3 renew_account

View File

@ -78,10 +78,10 @@ if ( $newpassword and not @errors) {
token => scalar $input->param('csrf_token'),
});
my $digest = Koha::AuthUtils::hash_password( scalar $input->param('newpassword') );
my $uid = $input->param('newuserid') || $bor->{userid};
my $password = $input->param('newpassword');
my $dbh = C4::Context->dbh;
if ( Koha::Patrons->find( $member )->update_password($uid, $digest) ) {
if ( Koha::Patrons->find( $member )->update_password($uid, $password) ) {
$template->param( newpassword => $newpassword );
if ( $destination eq 'circ' ) {
print $input->redirect("/cgi-bin/koha/circ/circulation.pl?findborrower=$cardnumber");

View File

@ -537,6 +537,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){
# updating any columns in the borrowers table,
# which can happen if we're only editing the
# patron attributes or messaging preferences sections
$patron->update_password($newdata{password}); # If != **** or '' # test here?
if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes);
}

View File

@ -10,7 +10,6 @@ use C4::Context;
use Koha::Patron::Password::Recovery
qw(SendPasswordRecoveryEmail ValidateBorrowernumber GetValidLinkInfo CompletePasswordRecovery DeleteExpiredPasswordRecovery);
use Koha::Patrons;
use Koha::AuthUtils qw(hash_password);
use Koha::Patrons;
my $query = new CGI;
use HTML::Entities;
@ -156,7 +155,7 @@ elsif ( $query->param('passwordReset') ) {
$error = 'password_too_weak' if $err eq 'too_weak';
$error = 'password_has_whitespaces' if $err eq 'has_whitespaces';
} else {
Koha::Patrons->find($borrower_number)->update_password( $username, hash_password($password) );
Koha::Patrons->find($borrower_number)->update_password( $username, $password );
CompletePasswordRecovery($uniqueKey);
$template->param(
password_reset_done => 1,

View File

@ -120,7 +120,7 @@ my $hash2 = hash_password('password');
{ # tests no_set_userenv parameter
my $patron = $builder->build( { source => 'Borrower' } );
Koha::Patrons->find( $patron->{borrowernumber} )->update_password( $patron->{userid}, $hash1 );
Koha::Patrons->find( $patron->{borrowernumber} )->update_password( $patron->{userid}, 'password' );
my $library = $builder->build(
{
source => 'Branch',

View File

@ -173,9 +173,9 @@ subtest 'update_password' => sub {
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' );
my $digest = $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' );
is( Koha::Patrons->find( $new_patron_1->borrowernumber )->password, $digest, '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' );
@ -1444,7 +1444,7 @@ subtest '->store' => sub {
# Test password
my $password = 'password';
$patron_1->password($password)->store;
$patron_1->update_password($patron_1->userid, $password);
like( $patron_1->password, qr|^\$2|, 'Password should be hashed using bcrypt (start with $2)' );
my $digest = $patron_1->password;
$patron_1->surname('xxx')->store;

View File

@ -9,7 +9,6 @@ use t::lib::Mocks;
use t::lib::TestBuilder;
use C4::Auth;
use Koha::AuthUtils qw/hash_password/;
use Koha::Database;
use Test::More tests => 27;
@ -369,9 +368,8 @@ my $schema = Koha::Database->schema;
my $builder = t::lib::TestBuilder->new;
# Borrower Creation
my $hash = hash_password('password');
our $patron = $builder->build( { source => 'Borrower' } );
Koha::Patrons->find( $patron->{borrowernumber} )->update_password( $patron->{userid}, $hash );
Koha::Patrons->find( $patron->{borrowernumber} )->update_password( $patron->{userid}, 'password' );
my $session = C4::Auth::get_session("");
$session->flush;

View File

@ -47,9 +47,8 @@ SKIP: {
like( $driver->get_title, qr(Log in to Koha), 'Hitting the main page should redirect to the login form');
my $password = Koha::AuthUtils::generate_password();
my $digest = Koha::AuthUtils::hash_password( $password );
my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { flags => 0 }});
$patron->update_password( $patron->userid, $digest );
$patron->update_password( $patron->userid, $password );
# Patron does not have permission to access staff interface
$s->auth( $patron->userid, $password );
@ -79,9 +78,8 @@ SKIP: {
like( $driver->get_title, qr(Koha online catalog), 'Hitting the main page should not redirect to the login form');
my $password = Koha::AuthUtils::generate_password();
my $digest = Koha::AuthUtils::hash_password( $password );
my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { flags => 0 }});
$patron->update_password( $patron->userid, $digest );
$patron->update_password( $patron->userid, $password );
# Using the modal
$driver->find_element('//a[@class="login-link loginModal-trigger"]')->click;

View File

@ -41,8 +41,7 @@ subtest 'OPAC - borrowernumber and branchcode as html attributes' => sub {
my $patron = $builder->build_object(
{ class => 'Koha::Patrons', value => { flags => 1 } } );
my $password = Koha::AuthUtils::generate_password();
my $digest = Koha::AuthUtils::hash_password($password);
$patron->update_password( $patron->userid, $digest );
$patron->update_password( $patron->userid, $password );
$s->opac_auth( $patron->userid, $password );
my $elt = $driver->find_element('//span[@class="loggedinusername"]');
is( $elt->get_attribute('data-branchcode'), $patron->library->branchcode,