Page MenuHomePhabricator

Allow multiple LDAP search filters, and complex search queries
ClosedPublic

Authored by epriestley on Feb 7 2014, 12:05 AM.
Tags
None
Referenced Files
F14057127: D8159.diff
Sun, Nov 17, 12:41 AM
F14045630: D8159.diff
Wed, Nov 13, 7:39 AM
F14011529: D8159.id20304.diff
Fri, Nov 1, 2:43 AM
F14009726: D8159.id20304.diff
Wed, Oct 30, 11:09 PM
F14008433: D8159.id20304.diff
Tue, Oct 29, 9:17 PM
F14007424: D8159.id20304.diff
Tue, Oct 29, 5:36 AM
F14003466: D8159.id20306.diff
Sat, Oct 26, 6:27 AM
F14002733: D8159.id20304.diff
Fri, Oct 25, 9:47 PM
Tokens
"Baby Tequila" token, awarded by frgtn.

Details

Reviewers
btrahan
Maniphest Tasks
Restricted Maniphest Task
Commits
rPHUf3eca1026279: Allow multiple LDAP search filters, and complex search queries
Summary

Ref T3208. I'm going to make some people try this before it gets anywhere near landing.

Test Plan

oh man LDAP

Diff Detail

Repository
rPHU libphutil
Branch
ldapfilter
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

Not ready to move forward without more testing in the wild.

This looks solid, looking forward to seeing this on master.

epriestley retitled this revision from [Draft] Allow multiple LDAP search filters, and complex search queries to Allow multiple LDAP search filters, and complex search queries.
epriestley edited edge metadata.

Seems to be working in the wild.

btrahan edited edge metadata.
This revision is now accepted and ready to land.Mar 17 2014, 10:04 PM
epriestley updated this revision to Diff 20306.

Closed by commit rPHUf3eca1026279 (authored by @epriestley).

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!