RedirectUtils needs to use KeycloakUriBuilder with no parameter parsing

Closes https://github.com/keycloak/keycloak/issues/22424

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.
     */

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");
+        return uri(uriTemplate, true);
-        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);
     }
 
-    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);
+                throw new IllegalArgumentException("Illegal uri template: " + uri);
-            if (!"".equals(group)) replacePath(group);
+            if (!"".equals(group)) replacePath(group, template);
         }
-        if (match.group(7) != null) replaceQuery(match.group(7));
+        if (match.group(7) != null) replaceQuery(match.group(7), template);
-        if (match.group(9) != null) fragment(match.group(9));
+        if (match.group(9) != null) fragment(match.group(9), template);
         return this;
     }
 
-    public KeycloakUriBuilder uri(String uriTemplate) throws IllegalArgumentException {
+    public KeycloakUriBuilder uri(String uri) throws IllegalArgumentException {
-        return uriTemplate(uriTemplate);
+        // 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;
     }
 

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());
+    }
 }

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();

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<String> 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));
+    }
 }