Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpSecurity Bean does not set DefaultAuthenticationEventPublisher #11449

Closed
andrecampanini opened this issue Jun 29, 2022 · 1 comment
Closed
Assignees
Labels
in: config An issue in spring-security-config status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@andrecampanini
Copy link

Context
The purpose of my module/starter is to be used by other applications in the company I work in order to publish automatically specific logs related to authentication and authorization via ApplicationListeners using Spring Security's and OAuth2's strategy for publishing authentication events (DefaultAuthenticationEventPublisher).

Expected behaviour: using old WebSecurityConfigurerAdapter
My old security config code based on Spring Boot 2.6 worked perfectly fine:

@Configuration @EnableWebSecurity
public class ResourceServerConfig extends WebSecurityConfigurerAdapter {

    @Override
    protected void configure(HttpSecurity http) throws Exception {
        http
            .authorizeRequests()
                . . . 
            .and()
                .oauth2ResourceServer()
                    .jwt();
    }
}

Description of the bug
After upgrading to Spring Boot 2.7 and replacing the usage of deprecated WebSecurityConfigurerAdapter class in favour of a config method @Bean that returns SecurityFilterChain as recommended in this article and release notes my applications don't publish automatically anymore because they do not have a valid AuthenticationEventPublisher as before.

How to reproduce it

@Configuration @EnableWebSecurity
public class ResourceServerConfig {

    @Bean
    public SecurityFilterChain configure(HttpSecurity http) throws Exception {
        http
            .authorizeRequests()
                . . . 
            .and()
                .oauth2ResourceServer()
                    .jwt();
        return http.build();
    }
}

Cause of the bug
The reason for this problem is: the object BearerTokenAuthenticationFilter uses an instance of ProviderManager as AuthenticationManager (so same way when using WebSecurityConfigurerAdapter). But as default an instance of ProviderManager declares its AuthenticationEventPublisher this way:

from: https://github.com/spring-projects/spring-security/blob/main/core/src/main/java/org/springframework/security/authentication/ProviderManager.java

public class ProviderManager implements AuthenticationManager, . . . {

    . . .

    private AuthenticationEventPublisher eventPublisher = new NullEventPublisher();

More details about the bug
So this is the problem: NullEventPublisher is a useless implementation which doesn't publish events.

Why this problem just now? When using WebSecurityConfigurerAdapter - this class was setting an instance of DefaultAuthenticationEventPublisher to ProviderManager's eventPublisher attribute over its default value NullEventPublisher's instance.

Note: I also tried using Spring Security autoconfiguration which is org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration#authenticationEventPublisher() from spring-boot-autoconfigure - but this one is never injected into ProviderManager.

Expected behaviour via a workaround - but I'm not fine with it
After some tests I was able to "fix the problem" with the following code:

@Configuration
@ConditionalOnClass({AuthenticationEventPublisher.class, JwtAuthenticationProvider.class})
 public class SpringConfiguration { //global configuration for several other applications using my module/starter
    @Bean
    public AuthenticationEventPublisher eventPublisher(ApplicationEventPublisher application) {
	    DefaultAuthenticationEventPublisher authentication = 
            new DefaultAuthenticationEventPublisher(application);
	
	    authentication.setDefaultAuthenticationFailureEvent(AuthenticationFailureBadCredentialsEvent.class);
	
        return authentication;
    }

    @Bean
    public ProviderManager providerManagerAvecDefaultAuthenticationPublisher(@Lazy JwtDecoder jwtDecoder, AuthenticationEventPublisher authenticationPublisher) {
        JwtAuthenticationProvider authenticationProvider = new JwtAuthenticationProvider(jwtDecoder);
        ProviderManager providerManager = new ProviderManager(Arrays.asList(authenticationProvider));
        providerManager.setAuthenticationEventPublisher(authenticationPublisher);
        return providerManager;
    }
}

And also had to adjust my security configuration:

@Configuration @EnableWebSecurity
public class ResourceServerConfig {

