Spring-security: SEC-1823: User roles in AD should handle nested groups

[spring-issuemaster]

Rick Jensen (Migrated from SEC-1823) said:

With Active Directory (AD), groups can be nested within each other. In fact, in larger organizations, it is quite common to use nested groups to manage users.

The default AD authorities populator only looks at a user's memberOf property to populate the list of granted authorities, using the group name as the authority name. The memberOf property is only populated with groups that the user is directly a member of, and does not include groups that the user is a member of through nesting of groups.

For example, you have the following group structure:

MyApplicationAdmins (group)
Members: DomainAdmins
DomainAdmins (group)
Members: User1
User1 (user)

Currently, the ActiveDirectoryLdapAuthenticationProvider will only populate the user's GrantedAuthorities with DomainAdmins, since that is the only group the user is directly a member of. Instead, the user should have a GrantedAuthorities list that contains both DomainAdmins and MyApplicationAdmins, since the user is in both groups through nesting.

With generic LDAP, there is no way to get nested groups other than by walking the LDAP tree, which requires multiple calls to the LDAP server and is very expensive. With AD however, there is a special matching rule object identifier that will walk a chain of ancestry objects. This page (http://msdn.microsoft.com/en-us/library/aa746475%28VS.85%29.aspx) describes it in more detail.

An example of how to use the filter to get all groups a user is in would be this filter:

(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={0}))

where {0} is the DN of the user.

To enable this, the loadUserAuthorities(...) method of the ActiveDirectoryLdapAuthenticationProvider needs to be modified. Since the full nested group information is not available in the DirContextOperations object, another AD server call is required, but it is only a single call and it fully populates the GrantedAuthorities with all of the groups a user is in, both directly and via nesting.

Here is an example of the updated method: (note: this was put together hastily and likely has issues, but it serves as a concrete starting point)

/**
 * Creates the user authority list from the values of the {@code memberOf} attribute obtained from the user's
 * Active Directory entry in a nested fashion.
 * @throws NamingException 
 */
public Collection<? extends GrantedAuthority> loadUserAuthorities(DirContextOperations userData, String username, String password) throws NamingException {
    SearchControls searchControls = new SearchControls();
    searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);

    String filter = "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={0}))";

    final String bindPrincipal = createBindPrincipal(username, domain);
    String searchRoot = rootDn != null ? rootDn : searchRootFromPrincipal(bindPrincipal);


    DirContext ctx = bindAsUser(username, password);

    final DistinguishedName ctxBaseDn = new DistinguishedName(ctx.getNameInNamespace());

    final DistinguishedName searchBaseDn = new DistinguishedName(searchRoot);

    final NamingEnumeration<SearchResult> resultsEnum = ctx.search(searchBaseDn, filter, new Object[]{userData.getDn()}, searchControls);

    if (logger.isDebugEnabled()) {
        logger.debug("Searching for entry under DN '" + ctxBaseDn
                + "', base = '" + searchBaseDn + "', filter = '" + filter + "'");
    }

    ArrayList<GrantedAuthority> authorities = new ArrayList<GrantedAuthority>();
    try {
        while (resultsEnum.hasMore()) {
            SearchResult searchResult = resultsEnum.next();
            // Work out the DN of the matched entry
            DistinguishedName dn = new DistinguishedName(new CompositeName(searchResult.getName()));

            if (searchRoot.length() > 0) {
                dn.prepend(searchBaseDn);
            }

            if (logger.isDebugEnabled()) {
                logger.debug("Found DN: " + dn);
            }

            authorities.add(new SimpleGrantedAuthority(dn.removeLast().getValue()));
        }
    } catch (PartialResultException e) {
        org.springframework.security.ldap.LdapUtils.closeEnumeration(resultsEnum);
        logger.info("Ignoring PartialResultException");
    }

    return authorities;
}

[spring-issuemaster]

Maxime Noirjean said:

Yeah, I have same problem.
Your code works perfectly but your code should be integrated in "ActiveDirectoryLdapAuthenticationProvider" class.

After more 6 month, this should already be done.

[spring-issuemaster]

Marcel Stör said:

Out customer has the same requirement as far as nested groups are concerned. However, we're not using the AD authentication provider as all requests are pre-authenticated (SSO). We only make calls to AD/LDAP to retrieve user details and authorities using the DefaultLdapAuthoritiesPopulator. Wondering if the approach detailed in this issue can somehow be integrated into DefaultLdapAuthoritiesPopulator...

[spring-issuemaster]

Rick Jensen said:

@Maxime - I haven't had time yet, but I intend to push a patch upstream for this at some point.

@Marcel - Yes, covering the scenario where users have already been authenticated and only need permissions loaded is important. I believe this covers that, but I'm open to suggestions if it doesn't.

[spring-issuemaster]

Marcel Stör said:

@Rick - not sure if we have a misunderstanding or not ;-)
My point was that in our scenario we're not using the ActiveDirectoryLdapAuthenticationProvider that you patched above. We're using our own PreAuthenticationProvider because, much to our surprise, we weren't able to configure what we need with standard SS classes. I outlined the solution here: http://forum.springsource.org/showthread.php?115826-Combine-pre-authentication-with-LDAP-for-user-details-and-authorities. Hence, if I understand your proposal correctly we'd need to patch the DefaultLdapAuthoritiesPopulator to make it work for us (didn't have time yet to really dig into this)?

[spring-issuemaster]

Yann Nicolas said:

This issue has 2 years but I have this exactly same problem. Is it planned to resolve it?
Thanks

[spring-issuemaster]

Benne Otten said:

I've stumbled upon thesame problem as outlined in this issue, but considering this issue has been open for almost three years now I am not hopefull it has been resolved yet and might not be resolved anytime soon.
Does anyone have a workaround for this? Any help is greatly appreciated.

[spring-issuemaster]

Jean-Pierre Bergamin said:

I use this class here: https://gist.github.com/ractive/258dd06c99d2939781c0
Put it in the package org.springframework.security.ldap.authentication.ad and you should be ready to go...

[spring-issuemaster]

Benne Otten said:

@Jean-Pierre, I'm now using the class you mentioned, and it looks promising.
It works, but I haven't had the chance yet to test it thouroughly.

Thanks for the input. ;)

[fpmoles]

what is the status on this issue (and the solutions documented) becoming part of the main line. We too have had to implement a work around for what is normal behavior in AD (nesting groups) and would prefer to use "official" code.

[rwinch]

@fpmoles Thanks for reaching out.

The proposed changes cannot be made in Spring Security because they are not passive. Ideally, we would use a strategy pattern to obtain the roles with a default implementation using the same behavior of the existing method. An additional implementation could then be provided that supports nested groups.

NOTE Simply setting a flag is not a good approach. This makes the code more complex and less flexible than using a strategy pattern.

One interface to look at is the 'LdapAuthoritiesPopulator`. This API might not work entirely due to the fact it doesn't expose the password. However, I haven't looked into this in any details.

If we get a Pull Request that:

• Is passive
• Documented
• Tested
• Uses a strategy pattern

I would be glad to consider this for Spring Security 4.2.0

[dparker-coder]

I had the same issue as @RickJensen, Spring was retrieving the groups assigned to the logged in user, but not any nested groups. I was able to add the following code in an override of getAdditionalRoles which solved the problem.

/**
 * This class extends a Spring class in order to override the getAdditionalRoles method. Spring's LDAP query
 * is only returning roles which are assigned to the user; it does not return any nested roles. The override
 * method uses a single AD LDAP query to retrieve all groups assigned to this user plus any nested groups.
 */
public class CustomLdapAuthoritiesPopulator extends DefaultLdapAuthoritiesPopulator {

    private static final Logger logger                      = LogManager.getLogger();
    private LdapTemplate        ldapTemplate;
    public static final String  SECURITY_GROUP_FLAG         = String.valueOf(1 << 31);
    public static final String  LDAP_MATCHING_RULE_BIT_AND  = ":1.2.840.113556.1.4.803:";
    public static final String  LDAP_MATCHING_RULE_IN_CHAIN = ":1.2.840.113556.1.4.1941:";
    public static final String  GROUP_SEARCH_BASE           = "OU=Groups,DC=mycomp,DC=com";
    public static final String  IS_GROUP                    = "(objectCategory=group)";
    /* Checks if logical AND of groupType bitfield and security bit is 1 */
    public static final String  IS_SECURITY_ENABLED         = "(groupType" + LDAP_MATCHING_RULE_BIT_AND
            + "=" + SECURITY_GROUP_FLAG + ")";

    public CustomLdapAuthoritiesPopulator(final ContextSource contextSource, final String groupSearchBase) {
        super(contextSource, groupSearchBase);
    }

    @Override
    protected Set<GrantedAuthority> getAdditionalRoles(final DirContextOperations userData,
                                                       final String username) {
        final Set<GrantedAuthority> authorities = new HashSet<>();
        // Build LDAP query filter for nested groups (roles). For ref, this is the filter string:
        // (&(objectCategory=group)(groupType:1.2.840.113556.1.4.803:=-2147483648)(member:1.2.840.113556.1.4.1941:={0}))
        final String filter = "(&" + IS_GROUP + IS_SECURITY_ENABLED
                + "(member" + LDAP_MATCHING_RULE_IN_CHAIN + "={0}))";

        final SearchControls searchControls = new SearchControls();
        searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);

        // Search LDAP for nested roles
        final LdapQuery ldapQuery = query().filter(filter, new Object[] { userData.getDn() });
        final List<String> roleList = ldapTemplate.search(GROUP_SEARCH_BASE,
                ldapQuery.filter().toString(),
                searchControls,
                new AttributesMapper<String>() {
                    @Override
                    public String mapFromAttributes(final Attributes attrs) throws NamingException {
                        return attrs.get("cn").get().toString();
                    }
                });

        for (final String role : roleList) {
            authorities.add(new SimpleGrantedAuthority(role));
        }

        logger.debug(" authorities after LDAP search of nested groups: {}", authorities);
        return authorities;
    }

    public void setLdapTemplate(final LdapTemplate ldapTemplate) {
        this.ldapTemplate = ldapTemplate;
    }
}

[fpmoles]

@rwinch this is what I was discussing last night