Support multiple RequestRejectedHandler beans by using CompositeRequestRejectedHandler

Author: Adam Ostrožlík

config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurity.java

@@ -95,8 +95,6 @@ public final class WebSecurity extends AbstractConfiguredSecurityBuilder<Filter,
 
 	private HttpFirewall httpFirewall;
 
-	private Collection<RequestRejectedHandler> requestRejectedHandlers;
-
 	private boolean debugEnabled;
 
 	private WebInvocationPrivilegeEvaluator privilegeEvaluator;
@@ -268,17 +266,6 @@ public WebSecurity postBuildAction(Runnable postBuildAction) {
 		return this;
 	}
 
-	/**
-	 * Sets the handlers to handle {@link org.springframework.security.web.firewall.RequestRejectedException}
-	 * @param requestRejectedHandlers
-	 * @return the {@link WebSecurity} for further customizations
-	 */
-	public WebSecurity requestRejectedHandlers(RequestRejectedHandler... requestRejectedHandlers) {
-		Assert.notNull(requestRejectedHandlers, "requestRejectedHandlers cannot be null");
-		this.requestRejectedHandlers = Arrays.asList(requestRejectedHandlers);
-		return this;
-	}
-
 	@Override
 	protected Filter performBuild() throws Exception {
 		Assert.state(!this.securityFilterChainBuilders.isEmpty(),

web/src/main/java/org/springframework/security/web/FilterChainProxy.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2004, 2005, 2006 Acegi Technology Pty Limited
+ * Copyright 2002-2020 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.
@@ -18,7 +18,6 @@
 
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
@@ -153,7 +152,7 @@ public class FilterChainProxy extends GenericFilterBean {
 
 	private HttpFirewall firewall = new StrictHttpFirewall();
 
-	private Collection<RequestRejectedHandler> requestRejectedHandlers = List.of(new DefaultRequestRejectedHandler());
+	private RequestRejectedHandler requestRejectedHandler = new DefaultRequestRejectedHandler();
 
 	public FilterChainProxy() {
 	}
@@ -184,25 +183,14 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
 			doFilterInternal(request, response, chain);
 		}
 		catch (RequestRejectedException ex) {
-			handleRequestRejectedException((HttpServletRequest) request, (HttpServletResponse) response, ex);
+			this.requestRejectedHandler.handle((HttpServletRequest) request, (HttpServletResponse) response, ex);
 		}
 		finally {
 			SecurityContextHolder.clearContext();
 			request.removeAttribute(FILTER_APPLIED);
 		}
 	}
 
-	private void handleRequestRejectedException(HttpServletRequest request, HttpServletResponse response, RequestRejectedException ex) throws ServletException, IOException {
-		var handlerOpt = this.requestRejectedHandlers.stream()
-			.filter(handler -> handler.shouldHandle(request))
-			.findFirst();
-		if (handlerOpt.isPresent()) {
-			handlerOpt.get().handle(request, response, ex);
-		} else {
-			new DefaultRequestRejectedHandler().handle(request, response, ex);
-		}
-	}
-
 	private void doFilterInternal(ServletRequest request, ServletResponse response, FilterChain chain)
 			throws IOException, ServletException {
 		FirewalledRequest firewallRequest = this.firewall.getFirewalledRequest((HttpServletRequest) request);
@@ -282,15 +270,12 @@ public void setFirewall(HttpFirewall firewall) {
 	/**
 	 * Sets the {@link RequestRejectedHandler} to be used for requests rejected by the
 	 * firewall.
-	 * @param requestRejectedHandlers the {@link RequestRejectedHandler}
+	 * @param requestRejectedHandler the {@link RequestRejectedHandler}
 	 * @since 5.2
 	 */
-	public void setRequestRejectedHandlers(Collection<RequestRejectedHandler> requestRejectedHandlers) {
-		Assert.notNull(requestRejectedHandlers, "requestRejectedHandlers may not be null");
-		if (requestRejectedHandlers.isEmpty()) {
-			return;
-		}
-		this.requestRejectedHandlers = requestRejectedHandlers;
+	public void setRequestRejectedHandler(RequestRejectedHandler requestRejectedHandler) {
+		Assert.notNull(requestRejectedHandler, "requestRejectedHandler may not be null");
+		this.requestRejectedHandler = requestRejectedHandler;
 	}
 
 	@Override

web/src/main/java/org/springframework/security/web/firewall/CompositeRequestRejectedHandler.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright 2002-2020 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.firewall;
+
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.springframework.security.web.header.HeaderWriter;
+import org.springframework.util.Assert;
+
+import java.io.IOException;
+import java.util.List;
+
+/**
+ * A {@link RequestRejectedHandler} that delegates to several other {@link RequestRejectedHandler}s.
+ *
+ * @author Adam Ostrožlík
+ * @since 5.7
+ */
+public class CompositeRequestRejectedHandler implements RequestRejectedHandler {
+
+	private final List<RequestRejectedHandler> requestRejectedhandlers;
+
+	/**
+	 * Creates a new instance.
+	 * @param requestRejectedhandlers the {@link RequestRejectedHandler} instances to handle {@link org.springframework.security.web.firewall.RequestRejectedException}
+	 */
+	public CompositeRequestRejectedHandler(List<RequestRejectedHandler> requestRejectedhandlers) {
+		Assert.notEmpty(requestRejectedhandlers, "requestRejectedhandlers cannot be empty");
+		this.requestRejectedhandlers = requestRejectedhandlers;
+	}
+
+	@Override
+	public void handle(HttpServletRequest request, HttpServletResponse response, RequestRejectedException requestRejectedException) throws IOException, ServletException {
+		for (RequestRejectedHandler requestRejectedhandler : requestRejectedhandlers) {
+			requestRejectedhandler.handle(request, response, requestRejectedException);
+		}
+	}
+}

web/src/main/java/org/springframework/security/web/firewall/RequestRejectedHandler.java

@@ -25,13 +25,8 @@
 /**
  * Used by {@link org.springframework.security.web.FilterChainProxy} to handle an
  * <code>RequestRejectedException</code>.
- * Supports multiple beans of this handler. The handler is chosen by the result of
- * <code>shouldHandle</code> method - <code>true</code> means it is chosen.
- * If there are more than one handler to choose from, the first matching is used.
- * If no suitable handler is found, the {@link DefaultRequestRejectedHandler} is used.
  *
  * @author Leonard Brünings
- * @author Adam Ostrožlík
  * @since 5.4
  */
 public interface RequestRejectedHandler {
@@ -47,12 +42,4 @@ public interface RequestRejectedHandler {
 	void handle(HttpServletRequest request, HttpServletResponse response,
 			RequestRejectedException requestRejectedException) throws IOException, ServletException;
 
-	/**
-	 * Determines if this handler should be invoked.
-	 * First available handler is invoked if there are multiple suitables ones.
-	 * @param request that resulted in an <code>RequestRejectedException</code>
-	 */
-	default boolean shouldHandle(HttpServletRequest request) {
-		return true;
-	}
 }

web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -49,7 +49,9 @@
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.BDDMockito.willAnswer;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
 
 /**
  * @author Luke Taylor
@@ -235,35 +237,19 @@ public void doFilterClearsSecurityContextHolderOnceOnForwards() throws Exception
 
 	@Test
 	public void setRequestRejectedHandlerDoesNotAcceptNull() {
-		assertThatIllegalArgumentException().isThrownBy(() -> this.fcp.setRequestRejectedHandlers(null));
+		assertThatIllegalArgumentException().isThrownBy(() -> this.fcp.setRequestRejectedHandler(null));
 	}
 
 	@Test
 	public void requestRejectedHandlerIsCalledIfFirewallThrowsRequestRejectedException() throws Exception {
 		HttpFirewall fw = mock(HttpFirewall.class);
 		RequestRejectedHandler rjh = mock(RequestRejectedHandler.class);
-		when(rjh.shouldHandle(this.request)).thenReturn(true);
 		this.fcp.setFirewall(fw);
-		this.fcp.setRequestRejectedHandlers(List.of(rjh));
+		this.fcp.setRequestRejectedHandler(rjh);
 		RequestRejectedException requestRejectedException = new RequestRejectedException("Contains illegal chars");
 		given(fw.getFirewalledRequest(this.request)).willThrow(requestRejectedException);
 		this.fcp.doFilter(this.request, this.response, this.chain);
 		verify(rjh).handle(eq(this.request), eq(this.response), eq((requestRejectedException)));
 	}
 
-	@Test
-	public void oneOfRequestRejectedHandlerIsCalledIfFirewallThrowsRequestRejectedException() throws Exception {
-		HttpFirewall fw = mock(HttpFirewall.class);
-		RequestRejectedHandler rjh1 = mock(RequestRejectedHandler.class);
-		RequestRejectedHandler rjh2 = mock(RequestRejectedHandler.class);
-		when(rjh1.shouldHandle(this.request)).thenReturn(false);
-		when(rjh2.shouldHandle(this.request)).thenReturn(true);
-		this.fcp.setFirewall(fw);
-		this.fcp.setRequestRejectedHandlers(List.of(rjh1, rjh2));
-		RequestRejectedException requestRejectedException = new RequestRejectedException("Contains illegal chars");
-		given(fw.getFirewalledRequest(this.request)).willThrow(requestRejectedException);
-		this.fcp.doFilter(this.request, this.response, this.chain);
-		verify(rjh2).handle(eq(this.request), eq(this.response), eq((requestRejectedException)));
-	}
-
 }