Skip to content

Commit ba5157a

Browse files
candrewscandrews
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 7dc4de0 commit ba5157a

File tree

4 files changed

+207
-45
lines changed

4 files changed

+207
-45
lines changed

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

Lines changed: 50 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.
@@ -33,6 +33,7 @@
3333
*
3434
* @author Joe Grandja
3535
* @author Rob Winch
36+
* @author Craig Andrews
3637
* @since 5.0
3738
* @see AuthorizationRequestRepository
3839
* @see OAuth2AuthorizationRequest
@@ -45,6 +46,8 @@ public final class HttpSessionOAuth2AuthorizationRequestRepository
4546

4647
private final String sessionAttributeName = DEFAULT_AUTHORIZATION_REQUEST_ATTR_NAME;
4748

49+
private boolean allowMultipleAuthorizationRequests;
50+
4851
@Override
4952
public OAuth2AuthorizationRequest loadAuthorizationRequest(HttpServletRequest request) {
5053
Assert.notNull(request, "request cannot be null");
@@ -67,9 +70,14 @@ public void saveAuthorizationRequest(OAuth2AuthorizationRequest authorizationReq
6770
}
6871
String state = authorizationRequest.getState();
6972
Assert.hasText(state, "authorizationRequest.state cannot be empty");
70-
Map<String, OAuth2AuthorizationRequest> authorizationRequests = this.getAuthorizationRequests(request);
71-
authorizationRequests.put(state, authorizationRequest);
72-
request.getSession().setAttribute(this.sessionAttributeName, authorizationRequests);
73+
if (this.allowMultipleAuthorizationRequests) {
74+
Map<String, OAuth2AuthorizationRequest> authorizationRequests = this.getAuthorizationRequests(request);
75+
authorizationRequests.put(state, authorizationRequest);
76+
request.getSession().setAttribute(this.sessionAttributeName, authorizationRequests);
77+
}
78+
else {
79+
request.getSession().setAttribute(this.sessionAttributeName, authorizationRequest);
80+
}
7381
}
7482

