Bugzilla – Attachment 65001 Details for
Bug 18880
Regression breaks local authentication fallback for all external authentications
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[ALTERNATIVE-PATCH] Bug 18880: Fix authentication fallback for external authentications
Bug-18880-Fix-authentication-fallback-for-external.patch (text/plain), 3.86 KB, created by
Jonathan Druart
on 2017-07-11 15:35:36 UTC
(
hide
)
Description:
[ALTERNATIVE-PATCH] Bug 18880: Fix authentication fallback for external authentications
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2017-07-11 15:35:36 UTC
Size:
3.86 KB
patch
obsolete
>From 693531586131b3235cf24557dd452a056fcc49d4 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Tue, 11 Jul 2017 12:24:36 -0300 >Subject: [PATCH] Bug 18880: Fix authentication fallback for external > authentications > >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. Thus the >"falsy" condition has to established with "@return = ()" instead. > >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 >--- > C4/Auth.pm | 38 +++++++++++++++++++++++--------------- > 1 file changed, 23 insertions(+), 15 deletions(-) > >diff --git a/C4/Auth.pm b/C4/Auth.pm >index b1e7824d4f..54787877d1 100644 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -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 == -1; > > } 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; > } >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18880
:
64719
|
64862
|
64877
|
64909
|
65000
|
65001
|
65011
|
65021
|
65037
|
65038
|
65053
|
65054