Spring-security: SEC-2224: ActiveDirectoryLdapAuthenticationProvider throws BadCredentialsException if userPrincipalName not equal to sAMAccountName + @domain

[spring-issuemaster]

Michael Solano (Migrated from SEC-2224) said:

When using the sAMAccountName for authentication via ActiveDirectoryLdapAuthenticationProvider, a BadCredentialsException will be thrown if the userPrincipalName is not the sAMAccountName with @domain post-fixed.

For example, if the sAMAccountName is "bwayne" but the userPrincipalName is "bruce.[email protected]", authentication will fail. The createBindPrincipal method assumes the userPrincipalName will be "[email protected]" and not "bruce.[email protected]".

The code below shows the details of that method:

    String createBindPrincipal(String username) {
        if (domain == null || username.toLowerCase().endsWith(domain)) {
            return username;
        }

        return username + "@" + domain;
    }

[spring-issuemaster]

Vladimir Terzic said:

below is my "fix" for this issue. Changing the searchFilter variable to include sAMAccountName (as an OR option) and passing plain username as an additional search parms is all that needs to change.

private DirContextOperations searchForUser(DirContext ctx, String username) throws NamingException {
        SearchControls searchCtls = new SearchControls();
        searchCtls.setSearchScope(SearchControls.SUBTREE_SCOPE);

        //have to use plain username for sAMAccountName since bind principal adds domain
        String searchFilter = "(&(objectClass=user)(|(userPrincipalName={0})(sAMAccountName={1})))";

        final String bindPrincipal = createBindPrincipal(username);

        String searchRoot = rootDn != null ? rootDn : searchRootFromPrincipal(bindPrincipal);

        return SpringSecurityLdapTemplate.searchForSingleEntryInternal(ctx, searchCtls, searchRoot, searchFilter,
                new Object[]{bindPrincipal, username});
    }

[spring-issuemaster]

Michael Solano said:

Hi Vladimir,

I did something similar as a temporary fix that got it working. I modified the search filter to only use sAMAccounName but had to code in some domain specific logic that, even if generalized, probably wouldn't work for everyone. I appreciate your feedback though!

[spring-issuemaster]

Robert Whitcomb said:

This is still an issue in Spring Security 3.2.3. Is there any chance this issue will be fixed in upcoming releases?

[spring-issuemaster]

Michael Solano said:

Hi Robert,

