Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 18880: Fix authentication fallback for external authentications

Browse source 
A regression in commit cfc484b17 / bug #18314 breaks the local
authentication fallback for all external authentications like LDAP, CAS
and Shibboleth.

The regression itself is a logical error as "@return = (0)" is
considered to be "false" when checked with "unless" (line 1814). That's
wrong as "unless" tests the number of elements in a list.

This patch tries to simplify the logic by adding a $passwd_ok and
$check_internal_as_fallback flags to be more verbose and hopefully more
understandable.
The goal here is simply to restore back the same logic as before cfc484b17

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Jonathan Druart 2017-07-11 12:24:36 -03:00
parent 70eafe4484
commit d852b33266
1 changed files with 23 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
C4/Auth.pm
View file

@ -1763,17 +1763,24 @@ sub checkpw {
my @return;
my $patron = Koha::Patrons->find({ userid => $userid });
my $check_internal_as_fallback = 0;
my $passwd_ok = 0;
# Note: checkpw_* routines returns:
# 1 if auth is ok
# 0 if auth is nok
# -1 if user bind failed (LDAP only)
# 2 if DB user is used (internal only)
if ( $patron and $patron->account_locked ) {
@return = (0);
# Nothing to check, account is locked
} elsif ($ldap) {
$debug and print STDERR "## checkpw - checking LDAP\n";
my ( $retval, $retcard, $retuserid ) = checkpw_ldap(@_); # EXTERNAL AUTH
if ( $retval ) {
@return = ( $retval, $retcard, $retuserid );
} else {
@return = (0);
}
$passwd_ok = 1 if $retval == 1;
$check_internal_as_fallback = 1 if $retval == 0;
} elsif ( $cas && $query && $query->param('ticket') ) {
$debug and print STDERR "## checkpw - checking CAS\n";
@ -1784,9 +1791,8 @@ sub checkpw {
my ( $retval, $retcard, $retuserid ) = checkpw_cas( $dbh, $ticket, $query, $type ); # EXTERNAL AUTH
if ( $retval ) {
@return = ( $retval, $retcard, $retuserid );
} else {
@return = (0);
}
$passwd_ok = $retval;
}
# If we are in a shibboleth session (shibboleth is enabled, and a shibboleth match attribute is present)
@ -1805,23 +1811,25 @@ sub checkpw {
my ( $retval, $retcard, $retuserid ) = C4::Auth_with_shibboleth::checkpw_shib($shib_login); # EXTERNAL AUTH
if ( $retval ) {
@return = ( $retval, $retcard, $retuserid );
} else {
@return = (0);
}
$passwd_ok = $retval;
}
} else {
$check_internal_as_fallback = 1;
}
# INTERNAL AUTH
@return = checkpw_internal( $dbh, $userid, $password, $no_set_userenv) unless @return;
if ( $check_internal_as_fallback ) {
@return = checkpw_internal( $dbh, $userid, $password, $no_set_userenv);
$passwd_ok = 1 if $return[0] > 0; # 1 or 2
}
if ( $return[0] == 0 ) {
unless ( $passwd_ok ) { # Auth failed
$patron->update({ login_attempts => $patron->login_attempts + 1 }) if $patron;
} elsif ( $return[0] == 1 ) {
if ( $patron ) {
# FIXME Koha::Object->update should return a Koha::Object to allow chaining
$patron->update({ login_attempts => 0 });
$patron->store;
}
} elsif ( $patron ) {
# FIXME Koha::Object->update should return a Koha::Object to allow chaining
$patron->update({ login_attempts => 0 });
$patron->store;
}
return @return;
}