Adding filters relative to custom ones is broken

[ngergs]

Describe the bug
Adding a filter relative (before/after) to a custom defined filter added previously does not work since spring security 5.5.0.

To Reproduce
Clone this minimal example project. Run

./gradlew run

and see that it breaks (works when downgrading spring boot to 2.4.5).

org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'springSecurityFilterChain' defined in class path resource [org/springframework/security/config/annotation/web/configuration/WebSecurityConfiguration.class]: Bean instantiation via factory method failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [javax.servlet.Filter]: Factory method 'springSecurityFilterChain' threw exception; nested exception is java.lang.NullPointerException
	at org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:658) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.ConstructorResolver.instantiateUsingFactoryMethod(ConstructorResolver.java:486) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.instantiateUsingFactoryMethod(AbstractAutowireCapableBeanFactory.java:1334) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1177) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:564) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:524) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:335) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:234) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:333) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:208) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:322) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:208) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:944) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:918) ~[spring-context-5.3.7.jar:5.3.7]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:583) ~[spring-context-5.3.7.jar:5.3.7]
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:145) ~[spring-boot-2.5.0.jar:2.5.0]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:758) ~[spring-boot-2.5.0.jar:2.5.0]
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:438) ~[spring-boot-2.5.0.jar:2.5.0]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:337) ~[spring-boot-2.5.0.jar:2.5.0]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1336) ~[spring-boot-2.5.0.jar:2.5.0]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1325) ~[spring-boot-2.5.0.jar:2.5.0]
	at de.selfenergy.debug.spring.security.filters.FiltersApplication.main(FiltersApplication.java:10) ~[main/:na]
Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [javax.servlet.Filter]: Factory method 'springSecurityFilterChain' threw exception; nested exception is java.lang.NullPointerException
	at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:185) ~[spring-beans-5.3.7.jar:5.3.7]
	at org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:653) ~[spring-beans-5.3.7.jar:5.3.7]
	... 21 common frames omitted
Caused by: java.lang.NullPointerException: null
	at org.springframework.security.config.annotation.web.builders.HttpSecurity.addFilterAtOffsetOf(HttpSecurity.java:2654) ~[spring-security-config-5.5.0.jar:5.5.0]
	at org.springframework.security.config.annotation.web.builders.HttpSecurity.addFilterAfter(HttpSecurity.java:2645) ~[spring-security-config-5.5.0.jar:5.5.0]
	at de.selfenergy.debug.spring.security.filters.SecurityConfiguration.configure(SecurityConfiguration.java:34) ~[main/:na]
	at org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter.getHttp(WebSecurityConfigurerAdapter.java:217) ~[spring-security-config-5.5.0.jar:5.5.0]
	at org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter.init(WebSecurityConfigurerAdapter.java:315) ~[spring-security-config-5.5.0.jar:5.5.0]
	at org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter.init(WebSecurityConfigurerAdapter.java:93) ~[spring-security-config-5.5.0.jar:5.5.0]
	at de.selfenergy.debug.spring.security.filters.SecurityConfiguration$$EnhancerBySpringCGLIB$$1613d8bd.init(<generated>) ~[main/:na]
	at org.springframework.security.config.annotation.AbstractConfiguredSecurityBuilder.init(AbstractConfiguredSecurityBuilder.java:338) ~[spring-security-config-5.5.0.jar:5.5.0]
	at org.springframework.security.config.annotation.AbstractConfiguredSecurityBuilder.doBuild(AbstractConfiguredSecurityBuilder.java:300) ~[spring-security-config-5.5.0.jar:5.5.0]
	at org.springframework.security.config.annotation.AbstractSecurityBuilder.build(AbstractSecurityBuilder.java:38) ~[spring-security-config-5.5.0.jar:5.5.0]
	at org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration.springSecurityFilterChain(WebSecurityConfiguration.java:127) ~[spring-security-config-5.5.0.jar:5.5.0]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:na]
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
	at java.base/java.lang.reflect.Method.invoke(Method.java:566) ~[na:na]
	at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:154) ~[spring-beans-5.3.7.jar:5.3.7]
	... 22 common frames omitted

Relevant code lines:

http.addFilterAfter(new SpringRelativeFilter(), SecurityContextHolderAwareRequestFilter.class)
       .addFilterAfter(new CustomRelativeFilter(), SpringRelativeFilter.class);

Expected behavior
Adding filters relative to custom defined ones should work.

Sample
minimal example project

If this is indeed a bug and you are interested I would offer to prepare a PR to adjust the behaviour :)

Related: gh-9633

[whcrow]

+1

[austinarbor]

As a temporary workaround, it looks like specifying the same afterFilter for both custom filters should achieve the same result as 5.4.x. Spring's OrderComparator is seemingly documented as unstable but, as of this writing the underlying data structure used to sort the filters is an ArrayList, which has a stable sort when called https://github.com/spring-projects/spring-security/blob/main/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java#L2619

So until this is resolved, the below code should work

http.addFilterAfter(new SpringRelativeFilter(), SecurityContextHolderAwareRequestFilter.class)
       .addFilterAfter(new CustomRelativeFilter(), SecurityContextHolderAwareRequestFilter.class);

[z0mb1ek]

same here with Keycloak spring security adapter

[ngergs]

The Keycloak spring security adapter can be fixed using the workaround from @austinarbor.
I added a keycloak branch to the minimal example project where this is demonstrated :)

[z0mb1ek]

sure, I wanted to point out the importance of the bug

[Stexxen]

Found this issue after running a CI test build with Spring Boot 2.5.0 and all our security filter related tests failed.
Thought I'd add another example :

Resolving was as per @austinarbor to only use Filters that come pre-supplied. From

.addFilterAfter(new DeviceAuthenticationFilter(authenticationManager), BasicAuthenticationFilter.class)
.addFilterBefore(wrappingFilter, DeviceAuthenticationFilter.class)

to

.addFilterAfter(new DeviceAuthenticationFilter(authenticationManager), BasicAuthenticationFilter.class)
.addFilterAfter(wrappingFilter, BasicAuthenticationFilter.class)

Executing in JDK 15 also give more info about the specifics of the NPE, but nothing that @whcrow hasn't already supplied

Of the other 100's of tests no others failed, so testamount to the excellent Spring devs that this is only issue experienced after such a big change.

[whcrow]

Found this issue after running a CI test build with Spring Boot 2.5.0 and all our security filter related tests failed.
Thought I'd add another example :

Resolving was as per @austinarbor to only use Filters that come pre-supplied. From

.addFilterAfter(new DeviceAuthenticationFilter(authenticationManager), BasicAuthenticationFilter.class)
.addFilterBefore(wrappingFilter, DeviceAuthenticationFilter.class)

to

.addFilterAfter(new DeviceAuthenticationFilter(authenticationManager), BasicAuthenticationFilter.class)
.addFilterAfter(wrappingFilter, BasicAuthenticationFilter.class)

Executing in JDK 15 also give more info about the specifics of the NPE, but nothing that @whcrow hasn't already supplied

Of the other 100's of tests no others failed, so testamount to the excellent Spring devs that this is only issue experienced after such a big change.

@Stexxen
Under jdk11 + springboot 2.4.6, It works fine.
But in springboot 2.5.0, A NullPointerException is triggered.
According to my test, The second parameter of addFilterBefore only supports built-in filters, such as UsernamePasswordAuthenticationFilter, custom filters are not supported.

[mnhock]

Some here by using the KeycloakWebSecurityConfigurerAdapter and updating to Spring Boot 2.5.0 with its Spring Security 5.5 transitive dependency.

[thocou]

An issue has been opened on Keycloak side too: https://issues.redhat.com/browse/KEYCLOAK-18283

[theexiile1305]

Added a pull request for fixing this issue #9832

[adhirajsinghchauhan]

Culprit: a31a855. That commit includes tests, but none of those tests operate on custom filters — so only filters "hardcoded" in FilterOrderRegistration will work as anchors for configuring before/after filters. However, custom filters are never added into the backing filterToOrder map (in FilterOrderRegistration), which is what causes the NPE at L2654:

spring-security/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java

Line 143 in 68f91ed

private FilterOrderRegistration filterOrders = new FilterOrderRegistration();

spring-security/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java

Lines 2653 to 2657 in 68f91ed

	private HttpSecurity addFilterAtOffsetOf(Filter filter, int offset, Class<? extends Filter> registeredFilter) {
	int order = this.filterOrders.getOrder(registeredFilter) + offset;
	this.filters.add(new OrderedFilter(filter, order));
	return this;
	}

[ch4mpy]

Created a PR with

• a test validating custom filter can be added before or after another custom filter
• a proposal for a fix having this test pass

P.S.
If anyone has a hint on how to configure Eclipse STS or Visual Studio Code, import Spring-security projects and run unit-tests from one of those IDEs, this would save me a lot of time :/

[marcusdacoregio]

Thanks to everyone that contributed to this issue. The fix was merged into the main branch and backported to 5.5.x, 5.4.x, 5.3.x, and 5.2.x branches.

@selfenergy, @whcrow, @austinarbor, @z0mb1ek, would you please test those changes in your applications by using the 5.5.1-SNAPSHOT version of Spring Security? It would be good if you folks could do it 😄.

[z0mb1ek]

@marcusdacoregio works like a charm, thx

[whcrow]

@marcusdacoregio

works with 5.5.1-SNAPSHOT, Thanks 👏

[ngergs]

@marcusdacoregio Works perfekt. Can also confirm that both filter are called in the correct order.
Thanks! 👍

[jomu78]

@marcusdacoregio: works with 5.5.1-SNAPSHOT connected to Keycloak

[bgarciaentornos]

Hi, I've discovered what seems like a very similar error case to this one that hasn't been addressed yet (as far as I know). It happens when adding ONLY a custom filter using the addFilterAfter method, the NPE is the same as before this fix. For example, if you comment the first filter on @ngergs little project you'd get that. Any help with this?

Thanks a lot and regards.

[marcusdacoregio]

Hi, @bgarciaentornos.

if you comment the first filter on @ngergs little project you'd get that

If you comment that line of code, the first filter will never get registered, therefore the NPE is expected in this scenario. If you think it is really a bug, please create a minimal, reproducible sample and open a new issue. Thank you!

[bgarciaentornos]

Yep, sorry, it wasn't exactly like that. Our case is with a OAuth2ClientContextFilter being registered as a Bean. We're going to check it again and open an issue if we find it reproducible.

Thanks for your help.