SEC-1641: DefaultLdapAuthoritiesPopulator does not accept groupSearchBase as null #1882

Closed
spring-issuemaster opened this Issue Dec 17, 2010 · 3 comments

1 participant

@spring-issuemaster

Michael Grove (Migrated from SEC-1641) said:

The DefaultLdapAuthoritiesPopulator Javadocs indicate a null groupSearchBase means that no searching will be performed. The implementation code, in particular getGroupMembershipRoles(), looks like it would handle that just fine, except there is an Assert.notNull in the groupSearchBase setter function. Being able to set the groupSearchBase to null is handy in situations where you don't really want to search but want to assign a defaultRole to all users. If the Assert goes away, one could do that very nicely with this class.

@spring-issuemaster

Luke Taylor said:

I've inlined the setter method into the constructor, removing the null check. In 3.1 the preferred approach for creating a default role, converting case, adding prefixes etc. will be to use a GrantedAuthoritiesMapper, so the equivalent functinality in other classes wil be deprecated. I've updated the DefaultLdapAuthoritiesPopulator to indicate this.

@spring-issuemaster

Michael Grove said:

Thanks Luke. I think I see a problem though in the update. Won't the below result in a NPE if groupSearchBase is null?

  this.groupSearchBase = groupSearchBase;
   if (groupSearchBase.length() == 0) {
       logger.info("groupSearchBase is empty. Searches will be performed from the context source base");
   }
@spring-issuemaster

Luke Taylor said:

Well spotted. That's what happens when you don't ensure a test fails before it passes :). It also needs to return a mutable empty set in the case of a null search base. I've committed a fix that hopefully works this time.

@spring-issuemaster spring-issuemaster added this to the 3.1.0.M2 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment