From f7490b7f7c15bb7431454fc3f992b05541c30e98 Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Mon, 17 Oct 2022 15:17:54 +0200 Subject: [PATCH] Fix issue where admin2 was not enabled by default if account2 was disabled (#14914) Refactoring ThemeSelector and DefaultThemeManager to re-use the same logic for selecting default theme as there used to be two places where one had a broken implementation Closes #14889 --- .../keycloak/theme/ThemeSelectorProvider.java | 13 ++++++++ .../keycloak/theme/DefaultThemeManager.java | 30 ++---------------- .../theme/DefaultThemeSelectorProvider.java | 7 +---- .../admin/AdminConsoleLandingPageTest.java | 31 ++++++++++++++----- 4 files changed, 39 insertions(+), 42 deletions(-) diff --git a/server-spi/src/main/java/org/keycloak/theme/ThemeSelectorProvider.java b/server-spi/src/main/java/org/keycloak/theme/ThemeSelectorProvider.java index 8d62605a84..4a96cd1b98 100755 --- a/server-spi/src/main/java/org/keycloak/theme/ThemeSelectorProvider.java +++ b/server-spi/src/main/java/org/keycloak/theme/ThemeSelectorProvider.java @@ -17,6 +17,9 @@ package org.keycloak.theme; +import org.keycloak.Config; +import org.keycloak.common.Profile; +import org.keycloak.common.Version; import org.keycloak.provider.Provider; /** @@ -32,4 +35,14 @@ public interface ThemeSelectorProvider extends Provider { */ String getThemeName(Theme.Type type); + default String getDefaultThemeName(Theme.Type type) { + String name = Config.scope("theme").get("default", Version.NAME.toLowerCase()); + if ((type == Theme.Type.ACCOUNT) && Profile.isFeatureEnabled(Profile.Feature.ACCOUNT2)) { + name = name.concat(".v2"); + } else if ((type == Theme.Type.ADMIN) && Profile.isFeatureEnabled(Profile.Feature.ADMIN2)) { + name = name.concat(".v2"); + } + return name; + } + } diff --git a/services/src/main/java/org/keycloak/theme/DefaultThemeManager.java b/services/src/main/java/org/keycloak/theme/DefaultThemeManager.java index 4a54c9f6bf..fdf9c06f0a 100755 --- a/services/src/main/java/org/keycloak/theme/DefaultThemeManager.java +++ b/services/src/main/java/org/keycloak/theme/DefaultThemeManager.java @@ -49,12 +49,10 @@ public class DefaultThemeManager implements ThemeManager { private final DefaultThemeManagerFactory factory; private final KeycloakSession session; private List providers; - private final String defaultTheme; public DefaultThemeManager(DefaultThemeManagerFactory factory, KeycloakSession session) { this.factory = factory; this.session = session; - this.defaultTheme = Config.scope("theme").get("default", Version.NAME.toLowerCase()); } @Override @@ -63,44 +61,20 @@ public class DefaultThemeManager implements ThemeManager { return getTheme(name, type); } - private String typeBasedDefault(Theme.Type type) { - boolean isProduct = Profile.isProduct(); - - if ((type == Theme.Type.ACCOUNT) && isAccount2Enabled()) { - return isProduct ? "rh-sso.v2" : "keycloak.v2"; - } - - return isProduct ? "rh-sso" : "keycloak"; - } - @Override public Theme getTheme(String name, Theme.Type type) { - if (name == null) { - name = defaultTheme; - } - Theme theme = factory.getCachedTheme(name, type); if (theme == null) { theme = loadTheme(name, type); if (theme == null) { - theme = loadTheme(typeBasedDefault(type), type); - if (theme == null) { - theme = loadTheme("base", type); - } + String defaultThemeName = session.getProvider(ThemeSelectorProvider.class).getDefaultThemeName(type); + theme = loadTheme(defaultThemeName, type); log.errorv("Failed to find {0} theme {1}, using built-in themes", type, name); } else { theme = factory.addCachedTheme(name, type, theme); } } - - if (!isAccount2Enabled() && theme.getName().equals("keycloak.v2")) { - theme = loadTheme("keycloak", type); - } - if (!isAccount2Enabled() && theme.getName().equals("rh-sso.v2")) { - theme = loadTheme("rh-sso", type); - } - return theme; } diff --git a/services/src/main/java/org/keycloak/theme/DefaultThemeSelectorProvider.java b/services/src/main/java/org/keycloak/theme/DefaultThemeSelectorProvider.java index 9ea9940c3e..b1a61dc81b 100644 --- a/services/src/main/java/org/keycloak/theme/DefaultThemeSelectorProvider.java +++ b/services/src/main/java/org/keycloak/theme/DefaultThemeSelectorProvider.java @@ -47,12 +47,7 @@ public class DefaultThemeSelectorProvider implements ThemeSelectorProvider { } if (name == null || name.isEmpty()) { - name = Config.scope("theme").get("default", Version.NAME.toLowerCase()); - if ((type == Theme.Type.ACCOUNT) && Profile.isFeatureEnabled(Profile.Feature.ACCOUNT2)) { - name = name.concat(".v2"); - } else if ((type == Theme.Type.ADMIN) && Profile.isFeatureEnabled(Profile.Feature.ADMIN2)) { - name = name.concat(".v2"); - } + name = getDefaultThemeName(type); } return name; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AdminConsoleLandingPageTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AdminConsoleLandingPageTest.java index ac0da592b1..2268b657da 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AdminConsoleLandingPageTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/AdminConsoleLandingPageTest.java @@ -13,11 +13,12 @@ import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.arquillian.annotation.DisableFeature; import java.io.IOException; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; -@DisableFeature(value = Profile.Feature.ACCOUNT2, skipRestart = true) // TODO remove this (KEYCLOAK-16228) public class AdminConsoleLandingPageTest extends AbstractKeycloakTest { private CloseableHttpClient client; @@ -44,16 +45,14 @@ public class AdminConsoleLandingPageTest extends AbstractKeycloakTest { public void landingPage() throws IOException { String body = SimpleHttp.doGet(suiteContext.getAuthServerInfo().getContextRoot() + "/auth/admin/master/console", client).asString(); - String authUrl = body.substring(body.indexOf("var authUrl = '") + 15); - authUrl = authUrl.substring(0, authUrl.indexOf("'")); + Map config = getConfig(body); + String authUrl = config.get("authUrl"); Assert.assertEquals(suiteContext.getAuthServerInfo().getContextRoot() + "/auth", authUrl); - String resourceUrl = body.substring(body.indexOf("var resourceUrl = '") + 19); - resourceUrl = resourceUrl.substring(0, resourceUrl.indexOf("'")); - Assert.assertTrue(resourceUrl.matches("/auth/resources/[^/]*/admin/([a-z]*|[a-z]*-[a-z]*)")); + String resourceUrl = config.get("resourceUrl"); + Assert.assertTrue(resourceUrl.matches("/auth/resources/[^/]*/admin/keycloak.v2")); - String consoleBaseUrl = body.substring(body.indexOf("var consoleBaseUrl = '") + 22); - consoleBaseUrl = consoleBaseUrl.substring(0, consoleBaseUrl.indexOf("'")); + String consoleBaseUrl = config.get("consoleBaseUrl"); Assert.assertEquals(consoleBaseUrl, "/auth/admin/master/console/"); Pattern p = Pattern.compile("link href=\"([^\"]*)\""); @@ -77,4 +76,20 @@ public class AdminConsoleLandingPageTest extends AbstractKeycloakTest { } } + private static Map getConfig(String body) { + Map variables = new HashMap<>(); + String start = ""; + + String config = body.substring(body.indexOf(start) + start.length()); + config = config.substring(0, config.indexOf(end)).trim(); + + Matcher matcher = Pattern.compile(".*\"(.*)\": \"(.*)\"").matcher(config); + while (matcher.find()) { + variables.put(matcher.group(1), matcher.group(2)); + } + + return variables; + } + }