Bug 24065: Fail shib login if multiple users matched

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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This commit is contained in:
Nick Clemens 2019-11-19 13:01:56 +00:00 committed by Martin Renvoize
parent 2e310fa7f3
commit d4589500d1
No known key found for this signature in database
GPG Key ID: 422B469130441A0F
2 changed files with 36 additions and 6 deletions

View File

@ -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);

View File

@ -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';