NPE on Theme after upgrade to 21 when parent or import theme not exis…

…ts (#17350) (#17379)

closes #17313

(cherry picked from commit 59f4fe1)

services/src/main/java/org/keycloak/services/resources/admin/info/ServerInfoAdminResource.java

@@ -180,15 +180,18 @@ private void setThemes(ServerInfoRepresentation info) {
             for (String name : themeNames) {
                 try {
                     Theme theme = session.theme().getTheme(name, type);
-                    ThemeInfoRepresentation ti = new ThemeInfoRepresentation();
-                    ti.setName(name);
+                    // Different name means the theme itself was not found and fallback to default theme was needed
+                    if (theme != null && name.equals(theme.getName())) {
+                        ThemeInfoRepresentation ti = new ThemeInfoRepresentation();
+                        ti.setName(name);
+
+                        String locales = theme.getProperties().getProperty("locales");
+                        if (locales != null) {
+                            ti.setLocales(locales.replaceAll(" ", "").split(","));
+                        }
 
-                    String locales = theme.getProperties().getProperty("locales");
-                    if (locales != null) {
-                        ti.setLocales(locales.replaceAll(" ", "").split(","));
+                        themes.add(ti);
                     }
-
-                    themes.add(ti);
                 } catch (IOException e) {
                     throw new WebApplicationException("Failed to load themes", e);
                 }

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

@@ -106,20 +106,19 @@ private Theme loadTheme(String name, Theme.Type type) {
         List<Theme> themes = new LinkedList<>();
         themes.add(theme);
 
-        if (theme.getImportName() != null) {
-            String[] s = theme.getImportName().split("/");
-            themes.add(findTheme(s[1], Theme.Type.valueOf(s[0].toUpperCase())));
-        }
+        if (!processImportedTheme(themes, theme, name, type)) return null;
 
         if (theme.getParentName() != null) {
             for (String parentName = theme.getParentName(); parentName != null; parentName = theme.getParentName()) {
+                String currentThemeName = theme.getName();
                 theme = findTheme(parentName, type);
+                if (theme == null) {
+                    log.warnf("Not found parent theme '%s' of theme '%s'. Unable to load %s theme '%s' due to this.", parentName, currentThemeName, type, name);
+                    return null;
+                }
                 themes.add(theme);
 
-                if (theme.getImportName() != null) {
-                    String[] s = theme.getImportName().split("/");
-                    themes.add(findTheme(s[1], Theme.Type.valueOf(s[0].toUpperCase())));
-                }
+                if (!processImportedTheme(themes, theme, name, type)) return null;
             }
         }
 
@@ -139,6 +138,19 @@ private Theme findTheme(String name, Theme.Type type) {
         return null;
     }
 
+    private boolean processImportedTheme(List<Theme> themes, Theme theme, String origThemeName, Theme.Type type) {
+        if (theme.getImportName() != null) {
+            String[] s = theme.getImportName().split("/");
+            Theme importedTheme = findTheme(s[1], Theme.Type.valueOf(s[0].toUpperCase()));
+            if (importedTheme == null) {
+                log.warnf("Not found theme '%s' referenced as import of theme '%s'. Unable to load %s theme '%s' due to this.", theme.getImportName(), theme.getName(), type, origThemeName);
+                return false;
+            }
+            themes.add(importedTheme);
+        }
+        return true;
+    }
+
     private static class ExtendingTheme implements Theme {
 
         private List<Theme> themes;

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/META-INF/keycloak-themes.json

@@ -2,5 +2,8 @@
   "themes": [{
     "name" : "address",
     "types": [ "admin", "account", "login" ]
+  }, {
+    "name" : "incorrect",
+    "types": [ "admin" ]
   }]
 }

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/theme/address/admin/theme.properties

@@ -0,0 +1,2 @@
+# This exists just to test backwards compatibility. Server should not break if non-existing theme is referenced from here
+parent=keycloak

testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/theme/incorrect/admin/theme.properties

@@ -0,0 +1,3 @@
+# This exists just to test backwards compatibility. Server should not break if non-existing theme is referenced from "import"
+import=admin/keycloak
+parent=base

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

@@ -29,6 +29,7 @@
 import org.keycloak.representations.idm.RoleRepresentation;
 import org.keycloak.representations.idm.UserFederationProviderFactoryRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
+import org.keycloak.representations.info.ThemeInfoRepresentation;
 
 import java.util.Arrays;
 import java.util.LinkedList;
@@ -92,6 +93,8 @@ private static String name(Object o1) {
             return ((ComponentRepresentation) o1).getName();
         } else if (o1 instanceof ClientScopeRepresentation) {
             return ((ClientScopeRepresentation) o1).getName();
+        } else if (o1 instanceof ThemeInfoRepresentation) {
+            return ((ThemeInfoRepresentation) o1).getName();
         }
 
         throw new IllegalArgumentException();

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ServerInfoTest.java

@@ -31,6 +31,7 @@
 import org.keycloak.testsuite.Assert;
 import org.keycloak.testsuite.util.KeyUtils;
 import org.keycloak.testsuite.util.KeystoreUtils;
+import org.keycloak.testsuite.util.WaitUtils;
 
 import java.util.List;
 import java.util.Map;
@@ -54,11 +55,12 @@ public void testServerInfo() {
         assertNotNull(info.getProviders().get("authenticator"));
 
         assertNotNull(info.getThemes());
+        // Not checking account themes for now as old account console is going to be removed soon, which would remove "keycloak" theme. So that is just to avoid another "test to update" when it is removed :)
         assertNotNull(info.getThemes().get("account"));
-        assertNotNull(info.getThemes().get("admin"));
-        assertNotNull(info.getThemes().get("email"));
-        assertNotNull(info.getThemes().get("login"));
-        assertNotNull(info.getThemes().get("welcome"));
+        Assert.assertNames(info.getThemes().get("admin"), "base", "keycloak.v2");
+        Assert.assertNames(info.getThemes().get("email"), "base", "keycloak");
+        Assert.assertNames(info.getThemes().get("login"), "address", "base", "environment-agnostic", "keycloak");
+        Assert.assertNames(info.getThemes().get("welcome"), "keycloak");
 
         assertNotNull(info.getEnums());
 

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

@@ -45,6 +45,20 @@ public void getTheme() {
         });
     }
 
+    @Test
+    public void testThemeFallback() {
+        testingClient.server().run(session -> {
+            try {
+                // Fallback to default theme when requested theme don't exists
+                Theme theme = session.theme().getTheme("address", Theme.Type.ADMIN);
+                Assert.assertNotNull(theme);
+                Assert.assertEquals("keycloak.v2", theme.getName());
+            } catch (IOException e) {
+                Assert.fail(e.getMessage());
+            }
+        });
+    }
+
     @Test
     public void getResourceAsStream() {
         testingClient.server().run(session -> {