Perform exact string match if redirect URI contains userinfo, encoded slashes or parent access (#131) (#28872)
Closes keycloak/keycloak-private#113 Closes keycloak/keycloak-private#134 Signed-off-by: rmartinc <rmartinc@redhat.com> Co-authored-by: Stian Thorgersen <stianst@gmail.com>
This commit is contained in:
parent
00d4cab55e
commit
fc6b6f0d94
8 changed files with 206 additions and 92 deletions
|
@ -46,6 +46,7 @@ public class Encode
|
||||||
private static final String[] matrixParameterEncoding = new String[128];
|
private static final String[] matrixParameterEncoding = new String[128];
|
||||||
private static final String[] queryNameValueEncoding = new String[128];
|
private static final String[] queryNameValueEncoding = new String[128];
|
||||||
private static final String[] queryStringEncoding = new String[128];
|
private static final String[] queryStringEncoding = new String[128];
|
||||||
|
private static final String[] userInfoStringEncoding = new String[128];
|
||||||
|
|
||||||
static
|
static
|
||||||
{
|
{
|
||||||
|
@ -158,6 +159,44 @@ public class Encode
|
||||||
}
|
}
|
||||||
queryStringEncoding[i] = URLEncoder.encode(String.valueOf((char) i));
|
queryStringEncoding[i] = URLEncoder.encode(String.valueOf((char) i));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* userinfo = *( unreserved / pct-encoded / sub-delims / ":" )
|
||||||
|
* unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
|
||||||
|
* pct-encoded = "%" HEXDIG HEXDIG
|
||||||
|
* sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
|
||||||
|
* / "*" / "+" / "," / ";" / "="
|
||||||
|
*/
|
||||||
|
for (int i = 0; i < 128; i++)
|
||||||
|
{
|
||||||
|
if (i >= 'a' && i <= 'z') continue;
|
||||||
|
if (i >= 'A' && i <= 'Z') continue;
|
||||||
|
if (i >= '0' && i <= '9') continue;
|
||||||
|
switch ((char) i)
|
||||||
|
{
|
||||||
|
case '-':
|
||||||
|
case '.':
|
||||||
|
case '_':
|
||||||
|
case '~':
|
||||||
|
case '!':
|
||||||
|
case '$':
|
||||||
|
case '&':
|
||||||
|
case '\'':
|
||||||
|
case '(':
|
||||||
|
case ')':
|
||||||
|
case '*':
|
||||||
|
case '+':
|
||||||
|
case ',':
|
||||||
|
case ';':
|
||||||
|
case '=':
|
||||||
|
case ':':
|
||||||
|
continue;
|
||||||
|
case ' ':
|
||||||
|
userInfoStringEncoding[i] = "%20";
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
userInfoStringEncoding[i] = URLEncoder.encode(String.valueOf((char) i));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -177,6 +216,24 @@ public class Encode
|
||||||
return encodeNonCodes(encodeFromArray(value, queryStringEncoding, false));
|
return encodeNonCodes(encodeFromArray(value, queryStringEncoding, false));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Keep encoded values "%..." and template parameters intact.
|
||||||
|
* @param value The user-info value to encode
|
||||||
|
* @return The user-info encoded
|
||||||
|
*/
|
||||||
|
public static String encodeUserInfo(String value) {
|
||||||
|
return encodeValue(value, userInfoStringEncoding);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Keep encoded values "%..." but not the template parameters.
|
||||||
|
* @param value The user-info to encode
|
||||||
|
* @return The user-info encoded
|
||||||
|
*/
|
||||||
|
public static String encodeUserInfoNotTemplateParameters(String value) {
|
||||||
|
return encodeNonCodes(encodeFromArray(value, userInfoStringEncoding, false));
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Keep encoded values "%...", matrix parameters, template parameters, and '/' characters intact.
|
* Keep encoded values "%...", matrix parameters, template parameters, and '/' characters intact.
|
||||||
*/
|
*/
|
||||||
|
@ -424,6 +481,30 @@ public class Encode
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Encodes everything in user-info
|
||||||
|
*
|
||||||
|
* @param nameOrValue
|
||||||
|
* @return
|
||||||
|
*/
|
||||||
|
public static String encodeUserInfoAsIs(String nameOrValue)
|
||||||
|
{
|
||||||
|
return encodeFromArray(nameOrValue, userInfoStringEncoding, true);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Keep any valid encodings from string i.e. keep "%2D" but don't keep "%p"
|
||||||
|
*
|
||||||
|
* @param segment
|
||||||
|
* @return
|
||||||
|
*/
|
||||||
|
public static String encodeUserInfoSaveEncodings(String segment)
|
||||||
|
{
|
||||||
|
String result = encodeFromArray(segment, userInfoStringEncoding, false);
|
||||||
|
result = encodeNonCodes(result);
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
public static String encodeFragmentAsIs(String nameOrValue)
|
public static String encodeFragmentAsIs(String nameOrValue)
|
||||||
{
|
{
|
||||||
return encodeFromArray(nameOrValue, queryNameValueEncoding, true);
|
return encodeFromArray(nameOrValue, queryNameValueEncoding, true);
|
||||||
|
|
|
@ -150,7 +150,7 @@ public class KeycloakUriBuilder {
|
||||||
if (at > -1) {
|
if (at > -1) {
|
||||||
String user = host.substring(0, at);
|
String user = host.substring(0, at);
|
||||||
host = host.substring(at + 1);
|
host = host.substring(at + 1);
|
||||||
this.userInfo = user;
|
replaceUserInfo(user, template);
|
||||||
}
|
}
|
||||||
Matcher hostPortMatch = hostPortPattern.matcher(host);
|
Matcher hostPortMatch = hostPortPattern.matcher(host);
|
||||||
if (hostPortMatch.matches()) {
|
if (hostPortMatch.matches()) {
|
||||||
|
@ -459,7 +459,7 @@ public class KeycloakUriBuilder {
|
||||||
} else if (userInfo != null || host != null || port != -1) {
|
} else if (userInfo != null || host != null || port != -1) {
|
||||||
buffer.append("//");
|
buffer.append("//");
|
||||||
if (userInfo != null)
|
if (userInfo != null)
|
||||||
replaceParameter(paramMap, fromEncodedMap, isTemplate, userInfo, buffer, encodeSlash).append("@");
|
replaceUserInfoParameter(paramMap, fromEncodedMap, isTemplate, userInfo, buffer).append("@");
|
||||||
if (host != null) {
|
if (host != null) {
|
||||||
if ("".equals(host)) throw new RuntimeException("empty host name");
|
if ("".equals(host)) throw new RuntimeException("empty host name");
|
||||||
replaceParameter(paramMap, fromEncodedMap, isTemplate, host, buffer, encodeSlash);
|
replaceParameter(paramMap, fromEncodedMap, isTemplate, host, buffer, encodeSlash);
|
||||||
|
@ -571,6 +571,33 @@ public class KeycloakUriBuilder {
|
||||||
return buffer;
|
return buffer;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected StringBuffer replaceUserInfoParameter(Map<String, ?> paramMap, boolean fromEncodedMap, boolean isTemplate, String string, StringBuffer buffer) {
|
||||||
|
Matcher matcher = createUriParamMatcher(string);
|
||||||
|
while (matcher.find()) {
|
||||||
|
String param = matcher.group(1);
|
||||||
|
Object valObj = paramMap.get(param);
|
||||||
|
if (valObj == null && !isTemplate) {
|
||||||
|
throw new IllegalArgumentException("NULL value for template parameter: " + param);
|
||||||
|
} else if (valObj == null && isTemplate) {
|
||||||
|
matcher.appendReplacement(buffer, matcher.group());
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
String value = valObj.toString();
|
||||||
|
if (value != null) {
|
||||||
|
if (!fromEncodedMap) {
|
||||||
|
value = Encode.encodeUserInfoAsIs(value);
|
||||||
|
} else {
|
||||||
|
value = Encode.encodeUserInfoSaveEncodings(value);
|
||||||
|
}
|
||||||
|
matcher.appendReplacement(buffer, value);
|
||||||
|
} else {
|
||||||
|
throw new IllegalArgumentException("path param " + param + " has not been provided by the parameter map");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
matcher.appendTail(buffer);
|
||||||
|
return buffer;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Return a unique order list of path params
|
* Return a unique order list of path params
|
||||||
*
|
*
|
||||||
|
@ -742,6 +769,15 @@ public class KeycloakUriBuilder {
|
||||||
return this;
|
return this;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public KeycloakUriBuilder replaceUserInfo(String userInfo, boolean template) {
|
||||||
|
if (userInfo == null) {
|
||||||
|
this.userInfo = null;
|
||||||
|
return this;
|
||||||
|
}
|
||||||
|
this.userInfo = template? Encode.encodeUserInfo(userInfo) : Encode.encodeUserInfoNotTemplateParameters(userInfo);
|
||||||
|
return this;
|
||||||
|
}
|
||||||
|
|
||||||
public URI build(Object[] values, boolean encodeSlashInPath) throws IllegalArgumentException {
|
public URI build(Object[] values, boolean encodeSlashInPath) throws IllegalArgumentException {
|
||||||
if (values == null) throw new IllegalArgumentException("values param is null");
|
if (values == null) throw new IllegalArgumentException("values param is null");
|
||||||
return buildFromValues(encodeSlashInPath, false, values);
|
return buildFromValues(encodeSlashInPath, false, values);
|
||||||
|
|
|
@ -80,4 +80,16 @@ public class KeycloakUriBuilderTest {
|
||||||
Assert.assertEquals("https://localhost:8443/%7Bpath%7D?key=%7Bquery%7D#%7Bfragment%7D", KeycloakUriBuilder.fromUri(
|
Assert.assertEquals("https://localhost:8443/%7Bpath%7D?key=%7Bquery%7D#%7Bfragment%7D", KeycloakUriBuilder.fromUri(
|
||||||
"https://localhost:8443/{path}?key={query}#{fragment}", false).buildAsString());
|
"https://localhost:8443/{path}?key={query}#{fragment}", false).buildAsString());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testUserInfo() {
|
||||||
|
Assert.assertEquals("https://user-info@localhost:8443/path?key=query#fragment", KeycloakUriBuilder.fromUri(
|
||||||
|
"https://{userinfo}@localhost:8443/{path}?key={query}#{fragment}").buildAsString("user-info", "path", "query", "fragment"));
|
||||||
|
Assert.assertEquals("https://user%20info%40%2F@localhost:8443/path?key=query#fragment", KeycloakUriBuilder.fromUri(
|
||||||
|
"https://{userinfo}@localhost:8443/{path}?key={query}#{fragment}").buildAsString("user info@/", "path", "query", "fragment"));
|
||||||
|
Assert.assertEquals("https://user-info%E2%82%AC@localhost:8443", KeycloakUriBuilder.fromUri(
|
||||||
|
"https://user-info%E2%82%AC@localhost:8443", false).buildAsString());
|
||||||
|
Assert.assertEquals("https://user-info%E2%82%AC@localhost:8443", KeycloakUriBuilder.fromUri(
|
||||||
|
"https://user-info€@localhost:8443", false).buildAsString());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -24,9 +24,13 @@ the name, set up a replacement string value. For example, a string value such as
|
||||||
|
|
||||||
*Home URL*:: Provides the default URL for when the auth server needs to redirect or link back to the client.
|
*Home URL*:: Provides the default URL for when the auth server needs to redirect or link back to the client.
|
||||||
|
|
||||||
*Valid Redirect URIs*:: Required field. Enter a URL pattern and click *+* to add and *-* to remove existing URLs and click *Save*. You can use wildcards at the end of the URL pattern. For example $$http://host.com/*$$
|
*Valid Redirect URIs*:: Required field. Enter a URL pattern and click *+* to add and *-* to remove existing URLs and click *Save*. Exact (case sensitive) string matching is used to compare valid redirect URIs.
|
||||||
+
|
+
|
||||||
Exclusive redirect URL patterns are typically more secure. See xref:unspecific-redirect-uris_{context}[Unspecific Redirect URIs] for more information.
|
You can use wildcards at the end of the URL pattern. For example `$$http://host.com/path/*$$`. To avoid security issues, if the passed redirect URI contains the *userinfo* part or its *path* manages access to parent directory (`/../`) no wildcard comparison is performed but the standard and secure exact string matching.
|
||||||
|
+
|
||||||
|
The full wildcard `$$*$$` valid redirect URI can also be configured to allow any *http* or *https* redirect URI. Please do not use it in production environments.
|
||||||
|
+
|
||||||
|
Exclusive redirect URI patterns are typically more secure. See xref:unspecific-redirect-uris_{context}[Unspecific Redirect URIs] for more information.
|
||||||
|
|
||||||
Web Origins:: Enter a URL pattern and click + to add and - to remove existing URLs. Click Save.
|
Web Origins:: Enter a URL pattern and click + to add and - to remove existing URLs. Click Save.
|
||||||
+
|
+
|
||||||
|
|
|
@ -15,3 +15,10 @@ Differently than the previous contract and behavior, this method is only invoked
|
||||||
was loaded from.
|
was loaded from.
|
||||||
|
|
||||||
endif::[]
|
endif::[]
|
||||||
|
= Changes in redirect URI verification when using wildcards
|
||||||
|
|
||||||
|
Because of security concerns, the redirect URI verification now performs a exact string matching (no wildcard involved) if the passed redirect uri contains a `userinfo` part or its `path` accesses parent directory (`/../`).
|
||||||
|
|
||||||
|
The full wildcard `*` can still be used as a valid redirect in development for http(s) URIs with those characteristics. In production environments a exact valid redirect URI without wildcard needs to be configured for any URI of that type.
|
||||||
|
|
||||||
|
Please note that wildcard valid redirect URIs are not recommended for production and not covered by the OAuth 2.0 specification.
|
||||||
|
|
|
@ -18,8 +18,6 @@
|
||||||
package org.keycloak.protocol.oidc.utils;
|
package org.keycloak.protocol.oidc.utils;
|
||||||
|
|
||||||
import org.jboss.logging.Logger;
|
import org.jboss.logging.Logger;
|
||||||
import org.keycloak.common.util.Encode;
|
|
||||||
import org.keycloak.common.util.KeycloakUriBuilder;
|
|
||||||
import org.keycloak.common.util.UriUtils;
|
import org.keycloak.common.util.UriUtils;
|
||||||
import org.keycloak.models.ClientModel;
|
import org.keycloak.models.ClientModel;
|
||||||
import org.keycloak.models.Constants;
|
import org.keycloak.models.Constants;
|
||||||
|
@ -34,6 +32,7 @@ import java.net.URI;
|
||||||
import java.util.Collection;
|
import java.util.Collection;
|
||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
import java.util.TreeSet;
|
import java.util.TreeSet;
|
||||||
|
import java.util.regex.Pattern;
|
||||||
import java.util.stream.Collectors;
|
import java.util.stream.Collectors;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -111,16 +110,13 @@ public class RedirectUtils {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Make the validations against fully decoded and normalized redirect-url. This also allows wildcards (case when client configured "Valid redirect-urls" contain wildcards)
|
// check if the passed URI allows wildcards
|
||||||
String decodedRedirectUri = decodeRedirectUri(redirectUri);
|
boolean allowWildcards = areWildcardsAllowed(originalRedirect);
|
||||||
URI decodedRedirect = toUri(decodedRedirectUri);
|
|
||||||
decodedRedirectUri = getNormalizedRedirectUri(decodedRedirect);
|
|
||||||
if (decodedRedirectUri == null) return null;
|
|
||||||
|
|
||||||
String r = decodedRedirectUri;
|
String r = redirectUri;
|
||||||
Set<String> resolveValidRedirects = resolveValidRedirects(session, rootUrl, validRedirects);
|
Set<String> resolveValidRedirects = resolveValidRedirects(session, rootUrl, validRedirects);
|
||||||
|
|
||||||
String valid = matchesRedirects(resolveValidRedirects, r, true);
|
String valid = matchesRedirects(resolveValidRedirects, r, allowWildcards);
|
||||||
|
|
||||||
if (valid == null && (r.startsWith(Constants.INSTALLED_APP_URL) || r.startsWith(Constants.INSTALLED_APP_LOOPBACK)) && r.indexOf(':', Constants.INSTALLED_APP_URL.length()) >= 0) {
|
if (valid == null && (r.startsWith(Constants.INSTALLED_APP_URL) || r.startsWith(Constants.INSTALLED_APP_LOOPBACK)) && r.indexOf(':', Constants.INSTALLED_APP_URL.length()) >= 0) {
|
||||||
int i = r.indexOf(':', Constants.INSTALLED_APP_URL.length());
|
int i = r.indexOf(':', Constants.INSTALLED_APP_URL.length());
|
||||||
|
@ -135,15 +131,7 @@ public class RedirectUtils {
|
||||||
|
|
||||||
r = sb.toString();
|
r = sb.toString();
|
||||||
|
|
||||||
valid = matchesRedirects(resolveValidRedirects, r, true);
|
valid = matchesRedirects(resolveValidRedirects, r, allowWildcards);
|
||||||
}
|
|
||||||
|
|
||||||
// Return the original redirectUri, which can be partially encoded - for example http://localhost:8280/foo/bar%20bar%2092%2F72/3 . Just make sure it is normalized
|
|
||||||
redirectUri = getNormalizedRedirectUri(originalRedirect);
|
|
||||||
|
|
||||||
// We try to check validity also for original (encoded) redirectUrl, but just in case it exactly matches some "Valid Redirect URL" specified for client (not wildcards allowed)
|
|
||||||
if (valid == null) {
|
|
||||||
valid = matchesRedirects(resolveValidRedirects, redirectUri, false);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (valid != null && !originalRedirect.isAbsolute()) {
|
if (valid != null && !originalRedirect.isAbsolute()) {
|
||||||
|
@ -154,7 +142,7 @@ public class RedirectUtils {
|
||||||
redirectUri = relativeToAbsoluteURI(session, rootUrl, redirectUri);
|
redirectUri = relativeToAbsoluteURI(session, rootUrl, redirectUri);
|
||||||
}
|
}
|
||||||
|
|
||||||
String scheme = decodedRedirect.getScheme();
|
String scheme = originalRedirect.getScheme();
|
||||||
if (valid != null && scheme != null) {
|
if (valid != null && scheme != null) {
|
||||||
// check the scheme is valid, it should be http(s) or explicitly allowed by the validation
|
// check the scheme is valid, it should be http(s) or explicitly allowed by the validation
|
||||||
if (!valid.startsWith(scheme + ":") && !"http".equalsIgnoreCase(scheme) && !"https".equalsIgnoreCase(scheme)) {
|
if (!valid.startsWith(scheme + ":") && !"http".equalsIgnoreCase(scheme) && !"https".equalsIgnoreCase(scheme)) {
|
||||||
|
@ -179,51 +167,22 @@ public class RedirectUtils {
|
||||||
try {
|
try {
|
||||||
uri = URI.create(redirectUri);
|
uri = URI.create(redirectUri);
|
||||||
} catch (IllegalArgumentException cause) {
|
} catch (IllegalArgumentException cause) {
|
||||||
logger.debug("Invalid redirect uri", cause);
|
logger.debugf(cause, "Invalid redirect uri %s", redirectUri);
|
||||||
} catch (Exception cause) {
|
} catch (Exception cause) {
|
||||||
logger.debug("Unexpected error when parsing redirect uri", cause);
|
logger.debugf(cause, "Unexpected error when parsing redirect uri %s", redirectUri);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return uri;
|
return uri;
|
||||||
}
|
}
|
||||||
|
|
||||||
private static String getNormalizedRedirectUri(URI uri) {
|
// any access to parent folder /../ is unsafe with or without encoding
|
||||||
String redirectUri = null;
|
private final static Pattern UNSAFE_PATH_PATTERN = Pattern.compile(
|
||||||
if (uri != null) {
|
"(/|%2[fF]|%5[cC]|\\\\)(%2[eE]|\\.){2}(/|%2[fF]|%5[cC]|\\\\)|(/|%2[fF]|%5[cC]|\\\\)(%2[eE]|\\.){2}$");
|
||||||
redirectUri = uri.normalize().toString();
|
|
||||||
}
|
|
||||||
return redirectUri;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Decode redirectUri. Only path is decoded as other elements can be encoded in the original URL or cannot be encoded at all.
|
private static boolean areWildcardsAllowed(URI redirectUri) {
|
||||||
// URL can be decoded multiple times (in case it was encoded multiple times, or some of it's parts were encoded multiple times)
|
// wildcars are only allowed if no user-info and no unsafe pattern in path
|
||||||
private static String decodeRedirectUri(String redirectUri) {
|
return redirectUri.getRawUserInfo() == null
|
||||||
if (redirectUri == null) return null;
|
&& (redirectUri.getRawPath() == null || !UNSAFE_PATH_PATTERN.matcher(redirectUri.getRawPath()).find());
|
||||||
int MAX_DECODING_COUNT = 5; // Max count of attempts for decoding URL (in case it was encoded multiple times)
|
|
||||||
|
|
||||||
try {
|
|
||||||
KeycloakUriBuilder uriBuilder = KeycloakUriBuilder.fromUri(redirectUri, false).preserveDefaultPort();
|
|
||||||
if (uriBuilder.getPath() == null) {
|
|
||||||
return redirectUri;
|
|
||||||
}
|
|
||||||
String encodedPath = uriBuilder.getPath();
|
|
||||||
String decodedPath;
|
|
||||||
|
|
||||||
for (int i = 0; i < MAX_DECODING_COUNT; i++) {
|
|
||||||
decodedPath = Encode.decode(encodedPath);
|
|
||||||
if (decodedPath.equals(encodedPath)) {
|
|
||||||
// URL path is decoded. We can return it in the original redirect URI
|
|
||||||
return uriBuilder.replacePath(decodedPath, false).buildAsString();
|
|
||||||
} else {
|
|
||||||
// Next attempt
|
|
||||||
encodedPath = decodedPath;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} catch (IllegalArgumentException iae) {
|
|
||||||
logger.debugf("Illegal redirect URI used: %s, Details: %s", redirectUri, iae.getMessage());
|
|
||||||
}
|
|
||||||
logger.debugf("Was not able to decode redirect URI: %s", redirectUri);
|
|
||||||
return null;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private static String relativeToAbsoluteURI(KeycloakSession session, String rootUrl, String relative) {
|
private static String relativeToAbsoluteURI(KeycloakSession session, String rootUrl, String relative) {
|
||||||
|
@ -240,24 +199,20 @@ public class RedirectUtils {
|
||||||
return sb.toString();
|
return sb.toString();
|
||||||
}
|
}
|
||||||
|
|
||||||
// removes the queryString, fragment and userInfo from the redirect
|
|
||||||
// to avoid comparing this when wildcards are used
|
|
||||||
private static String stripOffRedirectForWildcard(String redirect) {
|
|
||||||
return KeycloakUriBuilder.fromUri(redirect, false)
|
|
||||||
.preserveDefaultPort()
|
|
||||||
.userInfo(null)
|
|
||||||
.replaceQuery(null)
|
|
||||||
.fragment(null)
|
|
||||||
.buildAsString();
|
|
||||||
}
|
|
||||||
|
|
||||||
// return the String that matched the redirect or null if not matched
|
// return the String that matched the redirect or null if not matched
|
||||||
private static String matchesRedirects(Set<String> validRedirects, String redirect, boolean allowWildcards) {
|
private static String matchesRedirects(Set<String> validRedirects, String redirect, boolean allowWildcards) {
|
||||||
logger.tracef("matchesRedirects: redirect URL to check: %s, allow wildcards: %b, Configured valid redirect URLs: %s", redirect, allowWildcards, validRedirects);
|
logger.tracef("matchesRedirects: redirect URL to check: %s, allow wildcards: %b, Configured valid redirect URLs: %s", redirect, allowWildcards, validRedirects);
|
||||||
for (String validRedirect : validRedirects) {
|
for (String validRedirect : validRedirects) {
|
||||||
if (validRedirect.endsWith("*") && !validRedirect.contains("?") && allowWildcards) {
|
if ("*".equals(validRedirect)) {
|
||||||
// strip off the userInfo, query or fragment components - we don't check them when wildcards are effective
|
// the valid redirect * is a full wildcard for http(s) even if the redirect URI does not allow wildcards
|
||||||
String r = stripOffRedirectForWildcard(redirect);
|
return validRedirect;
|
||||||
|
} else if (validRedirect.endsWith("*") && !validRedirect.contains("?") && allowWildcards) {
|
||||||
|
// strip off the query or fragment components - we don't check them when wildcards are effective
|
||||||
|
int idx = redirect.indexOf('?');
|
||||||
|
if (idx == -1) {
|
||||||
|
idx = redirect.indexOf('#');
|
||||||
|
}
|
||||||
|
String r = idx == -1 ? redirect : redirect.substring(0, idx);
|
||||||
// strip off *
|
// strip off *
|
||||||
int length = validRedirect.length() - 1;
|
int length = validRedirect.length() - 1;
|
||||||
validRedirect = validRedirect.substring(0, length);
|
validRedirect = validRedirect.substring(0, length);
|
||||||
|
|
|
@ -86,6 +86,8 @@ public class RedirectUtilsTest {
|
||||||
Assert.assertEquals("custom1:/parent/child", RedirectUtils.verifyRedirectUri(session, null, "custom1:/parent/child", set, false));
|
Assert.assertEquals("custom1:/parent/child", RedirectUtils.verifyRedirectUri(session, null, "custom1:/parent/child", set, false));
|
||||||
Assert.assertEquals("custom2:/something", RedirectUtils.verifyRedirectUri(session, null, "custom2:/something", set, false));
|
Assert.assertEquals("custom2:/something", RedirectUtils.verifyRedirectUri(session, null, "custom2:/something", set, false));
|
||||||
Assert.assertEquals("https://keycloak.org/test", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test", set, false));
|
Assert.assertEquals("https://keycloak.org/test", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test", set, false));
|
||||||
|
Assert.assertEquals("https://keycloak.org/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/", set, false));
|
||||||
|
Assert.assertEquals("https://keycloak.org", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org", set, false));
|
||||||
|
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "custom1:/test", set, false));
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "custom1:/test", set, false));
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "custom1:/test1/test", set, false));
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "custom1:/test1/test", set, false));
|
||||||
|
@ -171,6 +173,8 @@ public class RedirectUtilsTest {
|
||||||
|
|
||||||
Assert.assertEquals("https://keycloak.org/path", RedirectUtils.verifyRedirectUri(session, "https://keycloak.org", "/path", set, false));
|
Assert.assertEquals("https://keycloak.org/path", RedirectUtils.verifyRedirectUri(session, "https://keycloak.org", "/path", set, false));
|
||||||
Assert.assertEquals("https://keycloak.org/path", RedirectUtils.verifyRedirectUri(session, "https://keycloak.org", "path", set, false));
|
Assert.assertEquals("https://keycloak.org/path", RedirectUtils.verifyRedirectUri(session, "https://keycloak.org", "path", set, false));
|
||||||
|
Assert.assertEquals("https://keycloak.org/test/../other", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/../other", set, false));
|
||||||
|
Assert.assertEquals("http://keycloak.org/test%2Fother", RedirectUtils.verifyRedirectUri(session, null, "http://keycloak.org/test%2Fother", set, false));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -183,8 +187,6 @@ public class RedirectUtilsTest {
|
||||||
|
|
||||||
Assert.assertEquals("https://keycloak.org/index.html", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/index.html", set, false));
|
Assert.assertEquals("https://keycloak.org/index.html", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/index.html", set, false));
|
||||||
Assert.assertEquals("https://test.com/index.html", RedirectUtils.verifyRedirectUri(session, null, "https://test.com/index.html", set, false));
|
Assert.assertEquals("https://test.com/index.html", RedirectUtils.verifyRedirectUri(session, null, "https://test.com/index.html", set, false));
|
||||||
Assert.assertEquals("https://something@keycloak.org/path", RedirectUtils.verifyRedirectUri(session, null, "https://something@keycloak.org/path", set, false));
|
|
||||||
Assert.assertEquals("https://some%20thing@test.com/path", RedirectUtils.verifyRedirectUri(session, null, "https://some%20thing@test.com/path", set, false));
|
|
||||||
Assert.assertEquals("https://something@keycloak.com/exact", RedirectUtils.verifyRedirectUri(session, null, "https://something@keycloak.com/exact", set, false));
|
Assert.assertEquals("https://something@keycloak.com/exact", RedirectUtils.verifyRedirectUri(session, null, "https://something@keycloak.com/exact", set, false));
|
||||||
|
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://something@other.com/", set, false));
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://something@other.com/", set, false));
|
||||||
|
@ -192,12 +194,15 @@ public class RedirectUtilsTest {
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org%2F@other.com", set, false));
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org%2F@other.com", set, false));
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://test@other.com", set, false));
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://test@other.com", set, false));
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://test.com@other.com", set, false));
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://test.com@other.com", set, false));
|
||||||
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://something@keycloak.org/path", set, false));
|
||||||
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://some%20thing@test.com/path", set, false));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testEncodedRedirectUri() {
|
public void testEncodedRedirectUri() {
|
||||||
Set<String> set = Stream.of(
|
Set<String> set = Stream.of(
|
||||||
"https://keycloak.org/test/*"
|
"https://keycloak.org/test/*",
|
||||||
|
"https://keycloak.org/exact/%5C%2F/.."
|
||||||
).collect(Collectors.toSet());
|
).collect(Collectors.toSet());
|
||||||
|
|
||||||
Assert.assertEquals("https://keycloak.org/test/index.html", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/index.html", set, false));
|
Assert.assertEquals("https://keycloak.org/test/index.html", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/index.html", set, false));
|
||||||
|
@ -205,16 +210,30 @@ public class RedirectUtilsTest {
|
||||||
Assert.assertEquals("https://keycloak.org/test?encodeTest=a%3Cb#encode2=a%3Cb", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test?encodeTest=a%3Cb#encode2=a%3Cb", set, false));
|
Assert.assertEquals("https://keycloak.org/test?encodeTest=a%3Cb#encode2=a%3Cb", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test?encodeTest=a%3Cb#encode2=a%3Cb", set, false));
|
||||||
Assert.assertEquals("https://keycloak.org/test/#encode2=a%3Cb", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/#encode2=a%3Cb", set, false));
|
Assert.assertEquals("https://keycloak.org/test/#encode2=a%3Cb", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/#encode2=a%3Cb", set, false));
|
||||||
|
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/../", set, false)); // direct
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/../", set, false));
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2E%2E/", set, false)); // encoded
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test\\..\\", set, false));
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test%2F%2E%2E%2F", set, false)); // encoded
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2E%2E/", set, false));
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/", set, false)); // double-encoded
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2e%2e/", set, false));
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/?some_query_param=some_value", set, false)); // double-encoded
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2E./", set, false));
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/#encodeTest=a%3Cb", set, false)); // double-encoded
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2E.", set, false));
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%25252E%25252E/", set, false)); // triple-encoded
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test\\%2E.", set, false));
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2525252525252E%2525252525252E/", set, false)); // seventh-encoded
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test%2f%2E%2e%2F", set, false));
|
||||||
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test%5C%2E.%5c", set, false));
|
||||||
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test%5C..", set, false));
|
||||||
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2F%2E%2E%2Fdocumentation", set, false));
|
||||||
|
Assert.assertEquals("https://keycloak.org/test/.../", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/.../", set, false));
|
||||||
|
Assert.assertEquals("https://keycloak.org/test/%2E../", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%2E../", set, false)); // encoded
|
||||||
|
Assert.assertEquals("https://keycloak.org/test/some%2Fthing/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/some%2Fthing/", set, false)); // encoded
|
||||||
|
Assert.assertEquals("https://keycloak.org/test/./", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/./", set, false));
|
||||||
|
Assert.assertEquals("https://keycloak.org/test/%252E%252E/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/", set, false)); // double-encoded
|
||||||
|
Assert.assertEquals("https://keycloak.org/test/%252E%252E/#encodeTest=a%3Cb", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%252E%252E/#encodeTest=a%3Cb", set, false)); // double-encoded
|
||||||
|
Assert.assertEquals("https://keycloak.org/test/%25252E%25252E/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test/%25252E%25252E/", set, false)); // triple-encoded
|
||||||
|
Assert.assertEquals("https://keycloak.org/exact/%5C%2F/..", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/exact/%5C%2F/..", set, false));
|
||||||
|
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak%2Eorg/test/", set, false));
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak%2Eorg/test/", set, false));
|
||||||
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org%2Ftest%2F%40sample.com", set, false));
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org%2Ftest%2F%40sample.com", set, false));
|
||||||
|
|
||||||
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test%2Fanother/../any/path/", set, false));
|
||||||
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/test%2Fanother/%2E%2E/any/path/", set, false));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -352,12 +352,12 @@ public class OAuthRedirectUriTest extends AbstractKeycloakTest {
|
||||||
checkRedirectUri("http://example.com/foo/../", false);
|
checkRedirectUri("http://example.com/foo/../", false);
|
||||||
checkRedirectUri("http://example.com/foo/%2E%2E/", false); // url-encoded "http://example.com/foobar/../"
|
checkRedirectUri("http://example.com/foo/%2E%2E/", false); // url-encoded "http://example.com/foobar/../"
|
||||||
checkRedirectUri("http://example.com/foo%2F%2E%2E%2F", false); // url-encoded "http://example.com/foobar/../"
|
checkRedirectUri("http://example.com/foo%2F%2E%2E%2F", false); // url-encoded "http://example.com/foobar/../"
|
||||||
checkRedirectUri("http://example.com/foo/%252E%252E/", false); // double-encoded "http://example.com/foobar/../"
|
checkRedirectUri("http://example.com/foo/%252E%252E/", true); // double-encoded "http://example.com/foobar/../"
|
||||||
checkRedirectUri("http://example.com/foo/%252E%252E/?some_query_param=some_value", false); // double-encoded "http://example.com/foobar/../?some_query_param=some_value"
|
checkRedirectUri("http://example.com/foo/%252E%252E/?some_query_param=some_value", true); // double-encoded "http://example.com/foobar/../?some_query_param=some_value"
|
||||||
checkRedirectUri("http://example.com/foo/%252E%252E/?encodeTest=a%3Cb", false); // double-encoded "http://example.com/foobar/../?encodeTest=a<b"
|
checkRedirectUri("http://example.com/foo/%252E%252E/?encodeTest=a%3Cb", true); // double-encoded "http://example.com/foobar/../?encodeTest=a<b"
|
||||||
checkRedirectUri("http://example.com/foo/%252E%252E/#encodeTest=a%3Cb", false); // double-encoded "http://example.com/foobar/../?encodeTest=a<b"
|
checkRedirectUri("http://example.com/foo/%252E%252E/#encodeTest=a%3Cb", true); // double-encoded "http://example.com/foobar/../?encodeTest=a<b"
|
||||||
checkRedirectUri("http://example.com/foo/%25252E%25252E/", false); // triple-encoded "http://example.com/foobar/../"
|
checkRedirectUri("http://example.com/foo/%25252E%25252E/", true); // triple-encoded "http://example.com/foobar/../"
|
||||||
checkRedirectUri("http://example.com/foo/%2525252525252E%2525252525252E/", false); // seventh-encoded "http://example.com/foobar/../"
|
checkRedirectUri("http://example.com/foo/%2525252525252E%2525252525252E/", true); // seventh-encoded "http://example.com/foobar/../"
|
||||||
|
|
||||||
checkRedirectUri("http://example.com/foo?encodeTest=a%3Cb", true);
|
checkRedirectUri("http://example.com/foo?encodeTest=a%3Cb", true);
|
||||||
checkRedirectUri("http://example.com/foo?encodeTest=a%3Cb#encode2=a%3Cb", true);
|
checkRedirectUri("http://example.com/foo?encodeTest=a%3Cb#encode2=a%3Cb", true);
|
||||||
|
|
Loading…
Reference in a new issue