From 9047dfce55c325b495ae883d7a41721706f22ef3 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 23 Mar 2022 15:58:30 +0000 Subject: [PATCH] Bug 29924: Update ILSDI to be aware of expired passwords To test: 1 - Enable ILSDI 2 - Set a patron password with expired password 3 - http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=AuthenticatePatron&username=usernam&password=password 4 - Confirm 'PasswordExpired' returned Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Tomas Cohen Arazi Signed-off-by: Fridolin Somers --- C4/Auth.pm | 9 ++++++--- C4/ILSDI/Services.pm | 5 ++++- t/db_dependent/Auth.t | 18 +++++++++++++++++- t/db_dependent/ILSDI_Services.t | 9 ++++++++- 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 4007179cc3..26066bb32d 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1149,7 +1149,7 @@ sub checkauth { } # $return: 1 = valid user - if ($return) { + if ($return > 0) { if ( $flags = haspermission( $userid, $flagsrequired ) ) { $auth_state = "logged_in"; @@ -1909,8 +1909,8 @@ sub checkpw { # 0 if auth is nok # -1 if user bind failed (LDAP only) - if ( $patron and ( $patron->account_locked || $patron->password_expired ) ) { - # Nothing to check, account is locked or password expired + if ( $patron and ( $patron->account_locked ) ) { + # Nothing to check, account is locked } elsif ($ldap && defined($password)) { my ( $retval, $retcard, $retuserid ) = checkpw_ldap(@_); # EXTERNAL AUTH if ( $retval == 1 ) { @@ -1963,6 +1963,9 @@ sub checkpw { if( $patron ) { if ( $passwd_ok ) { $patron->update({ login_attempts => 0 }); + if( $patron->password_expired ){ + @return = (-2); + } } elsif( !$patron->account_locked ) { $patron->update({ login_attempts => $patron->login_attempts + 1 }); } diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index d2fb35a707..02fe8e12ed 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -398,13 +398,16 @@ sub AuthenticatePatron { my $username = $cgi->param('username'); my $password = $cgi->param('password'); my ($status, $cardnumber, $userid) = C4::Auth::checkpw( C4::Context->dbh, $username, $password ); - if ( $status ) { + if ( $status == 1 ) { # Track the login C4::Auth::track_login_daily( $userid ); # Get the borrower my $patron = Koha::Patrons->find( { userid => $userid } ); return { id => $patron->borrowernumber }; } + elsif ( $status == -2 ){ + return { code => 'PasswordExpired' }; + } else { return { code => 'PatronNotFound' }; } diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index 46aa156b1d..ed1973ec5f 100755 --- a/t/db_dependent/Auth.t +++ b/t/db_dependent/Auth.t @@ -10,7 +10,7 @@ use CGI qw ( -utf8 ); use Test::MockObject; use Test::MockModule; use List::MoreUtils qw/all any none/; -use Test::More tests => 15; +use Test::More tests => 16; use Test::Warn; use t::lib::Mocks; use t::lib::TestBuilder; @@ -513,6 +513,22 @@ subtest 'Check value of login_attempts in checkpw' => sub { is( $patron->account_locked, 1, 'Check administrative lockout without pref' ); }; +subtest 'Check value of login_attempts in checkpw' => sub { + plan tests => 2; + + t::lib::Mocks::mock_preference('FailedLoginAttempts', 3); + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + $patron->set_password({ password => '123', skip_validation => 1 }); + + my @test = checkpw( $dbh, $patron->userid, '123', undef, 'opac', 1 ); + is( $test[0], 1, 'Patron authenticated correctly' ); + + $patron->password_expiration_date('2020-01-01')->store; + @test = checkpw( $dbh, $patron->userid, '123', undef, 'opac', 1 ); + is( $test[0], -2, 'Patron returned as expired correctly' ); + +}; + subtest '_timeout_syspref' => sub { plan tests => 6; diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t index 0824f9cdec..65120df27a 100755 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -43,7 +43,7 @@ my $builder = t::lib::TestBuilder->new; subtest 'AuthenticatePatron test' => sub { - plan tests => 16; + plan tests => 18; $schema->storage->txn_begin; @@ -112,6 +112,13 @@ subtest 'AuthenticatePatron test' => sub { is( $reply->{code}, 'PatronNotFound', "non-existing cardnumer/userid - PatronNotFound" ); is( $reply->{id}, undef, "id undef"); + $query->param( 'username', $borrower->{userid} ); + $query->param( 'password', $plain_password ); + $seen_patron->password_expiration_date('2020-01-01')->store; + $reply = C4::ILSDI::Services::AuthenticatePatron($query); + is( $reply->{code}, 'PasswordExpired', "correct credentials, expired password not authenticated" ); + is( $reply->{id}, undef, "id undef"); + $schema->storage->txn_rollback; }; -- 2.39.5