Deprecate HandlerMappingIntrospectorRequestTransformer

Closes gh-16536
This commit is contained in:
Josh Cummings
2025-04-03 17:09:19 -06:00
parent 1fb3fc80f9
commit f93a7a2f85
13 changed files with 540 additions and 116 deletions

View File

@@ -46,7 +46,6 @@ import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.util.Assert;
import org.springframework.web.filter.DelegatingFilterProxy;
import org.springframework.web.filter.GenericFilterBean;
import org.springframework.web.filter.ServletRequestPathFilter;
/**
* Delegates {@code Filter} requests to a list of Spring-managed filter beans. As of
@@ -163,8 +162,6 @@ public class FilterChainProxy extends GenericFilterBean {
private FilterChainDecorator filterChainDecorator = new VirtualFilterChainDecorator();
private Filter springWebFilter = new ServletRequestPathFilter();
public FilterChainProxy() {
}
@@ -213,29 +210,27 @@ public class FilterChainProxy extends GenericFilterBean {
throws IOException, ServletException {
FirewalledRequest firewallRequest = this.firewall.getFirewalledRequest((HttpServletRequest) request);
HttpServletResponse firewallResponse = this.firewall.getFirewalledResponse((HttpServletResponse) response);
this.springWebFilter.doFilter(firewallRequest, firewallResponse, (r, s) -> {
List<Filter> filters = getFilters(firewallRequest);
if (filters == null || filters.isEmpty()) {
if (logger.isTraceEnabled()) {
logger.trace(LogMessage.of(() -> "No security for " + requestLine(firewallRequest)));
}
firewallRequest.reset();
this.filterChainDecorator.decorate(chain).doFilter(firewallRequest, firewallResponse);
return;
List<Filter> filters = getFilters(firewallRequest);
if (filters == null || filters.isEmpty()) {
if (logger.isTraceEnabled()) {
logger.trace(LogMessage.of(() -> "No security for " + requestLine(firewallRequest)));
}
firewallRequest.reset();
this.filterChainDecorator.decorate(chain).doFilter(firewallRequest, firewallResponse);
return;
}
if (logger.isDebugEnabled()) {
logger.debug(LogMessage.of(() -> "Securing " + requestLine(firewallRequest)));
}
FilterChain reset = (req, res) -> {
if (logger.isDebugEnabled()) {
logger.debug(LogMessage.of(() -> "Securing " + requestLine(firewallRequest)));
logger.debug(LogMessage.of(() -> "Secured " + requestLine(firewallRequest)));
}
FilterChain reset = (req, res) -> {
if (logger.isDebugEnabled()) {
logger.debug(LogMessage.of(() -> "Secured " + requestLine(firewallRequest)));
}
// Deactivate path stripping as we exit the security filter chain
firewallRequest.reset();
chain.doFilter(req, res);
};
this.filterChainDecorator.decorate(reset, filters).doFilter(firewallRequest, firewallResponse);
});
// Deactivate path stripping as we exit the security filter chain
firewallRequest.reset();
chain.doFilter(req, res);
};
this.filterChainDecorator.decorate(reset, filters).doFilter(firewallRequest, firewallResponse);
}
/**

View File

@@ -35,7 +35,9 @@ import org.springframework.web.servlet.handler.HandlerMappingIntrospector;
* default throw {@link UnsupportedOperationException}.
*
* @author Rob Winch
* @deprecated please use {@link PathPatternRequestTransformer} instead
*/
@Deprecated(forRemoval = true)
public class HandlerMappingIntrospectorRequestTransformer
implements AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer {

View File

@@ -0,0 +1,70 @@
/*
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.security.web.access;
import java.util.HashMap;
import java.util.Map;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletRequestWrapper;
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
import org.springframework.web.util.ServletRequestPathUtils;
/**
* Prepares the privilege evaluator's request for {@link PathPatternRequestMatcher}
* authorization rules.
*
* @author Josh Cummings
* @since 6.5
*/
public final class PathPatternRequestTransformer
implements AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer {
@Override
public HttpServletRequest transform(HttpServletRequest request) {
HttpServletRequest wrapped = new AttributesSupportingHttpServletRequest(request);
ServletRequestPathUtils.parseAndCache(wrapped);
return wrapped;
}
private static final class AttributesSupportingHttpServletRequest extends HttpServletRequestWrapper {
private final Map<String, Object> attributes = new HashMap<>();
AttributesSupportingHttpServletRequest(HttpServletRequest request) {
super(request);
}
@Override
public Object getAttribute(String name) {
return this.attributes.get(name);
}
@Override
public void setAttribute(String name, Object value) {
this.attributes.put(name, value);
}
@Override
public void removeAttribute(String name) {
this.attributes.remove(name);
}
}
}

View File

@@ -17,20 +17,17 @@
package org.springframework.security.web.access;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import jakarta.servlet.ServletContext;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletRequestWrapper;
import org.springframework.security.authorization.AuthorizationManager;
import org.springframework.security.core.Authentication;
import org.springframework.security.web.FilterInvocation;
import org.springframework.security.web.util.matcher.RequestMatcherEntry;
import org.springframework.util.Assert;
import org.springframework.web.context.ServletContextAware;
import org.springframework.web.util.ServletRequestPathUtils;
/**
* A {@link WebInvocationPrivilegeEvaluator} which delegates to a list of
@@ -39,7 +36,11 @@ import org.springframework.web.util.ServletRequestPathUtils;
*
* @author Marcus Da Coregio
* @since 5.5.5
* @deprecated please use {@link AuthorizationManagerWebInvocationPrivilegeEvaluator} and
* adapt any delegate {@link WebInvocationPrivilegeEvaluator}s into
* {@link AuthorizationManager}s
*/
@Deprecated
public final class RequestMatcherDelegatingWebInvocationPrivilegeEvaluator
implements WebInvocationPrivilegeEvaluator, ServletContextAware {
@@ -120,8 +121,7 @@ public final class RequestMatcherDelegatingWebInvocationPrivilegeEvaluator
private List<WebInvocationPrivilegeEvaluator> getDelegate(String contextPath, String uri, String method) {
FilterInvocation filterInvocation = new FilterInvocation(contextPath, uri, method, this.servletContext);
HttpServletRequest request = new AttributesSupportingHttpServletRequest(filterInvocation.getHttpRequest());
ServletRequestPathUtils.parseAndCache(request);
HttpServletRequest request = filterInvocation.getHttpRequest();
for (RequestMatcherEntry<List<WebInvocationPrivilegeEvaluator>> delegate : this.delegates) {
if (delegate.getRequestMatcher().matches(request)) {
return delegate.getEntry();
@@ -135,29 +135,4 @@ public final class RequestMatcherDelegatingWebInvocationPrivilegeEvaluator
this.servletContext = servletContext;
}
private static final class AttributesSupportingHttpServletRequest extends HttpServletRequestWrapper {
private final Map<String, Object> attributes = new HashMap<>();
AttributesSupportingHttpServletRequest(HttpServletRequest request) {
super(request);
}
@Override
public Object getAttribute(String name) {
return this.attributes.get(name);
}
@Override
public void setAttribute(String name, Object value) {
this.attributes.put(name, value);
}
@Override
public void removeAttribute(String name) {
this.attributes.remove(name);
}
}
}

View File

@@ -31,6 +31,8 @@ import org.springframework.security.authentication.TestAuthentication;
import org.springframework.security.authorization.AuthorizationDecision;
import org.springframework.security.authorization.AuthorizationManager;
import org.springframework.security.web.access.AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer;
import org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager;
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -111,4 +113,19 @@ class AuthorizationManagerWebInvocationPrivilegeEvaluatorTests {
verify(this.authorizationManager).check(any(), eq(request));
}
// gh-16771
@Test
void isAllowedWhenInvokesDelegateThenCachesRequestPath() {
RequestMatcherDelegatingAuthorizationManager authorizationManager = RequestMatcherDelegatingAuthorizationManager
.builder()
.add(PathPatternRequestMatcher.withDefaults().matcher("/test/**"),
(authentication, context) -> this.authorizationManager.check(authentication, context.getRequest()))
.build();
AuthorizationManagerWebInvocationPrivilegeEvaluator privilegeEvaluator = new AuthorizationManagerWebInvocationPrivilegeEvaluator(
authorizationManager);
privilegeEvaluator.setRequestTransformer(new PathPatternRequestTransformer());
privilegeEvaluator.isAllowed("/test", TestAuthentication.authenticatedUser());
verify(this.authorizationManager).check(any(), any());
}
}

View File

@@ -22,16 +22,12 @@ import jakarta.servlet.http.HttpServletRequest;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.springframework.mock.web.MockServletContext;
import org.springframework.security.authentication.TestingAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcherEntry;
import org.springframework.web.util.ServletRequestPathUtils;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -178,19 +174,6 @@ class RequestMatcherDelegatingWebInvocationPrivilegeEvaluatorTests {
.withMessageContaining("requestMatcher cannot be null");
}
// gh-16771
@Test
void isAllowedWhenInvokesDelegateThenCachesRequestPath() {
PathPatternRequestMatcher path = PathPatternRequestMatcher.withDefaults().matcher("/path/**");
PathPatternRequestMatcher any = PathPatternRequestMatcher.withDefaults().matcher("/**");
WebInvocationPrivilegeEvaluator delegating = evaluator(deny(path), deny(any));
try (MockedStatic<ServletRequestPathUtils> utils = Mockito.mockStatic(ServletRequestPathUtils.class,
Mockito.CALLS_REAL_METHODS)) {
delegating.isAllowed("/uri", null);
utils.verify(() -> ServletRequestPathUtils.parseAndCache(any()), times(1));
}
}
@SuppressWarnings({ "rawtypes", "unchecked" })
private RequestMatcherDelegatingWebInvocationPrivilegeEvaluator evaluator(RequestMatcherEntry... entries) {
return new RequestMatcherDelegatingWebInvocationPrivilegeEvaluator(List.of(entries));