verifyRedirectUri should return null when the passed redirectUri is invalid

Closes https://github.com/keycloak/keycloak/issues/22778
This commit is contained in:
rmartinc 2023-09-19 12:09:50 +02:00 committed by Marek Posolda
parent d4a793be64
commit 082b0ed308
2 changed files with 22 additions and 2 deletions

View file

@ -107,6 +107,12 @@ public class RedirectUtils {
logger.debug("No Redirect URIs supplied"); logger.debug("No Redirect URIs supplied");
redirectUri = null; redirectUri = null;
} else { } else {
URI originalRedirect = toUri(redirectUri);
if (originalRedirect == null) {
// invalid URI passed as redirectUri
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) // 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); String decodedRedirectUri = decodeRedirectUri(redirectUri);
URI decodedRedirect = toUri(decodedRedirectUri); URI decodedRedirect = toUri(decodedRedirectUri);
@ -135,8 +141,7 @@ 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 // 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
URI redirect = toUri(redirectUri); redirectUri = getNormalizedRedirectUri(originalRedirect);
redirectUri = getNormalizedRedirectUri(redirect);
// 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) // 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) { if (valid == null) {

View file

@ -127,4 +127,19 @@ public class RedirectUtilsTest {
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/%7Babc%7D", set, false)); Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/%7Babc%7D", set, false));
} }
@Test
public void testverifyInvalidRedirectUri() {
Set<String> set = Stream.of(
"https://keycloak.org/*"
).collect(Collectors.toSet());
Assert.assertEquals("https://keycloak.org/path%20space/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path%20space/", set, false));
Assert.assertEquals("https://keycloak.org/path%3Cless/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path%3Cless/", set, false));
Assert.assertEquals("https://keycloak.org/path/index.jsp?param=v1+v2", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path/index.jsp?param=v1+v2", set, false));
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path space/", set, false));
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path<less/", set, false));
Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path/index.jsp?param=v1 v2", set, false));
}
} }