Attachment 95648 Details for Bug 24065 – Bug 24065: Fail shib login if multiple users matched

[patch] Bug 24065: Fail shib login if multiple users matched

Bug-24065-Fail-shib-login-if-multiple-users-matche.patch (text/plain), 3.87 KB, created by Marcel de Rooy on 2019-11-21 08:14:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2019-11-21 08:14:40 UTC

Size: 3.87 KB

patch

obsolete

>From 9d4eebb25ddb2efdde86b13c55d11cba3a648d10 Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Tue, 19 Nov 2019 13:01:56 +0000
>Subject: [PATCH] Bug 24065: Fail shib login if multiple users matched
>Content-Type: text/plain; charset=utf-8
>
>Ideally you could test against active shib, but is a small code change and
>covered by tests and should be readable
>
>To test:
>prove -v t/Auth_with_shibboleth.t
>
>Signed-off-by: Liz Rea <wizzyrea@gmail.com>
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> C4/Auth_with_shibboleth.pm | 11 ++++++++---
> t/Auth_with_shibboleth.t   | 31 ++++++++++++++++++++++++++++---
> 2 files changed, 36 insertions(+), 6 deletions(-)
>
>diff --git a/C4/Auth_with_shibboleth.pm b/C4/Auth_with_shibboleth.pm
>index 55cee73569..0c493f14b4 100644
>--- a/C4/Auth_with_shibboleth.pm
>+++ b/C4/Auth_with_shibboleth.pm
>@@ -103,9 +103,14 @@ sub checkpw_shib {
>     $debug and warn "User Shibboleth-authenticated as: $match";
> 
>     # Does the given shibboleth attribute value ($match) match a valid koha user ?
>-    my $borrower =
>-      Koha::Database->new()->schema()->resultset('Borrower')
>-      ->find( { $config->{matchpoint} => $match } );
>+    my $borrowers = Koha::Patrons->search( { $config->{matchpoint} => $match } );
>+    if ( $borrowers->count > 1 ){
>+        # If we have more than 1 borrower the matchpoint is not unique
>+        # we cannot know which patron is the correct one, so we should fail
>+         $debug and warn "There are several users with $config->{matchpoint} of $match, matchpoints must be unique";
>+        return 0;
>+    }
>+    my $borrower = $borrowers->next;
>     if ( defined($borrower) ) {
>         if ($config->{'sync'}) {
>             _sync($borrower->borrowernumber, $config, $match);
>diff --git a/t/Auth_with_shibboleth.t b/t/Auth_with_shibboleth.t
>index 9e95adf0e1..817394c883 100644
>--- a/t/Auth_with_shibboleth.t
>+++ b/t/Auth_with_shibboleth.t
>@@ -166,7 +166,7 @@ subtest "get_login_shib tests" => sub {
> 
> ## checkpw_shib
> subtest "checkpw_shib tests" => sub {
>-    plan tests => 21;
>+    plan tests => 24;
> 
>     my $shib_login;
>     my ( $retval, $retcard, $retuserid );
>@@ -174,8 +174,10 @@ subtest "checkpw_shib tests" => sub {
>     # Setup Mock Database Data
>     fixtures_ok [
>         'Borrower' => [
>-            [qw/cardnumber userid surname address city/],
>-            [qw/testcardnumber test1234 renvoize myaddress johnston/],
>+            [qw/cardnumber userid surname address city email/],
>+            [qw/testcardnumber test1234 renvoize myaddress johnston  /],
>+            [qw/testcardnumber1 test12345 clamp1 myaddress quechee kid@clamp.io/],
>+            [qw/testcardnumber2 test123456 clamp2 myaddress quechee kid@clamp.io/],
>         ],
>         'Category' => [ [qw/categorycode default_privacy/], [qw/S never/], ]
>       ],
>@@ -202,6 +204,29 @@ subtest "checkpw_shib tests" => sub {
>     [], "bad user with no debug";
>     is( $retval, "0", "user not authenticated" );
> 
>+    # duplicated matchpoint
>+    $matchpoint = 'email';
>+    $mapping{'email'} = { is => 'email' };
>+    $shib_login = 'kid@clamp.io';
>+    warnings_are {
>+        ( $retval, $retcard, $retuserid ) = checkpw_shib($shib_login);
>+    }
>+    [], "bad user with no debug";
>+    is( $retval, "0", "user not authenticated if duplicated matchpoint" );
>+    $C4::Auth_with_shibboleth::debug = '1';
>+    warnings_are {
>+        ( $retval, $retcard, $retuserid ) = checkpw_shib($shib_login);
>+    }
>+    [
>+        q/checkpw_shib/,
>+        q/koha borrower field to match: email/,
>+        q/shibboleth attribute to match: email/,
>+        q/User Shibboleth-authenticated as: kid@clamp.io/,
>+        q/There are several users with email of kid@clamp.io, matchpoints must be unique/
>+    ], "duplicated matchpoint warned with debug";
>+    $C4::Auth_with_shibboleth::debug = '0';
>+    reset_config();
>+
>     # autocreate user
>     $autocreate  = 1;
>     $shib_login  = 'test4321';
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 24065: 95546 | 95562 | 95648