Ref T3208. I'm going to make some people try this before it gets anywhere near landing.
Details
- Reviewers
btrahan - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- rPHUf3eca1026279: Allow multiple LDAP search filters, and complex search queries
oh man LDAP
Diff Detail
- Repository
- rPHU libphutil
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Turns out this actually killed our LDAP integration, we just didn't spot it until someone nuked their session and tried to log in again.
For whatever reason, we can only bind to our LDAP with either a fully anonymous bind (ldap_bind($conn); or simply not running it) or a completely authenticated user's DN (ldap_bind($conn, 'cn=Eric Stern,ou=People,dc=wepay,dc=com', 'mypassword');).
This seems to now try binding to ldap with the search query, e.g. ldap_bind($conn, 'uid=eric,ou=People,dc=wepay,dc=com', 'mypassword'); which is rejected by the server. Looking through, this is due to the removal of the $this->searchFirst check in loadLDAPUserData()
I've hacked it into working again by hardcoding my own credentials into the "anonymous" user, since that now appears to be the only way to attempt searching before attempting to bind with the user in the form, but that seems pretty terrible (my password is available in plaintext on the page's source code, and who knows where else). A "search before binding" option (in comparison to the anon user, which forces a bind during establishConnection) would solve the problem for us, although I don't know if it's an ideal solution either.
Thoughts? I'm happy to implement something myself if you have some guidance on a direction to go with it. Alternately, more info on what an anonymous username/password is actually supposed to be couldn't hurt either.
That sounds similar to this, if I'm reading it right?
https://secure.phabricator.com/T3208#29
Let me take a crack at this, and you can let me know if the fix works on your install?
The number of tickets you know off the top of your head never ceases to impress. Yeah, that's exactly it. I'll happily test a patch on our install. Thanks!