Bug 18314: Account lockout
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Tue, 21 Mar 2017 21:48:41 +0000 (18:48 -0300)
committerKyle M Hall <kyle@bywatersolutions.com>
Fri, 12 May 2017 14:58:44 +0000 (10:58 -0400)
To prevent brute force attacks on Koha accounts, staff and opac, we need to
implement an account lockout process to Koha.

After a number of failed login attempts a users account would become locked.
The user would then need to use the reset password functionality to send a reset
token to their email account. After a successful password reset the lockout flag
would be removed.

The number of failed login attempts before lockout is configurable using a new
system preference 'FailedLoginAttempts'.

How does it work?
When a patron enter an invalid password, the borrowers.login_attempts value
for this patron is incremented. When this value reach the value of the
pref FailedLoginAttempts, the password comparison is not done and the
authentication is rejected.
This login_attempts field is reset when a patron correctly logs in. When
the account is locked the patron has to reset his/her password using
the OpacResetPassword feature or ask a staff member to generate a new
password.
If the pref is not set (0, or '') the feature is considered as disabled,
but the failed login attempts are stored anyway.

Test plan:
0/ Apply patch and execute the update DB entry
1/ Switch on the feature by setting FailedLoginAttempts to 3
2/ Use an invalid password to login at the staff or OPAC interface
3/ After the third consecutive failures, you will be asked to reset your
password if OpacResetPassword is set, or contact a staff member
4/ Switch on OpacResetPassword and reset your password
5/ Confirm that you are able to login
6/ Play with the different combinations

QA details: The trick happens in C4::Auth::checkpw, to make things clear
I had to create a return value (note the awesome name: @return) and
replace the 3 successives if statements with elsif. Indeed if one of
the condition is reached, it will return inside the given block.

Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

C4/Auth.pm
Koha/Patron.pm
koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt

index 8a4166d..74c2792 100644 (file)
@@ -1209,6 +1209,8 @@ sub checkauth {
         push @inputs, { name => $name, value => $value };
     }
 
