Spring Authorization Server fails to start with multiple PasswordEncoder beans

[Dayde]

Describe the bug When having multiple PasswordEncoder declared as beans, Spring Authorization Server ignores the @Primary hint and fails to start because it finds multiple beans.

This behaviour is present since the beginning of the project. We’re using 0.4.5 but the reproducer joined below demonstrates the same problem with version 1.2.4.

To Reproduce Declare multiple PasswordEncorder as components with one as @Primary

    @Bean
    fun bcryptPasswordEncoder(): PasswordEncoder {
        return BCryptPasswordEncoder()
    }

    @Bean
    @Primary
    fun delegatingPasswordEncoder(): PasswordEncoder {
        return PasswordEncoderFactories.createDelegatingPasswordEncoder()
    }

Expected behavior The component declared as @Primary should be used.

Sample Reproducer available here -> https://gitlab.com/maltcommunity/public/oss/spring-authorization-server-bug-reproducer/

[Dayde]

The problem arises at that point -> https://github.com/spring-projects/spring-authorization-server/blob/6eda8c6a0ede33d0ba7b93bc81c74d30fe3a6772/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ClientAuthenticationConfigurer.java#L243

Because the getOptionalBean method does not rely on a simple getBean but on beansOfTypeIncludingAncestors https://github.com/spring-projects/spring-authorization-server/blob/main/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2ConfigurerUtils.java#L208

[jgrandja]

@Dayde

When having multiple PasswordEncoder declared as beans, Spring Authorization Server ignores the @Primary hint and fails to start because it finds multiple beans.

Can you please provide details on why you need to define multiple PasswordEncoder @Beans? Please provide each component that requires a PasswordEncoder (ClientSecretAuthenticationProvider being one of them)

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[spring-projects-issues]

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

[Dayde]

Hi, sorry for the late answer.

We have multiple implementations of DaoAuthenticationProvider in our codebase for different authentication mechanisms which historically had different password encoders.

[Dayde]

To be honest, "Why would one need different PasswordEncoder?" does not seem like a legitimate question to me. Having to migrate a user based from one encryption to another without downtime, having different kinds of authentication mechanism relying on different encryptions, even demonstrating the result of different password encoders for the sake of it, I’m pretty sure I’m missing a ton of legitimate use cases for wanting to register multiple PasswordEncoder as Bean.

Legitimate or not, the error being logged is misleading since the code ignores the @Primary used to indicate which implementation to use.

As you can see when you run the reproducer:

***************************
APPLICATION FAILED TO START
***************************

Description:

Method authorizationServerSecurityFilterChain in com.malt.reproducer.SecurityConfig required a single bean, but 2 were found:
        - bcryptPasswordEncoder: defined by method 'bcryptPasswordEncoder' in class path resource [com/malt/reproducer/MultiplePasswordEncodersConfiguration.class]
        - delegatingPasswordEncoder: defined by method 'delegatingPasswordEncoder' in class path resource [com/malt/reproducer/MultiplePasswordEncodersConfiguration.class]

This may be due to missing parameter name information

Action:

Consider marking one of the beans as @Primary, updating the consumer to accept multiple beans, or using @Qualifier to identify the bean that should be consumed

[jgrandja]

@Dayde

To be honest, "Why would one need different PasswordEncoder?" does not seem like a legitimate question to me.

I ask questions to understand your use case in more detail so I can better help. All questions are legitimate IMO.

the error being logged is misleading since the code ignores the @Primary used to indicate which implementation to use.

Ignoring @Primary is intentional. Spring Authorization cannot assume a @Primary PasswordEncoder @Bean is intended to be used for client credentials matching.

FYI, DaoAuthenticationProvider is intended to be used for user credentials (UserDetails) matching.

If you have multiple PasswordEncoder @Bean's then you need to explicitly set it via ClientSecretAuthenticationProvider.setPasswordEncoder().

[Dayde]

@jgrandja

Of course all questions are legitimate, I guess my English is not as good as I’d want to believe it is :smile: I did not mean any disrespect, anyway I think you got my point, even if it lacked the nuances I could have wrapped it with in my own language.

Did you look at the minimal reproducer I provided by any chance? How would you fix the issue in this most simple case? I tried to use the simplest example in the project’s documentation to reproduce the error.

FYI, DaoAuthenticationProvider is intended to be used for user credentials (UserDetails) matching.

If you have multiple PasswordEncoder @Bean's then you need to explicitly set it via ClientSecretAuthenticationProvider.setPasswordEncoder().

I’m afraid I’m not sure to understand how these pieces of information can help me fix the issue I’m facing.

Ignoring @Primary is intentional.

Then, if I may, the error log should not be telling to define one since it is ignored.

[jgrandja]

@Dayde No worries at all. No disrespect taken.

How would you fix the issue in this most simple case?

Instead of registering PasswordEncoder @Bean, you would configure it directly similar to this custom client authentication configuration, for example:

@Bean
public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) throws Exception {
    PasswordEncoder passwordEncoder = ...    // Initialize

    OAuth2AuthorizationServerConfigurer authorizationServerConfigurer =
            new OAuth2AuthorizationServerConfigurer();
    http.apply(authorizationServerConfigurer);

    authorizationServerConfigurer
            .clientAuthentication(clientAuthentication ->
                    clientAuthentication
                            .authenticationProviders(configurePasswordEncoder(passwordEncoder))
            );

    return http.build();
}

private Consumer<List<AuthenticationProvider>> configurePasswordEncoder(PasswordEncoder passwordEncoder) {
    return (authenticationProviders) ->
            authenticationProviders.forEach((authenticationProvider) -> {
                if (authenticationProvider instanceof ClientSecretAuthenticationProvider) {
                    ((ClientSecretAuthenticationProvider) authenticationProvider)
                            .setPasswordEncoder(passwordEncoder);
                }
            });
}

[Dayde]

@jgrandja

Ok that’s a bit less verbose than our current workaround I’ll take it.

However it still seems like a lot of code for something quite simple. Plus, I don’t think I would have been able to come up with this solution given the error I faced in the first place.

I’d be glad to send a pull request if you’d be kind enough to detail which solution you’d like implemented. Given your answers I’m afraid to work for nothing as you seem to be happy with how things are right now.

Different solutions I see:

1. Use the bean marked with the @Primary
2. Catch the exception to log something relevant for the next dev who will have the problem and add what you just described to the documentation
3. Anything else that would allow someone very remote from the project not to have to reopen a similar issue 😉

[jgrandja]

@Dayde

it still seems like a lot of code for something quite simple

From my perspective, the code in configurePasswordEncoder() is quite minimal. This is the standard way to customize an AuthenticationProvider. There are a number of setters in the various AuthenticationProvider's that allow for different implementations for collaborators. We don't want to expose specific configuration points in the configurers, e.g. clientAuthentication.passwordEncoder() because PasswordEncoder is not applicable to all clientAuthentication AuthenticationProvider's, such as JwtClientAssertionAuthenticationProvider. Hence, you need to directly set PasswordEncoder on ClientSecretAuthenticationProvider.

Use the bean marked with the @Primary

This is not an option as mentioned in my previous comment

Spring Authorization cannot assume a @Primary PasswordEncoder @Bean is intended to be used for client credentials matching.

FYI, DaoAuthenticationProvider is intended to be used for user credentials (UserDetails) matching.

A @Primary PasswordEncoder @Bean would be a better candidate for UserDetailsService. Please refer to the reference documentation for UserDetailsService.

Also, as an FYI, if I were to configure the following in your sample:

    @Bean
    fun users(): UserDetailsService {
        val user = User.withDefaultPasswordEncoder()
            .username("user1")
            .password("password")
            .roles("USER")
            .build()
        return InMemoryUserDetailsManager(user)
    }

The @Primary PasswordEncoder @Bean would still not be configured in the DaoAuthenticationProvider, which uses UserDetailsService because there is more than 1 bean configured. So it silently ignores it without throwing an exception. Try it out and put a break point in InitializeUserDetailsBeanManagerConfigurer.InitializeUserDetailsManagerConfigurer.configure().

Catch the exception to log something relevant for the next dev who will have the problem and add what you just described to the documentation

The error message is already being logged. If I run your sample I see this in the log:

2024-07-17T13:56:59.037-04:00 WARN 94990 --- [ main] ConfigServletWebServerApplicationContext : Exception encountered during context initialization - cancelling refresh attempt: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'authorizationServerSecurityFilterChain' defined in class path resource [com/malt/reproducer/SecurityConfig.class]: Failed to instantiate [org.springframework.security.web.SecurityFilterChain]: Factory method 'authorizationServerSecurityFilterChain' threw exception with message: No qualifying bean of type 'org.springframework.security.crypto.password.PasswordEncoder' available: Expected single matching bean of type 'org.springframework.security.crypto.password.PasswordEncoder' but found 2: bcryptPasswordEncoder,delegatingPasswordEncoder

The text in bold is from OAuth2ConfigurerUtils:

    static <T> T getOptionalBean(HttpSecurity httpSecurity, Class<T> type) {
        Map<String, T> beansMap = BeanFactoryUtils
            .beansOfTypeIncludingAncestors(httpSecurity.getSharedObject(ApplicationContext.class), type);
        if (beansMap.size() > 1) {
            throw new NoUniqueBeanDefinitionException(type, beansMap.size(),
                    "Expected single matching bean of type '" + type.getName() + "' but found " + beansMap.size() + ": "
                            + StringUtils.collectionToCommaDelimitedString(beansMap.keySet()));
        }
        return (!beansMap.isEmpty() ? beansMap.values().iterator().next() : null);
    }

I don't feel like any changes are needed at this point.