diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java index b1bc4e3aa8..2bca7fec10 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java @@ -72,10 +72,8 @@ public class RedirectUtils { if (validRedirect.startsWith("/")) { validRedirect = relativeToAbsoluteURI(session, rootUrl, validRedirect); logger.debugv("replacing relative valid redirect with: {0}", validRedirect); - resolveValidRedirects.add(validRedirect); - } else { - resolveValidRedirects.add(validRedirect); } + resolveValidRedirects.add(lowerCaseHostname(validRedirect)); } return resolveValidRedirects; } @@ -116,7 +114,7 @@ public class RedirectUtils { // Make the validations against fully decoded and normalized redirect-url. This also allows wildcards (case when client configured "Valid redirect-urls" contain wildcards) String decodedRedirectUri = decodeRedirectUri(redirectUri); URI decodedRedirect = toUri(decodedRedirectUri); - decodedRedirectUri = getNormalizedRedirectUri(decodedRedirect); + decodedRedirectUri = getNormalizedRedirectUri(decodedRedirect, true); if (decodedRedirectUri == null) return null; String r = decodedRedirectUri; @@ -141,21 +139,25 @@ public class RedirectUtils { } // 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); + redirectUri = getNormalizedRedirectUri(originalRedirect, false); // 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 && redirectUri.startsWith("/")) { + if (valid != null && !originalRedirect.isAbsolute()) { + // return absolute if the original URI is relative + if (!redirectUri.startsWith("/")) { + redirectUri = "/" + redirectUri; + } redirectUri = relativeToAbsoluteURI(session, rootUrl, redirectUri); } String scheme = decodedRedirect.getScheme(); if (valid != null && scheme != null) { // 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.toLowerCase() + ":") && !"http".equalsIgnoreCase(scheme) && !"https".equalsIgnoreCase(scheme)) { logger.debugf("Invalid URI because scheme is not allowed: %s", redirectUri); valid = null; } @@ -174,7 +176,7 @@ public class RedirectUtils { private static URI toUri(String redirectUri) { URI uri = null; if (redirectUri != null) { - try { + try { uri = URI.create(redirectUri); } catch (IllegalArgumentException cause) { logger.debug("Invalid redirect uri", cause); @@ -185,11 +187,13 @@ public class RedirectUtils { return uri; } - private static String getNormalizedRedirectUri(URI uri) { + private static String getNormalizedRedirectUri(URI uri, boolean lower) { String redirectUri = null; if (uri != null) { redirectUri = uri.normalize().toString(); - redirectUri = lowerCaseHostname(redirectUri); + if (lower) { + redirectUri = lowerCaseHostname(redirectUri); + } } return redirectUri; } @@ -204,9 +208,11 @@ public class RedirectUtils { KeycloakUriBuilder uriBuilder = KeycloakUriBuilder.fromUri(redirectUri, false).preserveDefaultPort(); String origQuery = uriBuilder.getQuery(); String origFragment = uriBuilder.getFragment(); + String origUserInfo = uriBuilder.getUserInfo(); String encodedRedirectUri = uriBuilder .replaceQuery(null) .fragment(null) + .userInfo(null) .buildAsString(); String decodedRedirectUri = null; @@ -217,6 +223,7 @@ public class RedirectUtils { return KeycloakUriBuilder.fromUri(decodedRedirectUri, false).preserveDefaultPort() .replaceQuery(origQuery) .fragment(origFragment) + .userInfo(origUserInfo) .buildAsString(); } else { // Next attempt @@ -231,12 +238,15 @@ public class RedirectUtils { } private static String lowerCaseHostname(String redirectUri) { - int n = redirectUri.indexOf('/', 7); - if (n == -1) { - return redirectUri.toLowerCase(); - } else { - return redirectUri.substring(0, n).toLowerCase() + redirectUri.substring(n); + // lower case host and scheme which are case-insensitive by spec + KeycloakUriBuilder uriBuilder = KeycloakUriBuilder.fromUri(redirectUri, false).preserveDefaultPort(); + if (uriBuilder.getScheme() != null) { + uriBuilder.scheme(uriBuilder.getScheme().toLowerCase()); } + if (uriBuilder.getHost() != null) { + uriBuilder.host(uriBuilder.getHost().toLowerCase()); + } + return uriBuilder.buildAsString(); } private static String relativeToAbsoluteURI(KeycloakSession session, String rootUrl, String relative) { diff --git a/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java b/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java index 4a5cd883c6..1e462fd609 100644 --- a/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java +++ b/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java @@ -142,4 +142,36 @@ public class RedirectUtilsTest { Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path set = Stream.of( + "*" + ).collect(Collectors.toSet()); + + 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 testVerifyDifferentCase() { + Set set = Stream.of( + "no.host.Name.App://TEST", + "no.host.Name.App:/Path/*", + "HTTPS://KeyCloak.org/*", + "HTTP://KeyCloak.org/with%20space/*", + "HTTP://KeyCloak.org/áéíóú", + "custom:*" + ).collect(Collectors.toSet()); + + Assert.assertEquals("no.host.Name.App://test", RedirectUtils.verifyRedirectUri(session, null, "no.host.Name.App://test", set, false)); + Assert.assertEquals("no.host.name.app://Test", RedirectUtils.verifyRedirectUri(session, null, "no.host.name.app://Test", set, false)); + Assert.assertEquals("no.host.name.app:/Path", RedirectUtils.verifyRedirectUri(session, null, "no.host.name.app:/Path", 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("CUSTOM:test", RedirectUtils.verifyRedirectUri(session, null, "CUSTOM:test", set, false)); + Assert.assertEquals("Custom://userInfo@KeyCLOAK.org/Path", RedirectUtils.verifyRedirectUri(session, null, "Custom://userInfo@KeyCLOAK.org/Path", set, false)); + Assert.assertEquals("Custom://user%20Info@keycloak.ORG", RedirectUtils.verifyRedirectUri(session, null, "Custom://user%20Info@keycloak.ORG", set, false)); + Assert.assertEquals("http://keycloak.org/áéíóú", RedirectUtils.verifyRedirectUri(session, null, "http://keycloak.org/áéíóú", set, false)); + } }