Request Cache supports matchingRequestParameterName

Closes gh-7157 gh-11453

Author: rwinch

Diff for: web/src/main/java/org/springframework/security/web/jackson2/DefaultSavedRequestMixin.java

@@ -17,6 +17,7 @@
 package org.springframework.security.web.jackson2;
 
 import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonTypeInfo;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 
@@ -43,4 +44,7 @@
 @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY, getterVisibility = JsonAutoDetect.Visibility.NONE)
 abstract class DefaultSavedRequestMixin {
 
+	@JsonInclude(JsonInclude.Include.NON_NULL)
+	String matchingRequestParameterName;
+
 }

Diff for: web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java

@@ -97,8 +97,15 @@ public class DefaultSavedRequest implements SavedRequest {
 
 	private final int serverPort;
 
-	@SuppressWarnings("unchecked")
+	private final String matchingRequestParameterName;
+
 	public DefaultSavedRequest(HttpServletRequest request, PortResolver portResolver) {
+		this(request, portResolver, null);
+	}
+
+	@SuppressWarnings("unchecked")
+	public DefaultSavedRequest(HttpServletRequest request, PortResolver portResolver,
+			String matchingRequestParameterName) {
 		Assert.notNull(request, "Request required");
 		Assert.notNull(portResolver, "PortResolver required");
 		// Cookies
@@ -131,6 +138,7 @@ public DefaultSavedRequest(HttpServletRequest request, PortResolver portResolver
 		this.serverName = request.getServerName();
 		this.contextPath = request.getContextPath();
 		this.servletPath = request.getServletPath();
+		this.matchingRequestParameterName = matchingRequestParameterName;
 	}
 
 	/**
@@ -147,6 +155,7 @@ private DefaultSavedRequest(Builder builder) {
 		this.serverName = builder.serverName;
 		this.servletPath = builder.servletPath;
 		this.serverPort = builder.serverPort;
+		this.matchingRequestParameterName = builder.matchingRequestParameterName;
 	}
 
 	/**
@@ -264,8 +273,9 @@ public List<Cookie> getCookies() {
 	 */
 	@Override
 	public String getRedirectUrl() {
+		String queryString = createQueryString(this.queryString, this.matchingRequestParameterName);
 		return UrlUtils.buildFullRequestUrl(this.scheme, this.serverName, this.serverPort, this.requestURI,
-				this.queryString);
+				queryString);
 	}
 
 	@Override
@@ -353,6 +363,19 @@ public String toString() {
 		return "DefaultSavedRequest [" + getRedirectUrl() + "]";
 	}
 
+	private static String createQueryString(String queryString, String matchingRequestParameterName) {
+		if (matchingRequestParameterName == null) {
+			return queryString;
+		}
+		if (queryString == null || queryString.length() == 0) {
+			return matchingRequestParameterName;
+		}
+		if (queryString.endsWith("&")) {
+			return queryString + matchingRequestParameterName;
+		}
+		return queryString + "&" + matchingRequestParameterName;
+	}
+
 	/**
 	 * @since 4.2
 	 */
@@ -388,6 +411,8 @@ public static class Builder {
 
 		private int serverPort = 80;
 
+		private String matchingRequestParameterName;
+
 		public Builder setCookies(List<SavedCookie> cookies) {
 			this.cookies = cookies;
 			return this;
@@ -458,6 +483,11 @@ public Builder setServerPort(int serverPort) {
 			return this;
 		}
 
+		public Builder setMatchingRequestParameterName(String matchingRequestParameterName) {
+			this.matchingRequestParameterName = matchingRequestParameterName;
+			return this;
+		}
+
 		public DefaultSavedRequest build() {
 			DefaultSavedRequest savedRequest = new DefaultSavedRequest(this);
 			if (!ObjectUtils.isEmpty(this.cookies)) {

Diff for: web/src/main/java/org/springframework/security/web/savedrequest/HttpSessionRequestCache.java

@@ -52,6 +52,8 @@ public class HttpSessionRequestCache implements RequestCache {
 
 	private String sessionAttrName = SAVED_REQUEST;
 
+	private String matchingRequestParameterName;
+
 	/**
 	 * Stores the current request, provided the configuration properties allow it.
 	 */
@@ -64,7 +66,8 @@ public void saveRequest(HttpServletRequest request, HttpServletResponse response
 			}
 			return;
 		}
-		DefaultSavedRequest savedRequest = new DefaultSavedRequest(request, this.portResolver);
+		DefaultSavedRequest savedRequest = new DefaultSavedRequest(request, this.portResolver,
+				this.matchingRequestParameterName);
 		if (this.createSessionAllowed || request.getSession(false) != null) {
 			// Store the HTTP request itself. Used by
 			// AbstractAuthenticationProcessingFilter
@@ -96,6 +99,12 @@ public void removeRequest(HttpServletRequest currentRequest, HttpServletResponse
 
 	@Override
 	public HttpServletRequest getMatchingRequest(HttpServletRequest request, HttpServletResponse response) {
+		if (this.matchingRequestParameterName != null
+				&& request.getParameter(this.matchingRequestParameterName) == null) {
+			this.logger.trace(
+					"matchingRequestParameterName is required for getMatchingRequest to lookup a value, but not provided");
+			return null;
+		}
 		SavedRequest saved = getRequest(request, response);
 		if (saved == null) {
 			this.logger.trace("No saved request");
@@ -161,4 +170,16 @@ public void setSessionAttrName(String sessionAttrName) {
 		this.sessionAttrName = sessionAttrName;
 	}
 
+	/**
+	 * Specify the name of a query parameter that is added to the URL that specifies the
+	 * request cache should be checked in
+	 * {@link #getMatchingRequest(HttpServletRequest, HttpServletResponse)}
+	 * @param matchingRequestParameterName the parameter name that must be in the request
+	 * for {@link #getMatchingRequest(HttpServletRequest, HttpServletResponse)} to check
+	 * the session.
+	 */
+	public void setMatchingRequestParameterName(String matchingRequestParameterName) {
+		this.matchingRequestParameterName = matchingRequestParameterName;
+	}
+
 }

Diff for: web/src/main/java/org/springframework/security/web/server/savedrequest/WebSessionServerRequestCache.java

@@ -34,8 +34,10 @@
 import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher.MatchResult;
 import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatchers;
 import org.springframework.util.Assert;
+import org.springframework.util.MultiValueMap;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.WebSession;
+import org.springframework.web.util.UriComponentsBuilder;
 
 /**
  * An implementation of {@link ServerRequestCache} that saves the
@@ -57,6 +59,8 @@ public class WebSessionServerRequestCache implements ServerRequestCache {
 
 	private ServerWebExchangeMatcher saveRequestMatcher = createDefaultRequestMacher();
 
+	private String matchingRequestParameterName;
+
 	/**
 	 * Sets the matcher to determine if the request should be saved. The default is to
 	 * match on any GET request.
@@ -81,19 +85,53 @@ public Mono<Void> saveRequest(ServerWebExchange exchange) {
 	public Mono<URI> getRedirectUri(ServerWebExchange exchange) {
 		return exchange.getSession()
 				.flatMap((session) -> Mono.justOrEmpty(session.<String>getAttribute(this.sessionAttrName)))
-				.map(URI::create);
+				.map(this::createRedirectUri);
 	}
 
 	@Override
 	public Mono<ServerHttpRequest> removeMatchingRequest(ServerWebExchange exchange) {
+		MultiValueMap<String, String> queryParams = exchange.getRequest().getQueryParams();
+		if (this.matchingRequestParameterName != null && !queryParams.containsKey(this.matchingRequestParameterName)) {
+			this.logger.trace(
+					"matchingRequestParameterName is required for getMatchingRequest to lookup a value, but not provided");
+			return Mono.empty();
+		}
+		ServerHttpRequest request = stripMatchingRequestParameterName(exchange.getRequest());
 		return exchange.getSession().map(WebSession::getAttributes).filter((attributes) -> {
-			String requestPath = pathInApplication(exchange.getRequest());
+			String requestPath = pathInApplication(request);
 			boolean removed = attributes.remove(this.sessionAttrName, requestPath);
 			if (removed) {
 				logger.debug(LogMessage.format("Request removed from WebSession: '%s'", requestPath));
 			}
 			return removed;
-		}).map((attributes) -> exchange.getRequest());
+		}).map((attributes) -> request);
+	}
+
+	/**
+	 * Specify the name of a query parameter that is added to the URL in
+	 * {@link #getRedirectUri(ServerWebExchange)} and is required for
+	 * {@link #removeMatchingRequest(ServerWebExchange)} to look up the
+	 * {@link ServerHttpRequest}.
+	 * @param matchingRequestParameterName the parameter name that must be in the request
+	 * for {@link #removeMatchingRequest(ServerWebExchange)} to check the session.
+	 */
+	public void setMatchingRequestParameterName(String matchingRequestParameterName) {
+		this.matchingRequestParameterName = matchingRequestParameterName;
+	}
+
+	private ServerHttpRequest stripMatchingRequestParameterName(ServerHttpRequest request) {
+		if (this.matchingRequestParameterName == null) {
+			return request;
+		}
+		// @formatter:off
+		URI uri = UriComponentsBuilder.fromUri(request.getURI())
+				.replaceQueryParam(this.matchingRequestParameterName)
+				.build()
+				.toUri();
+		return request.mutate()
+				.uri(uri)
+				.build();
+		// @formatter:on
 	}
 
 	private static String pathInApplication(ServerHttpRequest request) {
@@ -102,6 +140,18 @@ private static String pathInApplication(ServerHttpRequest request) {
 		return path + ((query != null) ? "?" + query : "");
 	}
 
+	private URI createRedirectUri(String uri) {
+		if (this.matchingRequestParameterName == null) {
+			return URI.create(uri);
+		}
+		// @formatter:off
+		return UriComponentsBuilder.fromUriString(uri)
+				.queryParam(this.matchingRequestParameterName)
+				.build()
+				.toUri();
+		// @formatter:on
+	}
+
 	private static ServerWebExchangeMatcher createDefaultRequestMacher() {
 		ServerWebExchangeMatcher get = ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, "/**");
 		ServerWebExchangeMatcher notFavicon = new NegatedServerWebExchangeMatcher(
@@ -111,4 +161,17 @@ private static ServerWebExchangeMatcher createDefaultRequestMacher() {
 		return new AndServerWebExchangeMatcher(get, notFavicon, html);
 	}
 
+	private static String createQueryString(String queryString, String matchingRequestParameterName) {
+		if (matchingRequestParameterName == null) {
+			return queryString;
+		}
+		if (queryString == null || queryString.length() == 0) {
+			return matchingRequestParameterName;
+		}
+		if (queryString.endsWith("&")) {
+			return queryString + matchingRequestParameterName;
+		}
+		return queryString + "&" + matchingRequestParameterName;
+	}
+
 }

Diff for: web/src/test/java/org/springframework/security/web/jackson2/DefaultSavedRequestMixinTests.java

@@ -55,22 +55,42 @@ public class DefaultSavedRequestMixinTests extends AbstractMixinTests {
 	// @formatter:on
 	// @formatter:off
 	private static final String REQUEST_JSON = "{" +
-		"\"@class\": \"org.springframework.security.web.savedrequest.DefaultSavedRequest\", "
-		+ "\"cookies\": " + COOKIES_JSON + ","
-		+ "\"locales\": [\"java.util.ArrayList\", [\"en\"]], "
-		+ "\"headers\": {\"@class\": \"java.util.TreeMap\", \"x-auth-token\": [\"java.util.ArrayList\", [\"12\"]]}, "
-		+ "\"parameters\": {\"@class\": \"java.util.TreeMap\"},"
-		+ "\"contextPath\": \"\", "
-		+ "\"method\": \"\", "
-		+ "\"pathInfo\": null, "
-		+ "\"queryString\": null, "
-		+ "\"requestURI\": \"\", "
-		+ "\"requestURL\": \"http://localhost\", "
-		+ "\"scheme\": \"http\", "
-		+ "\"serverName\": \"localhost\", "
-		+ "\"servletPath\": \"\", "
-		+ "\"serverPort\": 80"
-	+ "}";
+			"\"@class\": \"org.springframework.security.web.savedrequest.DefaultSavedRequest\", "
+			+ "\"cookies\": " + COOKIES_JSON + ","
+			+ "\"locales\": [\"java.util.ArrayList\", [\"en\"]], "
+			+ "\"headers\": {\"@class\": \"java.util.TreeMap\", \"x-auth-token\": [\"java.util.ArrayList\", [\"12\"]]}, "
+			+ "\"parameters\": {\"@class\": \"java.util.TreeMap\"},"
+			+ "\"contextPath\": \"\", "
+			+ "\"method\": \"\", "
+			+ "\"pathInfo\": null, "
+			+ "\"queryString\": null, "
+			+ "\"requestURI\": \"\", "
+			+ "\"requestURL\": \"http://localhost\", "
+			+ "\"scheme\": \"http\", "
+			+ "\"serverName\": \"localhost\", "
+			+ "\"servletPath\": \"\", "
+			+ "\"serverPort\": 80"
+			+ "}";
+	// @formatter:on
+	// @formatter:off
+	private static final String REQUEST_WITH_MATCHING_REQUEST_PARAM_NAME_JSON = "{" +
+			"\"@class\": \"org.springframework.security.web.savedrequest.DefaultSavedRequest\", "
+			+ "\"cookies\": " + COOKIES_JSON + ","
+			+ "\"locales\": [\"java.util.ArrayList\", [\"en\"]], "
+			+ "\"headers\": {\"@class\": \"java.util.TreeMap\", \"x-auth-token\": [\"java.util.ArrayList\", [\"12\"]]}, "
+			+ "\"parameters\": {\"@class\": \"java.util.TreeMap\"},"
+			+ "\"contextPath\": \"\", "
+			+ "\"method\": \"\", "
+			+ "\"pathInfo\": null, "
+			+ "\"queryString\": null, "
+			+ "\"requestURI\": \"\", "
+			+ "\"requestURL\": \"http://localhost\", "
+			+ "\"scheme\": \"http\", "
+			+ "\"serverName\": \"localhost\", "
+			+ "\"servletPath\": \"\", "
+			+ "\"serverPort\": 80, "
+			+ "\"matchingRequestParameterName\": \"success\""
+			+ "}";
 	// @formatter:on
 	@Test
 	public void matchRequestBuildWithConstructorAndBuilder() {
@@ -125,4 +145,17 @@ public void deserializeDefaultSavedRequest() throws IOException {
 		assertThat(request.getHeaderValues("x-auth-token")).hasSize(1).contains("12");
 	}
 
+	@Test
+	public void deserializeWhenMatchingRequestParameterNameThenRedirectUrlContainsParam() throws IOException {
+		DefaultSavedRequest request = (DefaultSavedRequest) this.mapper
+				.readValue(REQUEST_WITH_MATCHING_REQUEST_PARAM_NAME_JSON, Object.class);
+		assertThat(request.getRedirectUrl()).isEqualTo("http://localhost?success");
+	}
+
+	@Test
+	public void deserializeWhenNullMatchingRequestParameterNameThenRedirectUrlDoesNotContainParam() throws IOException {
+		DefaultSavedRequest request = (DefaultSavedRequest) this.mapper.readValue(REQUEST_JSON, Object.class);
+		assertThat(request.getRedirectUrl()).isEqualTo("http://localhost");
+	}
+
 }