7583
@Override
@@ -81,11 +89,15 @@ public OAuth2AuthorizationRequest removeAuthorizationRequest(HttpServletRequest
8189
}
8290
Map<String, OAuth2AuthorizationRequest> authorizationRequests = this.getAuthorizationRequests(request);
8391
OAuth2AuthorizationRequest originalRequest = authorizationRequests.remove(stateParameter);
84-
if (!authorizationRequests.isEmpty()) {
85-
request.getSession().setAttribute(this.sessionAttributeName, authorizationRequests);
92+
if (authorizationRequests.size() == 0) {
93+
request.getSession().removeAttribute(this.sessionAttributeName);
94+
}
95+
else if (authorizationRequests.size() == 1) {
96+
request.getSession().setAttribute(this.sessionAttributeName,
97+
authorizationRequests.values().iterator().next());
8698
}
8799
else {
88-
request.getSession().removeAttribute(this.sessionAttributeName);
100+
request.getSession().setAttribute(this.sessionAttributeName, authorizationRequests);
89101
}
90102
return originalRequest;
91103
}
@@ -115,12 +127,39 @@ private String getStateParameter(HttpServletRequest request) {
115127
*/
116128
private Map<String, OAuth2AuthorizationRequest> getAuthorizationRequests(HttpServletRequest request) {
117129
HttpSession session = request.getSession(false);
118-
Map<String, OAuth2AuthorizationRequest> authorizationRequests = (session != null)
119-
? (Map<String, OAuth2AuthorizationRequest>) session.getAttribute(this.sessionAttributeName) : null;
120-
if (authorizationRequests == null) {
130+
Object sessionAttributeValue = (session != null) ? session.getAttribute(this.sessionAttributeName) : null;
131+
if (sessionAttributeValue == null) {
121132
return new HashMap<>();
122133
}
123-
return authorizationRequests;
134+
else if (sessionAttributeValue instanceof OAuth2AuthorizationRequest) {
135+
OAuth2AuthorizationRequest auth2AuthorizationRequest = (OAuth2AuthorizationRequest) sessionAttributeValue;
136+
Map<String, OAuth2AuthorizationRequest> authorizationRequests = new HashMap<>(1);
137+
authorizationRequests.put(auth2AuthorizationRequest.getState(), auth2AuthorizationRequest);
138+
return authorizationRequests;
139+
}
140+
else if (sessionAttributeValue instanceof Map) {
141+
@SuppressWarnings("unchecked")
142+
Map<String, OAuth2AuthorizationRequest> authorizationRequests = (Map<String, OAuth2AuthorizationRequest>) sessionAttributeValue;
143+
return authorizationRequests;
144+
}
145+
else {
146+
throw new IllegalStateException(
147+
"authorizationRequests is supposed to be a Map or OAuth2AuthorizationRequest but actually is a "
148+
+ sessionAttributeValue.getClass());
149+
}
150+
}
151+
152+
/**
153+
* Configure if multiple {@link OAuth2AuthorizationRequest}s should be stored per
154+
* session. Default is false (not allow multiple {@link OAuth2AuthorizationRequest}
155+
* per session).
156+
* @param allowMultipleAuthorizationRequests true allows more than one
157+
* {@link OAuth2AuthorizationRequest} to be stored per session.
158+
* @since 5.5
159+
*/
160+
@Deprecated
161+
public void setAllowMultipleAuthorizationRequests(boolean allowMultipleAuthorizationRequests) {
162+
this.allowMultipleAuthorizationRequests = allowMultipleAuthorizationRequests;
124163
}
125164

126165
}

‎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: 5 additions & 34 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.
@@ -36,11 +36,12 @@
3636
* Tests for {@link HttpSessionOAuth2AuthorizationRequestRepository}.
3737
*
3838
* @author Joe Grandja
39+
* @author Craig Andrews
3940
*/
4041
@RunWith(MockitoJUnitRunner.class)
41-
public class HttpSessionOAuth2AuthorizationRequestRepositoryTests {
42+
public abstract class HttpSessionOAuth2AuthorizationRequestRepositoryTests {
4243

43-
private HttpSessionOAuth2AuthorizationRequestRepository authorizationRequestRepository = new HttpSessionOAuth2AuthorizationRequestRepository();
44+
protected HttpSessionOAuth2AuthorizationRequestRepository authorizationRequestRepository;
4445

4546
@Test
4647
public void loadAuthorizationRequestWhenHttpServletRequestIsNullThenThrowIllegalArgumentException() {
@@ -69,36 +70,6 @@ public void loadAuthorizationRequestWhenSavedThenReturnAuthorizationRequest() {
6970
assertThat(loadedAuthorizationRequest).isEqualTo(authorizationRequest);
7071
}
7172

72-
// gh-5110
73-
@Test
74-
public void loadAuthorizationRequestWhenMultipleSavedThenReturnMatchingAuthorizationRequest() {
75-
MockHttpServletRequest request = new MockHttpServletRequest();
76-
MockHttpServletResponse response = new MockHttpServletResponse();
77-
String state1 = "state-1122";
78-
OAuth2AuthorizationRequest authorizationRequest1 = createAuthorizationRequest().state(state1).build();
79-
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest1, request, response);
80-
String state2 = "state-3344";
81-
OAuth2AuthorizationRequest authorizationRequest2 = createAuthorizationRequest().state(state2).build();
82-
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest2, request, response);
83-
String state3 = "state-5566";
84-
OAuth2AuthorizationRequest authorizationRequest3 = createAuthorizationRequest().state(state3).build();
85-
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest3, request, response);
86-
request.addParameter(OAuth2ParameterNames.STATE, state1);
87-
OAuth2AuthorizationRequest loadedAuthorizationRequest1 = this.authorizationRequestRepository
88-
.loadAuthorizationRequest(request);
89-
assertThat(loadedAuthorizationRequest1).isEqualTo(authorizationRequest1);
90-
request.removeParameter(OAuth2ParameterNames.STATE);
91-
request.addParameter(OAuth2ParameterNames.STATE, state2);
92-
OAuth2AuthorizationRequest loadedAuthorizationRequest2 = this.authorizationRequestRepository
93-
.loadAuthorizationRequest(request);
94-
assertThat(loadedAuthorizationRequest2).isEqualTo(authorizationRequest2);
95-
request.removeParameter(OAuth2ParameterNames.STATE);
96-
request.addParameter(OAuth2ParameterNames.STATE, state3);
97-
OAuth2AuthorizationRequest loadedAuthorizationRequest3 = this.authorizationRequestRepository
98-
.loadAuthorizationRequest(request);
99-
assertThat(loadedAuthorizationRequest3).isEqualTo(authorizationRequest3);
100-
}
101-
10273
@Test
10374
public void loadAuthorizationRequestWhenSavedAndStateParameterNullThenReturnNull() {
10475
MockHttpServletRequest request = new MockHttpServletRequest();
@@ -237,7 +208,7 @@ public void removeAuthorizationRequestWhenNotSavedThenNotRemoved() {
237208
assertThat(removedAuthorizationRequest).isNull();
238209
}
239210

240-
private OAuth2AuthorizationRequest.Builder createAuthorizationRequest() {
211+
protected OAuth2AuthorizationRequest.Builder createAuthorizationRequest() {
241212
return OAuth2AuthorizationRequest.authorizationCode().authorizationUri("https://example.com/oauth2/authorize")
242213
.clientId("client-id-1234").state("state-1234");
243214
}

0 commit comments

Comments
 (0)