Skip to content

Commit b8eee20

Browse files
candrewsjgrandja
candrews
authored and
jgrandja
committed
HttpSessionOAuth2AuthorizationRequestRepository: store one request by default
Add setAllowMultipleAuthorizationRequests allowing applications to revert to the previous functionality should they need to do so. Closes gh-5145 Intentionally regresses gh-5110
1 parent b1c021e commit b8eee20

File tree

4 files changed

+211
-56
lines changed

4 files changed

+211
-56
lines changed

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/HttpSessionOAuth2AuthorizationRequestRepository.java

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -31,6 +31,7 @@
3131
*
3232
* @author Joe Grandja
3333
* @author Rob Winch
34+
* @author Craig Andrews
3435
* @since 5.0
3536
* @see AuthorizationRequestRepository
3637
* @see OAuth2AuthorizationRequest
@@ -41,6 +42,8 @@ public final class HttpSessionOAuth2AuthorizationRequestRepository implements Au
4142

4243
private final String sessionAttributeName = DEFAULT_AUTHORIZATION_REQUEST_ATTR_NAME;
4344

45+
private boolean allowMultipleAuthorizationRequests;
46+
4447
@Override
4548
public OAuth2AuthorizationRequest loadAuthorizationRequest(HttpServletRequest request) {
4649
Assert.notNull(request, "request cannot be null");
@@ -63,9 +66,14 @@ public void saveAuthorizationRequest(OAuth2AuthorizationRequest authorizationReq
6366
}
6467
String state = authorizationRequest.getState();
6568
Assert.hasText(state, "authorizationRequest.state cannot be empty");
66-
Map<String, OAuth2AuthorizationRequest> authorizationRequests = this.getAuthorizationRequests(request);
67-
authorizationRequests.put(state, authorizationRequest);
68-
request.getSession().setAttribute(this.sessionAttributeName, authorizationRequests);
69+
if (this.allowMultipleAuthorizationRequests) {
70+
Map<String, OAuth2AuthorizationRequest> authorizationRequests = this.getAuthorizationRequests(request);
71+
authorizationRequests.put(state, authorizationRequest);
72+
request.getSession().setAttribute(this.sessionAttributeName, authorizationRequests);
73+
}
74+
else {
75+
request.getSession().setAttribute(this.sessionAttributeName, authorizationRequest);
76+
}
6977
}
7078

7179
@Override
@@ -77,11 +85,16 @@ public OAuth2AuthorizationRequest removeAuthorizationRequest(HttpServletRequest
7785
}
7886
Map<String, OAuth2AuthorizationRequest> authorizationRequests = this.getAuthorizationRequests(request);
7987
OAuth2AuthorizationRequest originalRequest = authorizationRequests.remove(stateParameter);
80-
if (!authorizationRequests.isEmpty()) {
81-
request.getSession().setAttribute(this.sessionAttributeName, authorizationRequests);
82-
} else {
88+
if (authorizationRequests.size() == 0) {
8389
request.getSession().removeAttribute(this.sessionAttributeName);
8490
}
91+
else if (authorizationRequests.size() == 1) {
92+
request.getSession().setAttribute(this.sessionAttributeName,
93+
authorizationRequests.values().iterator().next());
94+
}
95+
else {
96+
request.getSession().setAttribute(this.sessionAttributeName, authorizationRequests);
97+
}
8598
return originalRequest;
8699
}
87100

