From 7bd313cd11b7b3f6f5d8a3b6e89aff9c25c4167f Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti Date: Fri, 26 Jun 2015 17:58:47 +0300 Subject: [PATCH] Bug 7174 - Authentication rewriting - LDAP Component - WIP --- C4/Auth.pm | 2 +- C4/Auth_with_ldap.pm | 11 ++++++++++- Koha/AuthUtils.pm | 31 +++++++++++++++++++++++++++++-- t/db_dependent/Auth_with_ldap.t | 18 +++++++++--------- 4 files changed, 49 insertions(+), 13 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 51f3f55..e069550 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1728,7 +1728,7 @@ sub checkpw { $type = 'opac' unless $type; if ($ldap) { $debug and print STDERR "## checkpw - checking LDAP\n"; - my ( $retval, $retcard, $retuserid ) = checkpw_ldap(@_); # EXTERNAL AUTH + my ( $retval, $retcard, $retuserid ) = checkpw_ldap($userid, $password); # EXTERNAL AUTH return 0 if $retval == -1; # Incorrect password for LDAP login attempt ($retval) and return ( $retval, $retcard, $retuserid ); } diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm index 58484a2..7f000c5 100644 --- a/C4/Auth_with_ldap.pm +++ b/C4/Auth_with_ldap.pm @@ -103,8 +103,17 @@ sub search_method { return $search; } +=head checkpw_ldap + +@RETURNS Integer, -1 if login failed + , 0 if connection to the LDAP server couldn't be reliably established. + or List of (-1|1|0, $cardnumber, $local_userid); + where $cardnumber is koha.borrowers.cardnumber + $local_userid is the koha.borrowers.userid +=cut + sub checkpw_ldap { - my ($dbh, $userid, $password) = @_; + my ($userid, $password) = @_; my @hosts = split(',', $prefhost); my $db = Net::LDAP->new(\@hosts); unless ( $db ) { diff --git a/Koha/AuthUtils.pm b/Koha/AuthUtils.pm index dbbc6a8..5342c04 100644 --- a/Koha/AuthUtils.pm +++ b/Koha/AuthUtils.pm @@ -142,14 +142,14 @@ sub generate_salt { my $isSuperuser = Koha::AuthUtils::checkKohaPassword($userid, $password); -Checks of the given username and password match anybody in the Koha DB +Checks if the given username and password match anybody in the Koha DB @PARAM1 String, user identifier, either the koha.borrowers.userid, or koha.borrowers.cardnumber @PARAM2 String, clear text password from the authenticating user @RETURN String 'superuser', if user is the Koha DB user (superuser account) undef, if the user is but a man. @THROWS Koha::Exception::LoginFailed, if no matching password was found for all username aliases in Koha. =cut - +TODO:: Should return the Koha::Borrower if login succeeded. sub checkKohaPassword { my ($userid, $password) = @_; @@ -213,6 +213,33 @@ sub checkHash { return $hash eq $stored_hash; } +=head checkLDAPPassword + + +Checks if the given username and password match anybody in the LDAP service +@PARAM1 String, user identifier +@PARAM2 String, clear text password from the authenticating user +@RETURN Boolean, 1 if login succeeded + 0 we couldn't reliably contact the LDAP server so we should + fallback to local Koha Password authentication. +@THROWS Koha::Exception::LoginFailed, if LDAP login failed +=cut + +sub checkLDAPPassword { + my ($userid, $password) = @_; + + #Lazy load dependencies because somebody might never need them. + require C4::Auth_with_ldap; + + my ($retval, $cardnumber, $local_userid) = C4::Auth_with_ldap::checkpw_ldap($userid, $password); # EXTERNAL AUTH + if ($retval == -1) { + Koha::Exception::LoginFailed->throw(error => "LDAP authentication failed for the given username and password"); + } + + return 1 if $retval; + return 0; +} + 1; __END__ diff --git a/t/db_dependent/Auth_with_ldap.t b/t/db_dependent/Auth_with_ldap.t index 4982248..5c648f5 100755 --- a/t/db_dependent/Auth_with_ldap.t +++ b/t/db_dependent/Auth_with_ldap.t @@ -74,7 +74,7 @@ subtest "checkpw_ldap tests" => sub { ## Connection fail tests $desired_connection_result = 'error'; - warning_is { $ret = C4::Auth_with_ldap::checkpw_ldap( $dbh, 'hola', password => 'hey' ) } + warning_is { $ret = C4::Auth_with_ldap::checkpw_ldap( 'hola', password => 'hey' ) } "LDAP connexion failed", "checkpw_ldap prints correct warning if LDAP conexion fails"; is( $ret, 0, "checkpw_ldap returns 0 if LDAP conexion fails"); @@ -96,7 +96,7 @@ subtest "checkpw_ldap tests" => sub { warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( - $dbh, 'hola', password => 'hey' ) } + 'hola', password => 'hey' ) } qr/Anonymous LDAP bind failed: LDAP error #1: error_name/, "checkpw_ldap prints correct warning if LDAP anonymous bind fails"; is( $ret, 0, "checkpw_ldap returns 0 if LDAP anonymous bind fails"); @@ -108,14 +108,14 @@ subtest "checkpw_ldap tests" => sub { $desired_count_result = 0; # user auth problem $non_anonymous_bind_result = 'success'; reload_ldap_module(); - is ( C4::Auth_with_ldap::checkpw_ldap( $dbh, 'hola', password => 'hey' ), + is ( C4::Auth_with_ldap::checkpw_ldap( 'hola', password => 'hey' ), 0, "checkpw_ldap returns 0 if user lookup returns 0"); $non_anonymous_bind_result = 'error'; reload_ldap_module(); warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( - $dbh, 'hola', password => 'hey' ) } + 'hola', password => 'hey' ) } qr/LDAP bind failed as kohauser hola: LDAP error #1: error_name/, "checkpw_ldap prints correct warning if LDAP bind fails"; is ( $ret, -1, "checkpw_ldap returns -1 LDAP bind fails for user (Bug 8148)"); @@ -130,7 +130,7 @@ subtest "checkpw_ldap tests" => sub { reload_ldap_module(); warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( - $dbh, 'hola', password => 'hey' ) } + 'hola', password => 'hey' ) } qr/LDAP bind failed as kohauser hola: LDAP error #1: error_name/, "checkpw_ldap prints correct warning if LDAP bind fails"; is ( $ret, 0, "checkpw_ldap returns 0 LDAP bind fails for user (Bug 12831)"); @@ -150,7 +150,7 @@ subtest "checkpw_ldap tests" => sub { reload_ldap_module(); warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( - $dbh, 'hola', password => 'hey' ) } + 'hola', password => 'hey' ) } qr/LDAP bind failed as ldapuser cn=Manager,dc=metavore,dc=com: LDAP error #1: error_name/, "checkpw_ldap prints correct warning if LDAP bind fails"; is ( $ret, 0, "checkpw_ldap returns 0 if bind fails"); @@ -162,7 +162,7 @@ subtest "checkpw_ldap tests" => sub { reload_ldap_module(); warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( - $dbh, 'hola', password => 'hey' ) } + 'hola', password => 'hey' ) } qr/LDAP Auth rejected : invalid password for user 'hola'. LDAP error #1: error_name/, "checkpw_ldap prints correct warning if LDAP bind fails"; is ( $ret, -1, "checkpw_ldap returns -1 if bind fails (Bug 8148)"); @@ -175,7 +175,7 @@ subtest "checkpw_ldap tests" => sub { reload_ldap_module(); warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( - $dbh, 'hola', password => 'hey' ) } + 'hola', password => 'hey' ) } qr/LDAP bind failed as ldapuser cn=Manager,dc=metavore,dc=com: LDAP error #1: error_name/, "checkpw_ldap prints correct warning if LDAP bind fails"; is ( $ret, 0, "checkpw_ldap returns 0 if bind fails"); @@ -187,7 +187,7 @@ subtest "checkpw_ldap tests" => sub { reload_ldap_module(); warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( - $dbh, 'hola', password => 'hey' ) } + 'hola', password => 'hey' ) } qr/LDAP Auth rejected : invalid password for user 'hola'. LDAP error #1: error_name/, "checkpw_ldap prints correct warning if LDAP bind fails"; is ( $ret, -1, "checkpw_ldap returns -1 if bind fails (Bug 8148)"); -- 1.7.9.5