NPE on Theme after upgrade to 21 when parent or import theme not exists (#17350)

* NPE on Theme after upgrade to 21 when parent or import theme not exists
closes #17313

* Update per review
This commit is contained in:
Marek Posolda
2023-03-01 16:46:37 +01:00
committed by GitHub
parent 0b78cf9b29
commit 59f4fe1c60
8 changed files with 61 additions and 19 deletions

View File

@@ -180,6 +180,8 @@ public class ServerInfoAdminResource {
for (String name : themeNames) {
try {
Theme theme = session.theme().getTheme(name, type);
// 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);
@@ -189,6 +191,7 @@ public class ServerInfoAdminResource {
}
themes.add(ti);
}
} catch (IOException e) {
throw new WebApplicationException("Failed to load themes", e);
}

View File

@@ -106,20 +106,19 @@ public class DefaultThemeManager implements ThemeManager {
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 @@ public class DefaultThemeManager implements ThemeManager {
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;

View File

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

View File

@@ -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

View File

@@ -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

View File

@@ -29,6 +29,7 @@ import org.keycloak.representations.idm.RealmRepresentation;
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 @@ public class Assert extends org.junit.Assert {
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();

View File

@@ -31,6 +31,7 @@ import org.keycloak.testsuite.AbstractKeycloakTest;
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 class ServerInfoTest extends AbstractKeycloakTest {
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());

View File

@@ -45,6 +45,20 @@ public class ThemeResourceProviderTest extends AbstractTestRealmKeycloakTest {
});
}
@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 -> {