Skip to content
This repository has been archived by the owner on May 31, 2022. It is now read-only.

Commit

Permalink
DefaultRedirectResolver matches on userinfo and query
Browse files Browse the repository at this point in the history
Fixes gh-1585
  • Loading branch information
phrinx authored and jgrandja committed Feb 19, 2019
1 parent 96f85b0 commit 16d39ad
Show file tree
Hide file tree
Showing 5 changed files with 201 additions and 33 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
Expand All @@ -21,13 +21,16 @@
import org.springframework.security.oauth2.common.exceptions.RedirectMismatchException;
import org.springframework.security.oauth2.provider.ClientDetails;
import org.springframework.util.Assert;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils;
import org.springframework.web.util.UriComponents;
import org.springframework.web.util.UriComponentsBuilder;

import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;

/**
Expand All @@ -47,7 +50,7 @@ public class DefaultRedirectResolver implements RedirectResolver {
/**
* Flag to indicate that requested URIs will match if they are a subdomain of the registered value.
*
* @param matchSubdomains the flag value to set (deafult true)
* @param matchSubdomains the flag value to set (default true)
*/
public void setMatchSubdomains(boolean matchSubdomains) {
this.matchSubdomains = matchSubdomains;
Expand Down Expand Up @@ -105,7 +108,8 @@ private boolean containsRedirectGrantType(Set<String> grantTypes) {
/**
* Whether the requested redirect URI "matches" the specified redirect URI. For a URL, this implementation tests if
* the user requested redirect starts with the registered redirect, so it would have the same host and root path if
* it is an HTTP URL. The port is also matched.
* it is an HTTP URL. The port, userinfo, query params also matched. Request redirect uri path can include
* additional parameters which are ignored for the match
* <p>
* For other (non-URL) cases, such as for some implicit clients, the redirect_uri must be an exact match.
*
Expand All @@ -115,22 +119,65 @@ private boolean containsRedirectGrantType(Set<String> grantTypes) {
*/
protected boolean redirectMatches(String requestedRedirect, String redirectUri) {
UriComponents requestedRedirectUri = UriComponentsBuilder.fromUriString(requestedRedirect).build();
String requestedRedirectUriScheme = (requestedRedirectUri.getScheme() != null ? requestedRedirectUri.getScheme() : "");
String requestedRedirectUriHost = (requestedRedirectUri.getHost() != null ? requestedRedirectUri.getHost() : "");
String requestedRedirectUriPath = (requestedRedirectUri.getPath() != null ? requestedRedirectUri.getPath() : "");

UriComponents registeredRedirectUri = UriComponentsBuilder.fromUriString(redirectUri).build();
String registeredRedirectUriScheme = (registeredRedirectUri.getScheme() != null ? registeredRedirectUri.getScheme() : "");
String registeredRedirectUriHost = (registeredRedirectUri.getHost() != null ? registeredRedirectUri.getHost() : "");
String registeredRedirectUriPath = (registeredRedirectUri.getPath() != null ? registeredRedirectUri.getPath() : "");

boolean portsMatch = this.matchPorts ? (registeredRedirectUri.getPort() == requestedRedirectUri.getPort()) : true;
boolean schemeMatch = isEqual(registeredRedirectUri.getScheme(), requestedRedirectUri.getScheme());
boolean userInfoMatch = isEqual(registeredRedirectUri.getUserInfo(), requestedRedirectUri.getUserInfo());
boolean hostMatch = hostMatches(registeredRedirectUri.getHost(), requestedRedirectUri.getHost());
boolean portMatch = matchPorts ? registeredRedirectUri.getPort() == requestedRedirectUri.getPort() : true;
boolean pathMatch = isEqual(registeredRedirectUri.getPath(),
StringUtils.cleanPath(requestedRedirectUri.getPath()));
boolean queryParamMatch = matchQueryParams(registeredRedirectUri.getQueryParams(),
requestedRedirectUri.getQueryParams());

return schemeMatch && userInfoMatch && hostMatch && portMatch && pathMatch && queryParamMatch;
}


/**
* Checks whether the registered redirect uri query params key and values contains match the requested set
*
* The requested redirect uri query params are allowed to contain additional params which will be retained
*
* @param registeredRedirectUriQueryParams
* @param requestedRedirectUriQueryParams
* @return whether the params match
*/
private boolean matchQueryParams(MultiValueMap<String, String> registeredRedirectUriQueryParams,
MultiValueMap<String, String> requestedRedirectUriQueryParams) {


Iterator<String> iter = registeredRedirectUriQueryParams.keySet().iterator();
while (iter.hasNext()) {
String key = iter.next();
List<String> registeredRedirectUriQueryParamsValues = registeredRedirectUriQueryParams.get(key);
List<String> requestedRedirectUriQueryParamsValues = requestedRedirectUriQueryParams.get(key);

if (!registeredRedirectUriQueryParamsValues.equals(requestedRedirectUriQueryParamsValues)) {
return false;
}
}

return true;
}



return registeredRedirectUriScheme.equals(requestedRedirectUriScheme) &&
hostMatches(registeredRedirectUriHost, requestedRedirectUriHost) &&
portsMatch &&
// Ensure exact path matching
registeredRedirectUriPath.equals(StringUtils.cleanPath(requestedRedirectUriPath));
/**
* Compares two strings but treats empty string or null equal
*
* @param str1
* @param str2
* @return true if strings are equal, false otherwise
*/
private boolean isEqual(String str1, String str2) {
if (StringUtils.isEmpty(str1) && StringUtils.isEmpty(str2)) {
return true;
} else if (!StringUtils.isEmpty(str1)) {
return str1.equals(str2);
} else {
return false;
}
}

/**
Expand All @@ -152,7 +199,7 @@ protected boolean hostMatches(String registered, String requested) {
*
* @param redirectUris the set of the registered URIs to try and find a match. This cannot be null or empty.
* @param requestedRedirect the URI used as part of the request
* @return the matching URI
* @return redirect uri
* @throws RedirectMismatchException if no match was found
*/
private String obtainMatchingRedirect(Set<String> redirectUris, String requestedRedirect) {
Expand All @@ -161,11 +208,26 @@ private String obtainMatchingRedirect(Set<String> redirectUris, String requested
if (redirectUris.size() == 1 && requestedRedirect == null) {
return redirectUris.iterator().next();
}

for (String redirectUri : redirectUris) {
if (requestedRedirect != null && redirectMatches(requestedRedirect, redirectUri)) {
return requestedRedirect;
// Initialize with the registered redirect-uri
UriComponentsBuilder redirectUriBuilder = UriComponentsBuilder.fromUriString(redirectUri);

UriComponents requestedRedirectUri = UriComponentsBuilder.fromUriString(requestedRedirect).build();

if (this.matchSubdomains) {
redirectUriBuilder.host(requestedRedirectUri.getHost());
}
if (!this.matchPorts) {
redirectUriBuilder.port(requestedRedirectUri.getPort());
}
redirectUriBuilder.replaceQuery(requestedRedirectUri.getQuery()); // retain additional params (if any)
redirectUriBuilder.fragment(null);
return redirectUriBuilder.build().toUriString();
}
}

throw new RedirectMismatchException("Invalid redirect: " + requestedRedirect
+ " does not match one of the registered values: " + redirectUris.toString());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class SubdomainRedirectResolverTests


@Test
public void testRedirectWatchdox() throws Exception
public void testRedirectMatch() throws Exception
{
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://watchdox.com"));
client.setRegisteredRedirectUri(redirectUris);
Expand All @@ -32,9 +32,9 @@ public void testRedirectWatchdox() throws Exception
}

@Test(expected=RedirectMismatchException.class)
public void testRedirectBadWatchdox() throws Exception
public void testRedirectNoMatch() throws Exception
{
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http//watchdox.com"));
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://watchdox.com"));
client.setRegisteredRedirectUri(redirectUris);
String requestedRedirect = "http://anywhere.google.com";
assertEquals(requestedRedirect, resolver.resolveRedirect(requestedRedirect, client));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
/*
* Copyright 2006-2011 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. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
* Copyright 2002-2019 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.security.oauth2.provider.endpoint;

Expand Down Expand Up @@ -180,4 +183,107 @@ public void testRedirectMatchPortsFalse() throws Exception {
String requestedRedirect = "http://anywhere.com:91";
assertEquals(requestedRedirect, resolver.resolveRedirect(requestedRedirect, client));
}

// gh-1566
@Test(expected = RedirectMismatchException.class)
public void testRedirectRegisteredUserInfoNotMatching() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://[email protected]"));
client.setRegisteredRedirectUri(redirectUris);
resolver.resolveRedirect("http://[email protected]", client);
}

// gh-1566
@Test(expected = RedirectMismatchException.class)
public void testRedirectRegisteredNoUserInfoNotMatching() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://[email protected]"));
client.setRegisteredRedirectUri(redirectUris);
resolver.resolveRedirect("http://anywhere.com", client);
}

// gh-1566
@Test()
public void testRedirectRegisteredUserInfoMatching() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://[email protected]"));
client.setRegisteredRedirectUri(redirectUris);
String requestedRedirect = "http://[email protected]";
assertEquals(requestedRedirect, resolver.resolveRedirect(requestedRedirect, client));
}

// gh-1566
@Test()
public void testRedirectRegisteredFragmentIgnoredAndStripped() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://[email protected]/foo/bar#baz"));
client.setRegisteredRedirectUri(redirectUris);
String requestedRedirect = "http://[email protected]/foo/bar";
assertEquals(requestedRedirect, resolver.resolveRedirect(requestedRedirect + "#bar", client));
}

// gh-1566
@Test()
public void testRedirectRegisteredQueryParamsMatching() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://anywhere.com/?p1=v1&p2=v2"));
client.setRegisteredRedirectUri(redirectUris);
String requestedRedirect = "http://anywhere.com/?p1=v1&p2=v2";
assertEquals(requestedRedirect, resolver.resolveRedirect(requestedRedirect, client));
}

// gh-1566
@Test()
public void testRedirectRegisteredQueryParamsMatchingIgnoringAdditionalParams() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://anywhere.com/?p1=v1&p2=v2"));
client.setRegisteredRedirectUri(redirectUris);
String requestedRedirect = "http://anywhere.com/?p1=v1&p2=v2&p3=v3";
assertEquals(requestedRedirect, resolver.resolveRedirect(requestedRedirect, client));
}

// gh-1566
@Test()
public void testRedirectRegisteredQueryParamsMatchingDifferentOrder() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://anywhere.com/?p1=v1&p2=v2"));
client.setRegisteredRedirectUri(redirectUris);
String requestedRedirect = "http://anywhere.com/?p2=v2&p1=v1";
assertEquals(requestedRedirect, resolver.resolveRedirect(requestedRedirect, client));
}

// gh-1566
@Test(expected = RedirectMismatchException.class)
public void testRedirectRegisteredQueryParamsWithDifferentValues() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://anywhere.com/?p1=v1&p2=v2"));
client.setRegisteredRedirectUri(redirectUris);
resolver.resolveRedirect("http://anywhere.com/?p1=v1&p2=v3", client);
}

// gh-1566
@Test(expected = RedirectMismatchException.class)
public void testRedirectRegisteredQueryParamsNotMatching() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://anywhere.com/?p1=v1"));
client.setRegisteredRedirectUri(redirectUris);
resolver.resolveRedirect("http://anywhere.com/?p2=v2", client);
}

// gh-1566
@Test(expected = RedirectMismatchException.class)
public void testRedirectRegisteredQueryParamsPartiallyMatching() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://anywhere.com/?p1=v1&p2=v2"));
client.setRegisteredRedirectUri(redirectUris);
resolver.resolveRedirect("http://anywhere.com/?p2=v2&p3=v3", client);
}

// gh-1566
@Test
public void testRedirectRegisteredQueryParamsMatchingWithMultipleValuesInRegistered() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://anywhere.com/?p1=v11&p1=v12"));
client.setRegisteredRedirectUri(redirectUris);
String requestedRedirect = "http://anywhere.com/?p1=v11&p1=v12";
assertEquals(requestedRedirect, resolver.resolveRedirect(requestedRedirect, client));
}

// gh-1566
@Test
public void testRedirectRegisteredQueryParamsMatchingWithParamWithNoValue() throws Exception {
Set<String> redirectUris = new HashSet<String>(Arrays.asList("http://anywhere.com/?p1&p2=v2"));
client.setRegisteredRedirectUri(redirectUris);
String requestedRedirect = "http://anywhere.com/?p1&p2=v2";
assertEquals(requestedRedirect, resolver.resolveRedirect(requestedRedirect, client));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class AuthorizationCodeProviderCookieTests extends AbstractEmptyAuthoriza
@Test
@OAuth2ContextConfiguration(resource = MyClientWithRegisteredRedirect.class, initialize = false)
public void testPostToProtectedResource() throws Exception {
approveAccessTokenGrant("http://anywhere", true);
approveAccessTokenGrant("http://anywhere?key=value", true);
assertNotNull(context.getAccessToken());
LinkedMultiValueMap<String, String> form = new LinkedMultiValueMap<>();
form.set("foo", "bar");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class AuthorizationCodeProviderCookieTests extends AbstractEmptyAuthoriza
@Test
@OAuth2ContextConfiguration(resource = MyClientWithRegisteredRedirect.class, initialize = false)
public void testPostToProtectedResource() throws Exception {
approveAccessTokenGrant("http://anywhere", true);
approveAccessTokenGrant("http://anywhere?key=value", true);
assertNotNull(context.getAccessToken());
LinkedMultiValueMap<String, String> form = new LinkedMultiValueMap<>();
form.set("foo", "bar");
Expand Down

0 comments on commit 16d39ad

Please sign in to comment.