@@ -107,11 +120,38 @@ private String getStateParameter(HttpServletRequest request) {
107120
*/
108121
private Map<String, OAuth2AuthorizationRequest> getAuthorizationRequests(HttpServletRequest request) {
109122
HttpSession session = request.getSession(false);
110-
Map<String, OAuth2AuthorizationRequest> authorizationRequests = session == null ? null :
111-
(Map<String, OAuth2AuthorizationRequest>) session.getAttribute(this.sessionAttributeName);
112-
if (authorizationRequests == null) {
123+
Object sessionAttributeValue = (session != null) ? session.getAttribute(this.sessionAttributeName) : null;
124+
if (sessionAttributeValue == null) {
113125
return new HashMap<>();
114126
}
115-
return authorizationRequests;
127+
else if (sessionAttributeValue instanceof OAuth2AuthorizationRequest) {
128+
OAuth2AuthorizationRequest auth2AuthorizationRequest = (OAuth2AuthorizationRequest) sessionAttributeValue;
129+
Map<String, OAuth2AuthorizationRequest> authorizationRequests = new HashMap<>(1);
130+
authorizationRequests.put(auth2AuthorizationRequest.getState(), auth2AuthorizationRequest);
131+
return authorizationRequests;
132+
}
133+
else if (sessionAttributeValue instanceof Map) {
134+
@SuppressWarnings("unchecked")
135+
Map<String, OAuth2AuthorizationRequest> authorizationRequests = (Map<String, OAuth2AuthorizationRequest>) sessionAttributeValue;
136+
return authorizationRequests;
137+
}
138+
else {
139+
throw new IllegalStateException(
140+
"authorizationRequests is supposed to be a Map or OAuth2AuthorizationRequest but actually is a "
141+
+ sessionAttributeValue.getClass());
142+
}
143+
}
144+
145+
/**
146+
* Configure if multiple {@link OAuth2AuthorizationRequest}s should be stored per
147+
* session. Default is false (not allow multiple {@link OAuth2AuthorizationRequest}
148+
* per session).
149+
* @param allowMultipleAuthorizationRequests true allows more than one
150+
* {@link OAuth2AuthorizationRequest} to be stored per session.
151+
* @since 5.5
152+
*/
153+
@Deprecated
154+
public void setAllowMultipleAuthorizationRequests(boolean allowMultipleAuthorizationRequests) {
155+
this.allowMultipleAuthorizationRequests = allowMultipleAuthorizationRequests;
116156
}
117157
}

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/HttpSessionOAuth2AuthorizationRequestRepositoryAllowMultipleAuthorizationRequestsTests.java

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Copyright 2002-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.oauth2.client.web;
18+
19+
import org.junit.Before;
20+
import org.junit.Test;
21+
22+
import org.springframework.mock.web.MockHttpServletRequest;
23+
import org.springframework.mock.web.MockHttpServletResponse;
24+
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
25+
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
26+
27+
import static org.assertj.core.api.Assertions.assertThat;
28+
29+
/**
30+
* Tests for {@link HttpSessionOAuth2AuthorizationRequestRepository} when
31+
* {@link HttpSessionOAuth2AuthorizationRequestRepository#setAllowMultipleAuthorizationRequests(boolean)}
32+
* is enabled.
33+
*
34+
* @author Joe Grandja
35+
* @author Craig Andrews
36+
*/
37+
public class HttpSessionOAuth2AuthorizationRequestRepositoryAllowMultipleAuthorizationRequestsTests
38+
extends HttpSessionOAuth2AuthorizationRequestRepositoryTests {
39+
40+
@Before
41+
public void setup() {
42+
this.authorizationRequestRepository = new HttpSessionOAuth2AuthorizationRequestRepository();
43+
this.authorizationRequestRepository.setAllowMultipleAuthorizationRequests(true);
44+
}
45+
46+
// gh-5110
47+
@Test
48+
public void loadAuthorizationRequestWhenMultipleSavedThenReturnMatchingAuthorizationRequest() {
49+
MockHttpServletRequest request = new MockHttpServletRequest();
50+
MockHttpServletResponse response = new MockHttpServletResponse();
51+
String state1 = "state-1122";
52+
OAuth2AuthorizationRequest authorizationRequest1 = createAuthorizationRequest().state(state1).build();
53+
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest1, request, response);
54+
String state2 = "state-3344";
55+
OAuth2AuthorizationRequest authorizationRequest2 = createAuthorizationRequest().state(state2).build();
56+
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest2, request, response);
57+
String state3 = "state-5566";
58+
OAuth2AuthorizationRequest authorizationRequest3 = createAuthorizationRequest().state(state3).build();
59+
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest3, request, response);
60+
request.addParameter(OAuth2ParameterNames.STATE, state1);
61+
OAuth2AuthorizationRequest loadedAuthorizationRequest1 = this.authorizationRequestRepository
62+
.loadAuthorizationRequest(request);
63+
assertThat(loadedAuthorizationRequest1).isEqualTo(authorizationRequest1);
64+
request.removeParameter(OAuth2ParameterNames.STATE);
65+
request.addParameter(OAuth2ParameterNames.STATE, state2);
66+
OAuth2AuthorizationRequest loadedAuthorizationRequest2 = this.authorizationRequestRepository
67+
.loadAuthorizationRequest(request);
68+
assertThat(loadedAuthorizationRequest2).isEqualTo(authorizationRequest2);
69+
request.removeParameter(OAuth2ParameterNames.STATE);
70+
request.addParameter(OAuth2ParameterNames.STATE, state3);
71+
OAuth2AuthorizationRequest loadedAuthorizationRequest3 = this.authorizationRequestRepository
72+
.loadAuthorizationRequest(request);
73+
assertThat(loadedAuthorizationRequest3).isEqualTo(authorizationRequest3);
74+
}
75+
76+
}

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/HttpSessionOAuth2AuthorizationRequestRepositoryDoNotAllowMultipleAuthorizationRequestsTests.java

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Copyright 2002-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.oauth2.client.web;
18+
19+
import org.junit.Before;
20+
import org.junit.Test;
21+
22+
import org.springframework.mock.web.MockHttpServletRequest;
23+
import org.springframework.mock.web.MockHttpServletResponse;
24+
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
25+
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
26+
27+
import static org.assertj.core.api.Assertions.assertThat;
28+
29+
/**
30+
* Tests for {@link HttpSessionOAuth2AuthorizationRequestRepository} when
31+
* {@link HttpSessionOAuth2AuthorizationRequestRepository#setAllowMultipleAuthorizationRequests(boolean)}
32+
* is disabled.
33+
*
34+
* @author Joe Grandja
35+
* @author Craig Andrews
36+
*/
37+
public class HttpSessionOAuth2AuthorizationRequestRepositoryDoNotAllowMultipleAuthorizationRequestsTests
38+
extends HttpSessionOAuth2AuthorizationRequestRepositoryTests {
39+
40+
@Before
41+
public void setup() {
42+
this.authorizationRequestRepository = new HttpSessionOAuth2AuthorizationRequestRepository();
43+
this.authorizationRequestRepository.setAllowMultipleAuthorizationRequests(false);
44+
}
45+
46+
// gh-5145
47+
@Test
48+
public void loadAuthorizationRequestWhenMultipleSavedThenReturnLastAuthorizationRequest() {
49+
MockHttpServletRequest request = new MockHttpServletRequest();
50+
MockHttpServletResponse response = new MockHttpServletResponse();
51+
String state1 = "state-1122";
52+
OAuth2AuthorizationRequest authorizationRequest1 = createAuthorizationRequest().state(state1).build();
53+
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest1, request, response);
54+
String state2 = "state-3344";
55+
OAuth2AuthorizationRequest authorizationRequest2 = createAuthorizationRequest().state(state2).build();
56+
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest2, request, response);
57+
String state3 = "state-5566";
58+
OAuth2AuthorizationRequest authorizationRequest3 = createAuthorizationRequest().state(state3).build();
59+
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest3, request, response);
60+
request.addParameter(OAuth2ParameterNames.STATE, state1);
61+
OAuth2AuthorizationRequest loadedAuthorizationRequest1 = this.authorizationRequestRepository
62+
.loadAuthorizationRequest(request);
63+
assertThat(loadedAuthorizationRequest1).isNull();
64+
request.removeParameter(OAuth2ParameterNames.STATE);
65+
request.addParameter(OAuth2ParameterNames.STATE, state2);
66+
OAuth2AuthorizationRequest loadedAuthorizationRequest2 = this.authorizationRequestRepository
67+
.loadAuthorizationRequest(request);
68+
assertThat(loadedAuthorizationRequest2).isNull();
69+
request.removeParameter(OAuth2ParameterNames.STATE);
70+
request.addParameter(OAuth2ParameterNames.STATE, state3);
71+
OAuth2AuthorizationRequest loadedAuthorizationRequest3 = this.authorizationRequestRepository
72+
.loadAuthorizationRequest(request);
73+
assertThat(loadedAuthorizationRequest3).isEqualTo(authorizationRequest3);
74+
}
75+
76+
}

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/HttpSessionOAuth2AuthorizationRequestRepositoryTests.java

