Merge branch '6.1.x' into 6.2.x

Add HandlerMappingIntrospector Caching Closes gh-14332
2026-08-04 01:07:02 +00:00 · 2023-12-14 16:09:58 -06:00
parent 0bd3b9e420 70dfb3d391
commit 6dd29520b0
10 changed files with 807 additions and 7 deletions
@@ -40,6 +40,8 @@ public final class AuthorizationManagerWebInvocationPrivilegeEvaluator

 	private ServletContext servletContext;

+	private HttpServletRequestTransformer requestTransformer = HttpServletRequestTransformer.IDENTITY;
+
 	public AuthorizationManagerWebInvocationPrivilegeEvaluator(
 			AuthorizationManager<HttpServletRequest> authorizationManager) {
 		Assert.notNull(authorizationManager, "authorizationManager cannot be null");
@@ -54,8 +56,8 @@ public final class AuthorizationManagerWebInvocationPrivilegeEvaluator
 	@Override
 	public boolean isAllowed(String contextPath, String uri, String method, Authentication authentication) {
 		FilterInvocation filterInvocation = new FilterInvocation(contextPath, uri, method, this.servletContext);
-		AuthorizationDecision decision = this.authorizationManager.check(() -> authentication,
-				filterInvocation.getHttpRequest());
+		HttpServletRequest httpRequest = this.requestTransformer.transform(filterInvocation.getHttpRequest());
+		AuthorizationDecision decision = this.authorizationManager.check(() -> authentication, httpRequest);
 		return decision == null || decision.isGranted();
 	}

@@ -64,4 +66,34 @@ public final class AuthorizationManagerWebInvocationPrivilegeEvaluator
 		this.servletContext = servletContext;
 	}

+	/**
+	 * Set a {@link HttpServletRequestTransformer} to be used prior to passing to the
+	 * {@link AuthorizationManager}.
+	 * @param requestTransformer the {@link HttpServletRequestTransformer} to use.
+	 */
+	public void setRequestTransformer(HttpServletRequestTransformer requestTransformer) {
+		Assert.notNull(requestTransformer, "requestTransformer cannot be null");
+		this.requestTransformer = requestTransformer;
+	}
+
+	/**
+	 * Used to transform the {@link HttpServletRequest} prior to passing it into the
+	 * {@link AuthorizationManager}.
+	 */
+	public interface HttpServletRequestTransformer {
+
+		HttpServletRequestTransformer IDENTITY = (request) -> request;
+
+		/**
+		 * Return the {@link HttpServletRequest} that is passed into the
+		 * {@link AuthorizationManager}
+		 * @param request the {@link HttpServletRequest} created by the
+		 * {@link WebInvocationPrivilegeEvaluator}
+		 * @return the {@link HttpServletRequest} that is passed into the
+		 * {@link AuthorizationManager}
+		 */
+		HttpServletRequest transform(HttpServletRequest request);
+
+	}
+
 }
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2002-2023 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.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.Map;
+
+import jakarta.servlet.DispatcherType;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletRequestWrapper;
+
+import org.springframework.util.Assert;
+import org.springframework.web.servlet.handler.HandlerMappingIntrospector;
+
+/**
+ * Transforms by passing it into
+ * {@link HandlerMappingIntrospector#setCache(HttpServletRequest)}. Before, it wraps the
+ * {@link HttpServletRequest} to ensure that the methods needed work since some methods by
+ * default throw {@link UnsupportedOperationException}.
+ *
+ * @author Rob Winch
+ */
+public class HandlerMappingIntrospectorRequestTransformer
+		implements AuthorizationManagerWebInvocationPrivilegeEvaluator.HttpServletRequestTransformer {
+
+	private final HandlerMappingIntrospector introspector;
+
+	public HandlerMappingIntrospectorRequestTransformer(HandlerMappingIntrospector introspector) {
+		Assert.notNull(introspector, "introspector canot be null");
+		this.introspector = introspector;
+	}
+
+	@Override
+	public HttpServletRequest transform(HttpServletRequest request) {
+		CacheableRequestWrapper cacheableRequest = new CacheableRequestWrapper(request);
+		this.introspector.setCache(cacheableRequest);
+		return cacheableRequest;
+	}
+
+	static final class CacheableRequestWrapper extends HttpServletRequestWrapper {
+
+		private final Map<String, Object> attributes = new HashMap<>();
+
+		/**
+		 * Constructs a request object wrapping the given request.
+		 * @param request the {@link HttpServletRequest} to be wrapped.
+		 * @throws IllegalArgumentException if the request is null
+		 */
+		CacheableRequestWrapper(HttpServletRequest request) {
+			super(request);
+		}
+
+		@Override
+		public DispatcherType getDispatcherType() {
+			return DispatcherType.REQUEST;
+		}
+
+		@Override
+		public Enumeration<String> getAttributeNames() {
+			return Collections.enumeration(this.attributes.keySet());
+		}
+
+		@Override
+		public Object getAttribute(String name) {
+			return this.attributes.get(name);
+		}
+
+		@Override
+		public void setAttribute(String name, Object o) {
+			this.attributes.put(name, o);
+		}
+
+		@Override
+		public void removeAttribute(String name) {
+			this.attributes.remove(name);
+		}
+
+	}
+
+}
@@ -25,14 +25,17 @@ import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;

+import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockServletContext;
 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 static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.verify;

@@ -45,6 +48,9 @@ class AuthorizationManagerWebInvocationPrivilegeEvaluatorTests {
 	@Mock
 	private AuthorizationManager<HttpServletRequest> authorizationManager;

+	@Mock
+	private HttpServletRequestTransformer requestTransformer;
+
 	@Test
 	void constructorWhenAuthorizationManagerNullThenIllegalArgument() {
 		assertThatIllegalArgumentException()
@@ -84,4 +90,20 @@ class AuthorizationManagerWebInvocationPrivilegeEvaluatorTests {
 		assertThat(captor.getValue().getServletContext()).isSameAs(servletContext);
 	}

+	@Test
+	void setRequestTransformerWhenNullThenIllegalArgumentException() {
+		assertThatIllegalArgumentException().isThrownBy(() -> this.privilegeEvaluator.setRequestTransformer(null));
+	}
+
+	@Test
+	void isAllowedWhenRequestTransformerThenUsesRequestTransformerResult() {
+		HttpServletRequest request = new MockHttpServletRequest();
+		given(this.requestTransformer.transform(any())).willReturn(request);
+		this.privilegeEvaluator.setRequestTransformer(this.requestTransformer);
+
+		this.privilegeEvaluator.isAllowed("/test", TestAuthentication.authenticatedUser());
+
+		verify(this.authorizationManager).check(any(), eq(request));
+	}
+
 }
@@ -0,0 +1,206 @@
+/*
+ * Copyright 2002-2023 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.Collections;
+
+import jakarta.servlet.DispatcherType;
+import jakarta.servlet.http.HttpServletRequest;
+import org.assertj.core.api.AssertionsForClassTypes;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.web.servlet.handler.HandlerMappingIntrospector;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+
+/**
+ * @author Rob Winch
+ */
+@ExtendWith(MockitoExtension.class)
+class HandlerMappingIntrospectorRequestTransformerTests {
+
+	@Mock
+	HandlerMappingIntrospector hmi;
+
+	HandlerMappingIntrospectorRequestTransformer transformer;
+
+	@BeforeEach
+	void setup() {
+		this.transformer = new HandlerMappingIntrospectorRequestTransformer(this.hmi);
+	}
+
+	@Test
+	void constructorWhenHmiIsNullThenIllegalArgumentException() {
+		AssertionsForClassTypes.assertThatExceptionOfType(IllegalArgumentException.class)
+			.isThrownBy(() -> new HandlerMappingIntrospectorRequestTransformer(null));
+	}
+
+	@Test
+	void transformThenNewRequestPassedToSetCache() {
+		MockHttpServletRequest request = new MockHttpServletRequest();
+
+		HttpServletRequest transformedRequest = this.transformer.transform(request);
+
+		ArgumentCaptor<HttpServletRequest> requestArg = ArgumentCaptor.forClass(HttpServletRequest.class);
+		verify(this.hmi).setCache(requestArg.capture());
+		assertThat(transformedRequest).isNotEqualTo(request);
+	}
+
+	@Test
+	void transformThenResultPassedToSetCache() {
+		MockHttpServletRequest request = new MockHttpServletRequest();
+
+		HttpServletRequest transformedRequest = this.transformer.transform(request);
+
+		ArgumentCaptor<HttpServletRequest> requestArg = ArgumentCaptor.forClass(HttpServletRequest.class);
+		verify(this.hmi).setCache(requestArg.capture());
+		assertThat(requestArg.getValue()).isEqualTo(transformedRequest);
+	}
+
+	/**
+	 * The request passed into the transformer does not allow interactions on certain
+	 * methods, we need to ensure that the methods used by
+	 * {@link HandlerMappingIntrospector#setCache(HttpServletRequest)} are overridden.
+	 */
+	@Test
+	void transformThenResultDoesNotDelegateToSetAttribute() {
+		HttpServletRequest request = mock(HttpServletRequest.class);
+
+		this.transformer.transform(request);
+
+		ArgumentCaptor<HttpServletRequest> requestArg = ArgumentCaptor.forClass(HttpServletRequest.class);
+		verify(this.hmi).setCache(requestArg.capture());
+		HttpServletRequest transformedRequest = requestArg.getValue();
+		String attrName = "any";
+		String attrValue = "value";
+		transformedRequest.setAttribute(attrName, attrValue);
+		verifyNoInteractions(request);
+		assertThat(transformedRequest.getAttribute(attrName)).isEqualTo(attrValue);
+	}
+
+	@Test
+	void transformThenSetAttributeWorks() {
+		HttpServletRequest request = mock(HttpServletRequest.class);
+
+		this.transformer.transform(request);
+
+		ArgumentCaptor<HttpServletRequest> requestArg = ArgumentCaptor.forClass(HttpServletRequest.class);
+		verify(this.hmi).setCache(requestArg.capture());
+		HttpServletRequest transformedRequest = requestArg.getValue();
+		String attrName = "any";
+		String attrValue = "value";
+		transformedRequest.setAttribute(attrName, attrValue);
+		assertThat(transformedRequest.getAttribute(attrName)).isEqualTo(attrValue);
+	}
+
+	/**
+	 * The request passed into the transformer does not allow interactions on certain
+	 * methods, we need to ensure that the methods used by
+	 * {@link HandlerMappingIntrospector#setCache(HttpServletRequest)} are overridden.
+	 */
+	@Test
+	void transformThenResultDoesNotDelegateToGetAttribute() {
+		HttpServletRequest request = mock(HttpServletRequest.class);
+
+		this.transformer.transform(request);
+
+		ArgumentCaptor<HttpServletRequest> requestArg = ArgumentCaptor.forClass(HttpServletRequest.class);
+		verify(this.hmi).setCache(requestArg.capture());
+		HttpServletRequest transformedRequest = requestArg.getValue();
+		transformedRequest.getAttribute("any");
+		verifyNoInteractions(request);
+	}
+
+	/**
+	 * The request passed into the transformer does not allow interactions on certain
+	 * methods, we need to ensure that the methods used by
+	 * {@link HandlerMappingIntrospector#setCache(HttpServletRequest)} are overridden.
+	 */
+	@Test
+	void transformThenResultDoesNotDelegateToGetAttributeNames() {
+		HttpServletRequest request = mock(HttpServletRequest.class);
+
+		this.transformer.transform(request);
+
+		ArgumentCaptor<HttpServletRequest> requestArg = ArgumentCaptor.forClass(HttpServletRequest.class);
+		verify(this.hmi).setCache(requestArg.capture());
+		HttpServletRequest transformedRequest = requestArg.getValue();
+		transformedRequest.getAttributeNames();
+		verifyNoInteractions(request);
+	}
+
+	@Test
+	void transformThenGetAttributeNamesWorks() {
+		HttpServletRequest request = mock(HttpServletRequest.class);
+
+		this.transformer.transform(request);
+
+		ArgumentCaptor<HttpServletRequest> requestArg = ArgumentCaptor.forClass(HttpServletRequest.class);
+		verify(this.hmi).setCache(requestArg.capture());
+		HttpServletRequest transformedRequest = requestArg.getValue();
+		String attrName = "any";
+		String attrValue = "value";
+		transformedRequest.setAttribute(attrName, attrValue);
+		assertThat(Collections.list(transformedRequest.getAttributeNames())).containsExactly(attrName);
+	}
+
+	/**
+	 * The request passed into the transformer does not allow interactions on certain
+	 * methods, we need to ensure that the methods used by
+	 * {@link HandlerMappingIntrospector#setCache(HttpServletRequest)} are overridden.
+	 */
+	@Test
+	void transformThenResultDoesNotDelegateToRemoveAttribute() {
+		HttpServletRequest request = mock(HttpServletRequest.class);
+
+		this.transformer.transform(request);
+
+		ArgumentCaptor<HttpServletRequest> requestArg = ArgumentCaptor.forClass(HttpServletRequest.class);
+		verify(this.hmi).setCache(requestArg.capture());
+		HttpServletRequest transformedRequest = requestArg.getValue();
+		transformedRequest.removeAttribute("any");
+		verifyNoInteractions(request);
+	}
+
+	/**
+	 * The request passed into the transformer does not allow interactions on certain
+	 * methods, we need to ensure that the methods used by
+	 * {@link HandlerMappingIntrospector#setCache(HttpServletRequest)} are overridden.
+	 */
+	@Test
+	void transformThenResultDoesNotDelegateToGetDispatcherType() {
+		HttpServletRequest request = mock(HttpServletRequest.class);
+
+		this.transformer.transform(request);
+
+		ArgumentCaptor<HttpServletRequest> requestArg = ArgumentCaptor.forClass(HttpServletRequest.class);
+		verify(this.hmi).setCache(requestArg.capture());
+		HttpServletRequest transformedRequest = requestArg.getValue();
+		assertThat(transformedRequest.getDispatcherType()).isEqualTo(DispatcherType.REQUEST);
+		verifyNoInteractions(request);
+	}
+
+}