Skip to content

Commit 008dbee

Browse files
ttddyyttddyy
committed
Allow query params in OAuth2 Authorization Request redirect_uri
Fixes gh-7329
1 parent 24500fa commit 008dbee

File tree

3 files changed

+320
-6
lines changed

3 files changed

+320
-6
lines changed

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2AuthorizationCodeGrantRedirectTests.java

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
/*
2+
* Copyright 2002-2019 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.config.annotation.web.configurers.oauth2.client;
18+
19+
import static org.mockito.ArgumentMatchers.any;
20+
import static org.mockito.Mockito.mock;
21+
import static org.mockito.Mockito.when;
22+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl;
23+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
24+
25+
import java.util.Collections;
26+
27+
import javax.servlet.http.HttpServletRequest;
28+
import javax.servlet.http.HttpServletResponse;
29+
30+
import org.junit.Before;
31+
import org.junit.Rule;
32+
import org.junit.Test;
33+
import org.springframework.beans.factory.annotation.Autowired;
34+
import org.springframework.context.annotation.Configuration;
35+
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
36+
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
37+
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
38+
import org.springframework.security.config.test.SpringTestRule;
39+
import org.springframework.security.oauth2.client.endpoint.OAuth2AccessTokenResponseClient;
40+
import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest;
41+
import org.springframework.security.oauth2.client.registration.ClientRegistration;
42+
import org.springframework.security.oauth2.client.registration.InMemoryClientRegistrationRepository;
43+
import org.springframework.security.oauth2.client.registration.TestClientRegistrations;
44+
import org.springframework.security.oauth2.client.web.AuthorizationRequestRepository;
45+
import org.springframework.security.oauth2.core.OAuth2AccessToken;
46+
import org.springframework.security.oauth2.core.endpoint.OAuth2AccessTokenResponse;
47+
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
48+
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
49+
import org.springframework.test.web.servlet.MockMvc;
50+
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
51+
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
52+
53+
/**
54+
* Tests for OAuth2 Authorization Code Grant final redirect
55+
*
56+
* @author Tadaya Tsuyukubo
57+
* @see org.springframework.security.oauth2.client.web.OAuth2AuthorizationCodeGrantFilter
58+
*/
59+
public class OAuth2AuthorizationCodeGrantRedirectTests {
60+
61+
private static AuthorizationRequestRepository<OAuth2AuthorizationRequest> authorizationRequestRepository;
62+
private static OAuth2AccessTokenResponseClient<OAuth2AuthorizationCodeGrantRequest> accessTokenResponseClient;
63+
64+
@Rule
65+
public final SpringTestRule spring = new SpringTestRule();
66+
67+
@Autowired
68+
private MockMvc mvc;
69+
70+
@Before
71+
public void setUp() {
72+
authorizationRequestRepository = mock(AuthorizationRequestRepository.class);
73+
accessTokenResponseClient = mock(OAuth2AccessTokenResponseClient.class);
74+
}
75+
76+
@Test
77+
public void redirect() throws Exception {
78+
perform("/redirect?code=MY-CODE&state=MY-STATE",
79+
"http://localhost/redirect",
80+
"http://localhost/redirect");
81+
}
82+
83+
@Test
84+
public void redirectWithParamAppended() throws Exception {
85+
perform("/redirect?code=MY-CODE&state=MY-STATE&extra=EXTRA",
86+
"http://localhost/redirect",
87+
"http://localhost/redirect?extra=EXTRA");
88+
}
89+
90+
@Test
91+
public void redirectWithParameters() throws Exception {
92+
perform("/redirect?foo=FOO&code=MY-CODE&state=MY-STATE",
93+
"http://localhost/redirect?foo=FOO",
94+
"http://localhost/redirect?foo=FOO");
95+
}
96+
97+
@Test
98+
public void redirectUrlWithParametersWithParamAppended() throws Exception {
99+
perform("/redirect?foo=FOO&code=MY-CODE&state=MY-STATE&extra=EXTRA",
100+
"http://localhost/redirect?foo=FOO",
101+
"http://localhost/redirect?foo=FOO&extra=EXTRA");
102+
}
103+
104+
private void perform(String requestUri, String authorizationRequestRedirectUri, String expectedRedirectUrl) throws Exception {
105+
this.spring.register(WebSecurityConfiguration.class).autowire();
106+
107+
OAuth2AuthorizationRequest authorizationRequest = OAuth2AuthorizationRequest
108+
.authorizationCode()
109+
.authorizationUri("http://localhost/auth")
110+
.clientId("example")
111+
.state("MY-STATE")
112+
.redirectUri(authorizationRequestRedirectUri)
113+
.attributes(Collections.singletonMap(OAuth2ParameterNames.REGISTRATION_ID, "registration-id")) // comes from TestClientRegistrations.clientRegistration
114+
.build();
115+
116+
when(authorizationRequestRepository.loadAuthorizationRequest(any(HttpServletRequest.class)))
117+
.thenReturn(authorizationRequest);
118+
when(authorizationRequestRepository.removeAuthorizationRequest(any(HttpServletRequest.class), any(HttpServletResponse.class)))
119+
.thenReturn(authorizationRequest);
120+
121+
OAuth2AccessTokenResponse accessTokenResponse = OAuth2AccessTokenResponse
122+
.withToken("MY-ACCESS-TOKEN")
123+
.tokenType(OAuth2AccessToken.TokenType.BEARER)
124+
.build();
125+
when(accessTokenResponseClient
126+
.getTokenResponse(any(OAuth2AuthorizationCodeGrantRequest.class)))
127+
.thenReturn(accessTokenResponse);
128+
129+
MockHttpServletRequestBuilder builder = MockMvcRequestBuilders.get(requestUri);
130+
this.mvc.perform(builder)
131+
.andExpect(status().is3xxRedirection())
132+
.andExpect(redirectedUrl(expectedRedirectUrl));
133+
}
134+
135+
@Configuration
136+
@EnableWebSecurity
137+
static class WebSecurityConfiguration extends WebSecurityConfigurerAdapter {
138+
@Override
139+
protected void configure(HttpSecurity http) throws Exception {
140+
141+
ClientRegistration clientRegistration = TestClientRegistrations.clientRegistration().build();
142+
143+
InMemoryClientRegistrationRepository inMemoryClientRegistrationRepository =
144+
new InMemoryClientRegistrationRepository(clientRegistration);
145+
146+
http.oauth2Client(oauth2client ->
147+
oauth2client
148+
.clientRegistrationRepository(inMemoryClientRegistrationRepository)
149+
.authorizationCodeGrant()
150+
.authorizationRequestRepository(authorizationRequestRepository)
151+
.accessTokenResponseClient(accessTokenResponseClient)
152+
);
153+
}
154+
}
155+
156+
}

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