Lines changed: 8 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -34,11 +34,12 @@
3434
* Tests for {@link HttpSessionOAuth2AuthorizationRequestRepository}.
3535
*
3636
* @author Joe Grandja
37+
* @author Craig Andrews
3738
*/
3839
@RunWith(MockitoJUnitRunner.class)
39-
public class HttpSessionOAuth2AuthorizationRequestRepositoryTests {
40-
private HttpSessionOAuth2AuthorizationRequestRepository authorizationRequestRepository =
41-
new HttpSessionOAuth2AuthorizationRequestRepository();
40+
public abstract class HttpSessionOAuth2AuthorizationRequestRepositoryTests {
41+
42+
protected HttpSessionOAuth2AuthorizationRequestRepository authorizationRequestRepository;
4243

4344
@Test(expected = IllegalArgumentException.class)
4445
public void loadAuthorizationRequestWhenHttpServletRequestIsNullThenThrowIllegalArgumentException() {
@@ -70,42 +71,6 @@ public void loadAuthorizationRequestWhenSavedThenReturnAuthorizationRequest() {
7071
assertThat(loadedAuthorizationRequest).isEqualTo(authorizationRequest);
7172
}
7273

73-
// gh-5110
74-
@Test
75-
public void loadAuthorizationRequestWhenMultipleSavedThenReturnMatchingAuthorizationRequest() {
76-
MockHttpServletRequest request = new MockHttpServletRequest();
77-
MockHttpServletResponse response = new MockHttpServletResponse();
78-
79-
String state1 = "state-1122";
80-
OAuth2AuthorizationRequest authorizationRequest1 = createAuthorizationRequest().state(state1).build();
81-
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest1, request, response);
82-
83-
String state2 = "state-3344";
84-
OAuth2AuthorizationRequest authorizationRequest2 = createAuthorizationRequest().state(state2).build();
85-
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest2, request, response);
86-
87-
String state3 = "state-5566";
88-
OAuth2AuthorizationRequest authorizationRequest3 = createAuthorizationRequest().state(state3).build();
89-
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest3, request, response);
90-
91-
request.addParameter(OAuth2ParameterNames.STATE, state1);
92-
OAuth2AuthorizationRequest loadedAuthorizationRequest1 =
93-
this.authorizationRequestRepository.loadAuthorizationRequest(request);
94-
assertThat(loadedAuthorizationRequest1).isEqualTo(authorizationRequest1);
95-
96-
request.removeParameter(OAuth2ParameterNames.STATE);
97-
request.addParameter(OAuth2ParameterNames.STATE, state2);
98-
OAuth2AuthorizationRequest loadedAuthorizationRequest2 =
99-
this.authorizationRequestRepository.loadAuthorizationRequest(request);
100-
assertThat(loadedAuthorizationRequest2).isEqualTo(authorizationRequest2);
101-
102-
request.removeParameter(OAuth2ParameterNames.STATE);
103-
request.addParameter(OAuth2ParameterNames.STATE, state3);
104-
OAuth2AuthorizationRequest loadedAuthorizationRequest3 =
105-
this.authorizationRequestRepository.loadAuthorizationRequest(request);
106-
assertThat(loadedAuthorizationRequest3).isEqualTo(authorizationRequest3);
107-
}
108-
10974
@Test
11075
public void loadAuthorizationRequestWhenSavedAndStateParameterNullThenReturnNull() {
11176
MockHttpServletRequest request = new MockHttpServletRequest();
@@ -284,11 +249,9 @@ public void removeAuthorizationRequestWhenNotSavedThenNotRemoved() {
284249
assertThat(removedAuthorizationRequest).isNull();
285250
}
286251

287-
private OAuth2AuthorizationRequest.Builder createAuthorizationRequest() {
288-
return OAuth2AuthorizationRequest.authorizationCode()
289-
.authorizationUri("https://example.com/oauth2/authorize")
290-
.clientId("client-id-1234")
291-
.state("state-1234");
252+
protected OAuth2AuthorizationRequest.Builder createAuthorizationRequest() {
253+
return OAuth2AuthorizationRequest.authorizationCode().authorizationUri("https://example.com/oauth2/authorize")
254+
.clientId("client-id-1234").state("state-1234");
292255
}
293256

294257
static class MockDistributedHttpSession extends MockHttpSession {

0 commit comments

Comments
 (0)