AbstractAuthenticationProcessingFilter produces error when registering a custom UsernamePasswordAuthenticationFilter Bean

[HomoEfficio]

Summary

Registering a custom UsernamePasswordAuthenticationFilter Bean produces java.lang.IllegalArgumentException: authenticationManager must be specified error.

Actual Behavior

When I add a custom UsernamePasswordAuthenticationFilter in the @Configuration class that extends WebSecurityConfigurerAdapter, Spring application fails to run.

2020-04-02 22:53:09.405 ERROR 58116 --- [  restartedMain] o.s.boot.SpringApplication               : Application run failed

org.springframework.context.ApplicationContextException: Unable to start web server; nested exception is org.springframework.boot.web.server.WebServerException: Unable to start embedded Tomcat
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.onRefresh(ServletWebServerApplicationContext.java:156) ~[spring-boot-2.2.6.RELEASE.jar:2.2.6.RELEASE]
...
Caused by: java.lang.IllegalArgumentException: authenticationManager must be specified
	at org.springframework.util.Assert.notNull(Assert.java:198) ~[spring-core-5.2.5.RELEASE.jar:5.2.5.RELEASE]
	at org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter.afterPropertiesSet(AbstractAuthenticationProcessingFilter.java:164) ~[spring-security-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1855) ~[spring-beans-5.2.5.RELEASE.jar:5.2.5.RELEASE]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1792) ~[spring-beans-5.2.5.RELEASE.jar:5.2.5.RELEASE]
	... 56 common frames omitted

Expected Behavior

Spring application should run successfully without error even after the custom UsernamePasswordAuthenticationFilter filter Bean has been added.

Configuration

@EnableWebSecurity
public class WebSecurityConfig extends WebSecurityConfigurerAdapter {

    @Override
    protected void configure(HttpSecurity http) throws Exception {
        http
                .authorizeRequests()
                    .anyRequest().authenticated()
                .and()
                .formLogin()
                .and()
                .httpBasic()
                .and()
                .addFilterBefore(getSimpleAuthFilter(), UsernamePasswordAuthenticationFilter.class);
    }

    @Bean
    public SimpleUsernamePasswordAuthFilter getSimpleAuthFilter() throws Exception {
        return new SimpleUsernamePasswordAuthFilter(authenticationManager());
    }
}

Version

spring-boot-2.2.6.RELEASE
spring-security-web:5.2.2.RELEASE

Sample

https://github.com/HomoEfficio/spring-security-practice

[rwinch]

Thanks for the report and the simple example to recreate your issue. This is not a bug. The issue was with SimpleUsernamePasswordAuthFilter which had a separate reference to AuthenticationManager. This means there was an unused (null) variable in UsernamePasswordAuthenticationFilter. It also meant that SimpleUsernamePasswordAuthFilter.setAuthenticationManager was setting an
AuthenticationManager that would never be used.

I sent a PR to your sample repository HomoEfficio/spring-security-practice#1

[HomoEfficio]

@rwinch
Thanks for the kind and warm answer, and PR.

Considering the responsibilities and usages of the AbstractAuthenticationProcessingFilter, authenticationManager property seems to be required, not optional.
So it is better to use constructor injection rather than to use setter injection, especially for the required properties.

But for now, the required property authenticationManager can only be set by setter method, which does not look so great in my opinion, and it leads to this uncomfortable issue.

What about adding another constructor that receives an AuthenticationManager to the AbstractAuthenticationProcessingFilter so we can seamlessly use constructor injection?
If you think this is reasonable, I will make another PR, including some follow-up changes for the UsernamePasswordAuthenticationFilter.

[rwinch]

Thanks for your offer to help! I am ok with adding a constructor that accepts AuthenticationManager, but we cannot remove the old constructor for passivity reasons. If you are alright with submitting a PR, I can review it once you submit it.