spring-projects / spring-projects/spring-security

Make ActiveDirectoryLdapAuthenticationProvider non-final to allow customization

Open
#17,850 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: waiting-for-triage type: enhancement
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

Description

Expected Behavior

Class ActiveDirectoryLdapAuthenticationProvider is non-final and can be extended

Current Behavior

Class ActiveDirectoryLdapAuthenticationProvider is final and cannot be extended

Context

NOTE: I am aware of existence of these tickets:
https://github.com/spring-projects/spring-security/issues/13844
https://github.com/spring-projects/spring-security/issues/3303

In our project ActiveDirectoryLdapAuthenticationProvider is used for authenticating against Active Directory since it uses Active Directory configuration conventions which allows for binding using just UserPrincipalName or even sAMAccountName. However, we use it only for authenticating and do not care about user details at all (we have our own custom context mapper which does not uses it). The problem is that ActiveDirectoryLdapAuthenticationProvider.doAuthentication() method is implemented as follows:

	@Override
	protected DirContextOperations doAuthentication(UsernamePasswordAuthenticationToken auth) {
		...
		try {
			ctx = bindAsUser(username, password);
			return searchForUser(ctx, username);
		}
                ...

which means it performs the search whether we need it or not and that forces us to correctly configure search filter even if we do not need it.

Here is a draft of copied and modified ActiveDirectoryLdapAuthenticationProvider class which works for us:

    @Override
    protected DirContextOperations doAuthentication(UsernamePasswordAuthenticationToken auth) {
        String username = auth.getName();
        String password = (String) auth.getCredentials();
        DirContext ctx = null;
        try {
            ctx = bindAsUser(username, password);
//            return searchForUser(ctx, username);
            return createStubUser(username);
        }
        catch (CommunicationException ex) {
            throw badLdapConnection(ex);
        }
//        catch (NamingException ex) {
//            this.logger.error("Failed to locate directory entry for authenticated user: " + username, ex);
//            throw badCredentials(ex);
//        }
        finally {
            LdapUtils.closeContext(ctx);
        }
    }

    private DirContextOperations createStubUser(String username) {
        // Minimal placeholder user, Spring only requires a non-null object
        return new DirContextAdapter() {{
            setAttributeValue("userPrincipalName", username);
        }};
    }

To sum it up, making ActiveDirectoryLdapAuthenticationProvider class non-final would allow us to cleanly do what we need instead of copying a code from spring classes.

There already is this ticket which was unfortunately closed due to different use case which was possible to solve by delegating:
https://github.com/spring-projects/spring-security/issues/3303

And also there is this ticket which is migrated from somewhere else which mentions removing final but the ticket itself was originated from a different use case which has already been resolved. This ticket has been open for a decade now so I suppose it's not active:
https://github.com/spring-projects/spring-security/issues/3303

Alternative approach could possibly be introducing ActiveDirectoryBindOnlyAuthenticationProvider that would not perform search, just binding.

Any response or effort in this matter will be much appreciated, thanks!

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with ActiveDirectoryLdapAuthenticationProvider and its doAuthentication entry point, then read issues 13844 and 3303 for prior decisions about extensibility. Verify whether the requested change supports bind-only customization without altering existing authentication behavior; done means the provider offers the requested extension path and project validation covers it.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spring
Domain
authentication, security
Issue type
Feature
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
50/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.