yammer / dropwizard-auth-ldap

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

authenticateAndReturnPermittedGroups and authenticate treat "RestrictedGroups" differently #7

Closed maheshkelkar closed 9 years ago

maheshkelkar commented 9 years ago

In LdapAuthenticator::authenticate() - if restrictedGroups is not configured, then intersection of user groups with restrcitedGroups is skipped. As a result user is authenticated successfully.

But, LdapAuthenticator:authenticateAndReturnPermittedGroups - if restrictedGroups is not configured, then intersection is still enforced and resultantly authentication is denied.

IMO, the behavior doesn't match and should be changed from:

   private Set<String> getGroupMembershipsIntersectingWithRestrictedGroups(
   ::
                if (configuration.getRestrictToGroups().contains(group)) {
                        overlappingGroups.add(group);
                }

to:

   private Set<String> getGroupMembershipsIntersectingWithRestrictedGroups(
   ::
                if (configuration.getRestrictToGroups().isEmpty() ||
                            configuration.getRestrictToGroups().contains(group)) {
                        overlappingGroups.add(group);
                }
chrisgray commented 9 years ago

I agree, this makes sense. I'll make this change in the following release.