I updated the affected versions to include 3.2.3 as well as 3.2.4 (which I'm using now).

I plan on upgrading to version 4 fairly soon and will test it out there as well.

Thanks,
Mike

Edit: Scratch that, I'm still using 3.1.4 (the rest of my spring stuff is on 3.2.4). So, I removed 3.2.4 from the affected versions and kept your 3.2.3.

[spring-issuemaster]

Ryan J. McDonough said:

I have gotten around the issue in 3.2.6 by doing the following:

 ActiveDirectoryLdapAuthenticationProvider provider =
                new ActiveDirectoryLdapAuthenticationProvider(ldapProperties.getDomain(),
                                                              ldapProperties.getUrl());
          provider.setConvertSubErrorCodesToExceptions(true);
          provider.setUseAuthenticationRequestCredentials(true);
          provider.setSearchFilter("(sAMAccountName={0})");
          return provider;

The addition of the filter fixes the issue. Prior to 3.2.6, Spring Security LDAP did something else and upgrading to Spring Boot 1.2.2 (which includes Spring Security 3.2.6) caused all kinds of grief.

[spring-issuemaster]

Rob Winch said:

damnhandy Thanks for the response. Injecting the search filter is the preferred way to handle this as of Spring Security 3.2.7+. I'm closing this issue issue as being superseded by SEC-1915

[gkibilov]

Still an issue using Spring 4.0.3, injecting sAMAccountName filter has no effect as searchForUser still uses only bindPrincipal when calling searchForSingleEntryInternal!

My filter property looks like this:
<property name="searchFilter" value="(&amp;(objectClass=user)(sAMAccountName={0}))" />
and searchForSingleEntryInternal throws IncorrectResultSizeDataAccessException.

Why not pass user name too? Then I could use a filter like this:
<property name="searchFilter" value="(&amp;(objectClass=user)(sAMAccountName={1}))" />

    private DirContextOperations searchForUser(DirContext context, String username)
            throws NamingException {
        SearchControls searchControls = new SearchControls();
        searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);

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

        try {
            return SpringSecurityLdapTemplate.searchForSingleEntryInternal(context,
                    searchControls, searchRoot, searchFilter,
                    new Object[] { bindPrincipal, userName});
        }

[taasjord]

This is still an issue. In my case the userPrincipalName has a different suffix than domain, ie. while bindPrincipal is "[email protected]", the userPrincipalName is "[email protected]" and sAMAccountName is "bwayne".

So I need to pass username to match sAMAccountName instead. My solution was just to copy the entire source of ActiveDirectoryLdapAuthenticationProvider (along with a package private exception) and change bindPrincipal to username.

[adamjhamer]

Hi,

I've been looking at the issues of sAMAccountName in search filters and the history of requests and changes to ActiveDirectoryLdapAuthenticationProvider, and I believe this issue can now be closed in favour of using the new rootDn and leaving the domain blank.

Flexibility was added to allow the use of a custom search filters, such as sAMAccountName, through #SEC-1915. A change was also made, #SEC-2987, to change the username to the bindPrincipal which then presented the search with username@domain and not just username. I understand the Pre-Win2k domain name is not part of the sAMAccountName attribute, so it is not correct to supply a domain in the search filter (we also have different domain names on the userPrincipalName to the sAMAccountName as @taasjord above). However a domain is required for the searchRoot (error thrown in ActiveDirectoryLdapAuthenticationProvider.searchRootFromPrincipal) - so we end up with a catch-22 -
hence the workaround @gkibilov provides above.

The latest code though now allows rootDN to be specified for the searchRoot whilst leaving the domain empty. This means we can keep the domain blank which satisfies the filter when using sAMAccountName, but satisfies searchRoot being provided with a rootDn. We copied the latest class in (fbb902c), and specified the rootDn, left the domain blank and its working.

Therefore this issue can be closed again, but I do feel that this position is fragile - so it would be good to allow the username to be also passed in to match the searchFilter as above by @gkibilov:
return SpringSecurityLdapTemplate.searchForSingleEntryInternal(context, searchControls, searchRoot, searchFilter, new Object[] { bindPrincipal, userName});

Thanks
Adam

[adamjhamer]

Further to my above comment, it was necessary for us to use a domain in the end because some AD accounts differed between their sAMAccountName and their 'User DN' name (not just the domain!). This meant the username passed in (expecting to match sAMAccountName) wasn't finding them since the user's DN was different. By providing the domain allowed both accounts to be found - a separate rootDn context was not enough.

Being required to provide the domain meant we were back with the problem of the searchFilter using the user@domain format, when sAMAccountName must only have the username. So we modified the latest code to add new Object[] { bindPrincipal, userName}) (as above) which allows the searchFilter to reference the second argument, the username, with (&(objectClass=user)(sAMAccountName={1})).

So whether this is considered a separate issue I'm not sure! Its not a domain mismatch (which is now catered for in the latest code) but a username DN mismatch which was solved by the change of code to new Object[] { bindPrincipal, userName})!

Thanks
Adam

[taasjord]

I just want to note that I did try to use rootDN and blank domain, but that doesn't work for me, because the domain is needed in bindAsUser (but cannot be in searchForUser).

The proposed change to add the necessary flexibility is very small and I don't see what harm it could make.

[sidamos]

I strongly vote for accepting this pull request. We have exactly the same problem.

If at least ActiveDirectoryLdapAuthenticationProvider was not final and the affected method was not private...

[mdeterman]

+1
I'm having the same issue.

[rehmatt]

+1
Same here

[shafsongithub]

This issue still persists even after new code checked-in in RC.

[taasjord]

@shafsongithub, did you set the search filter to use it?
Eg: provider.setSearchFilter("sAMAccountName={1}");

[shafsongithub]

No.. sounds like this will resolve my issue. I will try and let you guys know. BTW When this fix is coming in Release?

[shafsongithub]

It worked. Thanks for your timely comment.

[aivantsov]

Would you please integrate/cherry-pick the fix into the 4.2.x branch.
Thanks in advance.

N.B. Currently cannot switch to Spring Boot 2.x which would solve the issue due to a third party lib limitation.