yammer / dropwizard-auth-ldap

Dropwizard Authentication Module for LDAP using JNDI.
Apache License 2.0
36 stars 18 forks source link

restrictedToGroups not working #16

Closed odrewien closed 7 years ago

odrewien commented 7 years ago

If my ldap configuration contains restrictedToGroups data, the method "authentication" is not working. I debug the code and found in method filterByGroup this statement:

final String filter = String.format("(&(%s=%s)(|%s))", configuration.getGroupMembershipAttribute(), sanitizedUsername, groupFilter.toString());

This statement I changed to:

    final String filter = String.format("(&(%s=%s=%s,%s)(|%s))", configuration.getGroupMembershipAttribute(), configuration.getUserNameAttribute(), sanitizedUsername, configuration.getUserFilter(), groupFilter.toString());

Now it is working on my ldap (or maybe my ldap knowledge is insufficient).

chrisgray commented 7 years ago

Hmmm interesting. Are you sure that in the original query your GroupMembershipAttributes are correct? By default it's memberUid and the groupFilter is ou=groups,dc=example,dc=com if you configure those with what you're substituting with the userNameAttribute and userFilter does that work?

odrewien commented 7 years ago

Hi Christopher,

thx to have a look at this issue. I will try to explain this in detail....

I have exported my ldap data (see below):

dn: ou=groups,dc=odrewien,dc=de objectclass: organizationalUnit objectclass: top ou: groups

dn: cn=admin,ou=groups,dc=odrewien,dc=de cn: admin description: admin group member: uid=odrewien,ou=users,dc=odrewien,dc=de objectclass: groupOfNames objectclass: top

dn: cn=user,ou=groups,dc=odrewien,dc=de cn: user description: user group member: uid=odrewien,ou=users,dc=odrewien,dc=de objectclass: groupOfNames objectclass: top

dn: ou=users,dc=odrewien,dc=de objectclass: organizationalUnit objectclass: top ou: users

dn: uid=odrewien,ou=users,dc=odrewien,dc=de cn: Oliver Drewien givenname: Oliver objectclass: person objectclass: inetOrgPerson objectclass: organizationalPerson objectclass: top sn: Drewien uid: odrewien

My ldap configuration data are this:

ldapConfiguration: uri: ldap://myldap.de:389 cachePolicy: maximumSize=100, expireAfterWrite=1m userFilter: ou=users,dc=odrewien,dc=de groupFilter: ou=groups,dc=odrewien,dc=de userNameAttribute: uid groupNameAttribute: cn groupMembershipAttribute: member groupClassName: groupOfNames restrictToGroups:

When running the unchanged code the filter (final String filter = String.format("(&(%s=%s)(|%s))", configuration.getGroupMembershipAttribute(), sanitizedUsername, groupFilter.toString()); ) is like this: groupFilter: ou=groups,dc=odrewien,dc=de (see above) filter: (&(member=odrewien)(|(cn=admin)(cn=user))

This filter do NOT work. I assume the filter has to be: filter: (&(member=uid=odrewien,ou=users,dc=odrewien,dc=de)(|(cn=admin)(cn=user))

This filter is working.

Do I have made a mistake?

Regards

Oliver

On 08.02.2017 20:14, Christopher Gray wrote:

Hmmm interesting. Are you sure that in the original query your GroupMembershipAttributes are correct? By default it's memberUid https://github.com/yammer/dropwizard-auth-ldap/blob/master/src/main/java/com/yammer/dropwizard/authenticator/LdapConfiguration.java#L41 and the groupFilter is ou=groups,dc=example,dc=com https://github.com/yammer/dropwizard-auth-ldap/blob/master/src/main/java/com/yammer/dropwizard/authenticator/LdapConfiguration.java#L29 if you configure those with what you're substituting with the |userNameAttribute| and |userFilter| does that work?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/yammer/dropwizard-auth-ldap/issues/16#issuecomment-278431231, or mute the thread https://github.com/notifications/unsubscribe-auth/AXKi4VwIgR-xUtyKN9_I0ItWj0EufiPmks5rahQVgaJpZM4L6gC0.

chrisgray commented 7 years ago

Ah, no it looks like you're correct. It's because when you add members to your groups you have it added as:

member: uid=odrewien,ou=users,dc=odrewien,dc=de

so the changes you've done correctly let you search for that. The current setup would work if instead membership in groups was done like this:

member: odrewien

LDAP is hard because it's so configurable and this code assumes a particular layout which isn't compatible with yours. We'd have to refactor the code a bit to allow for an injectable sanitizedUsername into the groupMembershipAttribute=X where X would take the injectable sanitizedUsername, I think.

chrisgray commented 7 years ago

Closing to inactivity