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

Fail when any filter chain declared after an AnyRequestMatcher filter chain #15220

Closed
jzheaux opened this issue Jun 10, 2024 · 12 comments · Fixed by #15221
Closed

Fail when any filter chain declared after an AnyRequestMatcher filter chain #15220

jzheaux opened this issue Jun 10, 2024 · 12 comments · Fixed by #15221
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Jun 10, 2024

By default, a filter chain's securityMatcher defaults to any request:

http
//  .securityMatcher("/**") <-- implied by default
    .authorizeHttpRequests(...)
    .httpBasic(...)
    ...

This means that configurations that use multiple default filter chains can get unexpected results. Since technically both chains match any request, the first chain is always picked and the second one is never picked.

Spring Security should help by failing during startup when an application has any filter chain that is defined after an "any request" filter chain. For example, a configuration like this is problematic:

@Bean 
@Order(0)
SecurityFilterChain api(HttpSecurity http) throws Exception {
    http
        .authorizeHttpRequests(...)
        .httpBasic(...)

    return http.build();
}

@Bean 
@Order(1)
SecurityFilterChain app(HttpSecurity http) throws Exception {
    http
        .securityMatcher("/app/**")
        .authorizeHttpRequests(...)
        .formLogin(...)

    return http.build();
}

since the first filter chain will always consume any request and the second filter chain will never get exercised.

For clarity, an example fix in the above case could be:

@Bean 
@Order(0)
SecurityFilterChain api(HttpSecurity http) throws Exception {
    http
        .securityMatcher("/api/**")
        .authorizeHttpRequests(...)
        .httpBasic(...)

    return http.build();
}

@Bean 
@Order(1)
SecurityFilterChain app(HttpSecurity http) throws Exception {
    http
        .securityMatcher("/app/**")
        .authorizeHttpRequests(...)
        .formLogin(...)

    return http.build();
}

so that neither of them captures every request.

Or a more preferred one would be:

@Bean 
@Order(0)
SecurityFilterChain app(HttpSecurity http) throws Exception {
    http
        .securityMatcher("/app/**")
        .authorizeHttpRequests(...)
        .formLogin(...)

    return http.build();
}

@Bean 
@Order(1)
SecurityFilterChain api(HttpSecurity http) throws Exception {
    http
        .authorizeHttpRequests(...)
        .httpBasic(...)

    return http.build();
}

as this one still has an "any request" filter chain that acts as a catch-all and is correctly placed as the last filter chain.

I think this check could be done in WebSecurity#performBuild after all the SecurityFilterChain instances are fully built. Something like this might do the trick:

+ boolean anyRequestConfigured = false;
for (SecurityBuilder<? extends SecurityFilterChain> securityFilterChainBuilder : this.securityFilterChainBuilders) {
    SecurityFilterChain securityFilterChain = securityFilterChainBuilder.build();
+    Assert.isTrue(!anyRequestConfigured, "A filter chain that matches any request has already been configured, which means that this filter chain for [" + matcher + "] will never get invoked. Please use `HttpSecurity#securityMatcher` to ensure that there is only one filter chain configured for 'any request' and that the 'any request' filter chain is published last.")
+    if (securityFilterChain.getRequestMatcher() instanceof AnyRequestMatcher matcher) {
+        anyRequestConfigured = true;
+    }
    securityFilterChains.add(securityFilterChain);
    // ...
}
@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement status: ideal-for-contribution An issue that we actively are looking for someone to help us with labels Jun 10, 2024
franticticktick pushed a commit to franticticktick/spring-security that referenced this issue Jun 10, 2024
@franticticktick
Copy link
Contributor

Hi @jzheaux. This will only work with DefaultSecurityFilterChain, so I added the necessary check. But one question worries me - how many people used this, thinking that it was not a bug but a feature? And won't they get an unpleasant surprise?

@jzheaux
Copy link
Contributor Author

jzheaux commented Jun 17, 2024

Good question, @CrazyParanoid. Yes, it may be a surprise, though it's an important one since their configuration is broken. When an any-request filter chain comes before another filter chain, that second filter chain is never consulted, perhaps unbeknownst to the developer. Updating to the next minor release will help them fix this configuration bug.

@jzheaux jzheaux removed the status: ideal-for-contribution An issue that we actively are looking for someone to help us with label Jun 17, 2024
@jzheaux jzheaux self-assigned this Jun 17, 2024
jzheaux added a commit that referenced this issue Jun 17, 2024
Storing the request matcher outside of the for loop means that
if one of the SecurityFilterChain instances is not of type
DefaultSecurityFilterChain, then the error may print out an
earlier request matcher instead of the current one.

Instead, this commit changes to print out the entire filter chain
so that it can be inside of the for loop, regardless of type.

Issue gh-15220
@haynescd
Copy link

haynescd commented Nov 1, 2024

@jzheaux Appreciate this write up! Solved an issue we were having recently

@brunpoern
Copy link

Hi, I just stumbled across this change when upgrading to Spring Boot 3.4 but in our case the overwriting was intended. In production we use a filter chain which allows unauthenticated access only to some management endpoints. Any other request requires authentication. But in some tests/example applications we "overwrite" this "management-only" filter chain by a filter chain which allows request to all endpoints:

@Order(0)
@Bean
SecurityFilterChain managementOnlySecurityFilterChain(HttpSecurity http) throws Exception {
      http.securityMatcher("/actuator/**", "/startup-report/**")
          .authorizeHttpRequests(auth -> {
            auth.requestMatchers(mvc.pattern(HEALTH_ENDPOINT)).permitAll();
            // ...
            auth.anyRequest().authenticated();
          });
@Bean
@Order(Ordered.HIGHEST_PRECEDENCE)
SecurityFilterChain permitAllSecurityFilterChain(HttpSecurity http) throws Exception {
    http.authorizeHttpRequests(auth -> {
      auth.requestMatchers("/*/authenticationRequired").authenticated();
      auth.anyRequest().permitAll();
    });
 }

Now this approach doesn't work anymore. Is there any other way how to achieve this overwriting? I mean I could make the production filter chain somehow configurable to disable it in other places by configuration. Just wondering if I do something wrong here and there is a better solution?!

@jespersm
Copy link

@Order(0)
@Bean
SecurityFilterChain managementOnlySecurityFilterChain(HttpSecurity http) throws Exception {
    ...
@Bean
@Order(Ordered.HIGHEST_PRECEDENCE)
SecurityFilterChain permitAllSecurityFilterChain(HttpSecurity http) throws Exception {
...
 }

Now this approach doesn't work anymore. Is there any other way how to achieve this overwriting? I mean I could make the production filter chain somehow configurable to disable it in other places by configuration. Just wondering if I do something wrong here and there is a better solution?!

Ordered.HIGHEST_PRECEDENCE (Integer.MIN_VALUE) comes before 0, so you are blocking out the second filter, and that's why the IllegalArgumentException will be thrown now.

@brunpoern
Copy link

brunpoern commented Nov 28, 2024

You mean I block out the first filter, i.e. managementOnlySecurityFilterChain because Ordered.HIGHEST_PRECEDENCE comes first?! Actually this is what I want. I want to overwrite/blockout the managementOnlySecurityFilterChain because in my scenario access to all endpoints should be permitted.

The managementOnlySecurityFilterChain is always created in our production auto configuration. And before I could overwrite it from my actuall application by defining the higher precedence filter chain. Just wanted to confirm that I didn't misunderstood the new behavior (I understand the motivation behind it) and that there is no way to avoid it. Basically it's not possible anymore to have two filter chains which match any request.

@jespersm
Copy link

Basically it's not possible anymore to have two filter chains which match any request.

To do what you want to do, either suppress the managementOnlySecurityFilterChain bean in your tests, or "cheat" by using a filter which "effectively" blocks out the normal filter, like this:

@Bean
@Order(Ordered.HIGHEST_PRECEDENCE)
SecurityFilterChain permitAllSecurityFilterChain(HttpSecurity http) throws Exception {
    http.securityMatcher("/**")   // Matches all, but is not instanceof AnyRequestMatcher
    http.authorizeHttpRequests(auth -> {
      auth.requestMatchers("/*/authenticationRequired").authenticated();
      auth.anyRequest().permitAll();
    });
 }

@jespersm
Copy link

jespersm commented Nov 28, 2024

Thanks to @jzheaux for fixing this developer quality-of-life issue.

As an added bonus for anyone using HttpSecurity#securityMatcher(java.lang.String...) like in the examples above, attempting to filter a path served by a Servlet, in an application which also uses Spring MVC:

Remember to read the docs, and consider http.securityMatcher(antMatcher("/servletpath/**")) in place of http.securityMatcher("/servletpath/**"). Or spend a long time digging out the differerence, like I did.

@brunpoern
Copy link

@jespersm Thanks for the quick help. The "cheat" is what I was looking for 😀

I also read the docs now 😉 and just in case someone is looking for a way to combine multiple antMatchers, this seems to work:

http.securityMatchers(c -> c.requestMatchers(antMatcher("/actuator/**"), antMatcher("/startup-report/**")))

@sergei-zachesov
Copy link

I have an external library that implements basic and oauth2 authorization.

And I have a SecurityFilterChain for basic authorization:

@Bean
@ConditionalOnMissingBean
public AuthorizeRequestsCustomizer authorizeRequestsCustomizer(
    @Qualifier("basicAndFormMvc") MvcRequestMatcher.Builder mvc) {
  return customizer ->
      customizer.requestMatchers(mvc.pattern("/api/**")).authenticated().anyRequest().permitAll();
}

@Order(3)
@Bean("basicAndFormAuthFilter")
public SecurityFilterChain filterChain(
    HttpSecurity http, AuthorizeRequestsCustomizer authorizeRequestsCustomizer) throws Exception {

  http.sessionManagement(c -> c.sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED))
      .csrf(AbstractHttpConfigurer::disable)
      .cors(withDefaults())
      .httpBasic(withDefaults())
      .formLogin(withDefaults())
      .exceptionHandling(
          c ->
              c.defaultAuthenticationEntryPointFor(
                  customAuthenticationEntryPoint(), new AntPathRequestMatcher("/**")))
      .authorizeHttpRequests(authorizeRequestsCustomizer);

  return http.build();
}

But when starting, the error described above occurs:

 [                  main] [ERROR] o.springframework.boot.SpringApplication: [] Application run failed
org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'springSecurityFilterChain': Cannot create inner bean '(inner bean)#7f088b5c' while setting constructor argument with key [1]
	at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveInnerBeanValue(BeanDefinitionValueResolver.java:421)
	at org.springframework.beans.factory.support.BeanDefinitionValueResolver.lambda$resolveValueIfNecessary$1(BeanDefinitionValueResolver.java:153)
	at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveInnerBean(BeanDefinitionValueResolver.java:262)
	at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveValueIfNecessary(BeanDefinitionValueResolver.java:152)
	at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveManagedList(BeanDefinitionValueResolver.java:460)
	at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveValueIfNecessary(BeanDefinitionValueResolver.java:191)
	at org.springframework.beans.factory.support.ConstructorResolver.resolveConstructorArguments(ConstructorResolver.java:691)
	at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:206)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:1377)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1214)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:563)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:523)
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:336)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:289)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:334)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:312)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.instantiateSingleton(DefaultListableBeanFactory.java:1122)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingleton(DefaultListableBeanFactory.java:1093)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:1030)
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:987)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:627)
	at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:146)
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:752)
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:439)
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:318)
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1361)
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1350)
	at (.java:10)
Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name '(inner bean)#7f088b5c' defined in class path resource [org/springframework/security/config/annotation/web/configuration/WebSecurityConfiguration.class]: Failed to instantiate [jakarta.servlet.Filter]: Factory method 'springSecurityFilterChain' threw exception with message: A filter chain that matches any request [DefaultSecurityFilterChain defined as 'basicAndFormAuthFilter' in [class path resource [/BasicAndFormAuthConfig.class]] matching [any request] and having filters [DisableEncodeUrl, WebAsyncManagerIntegration, SecurityContextHolder, HeaderWriter, Cors, Logout, UsernamePasswordAuthentication, DefaultResources, DefaultLoginPageGenerating, DefaultLogoutPageGenerating, BasicAuthentication, RequestCacheAware, SecurityContextHolderAwareRequest, AnonymousAuthentication, SessionManagement, ExceptionTranslation]] has already been configured, which means that this filter chain [DefaultSecurityFilterChain defined as 'jwtSecurityFilterChain' in [class path resource [org/springframework/boot/autoconfigure/security/oauth2/resource/servlet/OAuth2ResourceServerJwtConfiguration$OAuth2SecurityFilterChainConfiguration.class]] matching [any request] and having filters [DisableEncodeUrl, WebAsyncManagerIntegration, SecurityContextHolder, HeaderWriter, Csrf, Logout, BearerTokenAuthentication, RequestCacheAware, SecurityContextHolderAwareRequest, AnonymousAuthentication, ExceptionTranslation, Authorization]] will never get invoked. Please use `HttpSecurity#securityMatcher` to ensure that there is only one filter chain configured for 'any request' and that the 'any request' filter chain is published last.
	at org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:657)
	at org.springframework.beans.factory.support.ConstructorResolver.instantiateUsingFactoryMethod(ConstructorResolver.java:489)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.instantiateUsingFactoryMethod(AbstractAutowireCapableBeanFactory.java:1357)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1187)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:563)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:523)
	at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveInnerBeanValue(BeanDefinitionValueResolver.java:407)
	... 29 common frames omitted
Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [jakarta.servlet.Filter]: Factory method 'springSecurityFilterChain' threw exception with message: A filter chain that matches any request [DefaultSecurityFilterChain defined as 'basicAndFormAuthFilter' in [class path resource [/BasicAndFormAuthConfig.class]] matching [any request] and having filters [DisableEncodeUrl, WebAsyncManagerIntegration, SecurityContextHolder, HeaderWriter, Cors, Logout, UsernamePasswordAuthentication, DefaultResources, DefaultLoginPageGenerating, DefaultLogoutPageGenerating, BasicAuthentication, RequestCacheAware, SecurityContextHolderAwareRequest, AnonymousAuthentication, SessionManagement, ExceptionTranslation]] has already been configured, which means that this filter chain [DefaultSecurityFilterChain defined as 'jwtSecurityFilterChain' in [class path resource [org/springframework/boot/autoconfigure/security/oauth2/resource/servlet/OAuth2ResourceServerJwtConfiguration$OAuth2SecurityFilterChainConfiguration.class]] matching [any request] and having filters [DisableEncodeUrl, WebAsyncManagerIntegration, SecurityContextHolder, HeaderWriter, Csrf, Logout, BearerTokenAuthentication, RequestCacheAware, SecurityContextHolderAwareRequest, AnonymousAuthentication, ExceptionTranslation, Authorization]] will never get invoked. Please use `HttpSecurity#securityMatcher` to ensure that there is only one filter chain configured for 'any request' and that the 'any request' filter chain is published last.
	at org.springframework.beans.factory.support.SimpleInstantiationStrategy.lambda$instantiate$0(SimpleInstantiationStrategy.java:199)
	at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiateWithFactoryMethod(SimpleInstantiationStrategy.java:88)
	at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:168)
	at org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:653)
	... 35 common frames omitted
Caused by: java.lang.IllegalArgumentException: A filter chain that matches any request [DefaultSecurityFilterChain defined as 'basicAndFormAuthFilter' in [class path resource [/BasicAndFormAuthConfig.class]] matching [any request] and having filters [DisableEncodeUrl, WebAsyncManagerIntegration, SecurityContextHolder, HeaderWriter, Cors, Logout, UsernamePasswordAuthentication, DefaultResources, DefaultLoginPageGenerating, DefaultLogoutPageGenerating, BasicAuthentication, RequestCacheAware, SecurityContextHolderAwareRequest, AnonymousAuthentication, SessionManagement, ExceptionTranslation]] has already been configured, which means that this filter chain [DefaultSecurityFilterChain defined as 'jwtSecurityFilterChain' in [class path resource [org/springframework/boot/autoconfigure/security/oauth2/resource/servlet/OAuth2ResourceServerJwtConfiguration$OAuth2SecurityFilterChainConfiguration.class]] matching [any request] and having filters [DisableEncodeUrl, WebAsyncManagerIntegration, SecurityContextHolder, HeaderWriter, Csrf, Logout, BearerTokenAuthentication, RequestCacheAware, SecurityContextHolderAwareRequest, AnonymousAuthentication, ExceptionTranslation, Authorization]] will never get invoked. Please use `HttpSecurity#securityMatcher` to ensure that there is only one filter chain configured for 'any request' and that the 'any request' filter chain is published last.
	at org.springframework.security.config.annotation.web.builders.WebSecurity.performBuild(WebSecurity.java:320)
	at org.springframework.security.config.annotation.web.builders.WebSecurity.performBuild(WebSecurity.java:94)
	at org.springframework.security.config.annotation.AbstractConfiguredSecurityBuilder.doBuild(AbstractConfiguredSecurityBuilder.java:354)
	at org.springframework.security.config.annotation.AbstractSecurityBuilder.build(AbstractSecurityBuilder.java:38)
	at org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration.springSecurityFilterChain(WebSecurityConfiguration.java:118)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.springframework.beans.factory.support.SimpleInstantiationStrategy.lambda$instantiate$0(SimpleInstantiationStrategy.java:171)
	... 38 common frames omitted

It comes from an existing SecurityFilterChain 'jwtSecurityFilterChain' in OAuth2ResourceServerJwtConfiguration.OAuth2SecurityFilterChainConfiguration:

	@Configuration(proxyBeanMethods = false)
	@ConditionalOnDefaultWebSecurity
	static class OAuth2SecurityFilterChainConfiguration {

		@Bean
		@ConditionalOnBean(JwtDecoder.class)
		SecurityFilterChain jwtSecurityFilterChain(HttpSecurity http) throws Exception {
			http.authorizeHttpRequests((requests) -> requests.anyRequest().authenticated());
			http.oauth2ResourceServer((resourceServer) -> resourceServer.jwt(withDefaults()));
			return http.build();
		}

	}

Since the following SecurityFilterChain is part of the library, I don't know how to get around this error using the tips above.

How can I get around this error?

@jespersm
Copy link

Since the following SecurityFilterChain is part of the library, I don't know how to get around this error using the tips above.

How can I get around this error?

If you want to disable the starter's config, you could use the corresponding Spring mechanisms for that, as discussed in https://stackoverflow.com/questions/67020215/how-to-turn-off-spring-security-auto-configuration

That's a better approach than the cheat I described above.

@sergei-zachesov
Copy link

Thanks @jespersm. I was thinking about this option, but stopped because of:

  1. I use my external library(there is no @SpringBootApplication), rather than implementing security in the project itself.
  2. It appears here inner class config with package-private modifier
  3. It is possible that after disabling the default configurations, something will not work.

I think I will resume work in this direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants