Spring Security Make ActiveDirectoryLdapAuthenticationProvider non-final to allow custom logic (i.e. Caching)

Expected Behavior

org.springframework.security.ldap.authentication.ad.ActiveDirectoryLdapAuthenticationProvider should not be final to allow extension with custom logic.

Current Behavior

org.springframework.security.ldap.authentication.ad.ActiveDirectoryLdapAuthenticationProvider is final, forcing me to clone the class.

Context

I am maintaining a couple of Spring Boot APIs that use the ActiveDirectoryLdapAuthenticationProvider to authenticate against our company's AD. These services are called multiple times in rapid succession by the same user. To optimise for this case, I would like to implement some LDAP/AD caching.

I would like to implement the caching logic by overriding the authenticate method of the AbstractLdapAuthenticationProvider class, which is the parent of ActiveDirectoryLdapAuthenticationProvider. I currently archive this by copying the ActiveDirectoryLdapAuthenticationProvider into my own codebase, but obviously this is far from best practice.

I don't see any reason why this class should be final, especially since the non-AD counterpart LdapAuthenticationProvider, which also extends AbstractLdapAuthenticationProvider, is not final either.

I see that #3191 had the same request, but was closed because it was outdated and lacked a concrete problem description. I also see #13732, but it is filed as a bug and I do not fully understand the author's case. But since we both want the same result, I will happily close this issue and comment there instead if desired.

If this enhancement is accepted, I will open a pull request to implement this rather small change.

Comment From: marcusdacoregio

Hi, @olebittner.

Can you elaborate more on how you want to implement the cache? Have you tried performing your cache logic elsewhere and then delegate to the AD implementation? Something like:

@Bean
CachedAuthenticationProvider cachedAuthenticationProvider() {
    return new CachedAuthenticationProvider();
}

static class CachedAuthenticationProvider implements AuthenticationProvider {

  private final ActiveDirectoryLdapAuthenticationProvider delegate = new ActiveDirectoryLdapAuthenticationProvider("example.com", "ldap://company.example.com/");

  @Override
  public Authentication authenticate(Authentication authentication) {
    Authentication cachedAuthentication = myCacheLogic();
    if (cachedAuthentication == null) {
      return this.delegate.authenticate(authentication);
    }
    return cachedAuthentication;
  }

  // ...

}

Comment From: olebittner

Hi @marcusdacoregio

Thanks for the reply. That's an elegant solution, I don't know why I didn't think of it myself.

I guess the underlying point is still valid, I don't see any reason to keep the class final. But since I no longer have a concrete use case for a non-final class, I guess we could close the issue and revisit the "final class" discussion when needed.