Lines changed: 102 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,18 @@
4141
import org.springframework.util.MultiValueMap;
4242
import org.springframework.util.StringUtils;
4343
import org.springframework.web.filter.OncePerRequestFilter;
44+
import org.springframework.web.util.UriComponents;
4445
import org.springframework.web.util.UriComponentsBuilder;
4546

4647
import javax.servlet.FilterChain;
4748
import javax.servlet.ServletException;
4849
import javax.servlet.http.HttpServletRequest;
4950
import javax.servlet.http.HttpServletResponse;
5051
import java.io.IOException;
52+
import java.util.Iterator;
53+
import java.util.List;
54+
import java.util.Map;
55+
import java.util.Objects;
5156

5257
/**
5358
* A {@code Filter} for the OAuth 2.0 Authorization Code Grant,
@@ -77,6 +82,7 @@
7782
* </ul>
7883
*
7984
* @author Joe Grandja
85+
* @author Tadaya Tsuyukubo
8086
* @since 5.1
8187
* @see OAuth2AuthorizationCodeAuthenticationToken
8288
* @see OAuth2AuthorizationCodeAuthenticationProvider
@@ -145,16 +151,106 @@ private boolean shouldProcessAuthorizationResponse(HttpServletRequest request) {
145151
if (authorizationRequest == null) {
146152
return false;
147153
}
148-
String requestUrl = UrlUtils.buildFullRequestUrl(request.getScheme(), request.getServerName(),
149-
request.getServerPort(), request.getRequestURI(), null);
150154
MultiValueMap<String, String> params = OAuth2AuthorizationResponseUtils.toMultiMap(request.getParameterMap());
151-
if (requestUrl.equals(authorizationRequest.getRedirectUri()) &&
152-
OAuth2AuthorizationResponseUtils.isAuthorizationResponse(params)) {
155+
156+
if (OAuth2AuthorizationResponseUtils.isAuthorizationResponse(params) &&
157+
isValidRedirectUrl(request, authorizationRequest.getRedirectUri())) {
153158
return true;
154159
}
155160
return false;
156161
}
157162

163+
private boolean isValidRedirectUrl(HttpServletRequest httpServletRequest, String redirectUrl) {
164+
UriComponents request = UriComponentsBuilder.fromHttpUrl(UrlUtils.buildFullRequestUrl(httpServletRequest))
165+
.replaceQueryParam(OAuth2ParameterNames.CODE)
166+
.replaceQueryParam(OAuth2ParameterNames.STATE)
167+
.build();
168+
169+
UriComponents redirect = UriComponentsBuilder.fromUriString(redirectUrl).build();
170+
171+
// Simple check first
172+
if (request.toUriString().equals(redirectUrl)) {
173+
return true;
174+
}
175+
176+
// Compare each part of url one by one
177+
178+
if (!Objects.equals(request.getScheme(), redirect.getScheme())) {
179+
return false;
180+
}
181+
182+
if (!Objects.equals(request.getUserInfo(), redirect.getUserInfo())) {
183+
return false;
184+
}
185+
186+
if (!Objects.equals(request.getHost(), redirect.getHost())) {
187+
return false;
188+
}
189+
190+
if (!Objects.equals(request.getPort(), redirect.getPort())) {
191+
return false;
192+
}
193+
194+
if (!Objects.equals(request.getPath(), redirect.getPath())) {
195+
return false;
196+
}
197+
198+
if (redirect.getQuery() == null) {
199+
return true; // no need to check request query params
200+
}
201+
202+
// Compare request param/values are in exact order specified in redirect url.
203+
// Request(Authorization Response) can have additional parameters appended which is allowed by spec.
204+
// Since urls are hierarchical, UriComponents uses HierarchicalUriComponents which keeps the exact
205+
// order of parameters.
206+
MultiValueMap<String, String> requestParamMap = request.getQueryParams();
207+
MultiValueMap<String, String> redirectParamMap = redirect.getQueryParams();
208+
209+
return containsRedirectUriParamsInOrder(requestParamMap, redirectParamMap);
210+
211+
}
212+
213+
private boolean containsRedirectUriParamsInOrder(MultiValueMap<String, String> requestMap, MultiValueMap<String, String> redirectMap) {
214+
Iterator<Map.Entry<String, List<String>>> requestIterator = requestMap.entrySet().iterator();
215+
Iterator<Map.Entry<String, List<String>>> redirectIterator = redirectMap.entrySet().iterator();
216+
217+
while (requestIterator.hasNext() && redirectIterator.hasNext()) {
218+
Map.Entry<String, List<String>> requestEntry = requestIterator.next();
219+
Map.Entry<String, List<String>> redirectEntry = redirectIterator.next();
220+
221+
String requestParam = requestEntry.getKey();
222+
String redirectParam = redirectEntry.getKey();
223+
224+
if (!requestParam.equals(redirectParam)) {
225+
return false; // param doesn't match
226+
}
227+
228+
List<String> requestValues = requestEntry.getValue();
229+
List<String> redirectValues = redirectEntry.getValue();
230+
231+
if (requestValues.size() < redirectValues.size()) {
232+
return false; // request param values don't have ones specified in redirect param
233+
}
234+
// request may have additional param values; thus, iterate over redirect values
235+
for (int i = 0; i < redirectValues.size(); i++) {
236+
String requestValue = requestValues.get(i);
237+
String redirectValue = redirectValues.get(i);
238+
239+
if (!requestValue.equals(redirectValue)) {
240+
return false; // request param value doesn't match
241+
}
242+
}
243+
}
244+
245+
// request may have additional params which is ok by spec
246+
247+
if (redirectIterator.hasNext()) {
248+
return false; // request has less params than redirect params
249+
}
250+
251+
return true;
252+
}
253+
158254
private void processAuthorizationResponse(HttpServletRequest request, HttpServletResponse response)
159255
throws IOException {
160256

@@ -166,7 +262,8 @@ private void processAuthorizationResponse(HttpServletRequest request, HttpServle
166262

167263
MultiValueMap<String, String> params = OAuth2AuthorizationResponseUtils.toMultiMap(request.getParameterMap());
168264
String redirectUri = UriComponentsBuilder.fromHttpUrl(UrlUtils.buildFullRequestUrl(request))
169-
.replaceQuery(null)
265+
.replaceQueryParam(OAuth2ParameterNames.CODE)
266+
.replaceQueryParam(OAuth2ParameterNames.STATE)
170267
.build()
171268
.toUriString();
172269
OAuth2AuthorizationResponse authorizationResponse = OAuth2AuthorizationResponseUtils.convert(params, redirectUri);

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

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@
5050
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
5151
import org.springframework.security.web.savedrequest.HttpSessionRequestCache;
5252
import org.springframework.security.web.savedrequest.RequestCache;
53+
import org.springframework.web.util.UriComponents;
54+
import org.springframework.web.util.UriComponentsBuilder;
5355

5456
import static org.assertj.core.api.Assertions.assertThat;
5557
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -362,13 +364,72 @@ public void doFilterWhenAuthorizationResponseSuccessAndAnonymousAccessNullAuthen
362364
assertThat(authorizedClients.values().iterator().next()).isSameAs(authorizedClient);
363365
}
364366

367+
@Test
368+
public void doFilterWhenAuthorizationResponseSuccessThenRequestParameterCombinations() throws Exception {
369+
// exact match
370+
assertThat(isValidRedirect("http://localhost", "http://localhost")).isTrue();
371+
assertThat(isValidRedirect("http://localhost/callback", "http://localhost/callback")).isTrue();
372+
assertThat(isValidRedirect("http://localhost?key=FOO", "http://localhost?key=FOO")).isTrue();
373+
374+
// additional param
375+
assertThat(isValidRedirect("http://localhost", "http://localhost?extra")).isTrue();
376+
assertThat(isValidRedirect("http://localhost?key=FOO", "http://localhost?key=FOO&extra")).isTrue();
377+
assertThat(isValidRedirect("http://localhost?key=FOO", "http://localhost?key=FOO&extra=EXTRA")).isTrue();
378+
assertThat(isValidRedirect("http://localhost?key=FOO", "http://localhost?key=FOO&key=EXTRA")).isTrue();
379+
380+
// multi parameter
381+
assertThat(isValidRedirect("http://localhost?key=FOO&key=BAR", "http://localhost?key=FOO&key=BAR")).isTrue();
382+
assertThat(isValidRedirect("http://localhost?key=FOO&key=BAR", "http://localhost?key=FOO&key=BAR&key=EXTRA")).isTrue();
383+
assertThat(isValidRedirect("http://localhost?key=FOO&key=BAR", "http://localhost?key=FOO&key=BAR&extra=EXTRA")).isTrue();
384+
385+
386+
// wrong parameter
387+
assertThat(isValidRedirect("http://localhost?key=FOO", "http://localhost?key=WRONG")).isFalse();
388+
assertThat(isValidRedirect("http://localhost?key=FOO", "http://localhost?extra=EXTRA&key=FOO")).isFalse();
389+
assertThat(isValidRedirect("http://localhost?key=FOO", "http://localhost?key=BAR&key=FOO")).isFalse();
390+
assertThat(isValidRedirect("http://localhost?key=FOO", "http://localhost?key=FOOABC")).isFalse();
391+
assertThat(isValidRedirect("http://localhost?key=FOO", "http://localhost")).isFalse();
392+
393+
assertThat(isValidRedirect("http://localhost?key=FOO&key=BAR", "http://localhost?key=BAR&key=FOO")).isFalse();
394+
assertThat(isValidRedirect("http://localhost?key=FOO&key=BAR", "http://localhost?key=BAZ&key=FOO&key=BAR")).isFalse();
395+
assertThat(isValidRedirect("http://localhost?key=FOO&key=BAR", "http://localhost?extra=EXTRA&key=FOO&key=BAR")).isFalse();
396+
}
397+
398+
private boolean isValidRedirect(String redirectUrl, String requestUrl) throws Exception {
399+
UriComponents redirect = UriComponentsBuilder.fromUriString(redirectUrl).build();
400+
UriComponents request = UriComponentsBuilder.fromUriString(requestUrl).build();
401+
402+
MockHttpServletRequest mockRequest = new MockHttpServletRequest("GET", request.getPath());
403+
mockRequest.setServletPath(request.getPath());
404+
mockRequest.addParameter(OAuth2ParameterNames.CODE, "code");
405+
mockRequest.addParameter(OAuth2ParameterNames.STATE, "state");
406+
mockRequest.setQueryString(request.getQuery());
407+
408+
MockHttpServletResponse mockResponse = new MockHttpServletResponse();
409+
FilterChain filterChain = mock(FilterChain.class);
410+
411+
this.setUpAuthorizationRequest(mockRequest, mockResponse, redirect.toUriString(), this.registration1);
412+
this.setUpAuthenticationResult(this.registration1);
413+
414+
this.filter.doFilter(mockRequest, mockResponse, filterChain);
415+
416+
String redirected = mockResponse.getRedirectedUrl();
417+
return requestUrl.equals(redirected);
418+
}
419+
365420
private void setUpAuthorizationRequest(HttpServletRequest request, HttpServletResponse response,
366421
ClientRegistration registration) {
422+
String redirectUri = request.getRequestURL().toString();
423+
setUpAuthorizationRequest(request, response, redirectUri, registration);
424+
}
425+
426+
private void setUpAuthorizationRequest(HttpServletRequest request, HttpServletResponse response,
427+
String redirectUri, ClientRegistration registration) {
367428
Map<String, Object> additionalParameters = new HashMap<>();
368429
additionalParameters.put(OAuth2ParameterNames.REGISTRATION_ID, registration.getRegistrationId());
369430
OAuth2AuthorizationRequest authorizationRequest = request()
370431
.additionalParameters(additionalParameters)
371-
.redirectUri(request.getRequestURL().toString()).build();
432+
.redirectUri(redirectUri).build();
372433
this.authorizationRequestRepository.saveAuthorizationRequest(authorizationRequest, request, response);
373434
}
374435

0 commit comments

Comments
 (0)