Additional restrictions when to issue a redirect to the caller on rolling updates

Closes #45574

Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
Signed-off-by: Alexander Schwartz <alexander.schwartz@gmx.net>
Co-authored-by: Pedro Ruivo <pruivo@users.noreply.github.com>

server-spi/src/main/java/org/keycloak/theme/Theme.java

@@ -85,6 +85,17 @@ public interface Theme {
 
     Properties getProperties() throws IOException;
 
+    /**
+     * Check if a resource exists in the theme.
+     * @param path path of the resource
+     * @return true if the resource exists
+     */
+    default boolean hasResource(String path) throws IOException {
+        try (InputStream is = getResourceAsStream(path)) {
+            return is != null;
+        }
+    }
+
     /**
      * Check if the given path contains a content hash.
      * If a resource is requested from this path, and it has a content hash, this guarantees that if the file

services/src/main/java/org/keycloak/services/resources/ThemeResource.java

@@ -116,30 +116,40 @@ public class ThemeResource {
                     return Response.status(Response.Status.NOT_FOUND).build();
                 }
 
-                if (!version.equals(Version.RESOURCES_VERSION) && !hasContentHash) {
+                // Only enter here if the requested version is different, doesn't have a content hash in the URL,
+                // and we didn't default to the default theme as the theme is unknown.
+                if (!version.equals(Version.RESOURCES_VERSION) && !hasContentHash && Objects.equals(theme.getName(), themeName)) {
                     // If it is not the right version, and it does not have a content hash, redirect.
                     // If it is not the right version, but it has a content hash, continue to see if it exists.
 
                     // A simpler way to check for encoded URL characters would be to retrieve the raw values.
                     // Unfortunately, RESTEasy doesn't support this, and UrlInfo will throw an IllegalArgumentException.
                     if (!uriInfo.getRequestUri().toURL().getPath().startsWith(base + UriBuilder.fromResource(ThemeResource.class)
-                            .path("/{version}/{themeType}/{themeName}/{path}").build(version,themeType, themeName, path).getPath())) {
+                            .path("/{version}/{themeType}/{themeName}/{path}").build(version, theme.getType().toString().toLowerCase(), theme.getName(), path).getPath())) {
                         // This prevents half-open redirects
                         log.debugf("No URL encoding should be necessary for the path, returning a 404: %s", uriInfo.getRequestUri().getPath());
                         return Response.status(Response.Status.NOT_FOUND).build();
                     }
 
+                    if (!theme.hasResource(path)) {
+                        // Prevent a redirect to a file that doesn't exist anyway
+                        log.debugf("Resource doesn't exist, returning a 404: %s", path);
+                        return Response.status(Response.Status.NOT_FOUND).build();
+                    }
+
                     URI redirectUri = UriBuilder.fromResource(ThemeResource.class)
                             .path("/{version}/{themeType}/{themeName}/{path}")
-                            .replaceQuery(uriInfo.getRequestUri().getRawQuery())
-                            // The 'path' can contain slashes, so encoding of slashes is set to false
-                            .build(new Object[]{Version.RESOURCES_VERSION, themeType, themeName, path}, false);
+                            // We will not add the query parameters to the redirect as it is difficult to sanitize them, and the theme handler doesn't need them.
+                            // The 'path' can contain slashes, so encoding of slashes is set to false.
+                            .build(new Object[]{Version.RESOURCES_VERSION, theme.getType().toString().toLowerCase(), theme.getName(), path}, false);
                     if (!redirectUri.normalize().equals(redirectUri)) {
                         // This prevents half-open redirects
                         log.debugf("Redirect URL should not require normalization, returning a 404: %s", redirectUri.toString());
                         return Response.status(Response.Status.NOT_FOUND).build();
                     }
 
+                    // From here, it should be safe to redirect as we only redirect to files that we know are present in the theme.
+
                     // The redirect will lead the browser to a resource that it then (when retrieved successfully) can cache again.
                     // This assumes that it is better to try to some content even if it is outdated or too new, instead of returning a 404.
                     // This should usually work for images, CSS or (simple) JavaScript referenced in the login theme that needs to be

services/src/main/java/org/keycloak/theme/DefaultThemeManager.java

@@ -232,6 +232,25 @@ public class DefaultThemeManager implements ThemeManager {
             return null;
         }
 
+        @Override
+        public boolean hasResource(String path) throws IOException {
+            for (Theme t : themes) {
+                if (t.hasResource(path)) {
+                    return true;
+                }
+            }
+
+            for (ThemeResourceProvider t : themeResourceProviders) {
+                try (InputStream resource = t.getResourceAsStream(path)) {
+                    if (resource != null) {
+                        return true;
+                    }
+                }
+            }
+
+            return false;
+        }
+
         @Override
         public InputStream getResourceAsStream(String path) throws IOException {
             for (Theme t : themes) {

services/src/main/java/org/keycloak/theme/FolderTheme.java

@@ -88,6 +88,12 @@ public class FolderTheme extends FileBasedTheme {
         return file.isFile() ? file.toURI().toURL() : null;
     }
 
+    @Override
+    public boolean hasResource(String path) throws IOException {
+        var file = ResourceLoader.getFile(resourcesDir, path);
+        return file != null && file.isFile();
+    }
+
     @Override
     public InputStream getResourceAsStream(String path) throws IOException {
         return ResourceLoader.getFileAsStream(resourcesDir, path);

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/theme/ThemeResourceProviderTest.java

@@ -151,11 +151,12 @@ public class ThemeResourceProviderTest extends AbstractTestRealmKeycloakTest {
         assertFound(suiteContext.getAuthServerInfo().getContextRoot().toString() + "/auth/resources/" + resourcesVersion + "/login/keycloak.v2/css%2Fstyles.css");
         assertNotFound(suiteContext.getAuthServerInfo().getContextRoot().toString() + "/auth/resources/" + "unkno" + "/login/keycloak.v2/css%2Fstyles.css");
         assertNotFound(suiteContext.getAuthServerInfo().getContextRoot().toString() + "/auth/resources/" + "unkn%2F" + "/login/keycloak.v2/css/styles.css");
+        assertNotFound(suiteContext.getAuthServerInfo().getContextRoot().toString() + "/auth/resources/" + "unkno" + "/login/keycloak.v2/css/unknown.css");
         // This on check will fail on Quarkus as Quarkus will normalize the URL before handing it to the REST endpoint
         // It will succeed on Undertow
         // assertNotFound(suiteContext.getAuthServerInfo().getContextRoot().toString() + "/auth/resources/" + "unkno" + "/login/keycloak.v2/css/../css/styles.css");
         assertRedirectAndValidateRedirect(suiteContext.getAuthServerInfo().getContextRoot().toString() + "/auth/resources/" + "unkno" + "/login/keycloak.v2/css/styles.css?name=%2Fvalue",
-                suiteContext.getAuthServerInfo().getContextRoot().toString() + "/auth/resources/" + resourcesVersion + "/login/keycloak.v2/css/styles.css?name=%2Fvalue");
+                suiteContext.getAuthServerInfo().getContextRoot().toString() + "/auth/resources/" + resourcesVersion + "/login/keycloak.v2/css/styles.css");
     }