Expire OAuth2AuthorizationRequest when saving to the session #9513
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* | ||
| * 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. | ||
|
|
@@ -16,23 +16,35 @@ | |
|
|
||
| package org.springframework.security.oauth2.client.web; | ||
|
|
||
| import java.io.Serializable; | ||
| import java.time.Clock; | ||
| import java.time.Duration; | ||
| import java.time.Instant; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| import javax.servlet.http.HttpServletRequest; | ||
| import javax.servlet.http.HttpServletResponse; | ||
| import javax.servlet.http.HttpSession; | ||
|
|
||
| import org.springframework.security.core.SpringSecurityCoreVersion; | ||
| import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; | ||
| import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames; | ||
| import org.springframework.util.Assert; | ||
|
|
||
| /** | ||
| * An implementation of an {@link AuthorizationRequestRepository} that stores | ||
| * {@link OAuth2AuthorizationRequest} in the {@code HttpSession}. | ||
| * <p> | ||
| * <b>NOTE:</b> {@link OAuth2AuthorizationRequest}s expire after two minutes, the default | ||
| * duration can be configured via {@link #setAuthorizationRequestTimeToLive(Duration)}. | ||
| * | ||
| * @author Joe Grandja | ||
| * @author Rob Winch | ||
| * @author Craig Andrews | ||
| * @since 5.0 | ||
| * @see AuthorizationRequestRepository | ||
| * @see OAuth2AuthorizationRequest | ||
|
|
@@ -45,15 +57,22 @@ public final class HttpSessionOAuth2AuthorizationRequestRepository | |
|
|
||
| private final String sessionAttributeName = DEFAULT_AUTHORIZATION_REQUEST_ATTR_NAME; | ||
|
|
||
| private Clock clock = Clock.systemUTC(); | ||
|
|
||
| private Duration authorizationRequestTimeToLive = Duration.ofSeconds(120); | ||
|
|
||
| private int maxActiveAuthorizationRequestsPerRegistrationIdPerSession = 3; | ||
|
|
||
| @Override | ||
| public OAuth2AuthorizationRequest loadAuthorizationRequest(HttpServletRequest request) { | ||
| Assert.notNull(request, "request cannot be null"); | ||
| String stateParameter = this.getStateParameter(request); | ||
| if (stateParameter == null) { | ||
| return null; | ||
| } | ||
| Map<String, OAuth2AuthorizationRequestReference> authorizationRequests = this.getAuthorizationRequests(request); | ||
| OAuth2AuthorizationRequestReference authorizationRequestReference = authorizationRequests.get(stateParameter); | ||
| return (authorizationRequestReference != null) ? authorizationRequestReference.authorizationRequest : null; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -67,8 +86,18 @@ public void saveAuthorizationRequest(OAuth2AuthorizationRequest authorizationReq | |
| } | ||
| String state = authorizationRequest.getState(); | ||
| Assert.hasText(state, "authorizationRequest.state cannot be empty"); | ||
| Map<String, OAuth2AuthorizationRequestReference> authorizationRequests = this.getAuthorizationRequests(request); | ||
| authorizationRequests.put(state, new OAuth2AuthorizationRequestReference(authorizationRequest, | ||
| this.clock.instant().plus(this.authorizationRequestTimeToLive))); | ||
| for (String registrationId : authorizationRequests.values().stream().map((r) -> r.getRegistrationId()) | ||
| .distinct().collect(Collectors.toList())) { | ||
| List<OAuth2AuthorizationRequestReference> references = authorizationRequests.values().stream() | ||
| .filter((r) -> Objects.equals(registrationId, r.getRegistrationId())).collect(Collectors.toList()); | ||
| if (references.size() > this.maxActiveAuthorizationRequestsPerRegistrationIdPerSession) { | ||
| references.stream().sorted((a, b) -> a.expiresAt.compareTo(b.expiresAt)).findFirst() | ||
| .map((r) -> r.getState()).ifPresent(authorizationRequests::remove); | ||
| } | ||
| } | ||
| request.getSession().setAttribute(this.sessionAttributeName, authorizationRequests); | ||
| } | ||
|
|
||
|
|
@@ -79,15 +108,16 @@ public OAuth2AuthorizationRequest removeAuthorizationRequest(HttpServletRequest | |
| if (stateParameter == null) { | ||
| return null; | ||
| } | ||
| Map<String, OAuth2AuthorizationRequestReference> authorizationRequests = this.getAuthorizationRequests(request); | ||
| OAuth2AuthorizationRequestReference authorizationRequestReference = authorizationRequests | ||
| .remove(stateParameter); | ||
| if (!authorizationRequests.isEmpty()) { | ||
| request.getSession().setAttribute(this.sessionAttributeName, authorizationRequests); | ||
| } | ||
| else { | ||
| request.getSession().removeAttribute(this.sessionAttributeName); | ||
| } | ||
| return (authorizationRequestReference != null) ? authorizationRequestReference.authorizationRequest : null; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -113,14 +143,81 @@ private String getStateParameter(HttpServletRequest request) { | |
| * @return a non-null and mutable map of {@link OAuth2AuthorizationRequest#getState()} | ||
| * to an {@link OAuth2AuthorizationRequest}. | ||
| */ | ||
| private Map<String, OAuth2AuthorizationRequestReference> getAuthorizationRequests(HttpServletRequest request) { | ||
| HttpSession session = request.getSession(false); | ||
| Map<String, OAuth2AuthorizationRequestReference> authorizationRequests = (session != null) | ||
| ? (Map<String, OAuth2AuthorizationRequestReference>) session.getAttribute(this.sessionAttributeName) | ||
| : null; | ||
| if (authorizationRequests == null) { | ||
| return new HashMap<>(); | ||
| } | ||
| // remove expired entries | ||
| authorizationRequests.entrySet().removeIf((entry) -> entry.getValue().expiresAt.isBefore(this.clock.instant())); | ||
| return authorizationRequests; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the {@link Clock} used in {@link Instant#now(Clock)} when setting the instant | ||
| * created for {@link OAuth2AuthorizationRequest}. | ||
| * @param clock the clock | ||
| * @since 5.5 | ||
| */ | ||
| void setClock(Clock clock) { | ||
|
There was a problem hiding this comment. Please remove this as it's not needed. The clock will always be in sync since this implementation operates independently within it's own application instance. Clock and clock skew is needed when components are interacting in a distributed way. There was a problem hiding this comment. This method is package private, so nothing other than Spring itself should be using it. Being able to set the clock is important for testing. See https://github.com/candrews/spring-security/blob/9b1a85d992f3ac0d725f67ac03ee6d9f44659b21/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/HttpSessionOAuth2AuthorizationRequestRepositoryTests.java#L249 Is there another approach that should be taken to make the clock available to be set by tests? |
||
| Assert.notNull(clock, "clock cannot be null"); | ||
| this.clock = clock; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the {@link Duration} for which {@link OAuth2AuthorizationRequest} should | ||
| * expire. | ||
| * @param authorizationRequestTimeToLive the {@link Duration} a | ||
| * {@link OAuth2AuthorizationRequest} is considered not expired. Must not be negative. | ||
| * @since 5.5 | ||
| */ | ||
| void setAuthorizationRequestTimeToLive(Duration authorizationRequestTimeToLive) { | ||
| Assert.notNull(authorizationRequestTimeToLive, "oAuth2AuthorizationRequestExpiresIn cannot be null"); | ||
| Assert.state(!authorizationRequestTimeToLive.isNegative(), | ||
| "oAuth2AuthorizationRequestExpiresIn cannot be negative"); | ||
| this.authorizationRequestTimeToLive = authorizationRequestTimeToLive; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the maximum number of {@link OAuth2AuthorizationRequest} that can be | ||
| * stored/active per registration id for a session. If the maximum number are present | ||
| * in a session when an attempt is made to save another one, then the oldest will be | ||
| * removed. | ||
| * @param maxActiveAuthorizationRequestsPerSession must not be negative. | ||
| */ | ||
| void setMaxActiveAuthorizationRequestsPerRegistrationIdPerSession( | ||
| int maxActiveAuthorizationRequestsPerRegistrationIdPerSession) { | ||
| Assert.state(maxActiveAuthorizationRequestsPerRegistrationIdPerSession > 0, | ||
| "maxActiveAuthorizationRequestsPerRegistrationIdPerSession must be greater than zero"); | ||
| this.maxActiveAuthorizationRequestsPerRegistrationIdPerSession = maxActiveAuthorizationRequestsPerRegistrationIdPerSession; | ||
| } | ||
|
|
||
| private static final class OAuth2AuthorizationRequestReference implements Serializable { | ||
|
|
||
| private static final long serialVersionUID = SpringSecurityCoreVersion.SERIAL_VERSION_UID; | ||
|
|
||
| private final Instant expiresAt; | ||
|
|
||
| private final OAuth2AuthorizationRequest authorizationRequest; | ||
|
|
||
| private OAuth2AuthorizationRequestReference(OAuth2AuthorizationRequest authorizationRequest, | ||
| Instant expiresAt) { | ||
| Assert.notNull(authorizationRequest, "authorizationRequest cannot be null"); | ||
| this.expiresAt = expiresAt; | ||
| this.authorizationRequest = authorizationRequest; | ||
| } | ||
|
|
||
| private String getRegistrationId() { | ||
| return this.authorizationRequest.getAttribute(OAuth2ParameterNames.REGISTRATION_ID); | ||
| } | ||
|
|
||
| private String getState() { | ||
| return this.authorizationRequest.getState(); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.