keycloak-aplcache

Merge pull request #3142 from vmuzikar/KEYCLOAK-3429 KEYCLOAK-3429

8/24/2016 4:53:29 AM

Details

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 441cd57..b7d9ad4 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
@@ -77,7 +77,7 @@ public class RedirectUtils {
         } else {
             redirectUri = lowerCaseHostname(redirectUri);
 
-            String r = redirectUri.indexOf('?') != -1 ? redirectUri.substring(0, redirectUri.indexOf('?')) : redirectUri;
+            String r = redirectUri;
             Set<String> resolveValidRedirects = resolveValidRedirects(uriInfo, rootUrl, validRedirects);
 
             boolean valid = matchesRedirects(resolveValidRedirects, r);
@@ -134,15 +134,17 @@ public class RedirectUtils {
 
     private static boolean matchesRedirects(Set<String> validRedirects, String redirect) {
         for (String validRedirect : validRedirects) {
-            if (validRedirect.endsWith("*")) {
+            if (validRedirect.endsWith("*") && !validRedirect.contains("?")) {
+                // strip off the query component - we don't check them when wildcards are effective
+                String r = redirect.contains("?") ? redirect.substring(0, redirect.indexOf("?")) : redirect;
                 // strip off *
                 int length = validRedirect.length() - 1;
                 validRedirect = validRedirect.substring(0, length);
-                if (redirect.startsWith(validRedirect)) return true;
+                if (r.startsWith(validRedirect)) return true;
                 // strip off trailing '/'
                 if (length - 1 > 0 && validRedirect.charAt(length - 1) == '/') length--;
                 validRedirect = validRedirect.substring(0, length);
-                if (validRedirect.equals(redirect)) return true;
+                if (validRedirect.equals(r)) return true;
             } else if (validRedirect.equals(redirect)) return true;
         }
         return false;
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java
index f80e789..520f910 100755
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java
@@ -97,6 +97,11 @@ public class OAuthRedirectUriTest extends AbstractKeycloakTest {
                 .secret("password");
         realm.client(installedApp6);
 
+        ClientBuilder installedApp7 = ClientBuilder.create().id("test-query-component").name("test-query-component")
+                .redirectUris("http://localhost?foo=bar", "http://localhost?foo=bar*")
+                .secret("password");
+        realm.client(installedApp7);
+
         testRealms.add(realm.build());
     }
 
@@ -186,6 +191,26 @@ public class OAuthRedirectUriTest extends AbstractKeycloakTest {
     }
 
     @Test
+    public void testQueryComponents() throws IOException {
+        // KEYCLOAK-3420
+        oauth.clientId("test-query-component");
+        checkRedirectUri("http://localhost?foo=bar", true);
+        checkRedirectUri("http://localhost?foo=bara", false);
+        checkRedirectUri("http://localhost?foo=bar/", false);
+        checkRedirectUri("http://localhost?foo2=bar2&foo=bar", false);
+        checkRedirectUri("http://localhost?foo=b", false);
+        checkRedirectUri("http://localhost?foo", false);
+        checkRedirectUri("http://localhost?foo=bar&bar=foo", false);
+        checkRedirectUri("http://localhost?foo&bar=foo", false);
+        checkRedirectUri("http://localhost?foo&bar", false);
+        checkRedirectUri("http://localhost", false);
+
+        // KEYCLOAK-3418
+        oauth.clientId("test-installed");
+        checkRedirectUri("http://localhost?foo=bar", false);
+    }
+
+    @Test
     public void testWildcard() throws IOException {
         oauth.clientId("test-wildcard");
         checkRedirectUri("http://example.com", false);