Strip off user-info from redirect URI when validating using wildcard (#61)
Closes keycloak/keycloak-private#58 Closes https://issues.redhat.com/browse/RHBK-679 Signed-off-by: rmartinc <rmartinc@redhat.com>
This commit is contained in:
parent
d411eafc42
commit
32a70cbedd
2 changed files with 33 additions and 2 deletions
|
@ -248,13 +248,24 @@ 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 (validRedirect.endsWith("*") && !validRedirect.contains("?") && allowWildcards) {
|
||||||
// strip off the query component - we don't check them when wildcards are effective
|
// strip off the userInfo, query or fragment components - we don't check them when wildcards are effective
|
||||||
String r = redirect.contains("?") ? redirect.substring(0, redirect.indexOf("?")) : redirect;
|
String r = stripOffRedirectForWildcard(redirect);
|
||||||
// strip off *
|
// strip off *
|
||||||
int length = validRedirect.length() - 1;
|
int length = validRedirect.length() - 1;
|
||||||
validRedirect = validRedirect.substring(0, length);
|
validRedirect = validRedirect.substring(0, length);
|
||||||
|
|
|
@ -174,4 +174,24 @@ 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));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testUserInfo() {
|
||||||
|
Set<String> set = Stream.of(
|
||||||
|
"https://keycloak.org/*",
|
||||||
|
"https://test*",
|
||||||
|
"https://something@keycloak.com/exact"
|
||||||
|
).collect(Collectors.toSet());
|
||||||
|
|
||||||
|
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://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.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://something@other.com/", set, false));
|
||||||
|
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org@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.com@other.com", set, false));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue