Skip to content

FilterInvocation should support getDispatcherType() #15042

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

Closed
chrylis opened this issue May 10, 2024 · 8 comments
Closed

FilterInvocation should support getDispatcherType() #15042

chrylis opened this issue May 10, 2024 · 8 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug

Comments

@chrylis
Copy link

chrylis commented May 10, 2024

Describe the bug
The core HttpSecurity builder supports dispatcherTypeMatchers, but FilterInvocation throws UnsupportedOperationException if they are invoked.

During an upgrade of an older Boot Servlet project, I ran into the problem where the Spring Boot /error mapping is no longer allowed by default, at least for 403 errors. I tried the suggested resolution of adding dispatcherTypeMatchers(ERROR).permitAll() to my SecurityFilterChain bean. This throws an exception in 5.7.11 (the default with the last Boot 2.7) and 5.8.12.

The problem appears to be that DefaultWebInvocationPrivilegeEvaluator uses a DummyRequest instead of the real request but does not implement core API methods; many/most other methods were supported as part of #8566.

To Reproduce

  1. Register dispatcherTypeMatchers(ERROR).permitAll() in a SecurityFilterChain.
  2. Make a request that triggers a 403 response as an unauthenticated user. (AnonymousAuthenticationToken)

Expected behavior
The matcher permits the error page to proceed.

Actual behavior

2024-05-10T18:43:15,654Z [http-nio-5000-exec-1] ERROR o.a.c.c.C.[Tomcat].[localhost] - Exception Processing ErrorPage[errorCode=0, location=/error]
java.lang.UnsupportedOperationException: public abstract javax.servlet.DispatcherType javax.servlet.ServletRequest.getDispatcherType() is not supported
	at org.springframework.security.web.FilterInvocation$UnsupportedOperationExceptionInvocationHandler.invoke(FilterInvocation.java:331)
	at com.sun.proxy.$Proxy84.getDispatcherType(Unknown Source)
	at javax.servlet.ServletRequestWrapper.getDispatcherType(ServletRequestWrapper.java:449)
	at org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher.matches(DispatcherTypeRequestMatcher.java:72)
	at org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource.getAttributes(DefaultFilterInvocationSecurityMetadataSource.java:84)
	at org.springframework.security.web.access.DefaultWebInvocationPrivilegeEvaluator.isAllowed(DefaultWebInvocationPrivilegeEvaluator.java:94)
	at org.springframework.security.web.access.DefaultWebInvocationPrivilegeEvaluator.isAllowed(DefaultWebInvocationPrivilegeEvaluator.java:69)
	at org.springframework.security.web.access.RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.isAllowed(RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.java:76)
	at org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter.isAllowed(ErrorPageSecurityFilter.java:88)
	at org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter.doFilter(ErrorPageSecurityFilter.java:76)
	at org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter.doFilter(ErrorPageSecurityFilter.java:70)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:352)
	at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.invoke(FilterSecurityInterceptor.java:108)
	at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.doFilter(FilterSecurityInterceptor.java:83)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:361)
	at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:126)
	at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:120)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:361)
	at org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:91)
	at org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:85)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:361)
	at org.springframework.security.web.authentication.AnonymousAuthenticationFilter.doFilter(AnonymousAuthenticationFilter.java:100)
@chrylis chrylis added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels May 10, 2024
@tehhowch
Copy link

tehhowch commented May 14, 2024

just ran into this myself!

Using 5.8.12 with latest Spring Boot 2.7

@marcusdacoregio marcusdacoregio self-assigned this May 15, 2024
@marcusdacoregio marcusdacoregio added in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels May 15, 2024
@marcusdacoregio
Copy link
Contributor

Thanks for the report @chrylis, do you have a minimal, reproducible sample that we can try it out? I'd like to see how exactly the WebInvocationPrivilegeEvaluator is invoked since it does not have access to the whole HttpServletRequest.

@marcusdacoregio marcusdacoregio added the status: waiting-for-feedback We need additional information before we can continue label May 15, 2024
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label May 22, 2024
@spring-projects-issues
Copy link

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues closed this as not planned Won't fix, can't repro, duplicate, stale May 29, 2024
@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels May 29, 2024
@teddy-materio
Copy link

Hi @marcusdacoregio. I ran into the same issue while following the migration steps for Spring Security 5.x to 6.x.

Minimal reproducible sample:
https://github.com/teddy-materio/issue-15042-repro

Steps to reproduce:

  1. Boot up the sample app
  2. Navigate to http://localhost:8080/demo

Expected Behavior:

  1. The DemoController will throw an exception
  2. The DemoErrorController should take over and render the application-provided error.html template

Actual Behavior:

  1. The DemoController will throw an exception
  2. While handling this original exception, an UnsupportedOperationException is thrown, the DemoErrorController is never reached, and a stock error page is rendered instead of the application-provided error.html template.
2024-07-09 12:43:14.552 ERROR 18013 --- [nio-8080-exec-1] o.a.c.c.C.[Tomcat].[localhost]           : Exception Processing ErrorPage[errorCode=0, location=/error]

java.lang.UnsupportedOperationException: public abstract javax.servlet.DispatcherType javax.servlet.ServletRequest.getDispatcherType() is not supported
	at org.springframework.security.web.FilterInvocation$UnsupportedOperationExceptionInvocationHandler.invoke(FilterInvocation.java:331) ~[spring-security-web-5.8.13.jar:5.8.13]
	at jdk.proxy2/jdk.proxy2.$Proxy63.getDispatcherType(Unknown Source) ~[na:na]
	at javax.servlet.ServletRequestWrapper.getDispatcherType(ServletRequestWrapper.java:449) ~[tomcat-embed-core-9.0.83.jar:4.0.FR]
	at org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher.matches(DispatcherTypeRequestMatcher.java:72) ~[spring-security-web-5.8.13.jar:5.8.13]
	at org.springframework.security.web.util.matcher.RequestMatcher.matcher(RequestMatcher.java:48) ~[spring-security-web-5.8.13.jar:5.8.13]
	at org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager.check(RequestMatcherDelegatingAuthorizationManager.java:76) ~[spring-security-web-5.8.13.jar:5.8.13]
	at org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager.check(RequestMatcherDelegatingAuthorizationManager.java:47) ~[spring-security-web-5.8.13.jar:5.8.13]
	at org.springframework.security.web.access.AuthorizationManagerWebInvocationPrivilegeEvaluator.isAllowed(AuthorizationManagerWebInvocationPrivilegeEvaluator.java:57) ~[spring-security-web-5.8.13.jar:5.8.13]
	at org.springframework.security.web.access.AuthorizationManagerWebInvocationPrivilegeEvaluator.isAllowed(AuthorizationManagerWebInvocationPrivilegeEvaluator.java:51) ~[spring-security-web-5.8.13.jar:5.8.13]
	at org.springframework.security.web.access.RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.isAllowed(RequestMatcherDelegatingWebInvocationPrivilegeEvaluator.java:76) ~[spring-security-web-5.8.13.jar:5.8.13]
	at org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter.isAllowed(ErrorPageSecurityFilter.java:88) ~[spring-boot-2.7.18.jar:2.7.18]
	at org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter.doFilter(ErrorPageSecurityFilter.java:76) ~[spring-boot-2.7.18.jar:2.7.18]
	at org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter.doFilter(ErrorPageSecurityFilter.java:70) ~[spring-boot-2.7.18.jar:2.7.18]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178) ~[tomcat-embed-core-9.0.83.jar:9.0.83]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153) ~[tomcat-embed-core-9.0.83.jar:9.0.83]

@marcusdacoregio
Copy link
Contributor

Thanks for the sample @teddy-materio. This problem is happening because the Spring Boot's ErrorPageSecurityFilter, responsible for authorization on the /error endpoint is using a rather limited API for invoking authorization rules (WebInvocationPrivilegeEvaluator). The ErrorPageSecurityFilter has been removed in Spring Boot 3.0, therefore that should not be a problem anymore.

@teddy-materio, to fix the error in your sample we can move the .requestMatchers("/demo", "/error").permitAll() line before .dispatcherTypeMatchers(DispatcherType.FORWARD, DispatcherType.ERROR).permitAll(), like so:

@Bean
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
  http
      .authorizeHttpRequests()
      .shouldFilterAllDispatcherTypes(true)
      .requestMatchers("/demo", "/error").permitAll()
      .dispatcherTypeMatchers(DispatcherType.FORWARD, DispatcherType.ERROR).permitAll() 
      .anyRequest().authenticated();

  return http.build();
}

This will make sure that we don't use the DispatcherTypeRequestMatcher to verify authorization on /error, since it will be verified by the MvcRequestMatcher before.

@teddy-materio
Copy link

That worked, thank you!! Really appreciate the quick response and solution 🙏 .

@jkelmPronto
Copy link

jkelmPronto commented Sep 14, 2024

Hi @marcusdacoregio ! Here's another way to reproduce this issue: through the org.springframework.security.taglibs.authz.JspAuthorizeTag.

Here's a sample call stack which triggers the exception:

Call stack

invoke:331, FilterInvocation$UnsupportedOperationExceptionInvocationHandler (org.springframework.security.web)

getDispatcherType:-1, $Proxy563 (jdk.proxy3)

getDispatcherType:438, ServletRequestWrapper (jakarta.servlet)

matches:72, DispatcherTypeRequestMatcher (org.springframework.security.web.util.matcher)

matcher:48, RequestMatcher (org.springframework.security.web.util.matcher)

check:80, RequestMatcherDelegatingAuthorizationManager (org.springframework.security.web.access.intercept)

check:49, RequestMatcherDelegatingAuthorizationManager (org.springframework.security.web.access.intercept)

isAllowed:60, AuthorizationManagerWebInvocationPrivilegeEvaluator (org.springframework.security.web.access)

isAllowed:109, RequestMatcherDelegatingWebInvocationPrivilegeEvaluator (org.springframework.security.web.access)

authorizeUsingUrlCheck:148, AbstractAuthorizeTag (org.springframework.security.taglibs.authz)

authorize:102, AbstractAuthorizeTag (org.springframework.security.taglibs.authz)

doStartTag:70, JspAuthorizeTag (org.springframework.security.taglibs.authz)

Security configuration:

@Bean
public SecurityFilterChain filterChain(HttpSecurity http...) {
    ...
    http.authorizeHttpRequests((auth) -> auth.dispatcherTypeMatchers(DispatcherType.FORWARD, DispatcherType.ERROR).permitAll();
    (more request matchers)
    ...
}

Referring back to the stack trace, a new instance of a FilterInvocation is created in org.springframework.security.web.access.AuthorizationManagerWebInvocationPrivilegeEvaluator#isAllowed(java.lang.String, java.lang.String, java.lang.String, org.springframework.security.core.Authentication) to produce an instance of the FilterInvocation.DummyRequest.

	@Override
	public boolean isAllowed(String contextPath, String uri, String method, Authentication authentication) {
		FilterInvocation filterInvocation = new FilterInvocation(contextPath, uri, method, this.servletContext);
		HttpServletRequest httpRequest = this.requestTransformer.transform(filterInvocation.getHttpRequest());
		AuthorizationDecision decision = this.authorizationManager.check(() -> authentication, httpRequest);
		return decision == null || decision.isGranted();
	}

When org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher#matches attempts to invoke jakarta.servlet.ServletRequest#getDispatcherType, the DummyRequest throws the UnsupportedOperationException via org.springframework.security.web.FilterInvocation.UnsupportedOperationExceptionInvocationHandler#invoke.

	@Override
	public boolean matches(HttpServletRequest request) {
		if (this.httpMethod != null && StringUtils.hasText(request.getMethod())
				&& this.httpMethod != HttpMethod.valueOf(request.getMethod())) {
			return false;
		}
		return this.dispatcherType == request.getDispatcherType();
	}
		@Override
		public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
			if (method.isDefault()) {
				return invokeDefaultMethod(proxy, method, args);
			}
			throw new UnsupportedOperationException(method + " is not supported");
		}

Is it possible that there was an oversight in the design of org.springframework.security.web.FilterInvocation.DummyRequestnested static class and its accompanying org.springframework.security.web.FilterInvocation.UnsupportedOperationExceptionInvocationHandler? The org.springframework.security.web.FilterInvocation.UnsupportedOperationExceptionInvocationHandler#invoke method has a Reflection check that verifies that the passed Method is a default method of DummyRequest's implemented interfaces. Between those implemented interfaces (jakarta.servlet.http.HttpServletRequest and jakarta.servlet.ServletRequest), there are only four default methods. Out of curiosity, what was the rationale for throwing an UnsupportedOperationException under that condition?


Interestingly, though, it's only using the JspAuthorizeTag's url attribute, not its access attribute, that triggers this problem.

If we use:
.dispatcherTypeMatchers(DispatcherType.FORWARD, DispatcherType.ERROR).permitAll()
Then we can't use:
<sec:authorize url="${myUrl}">
Looking at the source code of the JSP authorize tag though, this is fine:
<sec:authorize access="hasRole('MY_ROLE')">
This is because (from org.springframework.security.taglibs.authz.AbstractAuthorizeTag):

public boolean authorize() throws IOException {
    if (StringUtils.hasText(getAccess())) {
       return authorizeUsingAccessExpression();
    }
    if (StringUtils.hasText(getUrl())) {
       return authorizeUsingUrlCheck();
    }
    return false;
}

It's authorizeUsingUrlCheck() that invokes the problematic AuthorizationManagerWebInvocationPrivilegeEvaluator...

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: bug A general bug
Projects
None yet
Development

No branches or pull requests

6 participants