Deprecate AuthenticationProvider

[jzheaux]

AuthenticationManager and AuthenticationProvider have the same primary signature. In an effort to simplify the API, AuthenticationProvider should be deprecated.

Here is an initial list:

• Have existing AuthenticationProviders implement AuthorizationManager
• Add DelegatingAuthenticationManager to replace ProviderManager
• Introduce DSL support for specifying AuthenticationManager for each authentication mechanism
• Use AuthenticationManager by default for each authentication mechanism
• Deprecate authentication-provider XML support
• Deprecate DSL support for authenticationProvider()
• Consider creating AuthenticationProviderManagerAdapter to adapt AuthenticationProviders into AuthenticationManagers
• Consider @Bean support for multiple AuthenticationManagers

[jgrandja]

I agree that we should simplify the current design of AuthenticationManager and AuthenticationProvider, however, I'm not sure this makes sense:

Create AuthenticationManager implementations for existing AuthenticationProviders

Most (likely all) AuthenticationProvider implementations authenticate a specific credential (e.g. username/password, authorization_code, Jwt, etc.) and return the authenticated representation of it. However, they do not "manage" the credential or anything at all really. So I'm hung up on the naming here. I don't feel we should have an AuthenticationManager if it doesn't really manage anything.

A more logical name would be Authenticator since it authenticates a credential. The ProviderManager implementation could be re-implemented as a DelegatingAuthenticator.

I realize this is a much bigger change since we would introduce a new interface Authenticator and deprecate both AuthenticationManager and AuthenticationProvider. However, I don't feel we should introduce a whole new set of AuthenticationManager implementations.

Have we considered deprecating AuthenticationManager and keeping AuthenticationProvider but removing supports()?

[jzheaux]

Interesting idea; I can see your point about the semantics around the word "manager".

Yes, it does sound like a big change. Some other things that come to mind are AuthorizationManager, ReactiveAuthenticationManager, and ReactiveAuthorizationManager. There may be others. If we change servlet authentication to Authenticator, it seems like we'd need to change these others, following the same semantic reasoning.

Honestly, as nice as it would be to not have "XXXManager" components, it feels like too big of a change for too small of a benefit. Just my 2c, though.

[jgrandja]

I personally feel we should hold off on these changes until after 6.0 is out. I don't think we should compromise on the naming and use *Manager for the reasons I mentioned. More importantly, this is a major change that affects a large part of the core codebase and therefore is very risky to change at this point given we're close to RC1 phase. I think we should take the time to come up with a simple and intuitive design and work through a couple of 6.x releases to ensure we didn't break anything. This approach will reduce the risk and we'll be confident to remove AuthenticationManager and/or AuthenticationProvider when it's time to release 7.0.

[jzheaux]

Thanks, @jgrandja.

I don't see the name as a compromise, given that we have several other components that follow that naming convention. Stating that to stick with AuthenticationManager is a compromise is to also state that ReactiveAuthenticationManager, ReactiveAuthorizationManager, and AuthorizationManager are all compromises, too. I see XXXManager as the norm in Spring Security, even if the semantics of the word "Manager" have changed over the years.

To change that norm is to widen the scope of what is intended by this ticket. IOW, we can deprecate AuthenticationProvider and address Spring Security's overarching "manager" naming convention separately.

I think we should take the time to come up with a simple and intuitive design

I'm open to this discussion, but I'm not clear on what you mean by "more intuitive". Are you referring to something other than the name of the class? That is, I don't see this as much of a design discussion unless you are stating that the contract itself should be different. For example, the reactive bits provide ample design guidance.

More to the point, even if you are proposing a different contract, it still seems like this ticket is about the removal of existing unnecessary complexity. If we want to later introduce a new contract, we can do that as a separate effort.