Skip to content

Commit 7cfb17a

Browse files
Marek Saborwinch
Marek Sabo
authored and
rwinch
committed
Finer variables for OAuth2 redirectUriTemplate expansion
Fixes #6239
1 parent d285c6a commit 7cfb17a

File tree

3 files changed

+180
-26
lines changed

3 files changed

+180
-26
lines changed

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

+43-11
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import org.springframework.security.web.util.UrlUtils;
2828
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
2929
import org.springframework.util.Assert;
30+
import org.springframework.util.StringUtils;
31+
import org.springframework.web.util.UriComponents;
3032
import org.springframework.web.util.UriComponentsBuilder;
3133

3234
import javax.servlet.http.HttpServletRequest;
@@ -54,6 +56,7 @@
5456
*/
5557
public final class DefaultOAuth2AuthorizationRequestResolver implements OAuth2AuthorizationRequestResolver {
5658
private static final String REGISTRATION_ID_URI_VARIABLE_NAME = "registrationId";
59+
private static final char PATH_DELIMITER = '/';
5760
private final ClientRegistrationRepository clientRegistrationRepository;
5861
private final AntPathRequestMatcher authorizationRequestMatcher;
5962
private final StringKeyGenerator stateGenerator = new Base64StringKeyGenerator(Base64.getUrlEncoder());
@@ -127,7 +130,7 @@ private OAuth2AuthorizationRequest resolve(HttpServletRequest request, String re
127130
") for Client Registration with Id: " + clientRegistration.getRegistrationId());
128131
}
129132

130-
String redirectUriStr = this.expandRedirectUri(request, clientRegistration, redirectUriAction);
133+
String redirectUriStr = expandRedirectUri(request, clientRegistration, redirectUriAction);
131134

132135
OAuth2AuthorizationRequest authorizationRequest = builder
133136
.clientId(clientRegistration.getClientId())
@@ -149,20 +152,49 @@ private String resolveRegistrationId(HttpServletRequest request) {
149152
return null;
150153
}
151154

