spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.85k stars 5.91k forks source link

Circular reference when exposing a AuthenticationProvider with a dependency to HttpSecurity #16047

Closed dru1 closed 2 weeks ago

dru1 commented 2 weeks ago

Hi,

there seems to be a strange behavior, when there is a Security Configuration which exposes AuthenticationProviders as Spring @Beans, which use HttpSecurity as a method argument. The application does not start due to circular references, depending on which Spring Security Version is in use:

Spring Boot 3.2.7, Spring Security 6.2.5:

If there is only one AuthenticationProvider there is a circular reference:

@Configuration
@EnableWebSecurity
public class SecurityConfiguration {

    // Security configuration omitted (it doesn't matter)
    // Triggers a circular reference, when only one AuthenticationProvider bean is present

    @Bean
    public AnonymousAuthenticationProvider anonymousAuthenticationProviderOne(@Nonnull HttpSecurity http) throws Exception {
        return new AnonymousAuthenticationProvider("helloWorld1");
    }

}

Exception:

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

Description:

The dependencies of some of the beans in the application context form a cycle:

   org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration (field private org.springframework.security.config.annotation.web.builders.HttpSecurity org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration.httpSecurity)
┌─────┐
|  org.springframework.security.config.annotation.web.configuration.HttpSecurityConfiguration.httpSecurity defined in class path resource [org/springframework/security/config/annotation/web/configuration/HttpSecurityConfiguration.class]
↑     ↓
|  anonymousAuthenticationProviderOne defined in class path resource [org/example/SecurityConfiguration.class]
└─────┘

If there is more than one AuthenticationProvider, everything is fine:

@Configuration
@EnableWebSecurity
public class SecurityConfiguration {

    // Security configuration omitted (it doesn't matter)
    // No circular reference is triggered, when two AuthenticationProvider beans are present

    @Bean
    public AnonymousAuthenticationProvider anonymousAuthenticationProviderOne(@Nonnull HttpSecurity http) throws Exception {
        return new AnonymousAuthenticationProvider("helloWorld1");
    }

    @Bean
    public AnonymousAuthenticationProvider anonymousAuthenticationProviderTwo(@Nonnull HttpSecurity http) throws Exception {
        return new AnonymousAuthenticationProvider("helloWorld2");
    }

}

Spring Boot 3.3.5, Spring Security 6.3.4:

In this setup, it doesn't matter if there is only one AuthenticationProvider bean or two AuthenticationProvider beans. The application does not start in both cases due to circular references.

Analysis

I traced the cause to the InitializeAuthenticationProviderManagerConfigurer, which tries to default an AuthenticationProvider in the AuthenticationManagerBuilder, when there is exactly one AuthenticationProvider present. In version 6.2.5 the circular reference is triggered when the configurer tries to resolve the one Authentication bean. In version 6.3.4, the circular reference is triggered earlier.

Expected behavior

Correct me if I'm wrong, i found similar issues regarding circular references, but I think this is something new. Is the method signature AuthenticationProvider as @Bean with a dependency to HttpSecurity a bad practise? Is it a bad idea to expose an AuthenticationProvider at all? If the method argument HttpSecurity is removed, everything seems to work fine. The method argument HttpSecurity is primarily used to allow access to shared objects and to maintain a consistent method signature. I can provide a full example if necessary.

kse-music commented 2 weeks ago

I think the circular reference caused by introducing HttpSecurityas a parameter is not the point. What should be more concerned about here is how to handle the presence of multiple AuthenticationProvider beans. At the same time, PR #16050 delay initialization AuthenticationProvider is submitted in 6.3.x to ensure that the behavior is consistent

dru1 commented 2 weeks ago

That makes sense. If the initialization of the AuthenticationProvider bean is delayed with a Supplier as submitted in PR #16050 , the behavior is consistent between 6.2.x and 6.3.x, which improves the situation.

The other circular reference remains in PR #16050 , when there is only one AuthenticationProvider with a dependency to HttpSecurity as a method argument. I can avoid this situation, because in my use case there are several AuthenticationProviders anyway.

Thank you very much.

jzheaux commented 2 weeks ago

The construction of a SecurityFilterChain via an HttpSecurity instance depends on a fully-formulated AuthenticationManager. Given that, yes it does seem a bit smelly to publish an AuthenticationProvider in this way.

That said, I'd like to understand your use case better. Can you please say why your provider needs an instance of HttpSecurity?

dru1 commented 2 weeks ago

Sure. To be fair, the method argument HttpSecurity isn't used right now, but it could be in the future. Now the use case I'm facing is this:

Lets say, I want to have a security configuration in a library with multiple authentication providers, which can be enabled or disabled and it would be also helpful when beans in the configuration process can be customized. The following snippet shows the configuration process to achieve this by splitting the configuration process into multiple protected methods and passing the HttpSecurity builder as a context variable to every method, even if it's not used. HttpSecurity is primarily passed on to maintain a consistent method signature, so it's more of a design choice rather than a real requirement.

@Configuration
@EnableWebSecurity
public class SecurityConfiguration {

    // Application property
    private boolean daoAuthenticationEnabled;

    // Additional boolean flags to configure authentication

    @Bean
    public SecurityFilterChain securityFilterChain(@Nonnull HttpSecurity http) throws Exception {
        // Split the configuration in multiple protected methods with a consistent method signature
        configureAuthorization(http);
        configureAnonymousAuthentication(http);
        configuraDaoAuthentication(http);
        configureFormLogin(http);
        // Additional configuration methods
        return http.build();
    }

    // A typical configuration method looks like this
    protected void configureDaoAuthentication(@Nonnull HttpSecurity http) throws Exception {
        if (daoAuthenticationEnabled) {
            AuthenticationProvider daoAuthenticationProvider = daoAuthenticationProvider(http);
            http.authenticationProvider(daoAuthenticationProvider);
        }
    }

    // Some AuthenticationProvider's  implement InitializingBean or just need dependency injection, so they are annotated with @Bean
    @Bean
    public DaoAuthenticationProvider daoAuthenticationProvider(@Nonnull HttpSecurity http) throws Exception {
        DaoAuthenticationProvider daoAuthenticationProvider = new DaoAuthenticationProvider();
        daoAuthenticationProvider.setHideUserNotFoundExceptions(true);
        // Additional configuration but with no actual dependency to HttpSecurity at this point
        return daoAuthenticationProvider;
    }

}

I'm not sure if the method call daoAuthenticationProvider(http) is actually a good idea, but it didn't throw an error in 6.2.x, when there are multiple AuthenticationProvider beans present. What do you think? Is there a better way to achieve this?

jzheaux commented 2 weeks ago

You can take a look at WebSecurityConfigurerAdapter and WebMvcConfigurerAdapter (in Spring MVC) for guidance on the pattern you seem to be following.

As for the dependency cycle, HttpSecurity is a configuration object that is building a SecurityFilterChain. It would be very strange for singletons like DaoAuthenticationProvider or ProviderManager to need it at runtime. (for example, why would you be generating a new filter chain while the app is running?) I believe it's safe to exclude it from daoAuthenticationProvider's contract.

It feels like this ticket has moved into question territory at this point, so I'm going to close it and invite you to post to StackOverflow for any related discussions. You're welcome to post the SO link to this ticket so folks can continue to follow. Thanks again for reaching out!