Bug 20879 - Shibboleth in combination with LDAP as an alternative no longer works
Summary: Shibboleth in combination with LDAP as an alternative no longer works
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: Main
Hardware: PC Linux
: P1 - high major (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
: 20943 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-06-04 20:06 UTC by cking
Modified: 2019-10-14 19:57 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20879: Fix regression in shibboleth when ldap enabled (813 bytes, patch)
2018-06-21 07:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20879: Fix regression in shibboleth when ldap enabled (862 bytes, patch)
2018-06-21 10:10 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description cking 2018-06-04 20:06:32 UTC
Shibboleth authentication was working on 16.05.17, but after upgrading to 17.11.06, the OPAC returns the "Sorry, your Shibboleth identity does not match a valid library identity.  If you have a local login..."

koha-conf:
<useshibboleth>1</useshibboleth>
 <shibboleth>
   <matchpoint>cardnumber</matchpoint>
   <mapping>
      <cardnumber is="employeeNumber"></cardnumber>
   </mapping>
 </shibboleth>


Relevant attribute from /Shibboleth.sso/Session output:
Attributes
employeeNumber: 5000120216


MySQL output:
mysql> select cardnumber,borrowernumber from borrowers where cardnumber='5000120216';
+------------+----------------+
| cardnumber | borrowernumber |
+------------+----------------+
| 5000120216 |          12046 |
+------------+----------------+
1 row in set (0.00 sec)
Comment 1 Martin Renvoize 2018-06-05 12:16:19 UTC
I'm currently unable to replicate this.. could you let me know if you're using Plack or not to help narrow it down a bit.
Comment 2 Martin Renvoize 2018-06-05 12:17:38 UTC
It may also be worth enabling debugging (SetEnv DEBUG=1) and see what pops out in the logs.
Comment 3 cking 2018-06-05 15:19:14 UTC
I get the following in the opac-error log now with $debug=1.
The uninitialized value on line 86 appears to relate to this line in get_login_shib:
 $debug and warn $matchAttribute . " value: " . $ENV{$matchAttribute};

I don't believe we are using Plack, but I'm not familiar with it either.

[Tue Jun 05 09:44:25.029248 2018] [cgi:error] [pid 189874] [client 129.114.255.234:51435] AH01215: [Tue Jun  5 09:44:25 2018] Auth.pm: koha borrower field to match: cardnumber at /usr/share/koha/lib/C4/Auth_with_shibboleth.pm line 166, <DATA> line 755.: /usr/share/koha/opac/cgi-bin/opac/opac-search.pl
[Tue Jun 05 09:44:25.029403 2018] [cgi:error] [pid 189874] [client 129.114.255.234:51435] AH01215: [Tue Jun  5 09:44:25 2018] Auth.pm: shibboleth attribute to match: employeeNumber at /usr/share/koha/lib/C4/Auth_with_shibboleth.pm line 168, <DATA> line 755.: /usr/share/koha/opac/cgi-bin/opac/opac-search.pl
[Tue Jun 05 09:44:25.029520 2018] [cgi:error] [pid 189874] [client 129.114.255.234:51435] AH01215: [Tue Jun  5 09:44:25 2018] Auth.pm: koha borrower field to match: cardnumber at /usr/share/koha/lib/C4/Auth_with_shibboleth.pm line 166, <DATA> line 755.: /usr/share/koha/opac/cgi-bin/opac/opac-search.pl
[Tue Jun 05 09:44:25.029638 2018] [cgi:error] [pid 189874] [client 129.114.255.234:51435] AH01215: [Tue Jun  5 09:44:25 2018] Auth.pm: shibboleth attribute to match: employeeNumber at /usr/share/koha/lib/C4/Auth_with_shibboleth.pm line 168, <DATA> line 755.: /usr/share/koha/opac/cgi-bin/opac/opac-search.pl
[Tue Jun 05 09:44:25.029764 2018] [cgi:error] [pid 189874] [client 129.114.255.234:51435] AH01215: [Tue Jun  5 09:44:25 2018] Auth.pm: Use of uninitialized value in concatenation (.) or string at /usr/share/koha/lib/C4/Auth_with_shibboleth.pm line 86, <DATA> line 755.: /usr/share/koha/opac/cgi-bin/opac/opac-search.pl
[Tue Jun 05 09:44:25.029866 2018] [cgi:error] [pid 189874] [client 129.114.255.234:51435] AH01215: [Tue Jun  5 09:44:25 2018] Auth.pm: employeeNumber value:  at /usr/share/koha/lib/C4/Auth_with_shibboleth.pm line 86, <DATA> line 755.: /usr/share/koha/opac/cgi-bin/opac/opac-search.pl
[Tue Jun 05 09:44:25.092226 2018] [cgi:error] [pid 189874] [client 129.114.255.234:51435] AH01215: C4::Tags external dictionary = none: /usr/share/koha/opac/cgi-bin/opac/opac-search.pl
[Tue Jun 05 09:44:25.215447 2018] [cgi:error] [pid 189874] [client 129.114.255.234:51435] AH01215: [Tue Jun  5 09:44:25 2018] opac-search.pl: Use of uninitialized value in concatenation (.) or string at /usr/share/koha/lib/C4/Auth.pm line 550, <DATA> line 755.: /usr/share/koha/opac/cgi-bin/opac/opac-search.pl
Comment 4 Jonathan Druart 2018-06-05 15:24:52 UTC
I guess it comes from:
Auth.pm: employeeNumber value:  at /usr/share/koha/lib/C4/Auth_with_shibboleth.pm line 86

 85     $debug and warn $matchAttribute . " value: " . $ENV{$matchAttribute};

See bug 17776.
Comment 5 Jonathan Druart 2018-06-05 15:26:55 UTC
(In reply to cking from comment #3)
> I get the following in the opac-error log now with $debug=1.
> The uninitialized value on line 86 appears to relate to this line in
> get_login_shib:
>  $debug and warn $matchAttribute . " value: " . $ENV{$matchAttribute};
> 
> I don't believe we are using Plack, but I'm not familiar with it either.

I read to fast.
To know if you are using plack, go to the about page, tab "Server information", line "PSGI".
Comment 6 cking 2018-06-05 15:38:24 UTC
(In reply to Jonathan Druart from comment #5)
> (In reply to cking from comment #3)
> > I get the following in the opac-error log now with $debug=1.
> > The uninitialized value on line 86 appears to relate to this line in
> > get_login_shib:
> >  $debug and warn $matchAttribute . " value: " . $ENV{$matchAttribute};
> > 
> > I don't believe we are using Plack, but I'm not familiar with it either.
> 
> I read to fast.
> To know if you are using plack, go to the about page, tab "Server
> information", line "PSGI".

Ah, thanks.  No mention of PSGI in the About screen.
Comment 7 Martin Renvoize 2018-06-05 16:31:58 UTC
Well the fact that $ENV{$matchAttribute} is undefined is the root cause of the problem. I've never seen that happen outside of the plack context though so I'm not really sure what to advise.

Are you sure your SP is running and passing attributes through to the vhost in apache?
Comment 8 cking 2018-06-05 20:31:11 UTC
I must have copied the wrong log entries from the wrong timeframe.  Upon further testing, the value is actually present.  Could LDAP being enabled throw off the auth process for Shibboleth?
Comment 9 Martin Renvoize 2018-06-06 06:12:40 UTC
LDAP shouldn't throw off Shibboleth.. the code flow is to check for the shibboleth session first and only after testing for that fallback to the other authentication mechanisms.
Comment 11 cking 2018-06-06 16:06:28 UTC
(In reply to Martin Renvoize from comment #10)
> Are you seeing this line triggered in your logs at all?
> 
> http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/
> Auth_with_shibboleth.pm;h=6b20511df5854a666ddde4a15b72643b9ff25556;
> hb=5f846a33abe89599fa7b9a37cb3c00632d30ec90#l111

I am not seeing that line in the log.
Comment 12 cking 2018-06-20 20:52:00 UTC
From what I'm seeing in the logs:
1. It starts shibboleth validation (https://github.com/Koha-Community/Koha/blob/7f2ffda09f04adf0cfe244b48a09fb130752f330/C4/Auth.pm#L946) with $shib = 1 and $shib_login = 5000120216
2. Within the call to checkpw, it first hits elseif ($ldap) and userid and password are empty (https://github.com/Koha-Community/Koha/blob/7f2ffda09f04adf0cfe244b48a09fb130752f330/C4/Auth.pm#L1776)
3. Builds the principal_name as [empty userid]@domain.edu (https://github.com/Koha-Community/Koha/blob/7f2ffda09f04adf0cfe244b48a09fb130752f330/C4/Auth_with_ldap.pm#L146)
4. Fails LDAP bind with invalid principal name and empty password (https://github.com/Koha-Community/Koha/blob/7f2ffda09f04adf0cfe244b48a09fb130752f330/C4/Auth_with_ldap.pm#L169) with "LDAP error #48: LDAP_INAPPROPRIATE_AUTH The server requires the client which had attempted to bind anonymously or without supplying credentials to provide some form of credentials"
5. Back in checkpw (https://github.com/Koha-Community/Koha/blob/7f2ffda09f04adf0cfe244b48a09fb130752f330/C4/Auth.pm#L1778), retval is 0.
6. I do not make it into this elseif block (https://github.com/Koha-Community/Koha/blob/7f2ffda09f04adf0cfe244b48a09fb130752f330/C4/Auth.pm#L1801)
7. The next error in the log is an uninitialized value in subroutine entry at https://github.com/Koha-Community/Koha/blob/7f2ffda09f04adf0cfe244b48a09fb130752f330/C4/Auth.pm#L1911
8. And then https://github.com/Koha-Community/Koha/blob/7f2ffda09f04adf0cfe244b48a09fb130752f330/C4/Auth.pm#L1159
Comment 13 Martin Renvoize 2018-06-21 07:28:56 UTC
Created attachment 76220 [details] [review]
Bug 20879: Fix regression in shibboleth when ldap enabled
Comment 14 Martin Renvoize 2018-06-21 07:29:50 UTC
Very well sleuthed!

Could you try out the patch attached and let me know if it resolves the problem for you?
Comment 15 Martin Renvoize 2018-06-21 10:04:59 UTC
*** Bug 20943 has been marked as a duplicate of this bug. ***
Comment 16 Martin Renvoize 2018-06-21 10:06:39 UTC
I think this may well also affect CAS with LDAP as an alternative.. I've notified Matthias to take a look.
Comment 17 Nick Clemens 2018-06-21 10:09:16 UTC
Affects 17.05 as well
Comment 18 Nick Clemens 2018-06-21 10:10:19 UTC
Created attachment 76226 [details] [review]
Bug 20879: Fix regression in shibboleth when ldap enabled

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 19 cking 2018-06-21 18:40:52 UTC
Applied the patch and tested LDAP and Shibboleth logins, logoffs, etc. All seem to be working as expected now.  Thanks!
Comment 20 Katrin Fischer 2018-06-22 05:46:15 UTC
As LDAP/Shibboleth are so hard to test I suggest counting cking's test as a sign-off and Nick for QA. Would that be agreeable?
Comment 21 Jonathan Druart 2018-06-22 13:16:54 UTC
(In reply to Katrin Fischer from comment #20)
> As LDAP/Shibboleth are so hard to test I suggest counting cking's test as a
> sign-off and Nick for QA. Would that be agreeable?

Yes please :)
Comment 22 Nick Clemens 2018-06-22 22:04:38 UTC
Awesome work all!

Pushed to master for 18.11.x
Comment 23 Martin Renvoize 2018-06-27 10:36:00 UTC
Pushed to 18.05.x for 18.05.02
Comment 24 Fridolin Somers 2018-06-28 08:46:07 UTC
Pushed to 17.11.x for 17.11.08
Comment 25 Fridolin Somers 2018-09-24 13:01:50 UTC
Pushed to 17.05.x for 17.05.14