    @Autowired ProviderManager manager; //1

    @Bean
    public SecurityFilterChain configure(HttpSecurity http) throws Exception {
        http
            .authorizeRequests()
                . . . 
            .and()
                .oauth2ResourceServer()
                    .jwt()
                    .authenticationManager(manager); //2
        return http.build();
    }
}

But I have some big concerns:

  1. As my module/application is to be used by other applications - this "workaround" solution will force dozens of applications to add the lines followed by comments 1 and 2

  2. By adding a custom ProviderManager I am not aware of the risks of "forcing" a pre-built one for those applications
    So finally my question here is: Is there a way to bypass eventPublisher = new NullEventPublisher() from ProviderManager without forcing to configure oauth2ResourceServer().authenticationManager(manager) in all applications configuring its SecurityFilterChain?

  3. I had to use @Lazy for the injection of JwtDecoder - and also I'm not aware how this will affect different other applications

@andrecampanini andrecampanini added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jun 29, 2022
@andrecampanini andrecampanini changed the title Spring Boot 2.7 Spring Security and OAuth2: SecurityFilterChain instead of WebSecurityConfigurerAdapter hasn't properly AuthenticationEventPublisher Spring Boot 2.7, Spring Security and OAuth2: SecurityFilterChain instead of WebSecurityConfigurerAdapter hasn't properly AuthenticationEventPublisher Jun 29, 2022
@marcusdacoregio marcusdacoregio added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 30, 2022
@jzheaux jzheaux added this to the 5.8.x milestone Jul 7, 2022
@jzheaux jzheaux added in: config An issue in spring-security-config and removed in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Jul 7, 2022
@marcusdacoregio marcusdacoregio changed the title Spring Boot 2.7, Spring Security and OAuth2: SecurityFilterChain instead of WebSecurityConfigurerAdapter hasn't properly AuthenticationEventPublisher HttpSecurity does not set DefaultAuthenticationEventPublisher Aug 18, 2022
@marcusdacoregio marcusdacoregio changed the title HttpSecurity does not set DefaultAuthenticationEventPublisher HttpSecurity Bean does not set DefaultAuthenticationEventPublisher Aug 18, 2022
@marcusdacoregio marcusdacoregio self-assigned this Aug 18, 2022
@marcusdacoregio marcusdacoregio modified the milestones: 5.8.x, 5.8.0-M3 Aug 18, 2022
@marcusdacoregio
Copy link
Contributor

While working on this I found another problem. If you have a custom AuthenticationEventPublisher bean, that bean is only picked up if you have an UserDetailsService bean.

marcusdacoregio added a commit that referenced this issue Aug 19, 2022
…rBuilder

Prior to this, the HttpSecurity bean was not consistent with WebSecurityConfigurerAdapter's HttpSecurity because it did not setup a default AuthenticationEventPublisher. This also fixes a problem where the AuthenticationEventPublisher bean would only be considered if there was a UserDetailsService

Closes gh-11449
Closes gh-11726
marcusdacoregio added a commit that referenced this issue Aug 19, 2022
…rBuilder

Prior to this, the HttpSecurity bean was not consistent with WebSecurityConfigurerAdapter's HttpSecurity because it did not setup a default AuthenticationEventPublisher. This also fixes a problem where the AuthenticationEventPublisher bean would only be considered if there was a UserDetailsService

Closes gh-11449
Closes gh-11726
@github-actions github-actions bot added the status: backported An issue that has been backported to maintenance branches label Aug 19, 2022
marcusdacoregio added a commit that referenced this issue Aug 19, 2022
…rBuilder

Prior to this, the HttpSecurity bean was not consistent with WebSecurityConfigurerAdapter's HttpSecurity because it did not setup a default AuthenticationEventPublisher. This also fixes a problem where the AuthenticationEventPublisher bean would only be considered if there was a UserDetailsService

Closes gh-11449
Closes gh-11726
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
Status: Done
Development

No branches or pull requests

3 participants