Bug 18947

Summary: Unexpected Active Directory LDAP authentication failure mode
Product: Koha Reporter: Sven Coenye <helpdesk>
Component: AuthenticationAssignee: Nick Clemens <nick>
Status: Pushed to Stable --- QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: alex.arnaud, bob, dpavlin, fridolin.somers, jonathan.druart, katrin.fischer, liz, martin.renvoize, mtj, nick
Version: master   
Hardware: All   
OS: Linux   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979
Whiteboard:
Change sponsored?: --- Patch complexity: ---
Bot Control: --- When did the bot last check this:
Who signed the patch off: Text to go in the release notes:
Bug Depends on: 6979    
Bug Blocks:    
Attachments: Bug 18947 - LDAP: do not assume anonymous bind if no user or password
Bug 18947 - LDAP: do not assume anonymous bind if no user or password
Bug 18947 - LDAP: do not assume anonymous bind if no user or password

Description Sven Coenye 2017-07-17 18:31:19 UTC
Commit 68c365ea8ab536a85d92d3769b0bbaa0e1691116 introduced the following line in Auth_with_ldap.pm:

$ldap->{anonymous_bind} = 1 unless $ldapname && $ldappassword;

There is no comment in the code or the Git commit log as to why.

This change silently overrides the settings in koha-conf.xml and causes a previously functional setup with LDAP authentication against Active Directory to fail unless koha-conf.xml defines values for <user> and <pass>. As user and pass are only used if auth_by_bind and anonymous_bind are both false, their values should not come into play in the other cases.

Prior to this change, KOHA would bind against AD with the user supplied credentials, as expected. With the change, KOHA attempts an anonymous bind and then tries to search for the supplied user name. As there was no valid bind in place, AD rejects the attempt.

Workaround: enter some value for <user> and <pass> in the ldapserver configuration block. These do not have to be real credentials as KOHA will not attempt to use them now that auth_by_bind has priority again.

Note: there is an apparent duplicate attempt to do the same further down, by setting $config{anonymous} to ($ldapname and $ldappassword) ? 0 : 1, although that key is not used anywhere in Auth_by_ldap.pm
Comment 1 Nick Clemens 2017-11-06 14:34:03 UTC
This broke LDAP configuration after upgrade to 17.05, will tyr to provide a patch shortly
Comment 2 Liz Rea 2017-11-30 22:39:25 UTC
Thanks for writing this down Nick, just had this problem and couldn't even get in with the DB admin.
Comment 3 Nick Clemens 2017-12-13 13:38:45 UTC
Created attachment 69777 [details] [review]
Bug 18947 - LDAP: do not assume anonymous bind if no user or password

To test:
Ideally tested on a working ldap server with bind by auth and no
anonymous bind
1  - Define an LDAP config with bind by auth
2  - Don't define user/pass
3  - Define anonymous_bind = 0
4  - Attempt bind by auth
5  - Error is something like:
LDAP search failed to return object : XXXXXXXXX: LdapErr: XXXX-XXXXXX,
     comment: In order to perform this operation a successful bind must
     be completed on the connection., data 0, v2580 at
     /usr/share/koha/lib/C4/Auth_with_ldap.pm line 102.
6  - Define user/pass
7  - Now bind by auth should work
8  - remove user/pass
9  - Apply patch
10 - Attempt again
11 - Bind by auth shoudl succeed

prove -v t/db_dependent/Auth_with_ldap.t
Comment 4 Martin Renvoize 2018-07-12 09:52:21 UTC
Created attachment 76879 [details] [review]
Bug 18947 - LDAP: do not assume anonymous bind if no user or password

To test:
Ideally tested on a working ldap server with bind by auth and no
anonymous bind
1  - Define an LDAP config with bind by auth
2  - Don't define user/pass
3  - Define anonymous_bind = 0
4  - Attempt bind by auth
5  - Error is something like:
LDAP search failed to return object : XXXXXXXXX: LdapErr: XXXX-XXXXXX,
     comment: In order to perform this operation a successful bind must
     be completed on the connection., data 0, v2580 at
     /usr/share/koha/lib/C4/Auth_with_ldap.pm line 102.
6  - Define user/pass
7  - Now bind by auth should work
8  - remove user/pass
9  - Apply patch
10 - Attempt again
11 - Bind by auth shoudl succeed

prove -v t/db_dependent/Auth_with_ldap.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2018-07-12 09:52:57 UTC
Can't believe I hadn't come across this until now!

Signing Off.
Comment 6 Liz Rea 2018-07-13 00:36:31 UTC
I just had to add the extra configs because of this last night lol.
Comment 7 Katrin Fischer 2018-07-13 06:03:56 UTC
Hi Liz, could you sign off so we can count Martin for QA?
Comment 8 Katrin Fischer 2018-07-15 17:55:26 UTC
Is this also valid for master?
Comment 9 Liz Rea 2018-07-15 20:53:41 UTC
I don't have a non-production LDAP to test against :(
Comment 10 Jonathan Druart 2018-08-06 13:25:07 UTC
Alex, could you have a look at this one and QA it?
Comment 11 Brendan Gallagher 2018-08-17 19:47:52 UTC
Created attachment 78001 [details] [review]
Bug 18947 - LDAP: do not assume anonymous bind if no user or password

To test:
Ideally tested on a working ldap server with bind by auth and no
anonymous bind
1  - Define an LDAP config with bind by auth
2  - Don't define user/pass
3  - Define anonymous_bind = 0
4  - Attempt bind by auth
5  - Error is something like:
LDAP search failed to return object : XXXXXXXXX: LdapErr: XXXX-XXXXXX,
     comment: In order to perform this operation a successful bind must
     be completed on the connection., data 0, v2580 at
     /usr/share/koha/lib/C4/Auth_with_ldap.pm line 102.
6  - Define user/pass
7  - Now bind by auth should work
8  - remove user/pass
9  - Apply patch
10 - Attempt again
11 - Bind by auth shoudl succeed

prove -v t/db_dependent/Auth_with_ldap.t

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Comment 12 Katrin Fischer 2018-08-19 11:25:03 UTC
Thx, Brendan. Martin, are you ok with switching to PQA counting yours as QA?
Comment 13 Martin Renvoize 2018-08-20 09:02:49 UTC
Certainly, the code is solid and I'm more than happy for my SO to count as QA :)
Comment 14 Nick Clemens 2018-08-21 13:54:25 UTC
Awesome work all!

Pushed to master for 18.11
Comment 15 Martin Renvoize 2018-08-24 08:58:39 UTC
Pushed to 18.05.x for 18.05.03
Comment 16 Fridolin SOMERS 2018-08-28 12:59:20 UTC
Pushed to 17.11.x for 17.11.09