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

Support for full DN of the user in the member field when used with groupOfNames object class #5

Merged
merged 10 commits into from
Sep 30, 2016

Conversation

d-lorenc
Copy link
Contributor

@d-lorenc d-lorenc commented Jan 3, 2015

I found out that the current implementation seems to assume that groups are defined using posixGroup/memberUID, where memberUID is a short unique user ID. As I'm using groupOfNames for groups in LDAP I had to make it work with a member field which has to be defined as a full DN of the user. It's a small change, and others might find it useful too, so the pull request. Sorry for no unit tests, but your current structure didn't allow me to test it - that's my excuse :) However I've tested that change in my project in more end2end scenario and all my functional tests are green now.

@@ -84,6 +87,14 @@ private boolean filterByGroup(InitialDirContext context, String sanitizedUsernam
}
}

private String userNameBaseOnGroupClass(String userName) {
if ("groupOfNames".equalsIgnoreCase(configuration.getGroupClassName())
&& "member".equalsIgnoreCase(configuration.getGroupMembershipAttribute())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"groupOfNames" and "member" seem pretty specific to your LDAP setup use case. Other setups can use other fields for their member identity :(

@chrisgray
Copy link
Contributor

Maybe we can add in the ability to switch the fullDN based on a configuration parameter rather than magical group member fields?

@chrisgray chrisgray merged commit df27372 into yammer:master Sep 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants