Store one request by default in WebSessionOAuth2ServerAuthorizationRequestRepository

Related to gh-9649
Closes gh-9857

Author: sjohnr

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/server/WebSessionOAuth2ServerAuthorizationRequestRepository.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -34,6 +34,7 @@
  * {@link OAuth2AuthorizationRequest} in the {@code WebSession}.
  *
  * @author Rob Winch
+ * @author Steve Riesenberg
  * @since 5.1
  * @see AuthorizationRequestRepository
  * @see OAuth2AuthorizationRequest
@@ -46,14 +47,18 @@ public final class WebSessionOAuth2ServerAuthorizationRequestRepository
 
 	private final String sessionAttributeName = DEFAULT_AUTHORIZATION_REQUEST_ATTR_NAME;
 
+	private boolean allowMultipleAuthorizationRequests;
+
 	@Override
 	public Mono<OAuth2AuthorizationRequest> loadAuthorizationRequest(ServerWebExchange exchange) {
 		String state = getStateParameter(exchange);
 		if (state == null) {
 			return Mono.empty();
 		}
 		// @formatter:off
-		return getStateToAuthorizationRequest(exchange)
+		return this.getSessionAttributes(exchange)
+				.filter((sessionAttrs) -> sessionAttrs.containsKey(this.sessionAttributeName))
+				.map(this::getAuthorizationRequests)
 				.filter((stateToAuthorizationRequest) -> stateToAuthorizationRequest.containsKey(state))
 				.map((stateToAuthorizationRequest) -> stateToAuthorizationRequest.get(state));
 		// @formatter:on
@@ -63,10 +68,20 @@ public Mono<OAuth2AuthorizationRequest> loadAuthorizationRequest(ServerWebExchan
 	public Mono<Void> saveAuthorizationRequest(OAuth2AuthorizationRequest authorizationRequest,
 			ServerWebExchange exchange) {
 		Assert.notNull(authorizationRequest, "authorizationRequest cannot be null");
+		Assert.notNull(exchange, "exchange cannot be null");
 		// @formatter:off
-		return saveStateToAuthorizationRequest(exchange)
-				.doOnNext((stateToAuthorizationRequest) -> stateToAuthorizationRequest
-						.put(authorizationRequest.getState(), authorizationRequest))
+		return getSessionAttributes(exchange)
+				.doOnNext((sessionAttrs) -> {
+					if (this.allowMultipleAuthorizationRequests) {
+						Map<String, OAuth2AuthorizationRequest> authorizationRequests = this.getAuthorizationRequests(
+								sessionAttrs);
+						authorizationRequests.put(authorizationRequest.getState(), authorizationRequest);
+						sessionAttrs.put(this.sessionAttributeName, authorizationRequests);
+					}
+					else {
+						sessionAttrs.put(this.sessionAttributeName, authorizationRequest);
+					}
+				})
 				.then();
 		// @formatter:on
 	}
@@ -78,30 +93,21 @@ public Mono<OAuth2AuthorizationRequest> removeAuthorizationRequest(ServerWebExch
 			return Mono.empty();
 		}
 		// @formatter:off
-		return exchange.getSession()
-				.map(WebSession::getAttributes)
-				.handle((sessionAttrs, sink) -> {
-					Map<String, OAuth2AuthorizationRequest> stateToAuthzRequest = sessionAttrsMapStateToAuthorizationRequest(
+		return getSessionAttributes(exchange)
+				.flatMap((sessionAttrs) -> {
+					Map<String, OAuth2AuthorizationRequest> authorizationRequests = this.getAuthorizationRequests(
 							sessionAttrs);
-					if (stateToAuthzRequest == null) {
-						sink.complete();
-						return;
-					}
-					OAuth2AuthorizationRequest removedValue = stateToAuthzRequest.remove(state);
-					if (stateToAuthzRequest.isEmpty()) {
+					OAuth2AuthorizationRequest originalRequest = authorizationRequests.remove(state);
+					if (authorizationRequests.isEmpty()) {
 						sessionAttrs.remove(this.sessionAttributeName);
 					}
-					else if (removedValue != null) {
-						// gh-7327 Overwrite the existing Map to ensure the state is saved for
-						// distributed sessions
-						sessionAttrs.put(this.sessionAttributeName, stateToAuthzRequest);
-					}
-					if (removedValue == null) {
-						sink.complete();
+					else if (authorizationRequests.size() == 1) {
+						sessionAttrs.put(this.sessionAttributeName, authorizationRequests.values().iterator().next());
 					}
 					else {
-						sink.next(removedValue);
+						sessionAttrs.put(this.sessionAttributeName, authorizationRequests);
 					}
+					return Mono.justOrEmpty(originalRequest);
 				});
 		// @formatter:on
 	}
@@ -120,36 +126,41 @@ private Mono<Map<String, Object>> getSessionAttributes(ServerWebExchange exchang
 		return exchange.getSession().map(WebSession::getAttributes);
 	}
 
-	private Mono<Map<String, OAuth2AuthorizationRequest>> getStateToAuthorizationRequest(ServerWebExchange exchange) {
-		Assert.notNull(exchange, "exchange cannot be null");
-
-		// @formatter:off
-		return getSessionAttributes(exchange)
-				.flatMap((sessionAttrs) -> Mono.justOrEmpty(this.sessionAttrsMapStateToAuthorizationRequest(sessionAttrs)));
-		// @formatter:on
-	}
-
-	private Mono<Map<String, OAuth2AuthorizationRequest>> saveStateToAuthorizationRequest(ServerWebExchange exchange) {
-		Assert.notNull(exchange, "exchange cannot be null");
-		// @formatter:off
-		return getSessionAttributes(exchange)
-				.doOnNext((sessionAttrs) -> {
-					Object stateToAuthzRequest = sessionAttrs.get(this.sessionAttributeName);
-					if (stateToAuthzRequest == null) {
-						stateToAuthzRequest = new HashMap<String, OAuth2AuthorizationRequest>();
-					}
-					// No matter stateToAuthzRequest was in session or not, we should always put
-					// it into session again
-					// in case of redis or hazelcast session. #6215
-					sessionAttrs.put(this.sessionAttributeName, stateToAuthzRequest);
-				})
-				.flatMap((sessionAttrs) -> Mono.justOrEmpty(this.sessionAttrsMapStateToAuthorizationRequest(sessionAttrs)));
-		// @formatter:on
+	private Map<String, OAuth2AuthorizationRequest> getAuthorizationRequests(Map<String, Object> sessionAttrs) {
+		Object sessionAttributeValue = sessionAttrs.get(this.sessionAttributeName);
+		if (sessionAttributeValue == null) {
+			return new HashMap<>();
+		}
+		else if (sessionAttributeValue instanceof OAuth2AuthorizationRequest) {
+			OAuth2AuthorizationRequest oauth2AuthorizationRequest = (OAuth2AuthorizationRequest) sessionAttributeValue;
+			Map<String, OAuth2AuthorizationRequest> authorizationRequests = new HashMap<>(1);
+			authorizationRequests.put(oauth2AuthorizationRequest.getState(), oauth2AuthorizationRequest);
+			return authorizationRequests;
+		}
+		else if (sessionAttributeValue instanceof Map) {
+			@SuppressWarnings("unchecked")
+			Map<String, OAuth2AuthorizationRequest> authorizationRequests = (Map<String, OAuth2AuthorizationRequest>) sessionAttrs
+					.get(this.sessionAttributeName);
+			return authorizationRequests;
+		}
+		else {
+			throw new IllegalStateException(
+					"authorizationRequests is supposed to be a Map or OAuth2AuthorizationRequest but actually is a "
+							+ sessionAttributeValue.getClass());
+		}
 	}
 
-	private Map<String, OAuth2AuthorizationRequest> sessionAttrsMapStateToAuthorizationRequest(
-			Map<String, Object> sessionAttrs) {
-		return (Map<String, OAuth2AuthorizationRequest>) sessionAttrs.get(this.sessionAttributeName);
+	/**
+	 * Configure if multiple {@link OAuth2AuthorizationRequest}s should be stored per
+	 * session. Default is false (not allow multiple {@link OAuth2AuthorizationRequest}
+	 * per session).
+	 * @param allowMultipleAuthorizationRequests true allows more than one
+	 * {@link OAuth2AuthorizationRequest} to be stored per session.
+	 * @since 5.5
+	 */
+	@Deprecated
+	public void setAllowMultipleAuthorizationRequests(boolean allowMultipleAuthorizationRequests) {
+		this.allowMultipleAuthorizationRequests = allowMultipleAuthorizationRequests;
 	}
 
 }