diff --git a/common/src/main/java/org/keycloak/common/util/Encode.java b/common/src/main/java/org/keycloak/common/util/Encode.java index 5999b2f2ac..b17d61be5b 100755 --- a/common/src/main/java/org/keycloak/common/util/Encode.java +++ b/common/src/main/java/org/keycloak/common/util/Encode.java @@ -168,6 +168,15 @@ public class Encode return encodeValue(value, queryStringEncoding); } + /** + * Keep encoded values "%..." but not the template parameters. + * @param value + * @return + */ + public static String encodeQueryStringNotTemplateParameters(String value) { + return encodeNonCodes(encodeFromArray(value, queryStringEncoding, false)); + } + /** * Keep encoded values "%...", matrix parameters, template parameters, and '/' characters intact. */ @@ -192,6 +201,16 @@ public class Encode return encodeValue(value, queryStringEncoding); } + /** + * Keep encoded values "%..." but not the template parameters. + * @param value + * @return + */ + public static String encodeFragmentNotTemplateParameters(String value) + { + return encodeNonCodes(encodeFromArray(value, queryStringEncoding, false)); + } + /** * Keep encoded values "%..." and template parameters intact. */ diff --git a/common/src/main/java/org/keycloak/common/util/KeycloakUriBuilder.java b/common/src/main/java/org/keycloak/common/util/KeycloakUriBuilder.java index 1a56f01634..659c1d8c41 100755 --- a/common/src/main/java/org/keycloak/common/util/KeycloakUriBuilder.java +++ b/common/src/main/java/org/keycloak/common/util/KeycloakUriBuilder.java @@ -51,7 +51,12 @@ public class KeycloakUriBuilder { } public static KeycloakUriBuilder fromUri(String uriTemplate) { - return new KeycloakUriBuilder().uri(uriTemplate); + // default fromUri manages template params {} + return new KeycloakUriBuilder().uri(uriTemplate, true); + } + + public static KeycloakUriBuilder fromUri(String uri, boolean template) { + return new KeycloakUriBuilder().uri(uri, template); } public static KeycloakUriBuilder fromPath(String path) throws IllegalArgumentException { @@ -131,28 +136,10 @@ public class KeycloakUriBuilder { * @return */ public KeycloakUriBuilder uriTemplate(String uriTemplate) { - if (uriTemplate == null) throw new IllegalArgumentException("uriTemplate parameter is null"); - Matcher opaque = opaqueUri.matcher(uriTemplate); - if (opaque.matches()) { - this.authority = null; - this.host = null; - this.port = -1; - this.userInfo = null; - this.query = null; - this.scheme = opaque.group(1); - this.ssp = opaque.group(2); - return this; - } else { - Matcher match = hierarchicalUri.matcher(uriTemplate); - if (match.matches()) { - ssp = null; - return parseHierarchicalUri(uriTemplate, match); - } - } - throw new IllegalArgumentException("Illegal uri template: " + uriTemplate); + return uri(uriTemplate, true); } - protected KeycloakUriBuilder parseHierarchicalUri(String uriTemplate, Matcher match) { + protected KeycloakUriBuilder parseHierarchicalUri(String uri, Matcher match, boolean template) { boolean scheme = match.group(2) != null; if (scheme) this.scheme = match.group(2); String authority = match.group(4); @@ -171,7 +158,7 @@ public class KeycloakUriBuilder { try { this.port = Integer.parseInt(hostPortMatch.group(2)); } catch (NumberFormatException e) { - throw new IllegalArgumentException("Illegal uri template: " + uriTemplate, e); + throw new IllegalArgumentException("Illegal uri template: " + uri, e); } } else { this.host = host; @@ -180,16 +167,39 @@ public class KeycloakUriBuilder { if (match.group(5) != null) { String group = match.group(5); if (!scheme && !"".equals(group) && !group.startsWith("/") && group.indexOf(':') > -1) - throw new IllegalArgumentException("Illegal uri template: " + uriTemplate); - if (!"".equals(group)) replacePath(group); + throw new IllegalArgumentException("Illegal uri template: " + uri); + if (!"".equals(group)) replacePath(group, template); } - if (match.group(7) != null) replaceQuery(match.group(7)); - if (match.group(9) != null) fragment(match.group(9)); + if (match.group(7) != null) replaceQuery(match.group(7), template); + if (match.group(9) != null) fragment(match.group(9), template); return this; } - public KeycloakUriBuilder uri(String uriTemplate) throws IllegalArgumentException { - return uriTemplate(uriTemplate); + public KeycloakUriBuilder uri(String uri) throws IllegalArgumentException { + // default uri manages template params {} + return uri(uri, true); + } + + public KeycloakUriBuilder uri(String uri, boolean template) throws IllegalArgumentException { + if (uri == null) throw new IllegalArgumentException("uri parameter is null"); + Matcher opaque = opaqueUri.matcher(uri); + if (opaque.matches()) { + this.authority = null; + this.host = null; + this.port = -1; + this.userInfo = null; + this.query = null; + this.scheme = opaque.group(1); + this.ssp = opaque.group(2); + return this; + } else { + Matcher match = hierarchicalUri.matcher(uri); + if (match.matches()) { + ssp = null; + return parseHierarchicalUri(uri, match, template); + } + } + throw new IllegalArgumentException("Illegal uri template: " + uri); } public KeycloakUriBuilder uri(URI uri) throws IllegalArgumentException { @@ -356,20 +366,30 @@ public class KeycloakUriBuilder { } public KeycloakUriBuilder replaceQuery(String query) throws IllegalArgumentException { + // default replaceQuery manages template params {} + return replaceQuery(query, true); + } + + public KeycloakUriBuilder replaceQuery(String query, boolean template) throws IllegalArgumentException { if (query == null || query.length() == 0) { this.query = null; return this; } - this.query = Encode.encodeQueryString(query); + this.query = template? Encode.encodeQueryString(query) : Encode.encodeQueryStringNotTemplateParameters(query); return this; } public KeycloakUriBuilder fragment(String fragment) throws IllegalArgumentException { + // default fragment manages template params {} + return fragment(fragment, true); + } + + public KeycloakUriBuilder fragment(String fragment, boolean template) throws IllegalArgumentException { if (fragment == null) { this.fragment = null; return this; } - this.fragment = Encode.encodeFragment(fragment); + this.fragment = template? Encode.encodeFragment(fragment) : Encode.encodeFragmentNotTemplateParameters(fragment); return this; } @@ -709,11 +729,16 @@ public class KeycloakUriBuilder { } public KeycloakUriBuilder replacePath(String path) { + // default replacePath manages template expression {} + return replacePath(path, true); + } + + public KeycloakUriBuilder replacePath(String path, boolean template) { if (path == null) { this.path = null; return this; } - this.path = Encode.encodePath(path); + this.path = template? Encode.encodePath(path) : Encode.encodePathSaveEncodings(path); return this; } diff --git a/common/src/test/java/org/keycloak/common/util/KeycloakUriBuilderTest.java b/common/src/test/java/org/keycloak/common/util/KeycloakUriBuilderTest.java index 414478fbaf..5e418d73e5 100644 --- a/common/src/test/java/org/keycloak/common/util/KeycloakUriBuilderTest.java +++ b/common/src/test/java/org/keycloak/common/util/KeycloakUriBuilderTest.java @@ -72,4 +72,12 @@ public class KeycloakUriBuilderTest { Assert.assertEquals("https://localhost/path", KeycloakUriBuilder.fromUri("https://localhost/path").buildAsString()); Assert.assertEquals("https://localhost/path", KeycloakUriBuilder.fromUri("https://localhost/path").preserveDefaultPort().buildAsString()); } + + @Test + public void testTemplateAndNotTemplate() { + Assert.assertEquals("https://localhost:8443/path?key=query#fragment", KeycloakUriBuilder.fromUri( + "https://localhost:8443/{path}?key={query}#{fragment}").buildAsString("path", "query", "fragment")); + Assert.assertEquals("https://localhost:8443/%7Bpath%7D?key=%7Bquery%7D#%7Bfragment%7D", KeycloakUriBuilder.fromUri( + "https://localhost:8443/{path}?key={query}#{fragment}", false).buildAsString()); + } } 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 447685e6d8..7c9c4755e7 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 @@ -196,7 +196,7 @@ public class RedirectUtils { 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).preserveDefaultPort(); + KeycloakUriBuilder uriBuilder = KeycloakUriBuilder.fromUri(redirectUri, false).preserveDefaultPort(); String origQuery = uriBuilder.getQuery(); String origFragment = uriBuilder.getFragment(); String encodedRedirectUri = uriBuilder @@ -209,7 +209,7 @@ public class RedirectUtils { decodedRedirectUri = Encode.decode(encodedRedirectUri); if (decodedRedirectUri.equals(encodedRedirectUri)) { // URL is decoded. We can return it (after attach original query and fragment) - return KeycloakUriBuilder.fromUri(decodedRedirectUri).preserveDefaultPort() + return KeycloakUriBuilder.fromUri(decodedRedirectUri, false).preserveDefaultPort() .replaceQuery(origQuery) .fragment(origFragment) .buildAsString(); 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 ea23040bad..1d953cd897 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 @@ -114,4 +114,17 @@ public class RedirectUtilsTest { Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "custom3:/test", set, false)); } + + @Test + public void testverifyRedirectUriWithCurlyBrackets() { + Set set = Stream.of( + "https://keycloak.org/%7B123%7D", + "https://keycloak.org/parent/*" + ).collect(Collectors.toSet()); + + Assert.assertEquals("https://keycloak.org/%7B123%7D", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/%7B123%7D", set, false)); + Assert.assertEquals("https://keycloak.org/parent/%7B123%7D", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/parent/%7B123%7D", set, false)); + + Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/%7Babc%7D", set, false)); + } }