From f3adcc0f4d9afd0a68197656f040e43d9964c52c Mon Sep 17 00:00:00 2001
From: Hynek Mlnarik <hmlnarik@redhat.com>
Date: Tue, 18 May 2021 18:31:21 +0000
Subject: [PATCH] KEYCLOAK-17748 Optimize validation of redirect URIs in logout endpoint

---
 src/main/java/org/keycloak/protocol/cas/utils/RedirectUtils.java |   63 ++++++++++++++++++++-----------
 1 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/src/main/java/org/keycloak/protocol/cas/utils/RedirectUtils.java b/src/main/java/org/keycloak/protocol/cas/utils/RedirectUtils.java
index 5f89335..da0f11a 100644
--- a/src/main/java/org/keycloak/protocol/cas/utils/RedirectUtils.java
+++ b/src/main/java/org/keycloak/protocol/cas/utils/RedirectUtils.java
@@ -24,14 +24,15 @@
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.KeycloakUriInfo;
 import org.keycloak.models.RealmModel;
+import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.services.Urls;
 import org.keycloak.services.util.ResolveRelative;
 
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@@ -71,21 +72,30 @@
     }
 
     private static Set<String> getValidateRedirectUris(KeycloakSession session) {
-        Set<String> redirects = new HashSet<>();
-        for (ClientModel client : session.getContext().getRealm().getClients()) {
-            if (client.isEnabled()) {
-                redirects.addAll(resolveValidRedirects(session, client.getRootUrl(), client.getRedirectUris()));
-            }
-        }
-        return redirects;
+        RealmModel realm = session.getContext().getRealm();
+        return session.clientStorageManager().getAllRedirectUrisOfEnabledClients(realm).entrySet().stream()
+          .filter(me -> me.getKey().isEnabled() && OIDCLoginProtocol.LOGIN_PROTOCOL.equals(me.getKey().getProtocol()) && !me.getKey().isBearerOnly() && (me.getKey().isStandardFlowEnabled() || me.getKey().isImplicitFlowEnabled()))
+          .map(me -> resolveValidRedirects(session, me.getKey().getRootUrl(), me.getValue()))
+          .flatMap(Collection::stream)
+          .collect(Collectors.toSet());
     }
 
-    private static String verifyRedirectUri(KeycloakSession session, String rootUrl, String redirectUri, Set<String> validRedirects, boolean requireRedirectUri) {
+    public static String verifyRedirectUri(KeycloakSession session, String rootUrl, String redirectUri, Set<String> validRedirects, boolean requireRedirectUri) {
         KeycloakUriInfo uriInfo = session.getContext().getUri();
         RealmModel realm = session.getContext().getRealm();
 
-        if (redirectUri != null)
-            redirectUri = normalizeUrl(redirectUri);
+        if (redirectUri != null) {
+            try {
+                URI uri = URI.create(redirectUri);
+                redirectUri = uri.normalize().toString();
+            } catch (IllegalArgumentException cause) {
+                logger.debug("Invalid redirect uri", cause);
+                return null;
+            } catch (Exception cause) {
+                logger.debug("Unexpected error when parsing redirect uri", cause);
+                return null;
+            }
+        }
 
         if (redirectUri == null) {
             if (!requireRedirectUri) {
@@ -107,7 +117,7 @@
 
             boolean valid = matchesRedirects(resolveValidRedirects, r);
 
-            if (!valid && r.startsWith(Constants.INSTALLED_APP_URL) && r.indexOf(':', Constants.INSTALLED_APP_URL.length()) >= 0) {
+            if (!valid && (r.startsWith(Constants.INSTALLED_APP_URL) || r.startsWith(Constants.INSTALLED_APP_LOOPBACK)) && r.indexOf(':', Constants.INSTALLED_APP_URL.length()) >= 0) {
                 int i = r.indexOf(':', Constants.INSTALLED_APP_URL.length());
 
                 StringBuilder sb = new StringBuilder();
@@ -179,19 +189,26 @@
     private static String getSingleValidRedirectUri(Collection<String> validRedirects) {
         if (validRedirects.size() != 1) return null;
         String validRedirect = validRedirects.iterator().next();
-        int idx = validRedirect.indexOf("/*");
-        if (idx > -1) {
-            validRedirect = validRedirect.substring(0, idx);
-        }
-        return validRedirect;
+        return validateRedirectUriWildcard(validRedirect);
     }
 
-    private static String normalizeUrl(String url) {
-        try {
-            URI uri = new URI(url);
-            return uri.normalize().toString();
-        } catch (URISyntaxException e) {
-            throw new IllegalArgumentException("Invalid URL syntax: " + e.getMessage());
+    public static String validateRedirectUriWildcard(String redirectUri) {
+        if (redirectUri == null)
+            return null;
+
+        int idx = redirectUri.indexOf("/*");
+        if (idx > -1) {
+            redirectUri = redirectUri.substring(0, idx);
         }
+        return redirectUri;
+    }
+
+    private static String getFirstValidRedirectUri(Collection<String> validRedirects) {
+        final String redirectUri = validRedirects.stream().findFirst().orElse(null);
+        return (redirectUri != null) ? validateRedirectUriWildcard(redirectUri) : null;
+    }
+
+    public static String getFirstValidRedirectUri(KeycloakSession session, String rootUrl, Set<String> validRedirects) {
+        return getFirstValidRedirectUri(resolveValidRedirects(session, rootUrl, validRedirects));
     }
 }

--
Gitblit v1.10.0