Attachment #87178 for bug #21336

View | Details | Raw Unified | Return to bug 21336
Collapse All | Expand All

Lines 1845-1851 sub checkpw { Link Here

(-)a/C4/Auth.pm (-1 / +1 lines)
1845	if( $patron ) {	1845	if( $patron ) {
1846	if ( $passwd_ok ) {	1846	if ( $passwd_ok ) {
1847	$patron->update({ login_attempts => 0 });	1847	$patron->update({ login_attempts => 0 });
1848	} else {	1848	} elsif( !$patron->account_locked ) {
1849	$patron->update({ login_attempts => $patron->login_attempts + 1 });	1849	$patron->update({ login_attempts => $patron->login_attempts + 1 });
1850	}	1850	}
1851	}	1851	}

Line 0 Link Here

(-)a/installer/data/mysql/atomicupdate/bug_21336c.perl (+6 lines)
	1	$DBversion = 'XXX'; # will be replaced by the RM
	2	if( CheckVersion( $DBversion ) ) {
	3	$dbh->do( "UPDATE borrowers SET login_attempts = ? WHERE login_attempts > ?", undef, C4::Context->preference('FailedLoginAttempts'), C4::Context->preference('FailedLoginAttempts') );
	4	SetVersion( $DBversion );
	5	print "Upgrade to $DBversion done (Bug 21336 - Reset login_attempts)\n";
	6	}




use Test::MockObject;
use Test::MockModule;
use List::MoreUtils qw/all any none/;
use Test::More tests => 21;
use Test::Warn;
use t::lib::Mocks;
use t::lib::TestBuilder;

# FIXME: SessionStorage defaults to mysql, but it seems to break transaction
# handling
t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' );
t::lib::Mocks::mock_preference( 'GDPR_Policy', '' ); # Disabled

$schema->storage->txn_begin;



ok(C4::Auth::checkpw_hash('password', $hash1), 'password validates with first hash');
ok(C4::Auth::checkpw_hash('password', $hash2), 'password validates with second hash');

subtest 'Check value of login_attempts in checkpw' => sub {
    plan tests => 6;

    t::lib::Mocks::mock_preference('FailedLoginAttempts', 3);

    # Only interested here in regular login
    $C4::Auth::cas  = 0;
    $C4::Auth::ldap = 0;

    my $patron = $builder->build_object({ class => 'Koha::Patrons' });
    $patron->login_attempts(2);
    $patron->password('123')->store; # yes, deliberately not hashed

    is( $patron->account_locked, 0, 'Patron not locked' );
    my @test = checkpw( $dbh, $patron->userid, '123', undef, 'opac', 1 );
        # Note: 123 will not be hashed to 123 !
    is( $test[0], 0, 'checkpw should have failed' );
    $patron->discard_changes; # refresh
    is( $patron->login_attempts, 3, 'Login attempts increased' );
    is( $patron->account_locked, 1, 'Check locked status' );

    # And another try to go over the limit: different return value!
    @test = checkpw( $dbh, $patron->userid, '123', undef, 'opac', 1 );
    is( @test, 0, 'checkpw failed again and returns nothing now' );
    $patron->discard_changes; # refresh
    is( $patron->login_attempts, 3, 'Login attempts not increased anymore' );
};

$schema->storage->txn_rollback;

Lines 1121-1127 subtest 'libraries_where_can_see_patrons + can_see_patron_infos + search_limited Link Here

(-)a/t/db_dependent/Koha/Patrons.t (-2 / +3 lines)
1121	};	1121	};
1122		1122
1123	subtest 'account_locked' => sub {	1123	subtest 'account_locked' => sub {
1124	plan tests => 8;	1124	plan tests => 9;
1125	my $patron = $builder->build({ source => 'Borrower', value => { login_attempts => 0 } });	1125	my $patron = $builder->build({ source => 'Borrower', value => { login_attempts => 0 } });
1126	$patron = Koha::Patrons->find( $patron->{borrowernumber} );	1126	$patron = Koha::Patrons->find( $patron->{borrowernumber} );
1127	for my $value ( undef, '', 0 ) {	1127	for my $value ( undef, '', 0 ) {
Lines 1136-1141 subtest 'account_locked' => sub { Link Here
1136	is( $patron->account_locked, 0, 'Patron has 2 failed attempts, account should not be considered locked yet' );	1136	is( $patron->account_locked, 0, 'Patron has 2 failed attempts, account should not be considered locked yet' );
1137	$patron->login_attempts(3)->store;	1137	$patron->login_attempts(3)->store;
1138	is( $patron->account_locked, 1, 'Patron has 3 failed attempts, account should be considered locked yet' );	1138	is( $patron->account_locked, 1, 'Patron has 3 failed attempts, account should be considered locked yet' );
		1139	$patron->login_attempts(4)->store;
		1140	is( $patron->account_locked, 1, 'Patron could not have 4 failed attempts, but account should still be considered locked' );
1139		1141
1140	$patron->delete;	1142	$patron->delete;
1141	};	1143	};
1142	-

Return to bug 21336

Lines 10-16 use CGI qw ( -utf8 ); Link Here

(-)a/t/db_dependent/Auth.t (-1 / +32 lines)
10	use Test::MockObject;	10	use Test::MockObject;
11	use Test::MockModule;	11	use Test::MockModule;
12	use List::MoreUtils qw/all any none/;	12	use List::MoreUtils qw/all any none/;
13	use Test::More tests => 20;	13	use Test::More tests => 21;
14	use Test::Warn;	14	use Test::Warn;
15	use t::lib::Mocks;	15	use t::lib::Mocks;
16	use t::lib::TestBuilder;	16	use t::lib::TestBuilder;
Lines 32-37 my $dbh = C4::Context->dbh; Link Here
32	# FIXME: SessionStorage defaults to mysql, but it seems to break transaction	32	# FIXME: SessionStorage defaults to mysql, but it seems to break transaction
33	# handling	33	# handling
34	t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' );	34	t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' );
		35	t::lib::Mocks::mock_preference( 'GDPR_Policy', '' ); # Disabled
35		36
36	$schema->storage->txn_begin;	37	$schema->storage->txn_begin;
37		38
Lines 315-317 my $hash2 = hash_password('password'); Link Here
315		316
316	ok(C4::Auth::checkpw_hash('password', $hash1), 'password validates with first hash');	317	ok(C4::Auth::checkpw_hash('password', $hash1), 'password validates with first hash');
317	ok(C4::Auth::checkpw_hash('password', $hash2), 'password validates with second hash');	318	ok(C4::Auth::checkpw_hash('password', $hash2), 'password validates with second hash');
		319
		320	subtest 'Check value of login_attempts in checkpw' => sub {
		321	plan tests => 6;
		322
		323	t::lib::Mocks::mock_preference('FailedLoginAttempts', 3);
		324
		325	# Only interested here in regular login
		326	$C4::Auth::cas = 0;
		327	$C4::Auth::ldap = 0;
		328
		329	my $patron = $builder->build_object({ class => 'Koha::Patrons' });
		330	$patron->login_attempts(2);
		331	$patron->password('123')->store; # yes, deliberately not hashed
		332
		333	is( $patron->account_locked, 0, 'Patron not locked' );
		334	my @test = checkpw( $dbh, $patron->userid, '123', undef, 'opac', 1 );
		335	# Note: 123 will not be hashed to 123 !
		336	is( $test[0], 0, 'checkpw should have failed' );
		337	$patron->discard_changes; # refresh
		338	is( $patron->login_attempts, 3, 'Login attempts increased' );
		339	is( $patron->account_locked, 1, 'Check locked status' );
		340
		341	# And another try to go over the limit: different return value!
		342	@test = checkpw( $dbh, $patron->userid, '123', undef, 'opac', 1 );
		343	is( @test, 0, 'checkpw failed again and returns nothing now' );
		344	$patron->discard_changes; # refresh
		345	is( $patron->login_attempts, 3, 'Login attempts not increased anymore' );
		346	};
		347
		348	$schema->storage->txn_rollback;