search

kvspb / nginx-auth-ldap

LDAP authentication module for nginx
BSD 2-Clause "Simplified" License
739 stars 253 forks source link

Revert #199 to fix group membership lookups #234

Closed davidjb closed 4 years ago

davidjb commented 4 years ago

This reverts commit bf64cf217abbe79917f9d44a651c2ecbb82ec993, reversing changes made to f022103e31e71e70a4bb64cd4c7792fa3f238b4a.

The PR in #199 contained changes that weren't right - breaking the behaviour of LDAP setups when group_attribute_is_dn on is enabled (eg what this line of code https://github.com/kvspb/nginx-auth-ldap/commit/bf64cf217abbe79917f9d44a651c2ecbb82ec993#diff-c05c0daefb48996cbf510b81002b49bcR2230 is conditionally targeting). The PR in effect changes the subsequent LDAP query (eg user_val) from correctly looking up the user's DN as a group attribute in LDAP to querying for groups with a group_attribute equalling the group's DN (eg ctx->dn is the group's dn at this point, see here). This isn't right and won't work, hence the breaking change.

This PR reverts the previous change to make this work correctly again.

Fwiw, the originally-referenced issue #180 in PR #199 seems to be a completely different issue, relating to escaping and parentheses so I'm not sure what/how the original author was trying to fix.

mmguero commented 4 years ago

I've been pulling my hair out trying to figure out why group membership lookups which have worked for a long time are suddenly broken for me. I'm going to test reverting this commit in my branch. Thank you.