152-
private String expandRedirectUri(HttpServletRequest request, ClientRegistration clientRegistration, String action) {
153-
// Supported URI variables -> baseUrl, action, registrationId
154-
// Used in -> CommonOAuth2Provider.DEFAULT_REDIRECT_URL = "{baseUrl}/{action}/oauth2/code/{registrationId}"
155+
/**
156+
* Expands the {@link ClientRegistration#getRedirectUriTemplate()} with following provided variables:<br/>
157+
* - baseUrl (e.g. https://localhost/app) <br/>
158+
* - baseScheme (e.g. https) <br/>
159+
* - baseHost (e.g. localhost) <br/>
160+
* - basePort (e.g. :8080) <br/>
161+
* - basePath (e.g. /app) <br/>
162+
* - registrationId (e.g. google) <br/>
163+
* - action (e.g. login) <br/>
164+
* <p/>
165+
* Null variables are provided as empty strings.
166+
* <p/>
167+
* Default redirectUriTemplate is: {@link org.springframework.security.config.oauth2.client}.CommonOAuth2Provider#DEFAULT_REDIRECT_URL
168+
*
169+
* @return expanded URI
170+
*/
171+
private static String expandRedirectUri(HttpServletRequest request, ClientRegistration clientRegistration, String action) {
155172
Map<String, String> uriVariables = new HashMap<>();
156173
uriVariables.put("registrationId", clientRegistration.getRegistrationId());
157-
String baseUrl = UriComponentsBuilder.fromHttpUrl(UrlUtils.buildFullRequestUrl(request))
158-
.replaceQuery(null)
174+
175+
UriComponents uriComponents = UriComponentsBuilder.fromHttpUrl(UrlUtils.buildFullRequestUrl(request))
159176
.replacePath(request.getContextPath())
160-
.build()
161-
.toUriString();
162-
uriVariables.put("baseUrl", baseUrl);
163-
if (action != null) {
164-
uriVariables.put("action", action);
177+
.replaceQuery(null)
178+
.fragment(null)
179+
.build();
180+
String scheme = uriComponents.getScheme();
181+
uriVariables.put("baseScheme", scheme == null ? "" : scheme);
182+
String host = uriComponents.getHost();
183+
uriVariables.put("baseHost", host == null ? "" : host);
184+
// following logic is based on HierarchicalUriComponents#toUriString()
185+
int port = uriComponents.getPort();
186+
uriVariables.put("basePort", port == -1 ? "" : ":" + port);
187+
String path = uriComponents.getPath();
188+
if (StringUtils.hasLength(path)) {
189+
if (path.charAt(0) != PATH_DELIMITER) {
190+
path = PATH_DELIMITER + path;
191+
}
165192
}
193+
uriVariables.put("basePath", path == null ? "" : path);
194+
uriVariables.put("baseUrl", uriComponents.toUriString());
195+
196+
uriVariables.put("action", action == null ? "" : action);
197+
166198
return UriComponentsBuilder.fromUriString(clientRegistration.getRedirectUriTemplate())
167199
.buildAndExpand(uriVariables)
168200
.toUriString();

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

+44-13
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@
3030
import org.springframework.security.web.server.util.matcher.PathPatternParserServerWebExchangeMatcher;
3131
import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatcher;
3232
import org.springframework.util.Assert;
33+
import org.springframework.util.StringUtils;
3334
import org.springframework.web.server.ResponseStatusException;
3435
import org.springframework.web.server.ServerWebExchange;
36+
import org.springframework.web.util.UriComponents;
3537
import org.springframework.web.util.UriComponentsBuilder;
3638
import reactor.core.publisher.Mono;
3739

@@ -63,8 +65,9 @@ public class DefaultServerOAuth2AuthorizationRequestResolver
6365
/**
6466
* The default pattern used to resolve the {@link ClientRegistration#getRegistrationId()}
6567
*/
66-
public static final String DEFAULT_AUTHORIZATION_REQUEST_PATTERN = "/oauth2/authorization/{" + DEFAULT_REGISTRATION_ID_URI_VARIABLE_NAME
67-
+ "}";
68+
public static final String DEFAULT_AUTHORIZATION_REQUEST_PATTERN = "/oauth2/authorization/{" + DEFAULT_REGISTRATION_ID_URI_VARIABLE_NAME + "}";
69+
70+
private static final char PATH_DELIMITER = '/';
6871

6972
private final ServerWebExchangeMatcher authorizationRequestMatcher;
7073

@@ -121,8 +124,7 @@ private Mono<ClientRegistration> findByRegistrationId(ServerWebExchange exchange
121124

122125
private OAuth2AuthorizationRequest authorizationRequest(ServerWebExchange exchange,
123126
ClientRegistration clientRegistration) {
124-
String redirectUriStr = this
125-
.expandRedirectUri(exchange.getRequest(), clientRegistration);
127+
String redirectUriStr = expandRedirectUri(exchange.getRequest(), clientRegistration);
126128

127129
Map<String, Object> attributes = new HashMap<>();
128130
attributes.put(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId());
@@ -153,23 +155,52 @@ else if (AuthorizationGrantType.IMPLICIT.equals(clientRegistration.getAuthorizat
153155
.build();
154156
}
155157

156-
private String expandRedirectUri(ServerHttpRequest request, ClientRegistration clientRegistration) {
157-
// Supported URI variables -> baseUrl, action, registrationId
158-
// Used in -> CommonOAuth2Provider.DEFAULT_REDIRECT_URL = "{baseUrl}/{action}/oauth2/code/{registrationId}"
158+
/**
159+
* Expands the {@link ClientRegistration#getRedirectUriTemplate()} with following provided variables:<br/>
160+
* - baseUrl (e.g. https://localhost/app) <br/>
161+
* - baseScheme (e.g. https) <br/>
162+
* - baseHost (e.g. localhost) <br/>
163+
* - basePort (e.g. :8080) <br/>
164+
* - basePath (e.g. /app) <br/>
165+
* - registrationId (e.g. google) <br/>
166+
* - action (e.g. login) <br/>
167+
* <p/>
168+
* Null variables are provided as empty strings.
169+
* <p/>
170+
* Default redirectUriTemplate is: {@link org.springframework.security.config.oauth2.client}.CommonOAuth2Provider#DEFAULT_REDIRECT_URL
171+
*
172+
* @return expanded URI
173+
*/
174+
private static String expandRedirectUri(ServerHttpRequest request, ClientRegistration clientRegistration) {
159175
Map<String, String> uriVariables = new HashMap<>();
160176
uriVariables.put("registrationId", clientRegistration.getRegistrationId());
161177

162-
String baseUrl = UriComponentsBuilder.fromUri(request.getURI())
178+
UriComponents uriComponents = UriComponentsBuilder.fromUri(request.getURI())
163179
.replacePath(request.getPath().contextPath().value())
164180
.replaceQuery(null)
165-
.build()
166-
.toUriString();
167-
uriVariables.put("baseUrl", baseUrl);
181+
.fragment(null)
182+
.build();
183+
String scheme = uriComponents.getScheme();
184+
uriVariables.put("baseScheme", scheme == null ? "" : scheme);
185+
String host = uriComponents.getHost();
186+
uriVariables.put("baseHost", host == null ? "" : host);
187+
// following logic is based on HierarchicalUriComponents#toUriString()
188+
int port = uriComponents.getPort();
189+
uriVariables.put("basePort", port == -1 ? "" : ":" + port);
190+
String path = uriComponents.getPath();
191+
if (StringUtils.hasLength(path)) {
192+
if (path.charAt(0) != PATH_DELIMITER) {
193+
path = PATH_DELIMITER + path;
194+
}
195+
}
196+
uriVariables.put("basePath", path == null ? "" : path);
197+
uriVariables.put("baseUrl", uriComponents.toUriString());
168198

199+
String action = "";
169200
if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
170-
String loginAction = "login";
171-
uriVariables.put("action", loginAction);
201+
action = "login";
172202
}
203+
uriVariables.put("action", action);
173204

174205
return UriComponentsBuilder.fromUriString(clientRegistration.getRedirectUriTemplate())
175206
.buildAndExpand(uriVariables)

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

+93-2
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,17 @@
4141
public class DefaultOAuth2AuthorizationRequestResolverTests {
4242
private ClientRegistration registration1;
4343
private ClientRegistration registration2;
44+
private ClientRegistration fineRedirectUriTemplateRegistration;
4445
private ClientRegistration pkceRegistration;
4546
private ClientRegistrationRepository clientRegistrationRepository;
46-
private String authorizationRequestBaseUri = "/oauth2/authorization";
47+
private final String authorizationRequestBaseUri = "/oauth2/authorization";
4748
private DefaultOAuth2AuthorizationRequestResolver resolver;
4849

4950
@Before
5051
public void setUp() {
5152
this.registration1 = TestClientRegistrations.clientRegistration().build();
5253
this.registration2 = TestClientRegistrations.clientRegistration2().build();
54+
this.fineRedirectUriTemplateRegistration = fineRedirectUriTemplateClientRegistration().build();
5355
this.pkceRegistration = TestClientRegistrations.clientRegistration()
5456
.registrationId("pkce-client-registration-id")
5557
.clientId("pkce-client-id")
@@ -58,7 +60,7 @@ public void setUp() {
5860
.build();
5961

6062
this.clientRegistrationRepository = new InMemoryClientRegistrationRepository(
61-
this.registration1, this.registration2, this.pkceRegistration);
63+
this.registration1, this.registration2, this.fineRedirectUriTemplateRegistration, this.pkceRegistration);
6264
this.resolver = new DefaultOAuth2AuthorizationRequestResolver(
6365
this.clientRegistrationRepository, this.authorizationRequestBaseUri);
6466
}
@@ -152,6 +154,80 @@ public void resolveWhenAuthorizationRequestRedirectUriTemplatedThenRedirectUriEx
152154
"http://localhost/login/oauth2/code/" + clientRegistration.getRegistrationId());
153155
}
154156

157+
@Test
158+
public void resolveWhenAuthorizationRequestRedirectUriTemplatedThenHttpRedirectUriWithExtraVarsExpanded() {
159+
ClientRegistration clientRegistration = this.fineRedirectUriTemplateRegistration;
160+
String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
161+
MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
162+
request.setServerPort(8080);
163+
request.setServletPath(requestUri);
164+
165+
OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
166+
assertThat(authorizationRequest.getRedirectUri()).isNotEqualTo(clientRegistration.getRedirectUriTemplate());
167+
assertThat(authorizationRequest.getRedirectUri()).isEqualTo(
168+
"http://localhost:8080/login/oauth2/code/" + clientRegistration.getRegistrationId());
169+
}
170+
171+
@Test
172+
public void resolveWhenAuthorizationRequestRedirectUriTemplatedThenHttpsRedirectUriWithExtraVarsExpanded() {
173+
ClientRegistration clientRegistration = this.fineRedirectUriTemplateRegistration;
174+
String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
175+
MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
176+
request.setScheme("https");
177+
request.setServerPort(8081);
178+
request.setServletPath(requestUri);
179+
180+
OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
181+
assertThat(authorizationRequest.getRedirectUri()).isNotEqualTo(clientRegistration.getRedirectUriTemplate());
182+
assertThat(authorizationRequest.getRedirectUri()).isEqualTo(
183+
"https://localhost:8081/login/oauth2/code/" + clientRegistration.getRegistrationId());
184+
}
185+
186+
@Test
187+
public void resolveWhenAuthorizationRequestIncludesPort80ThenExpandedRedirectUriWithExtraVarsExcludesPort() {
188+
ClientRegistration clientRegistration = this.fineRedirectUriTemplateRegistration;
189+
String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
190+
MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
191+
request.setScheme("http");
192+
request.setServerPort(80);
193+
request.setServletPath(requestUri);
194+
195+
OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
196+
assertThat(authorizationRequest.getRedirectUri()).isNotEqualTo(clientRegistration.getRedirectUriTemplate());
197+
assertThat(authorizationRequest.getRedirectUri()).isEqualTo(
198+
"http://localhost/login/oauth2/code/" + clientRegistration.getRegistrationId());
199+
}
200+
201+
@Test
202+
public void resolveWhenAuthorizationRequestIncludesPort443ThenExpandedRedirectUriWithExtraVarsExcludesPort() {
203+
ClientRegistration clientRegistration = this.fineRedirectUriTemplateRegistration;
204+
String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
205+
MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
206+
request.setScheme("https");
207+
request.setServerPort(443);
208+
request.setServletPath(requestUri);
209+
210+
OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
211+
assertThat(authorizationRequest.getRedirectUri()).isNotEqualTo(clientRegistration.getRedirectUriTemplate());
212+
assertThat(authorizationRequest.getRedirectUri()).isEqualTo(
213+
"https://localhost/login/oauth2/code/" + clientRegistration.getRegistrationId());
214+
}
215+
216+
@Test
217+
public void resolveWhenAuthorizationRequestHasNoPortThenExpandedRedirectUriWithExtraVarsExcludesPort() {
218+
ClientRegistration clientRegistration = this.fineRedirectUriTemplateRegistration;
219+
String requestUri = this.authorizationRequestBaseUri + "/" + clientRegistration.getRegistrationId();
220+
MockHttpServletRequest request = new MockHttpServletRequest("GET", requestUri);
221+
request.setScheme("https");
222+
request.setServerPort(-1);
223+
request.setServletPath(requestUri);
224+
225+
OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
226+
assertThat(authorizationRequest.getRedirectUri()).isNotEqualTo(clientRegistration.getRedirectUriTemplate());
227+
assertThat(authorizationRequest.getRedirectUri()).isEqualTo(
228+
"https://localhost/login/oauth2/code/" + clientRegistration.getRegistrationId());
229+
}
230+
155231
// gh-5520
156232
@Test
157233
public void resolveWhenAuthorizationRequestRedirectUriTemplatedThenRedirectUriExpandedExcludesQueryString() {
@@ -301,4 +377,19 @@ public void resolveWhenAuthorizationRequestWithValidPkceClientThenResolves() {
301377
"code_challenge_method=S256&" +
302378
"code_challenge=([a-zA-Z0-9\\-\\.\\_\\~]){43}");
303379
}
380+
381+
private static ClientRegistration.Builder fineRedirectUriTemplateClientRegistration() {
382+
return ClientRegistration.withRegistrationId("fine-redirect-uri-template-client-registration")
383+
.redirectUriTemplate("{baseScheme}://{baseHost}{basePort}{basePath}/{action}/oauth2/code/{registrationId}")
384+
.clientAuthenticationMethod(ClientAuthenticationMethod.BASIC)
385+
.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
386+
.scope("read:user")
387+
.authorizationUri("https://example.com/login/oauth/authorize")
388+
.tokenUri("https://example.com/login/oauth/access_token")
389+
.userInfoUri("https://api.example.com/user")
390+
.userNameAttributeName("id")
391+
.clientName("Fine Redirect Uri Template Client")
392+
.clientId("fine-redirect-uri-template-client")
393+
.clientSecret("client-secret");
394+
}
304395
}

0 commit comments

Comments
 (0)