Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDAP auth group filtering: support for $userdn #676

Merged
merged 1 commit into from Apr 20, 2017

Conversation

epol
Copy link
Contributor

@epol epol commented Mar 29, 2017

In the LDAP authentication module the group filtering can use the $userdn variable that contains the user DN.
This feature is useful in a Active Directory environment where the groups contain the user DNs as members.

In the LDAP authentication module the group filtering can use the $userdn variable that contains the user DN.
This feature is useful in a Active Directory environment where the groups contain the user DNs as members.
@Shnoulle
Copy link
Collaborator

And next time ? We add more and more var ?

Personally : i don't want to improve LDAPAuth like ths because

  1. It's not a global system Allow any filter
  2. LDAPAuth can be easily cloned and update for each situation purpose.

@epol
Copy link
Contributor Author

epol commented Mar 30, 2017

Hi Denis and thank you for your reply.

tl;dr: I think that many users are using Active Directory as the authentication and authorization method, so merging this edit would make their life easier configuring LimeSurvey.

Long answer
I agree with you that adding a lot of vars to cover every possible use would be clumsy and potentially harmful on the performance point of view (you may have to do more LDAP queries).
But, on the other hand, I think that my extension covers enough use cases to deserve some thinking. Indeed I think that there are a lot of users using Active Directory as their LDAP database and they may want to use groups (where members are pointed using the DN); adding the $userdn attribute doesn't add any additional LDAP query (the variable is already available from a previous query), so the effect on performances should be negligible.

@LouisGac LouisGac merged commit fe8e8b8 into LimeSurvey:master Apr 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants