From 9047dfce55c325b495ae883d7a41721706f22ef3 Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
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 <andrew@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
---
 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