+    my $patron = Koha::Patrons->find({ userid => $q_userid }); # Not necessary logged in!
+
     my $LibraryNameTitle = C4::Context->preference("LibraryName");
     $LibraryNameTitle =~ s/<(?:\/?)(?:br|p)\s*(?:\/?)>/ /sgi;
     $LibraryNameTitle =~ s/<(?:[^<>'"]|'(?:[^']*)'|"(?:[^"]*)")*>//sg;
@@ -1258,6 +1260,7 @@ sub checkauth {
         PatronSelfRegistration                => C4::Context->preference("PatronSelfRegistration"),
         PatronSelfRegistrationDefaultCategory => C4::Context->preference("PatronSelfRegistrationDefaultCategory"),
         opac_css_override                     => $ENV{'OPAC_CSS_OVERRIDE'},
+        too_many_login_attempts               => ( $patron and $patron->account_locked ),
     );
 
     $template->param( SCO_login => 1 ) if ( $query->param('sco_user_login') );
@@ -1756,28 +1759,39 @@ sub get_session {
 sub checkpw {
     my ( $dbh, $userid, $password, $query, $type, $no_set_userenv ) = @_;
     $type = 'opac' unless $type;
-    if ($ldap) {
+
+    my @return;
+    my $patron = Koha::Patrons->find({ userid => $userid });
+
+    if ( $patron and $patron->account_locked ) {
+        @return = (0);
+    } elsif ($ldap) {
         $debug and print STDERR "## checkpw - checking LDAP\n";
         my ( $retval, $retcard, $retuserid ) = checkpw_ldap(@_);    # EXTERNAL AUTH
-        return 0 if $retval == -1;                                  # Incorrect password for LDAP login attempt
-        ($retval) and return ( $retval, $retcard, $retuserid );
-    }
+        if ( $retval ) {
+            @return = ( $retval, $retcard, $retuserid );
+        } else {
+            @return = (0);
+        }
 
-    if ( $cas && $query && $query->param('ticket') ) {
+    } elsif ( $cas && $query && $query->param('ticket') ) {
         $debug and print STDERR "## checkpw - checking CAS\n";
 
         # In case of a CAS authentication, we use the ticket instead of the password
         my $ticket = $query->param('ticket');
         $query->delete('ticket');                                   # remove ticket to come back to original URL
         my ( $retval, $retcard, $retuserid ) = checkpw_cas( $dbh, $ticket, $query, $type );    # EXTERNAL AUTH
-        ($retval) and return ( $retval, $retcard, $retuserid );
-        return 0;
+        if ( $retval ) {
+            @return = ( $retval, $retcard, $retuserid );
+        } else {
+            @return = (0);
+        }
     }
 
     # If we are in a shibboleth session (shibboleth is enabled, and a shibboleth match attribute is present)
     # Check for password to asertain whether we want to be testing against shibboleth or another method this
     # time around.
-    if ( $shib && $shib_login && !$password ) {
+    elsif ( $shib && $shib_login && !$password ) {
 
         $debug and print STDERR "## checkpw - checking Shibboleth\n";
 
@@ -1788,13 +1802,23 @@ sub checkpw {
         # Then, we check if it matches a valid koha user
         if ($shib_login) {
             my ( $retval, $retcard, $retuserid ) = C4::Auth_with_shibboleth::checkpw_shib($shib_login);    # EXTERNAL AUTH
-            ($retval) and return ( $retval, $retcard, $retuserid );
-            return 0;
+            if ( $retval ) {
+                @return = ( $retval, $retcard, $retuserid );
+            } else {
+                @return = (0);
+            }
         }
     }
 
     # INTERNAL AUTH
-    return checkpw_internal( $dbh, $userid, $password, $no_set_userenv);
+    @return = checkpw_internal( $dbh, $userid, $password, $no_set_userenv) unless @return;
+
+    if ( $return[0] == 0 ) {
+        $patron->update({ login_attempts => $patron->login_attempts + 1 }) if $patron;
+    } elsif ( $return[1] == 1 ) {
+        $patron->update({ login_attempts => 0 })->store if $patron;
+    }
+    return @return;
 }
 
 sub checkpw_internal {
index b97bceb..860a4b6 100644 (file)
@@ -630,6 +630,24 @@ sub get_enrollable_clubs {
     return wantarray ? $e->as_list : $e;
 }
 
+=head3 account_locked
+
+my $is_locked = $patron->account_locked
+
+Return true if the patron has reach the maximum number of login attempts (see pref FailedLoginAttempts).
+Otherwise return false.
+If the pref is not set (empty string, null or 0), the feature is considered as disabled.
+
+=cut
+
+sub account_locked {
+    my ($self) = @_;
+    my $FailedLoginAttempts = C4::Context->preference('FailedLoginAttempts');
+    return ( $FailedLoginAttempts
+          and $self->login_attempts
+          and $self->login_attempts >= $FailedLoginAttempts )? 1 : 0;
+}
+
 =head3 type
 
 =cut
index c6a050c..7d67165 100644 (file)
@@ -1,3 +1,4 @@
+[% USE Koha %]
 [% USE Branches %]
 [% SET footerjs = 1 %]
 [% INCLUDE 'doc-head-open.inc' %]
@@ -5,7 +6,8 @@
     [% IF ( nopermission ) %]Access denied[% END %]
     [% IF ( timed_out ) %]Session timed out[% END %]
     [% IF ( different_ip ) %]IP address change[% END %]
-    [% IF ( invalid_username_or_password ) %]Invalid username or password[% END %]
+    [% IF too_many_login_attempts %]This account has been locked.
+    [% ELSIF invalid_username_or_password %]Invalid username or password[% END %]
     [% IF ( loginprompt ) %]Log in to Koha[% END %]
 </title>
 [% INCLUDE 'doc-head-close.inc' %]
@@ -37,7 +39,9 @@
 <div id="login_error"><strong>Error: </strong>Autolocation is switched on and you are logging in with an IP address that doesn't match your library. </div>
 [% END %]
 
-[% IF ( invalid_username_or_password ) %]
+[% IF too_many_login_attempts %]
+<div id="login_error"><strong>Error: </strong>This account has been locked!</div>
+[% ELSIF invalid_username_or_password %]
 <div id="login_error"><strong>Error: </strong>Invalid username or password</div>
 [% END %]
 
index 6d941bf..31cfbc6 100644 (file)
                                 </div>
                             [% END %]
 
-                            [% IF ( invalid_username_or_password ) %]
+
+                            [% IF too_many_login_attempts %]
+                                <div class="alert alert-info">
+                                This account has been locked!
+                                [% IF Koha.Preference('OpacResetPassword') %]
+                                    <a href="/cgi-bin/koha/opac-password-recovery.pl">You must reset your password</a>.
+                                [% ELSE %]
+                                    Please contact a library staff member.
+                                [% END %]
+                                </div>
+                            [% ELSIF invalid_username_or_password %]
                                 <!-- This is what is displayed if user doesnt have permission -->
                                 <div class="alert alert-info">
                                     <p>You entered an incorrect username or password. Please try again! And remember, passwords are case sensitive.</p>