-
Notifications
You must be signed in to change notification settings - Fork 178
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
Docs updates, and a few tweaks to the allow config #286
Conversation
…s used downstream
@@ -264,7 +266,7 @@ def _ensure_allowed_groups_requirements(self, change): | |||
c.LDAPAuthenticator.lookup_dn_search_password = 'secret' | |||
c.LDAPAuthenticator.user_search_base = 'ou=people,dc=wikimedia,dc=org' | |||
c.LDAPAuthenticator.user_attribute = 'uid' | |||
c.LDAPAuthenticator.lookup_dn_user_dn_attribute = 'cn' | |||
c.LDAPAuthenticator.lookup_dn_user_dn_attribute = 'sAMAccountName' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I presume this is meant to be an AD example, and this should never be cn
? Or should it be uid
?
) | ||
username = username[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is it acceptable to return just the first match?
# if all groups are needed (e.g. for manage_groups) | ||
# we should keep fetching membership | ||
break | ||
# Returned in auth_state, so fetch the full list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative, if we don't want to iterate through all groups, is to change the property to is_in_allowed_groups: true|false
More details in commit messages.