From 4c8724e7b1a1375810fade399ecced78a792e852 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Thu, 16 Nov 2023 18:46:34 -0300 Subject: [PATCH] Do not run value transformation if a option is being requested by its name and there is no dependency on other options Closes #24757 Co-authored-by: Steven Hawkins Signed-off-by: Pedro Igor --- .../org/keycloak/config/ProxyOptions.java | 18 ++++++++++-- .../runtime/configuration/Configuration.java | 10 ++++++- .../configuration/mappers/PropertyMapper.java | 23 ++++++--------- .../mappers/ProxyPropertyMappers.java | 28 +++++++------------ .../hostname/DefaultHostnameProvider.java | 20 +++++++------ .../configuration/test/ConfigurationTest.java | 14 ++++++++++ 6 files changed, 70 insertions(+), 43 deletions(-) diff --git a/quarkus/config-api/src/main/java/org/keycloak/config/ProxyOptions.java b/quarkus/config-api/src/main/java/org/keycloak/config/ProxyOptions.java index 96a9375bb3..ea719d0e16 100644 --- a/quarkus/config-api/src/main/java/org/keycloak/config/ProxyOptions.java +++ b/quarkus/config-api/src/main/java/org/keycloak/config/ProxyOptions.java @@ -3,10 +3,24 @@ package org.keycloak.config; public class ProxyOptions { public enum Mode { - none, + none(false), edge, reencrypt, - passthrough; + passthrough(false); + + private final boolean proxyHeadersEnabled; + + Mode(boolean proxyHeadersEnabled) { + this.proxyHeadersEnabled = proxyHeadersEnabled; + } + + Mode() { + this(true); + } + + public boolean isProxyHeadersEnabled() { + return proxyHeadersEnabled; + } } public static final Option PROXY = new OptionBuilder<>("proxy", Mode.class) diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/Configuration.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/Configuration.java index 2e276174f8..67e26da337 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/Configuration.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/Configuration.java @@ -55,7 +55,15 @@ public final class Configuration { Optional value = getRawPersistedProperty(name); if (value.isEmpty()) { - value = getRawPersistedProperty(getMappedPropertyName(name)); + PropertyMapper mapper = PropertyMappers.getMapper(name); + + if (mapper != null) { + value = getRawPersistedProperty(mapper.getFrom()); + + if (value.isEmpty() && mapper.getTo() != null) { + value = getRawPersistedProperty(mapper.getTo()); + } + } } if (value.isEmpty()) { diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java index fe90b05189..b664b8360e 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java @@ -111,10 +111,10 @@ public class PropertyMapper { } } - return transformValue(ofNullable(parentValue == null ? null : parentValue.getValue()), context); + return transformValue(name, ofNullable(parentValue == null ? null : parentValue.getValue()), context); } - ConfigValue defaultValue = transformValue(this.option.getDefaultValue().map(Objects::toString), context); + ConfigValue defaultValue = transformValue(name, this.option.getDefaultValue().map(Objects::toString), context); if (defaultValue != null) { return defaultValue; @@ -124,19 +124,13 @@ public class PropertyMapper { ConfigValue current = context.proceed(name); if (current != null) { - return transformValue(ofNullable(current.getValue()), context); + return transformValue(name, ofNullable(current.getValue()), context).withConfigSourceName(current.getConfigSourceName()); } return current; } - Optional configValue = ofNullable(config.getValue()); - - if (config.getName().equals(name)) { - return config; - } - - ConfigValue transformedValue = transformValue(configValue, context); + ConfigValue transformedValue = transformValue(name, ofNullable(config.getValue()), context).withConfigSourceName(config.getConfigSourceName()); // we always fallback to the current value from the property we are mapping if (transformedValue == null) { @@ -196,13 +190,14 @@ public class PropertyMapper { return mask; } - private ConfigValue transformValue(Optional value, ConfigSourceInterceptorContext context) { + private ConfigValue transformValue(String name, Optional value, ConfigSourceInterceptorContext context) { if (value == null) { return null; } - if (mapper == null) { - return ConfigValue.builder().withName(to).withValue(value.orElse(null)).build(); + if (mapper == null || (mapFrom == null && name.equals(getFrom()))) { + // no mapper set or requesting a property that does not depend on other property, just return the value from the config source + return ConfigValue.builder().withName(name).withValue(value.orElse(null)).build(); } Optional mappedValue = mapper.apply(value, context); @@ -211,7 +206,7 @@ public class PropertyMapper { return null; } - return ConfigValue.builder().withName(to).withValue(mappedValue.get()).withRawValue(value.orElse(null)).build(); + return ConfigValue.builder().withName(name).withValue(mappedValue.get()).withRawValue(value.orElse(null)).build(); } private ConfigValue convertValue(ConfigValue configValue) { diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ProxyPropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ProxyPropertyMappers.java index a1d72829f3..09b5c9e0de 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ProxyPropertyMappers.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ProxyPropertyMappers.java @@ -4,7 +4,6 @@ import io.smallrye.config.ConfigSourceInterceptorContext; import java.util.Optional; -import static java.util.Optional.of; import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper.fromOption; import static org.keycloak.quarkus.runtime.integration.QuarkusPlatform.addInitializationException; @@ -19,40 +18,33 @@ final class ProxyPropertyMappers { return new PropertyMapper[] { fromOption(ProxyOptions.PROXY) .to("quarkus.http.proxy.proxy-address-forwarding") - .transformer(ProxyPropertyMappers::getValidProxyModeValue) + .transformer(ProxyPropertyMappers::isProxyHeadersEnabled) .paramLabel("mode") .build(), fromOption(ProxyOptions.PROXY_FORWARDED_HOST) .to("quarkus.http.proxy.enable-forwarded-host") .mapFrom("proxy") - .transformer(ProxyPropertyMappers::getValidProxyModeValue) + .transformer(ProxyPropertyMappers::isProxyHeadersEnabled) .build(), fromOption(ProxyOptions.PROXY_FORWARDED_HEADER_ENABLED) .to("quarkus.http.proxy.allow-forwarded") .mapFrom("proxy") - .transformer(ProxyPropertyMappers::getValidProxyModeValue) + .transformer(ProxyPropertyMappers::isProxyHeadersEnabled) .build(), fromOption(ProxyOptions.PROXY_X_FORWARDED_HEADER_ENABLED) .to("quarkus.http.proxy.allow-x-forwarded") .mapFrom("proxy") - .transformer(ProxyPropertyMappers::getValidProxyModeValue) + .transformer(ProxyPropertyMappers::isProxyHeadersEnabled) .build() }; } - private static Optional getValidProxyModeValue(Optional value, ConfigSourceInterceptorContext context) { - String mode = value.get(); - - switch (mode) { - case "none": - case "passthrough": - return of(Boolean.FALSE.toString()); - case "edge": - case "reencrypt": - return of(Boolean.TRUE.toString()); - default: - addInitializationException(Messages.invalidProxyMode(mode)); - return of(Boolean.FALSE.toString()); + private static Optional isProxyHeadersEnabled(Optional value, ConfigSourceInterceptorContext context) { + try { + return Optional.of(String.valueOf(ProxyOptions.Mode.valueOf(value.get()).isProxyHeadersEnabled())); + } catch (IllegalArgumentException iae) { + addInitializationException(Messages.invalidProxyMode(value.get())); + return Optional.of(Boolean.FALSE.toString()); } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/hostname/DefaultHostnameProvider.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/hostname/DefaultHostnameProvider.java index 2591a9be79..6e58890f36 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/hostname/DefaultHostnameProvider.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/hostname/DefaultHostnameProvider.java @@ -18,6 +18,9 @@ package org.keycloak.quarkus.runtime.hostname; import static org.keycloak.common.util.UriUtils.checkUrl; +import static org.keycloak.config.ProxyOptions.PROXY; +import static org.keycloak.quarkus.runtime.configuration.Configuration.getConfigValue; +import static org.keycloak.quarkus.runtime.configuration.Configuration.getKcConfigValue; import static org.keycloak.urls.UrlType.ADMIN; import static org.keycloak.urls.UrlType.LOCAL_ADMIN; import static org.keycloak.urls.UrlType.BACKEND; @@ -36,9 +39,10 @@ import org.keycloak.Config; import org.keycloak.common.enums.SslRequired; import org.keycloak.common.util.Resteasy; import org.keycloak.config.HostnameOptions; +import org.keycloak.config.ProxyOptions; +import org.keycloak.config.ProxyOptions.Mode; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; -import org.keycloak.quarkus.runtime.configuration.Configuration; import org.keycloak.urls.HostnameProvider; import org.keycloak.urls.HostnameProviderFactory; import org.keycloak.urls.UrlType; @@ -237,15 +241,15 @@ public final class DefaultHostnameProvider implements HostnameProvider, Hostname @Override public void init(Config.Scope config) { - boolean isHttpEnabled = Boolean.parseBoolean(Configuration.getConfigValue("kc.http-enabled").getValue()); - String configPath = Configuration.getConfigValue("kc.http-relative-path").getValue(); + boolean isHttpEnabled = Boolean.parseBoolean(getConfigValue("kc.http-enabled").getValue()); + String configPath = getConfigValue("kc.http-relative-path").getValue(); if (!configPath.startsWith("/")) { configPath = "/" + configPath; } - String httpPort = Configuration.getConfigValue("kc.https-port").getValue(); - String configPort = isHttpEnabled ? Configuration.getConfigValue("kc.http-port").getValue() : httpPort ; + String httpsPort = getConfigValue("kc.https-port").getValue(); + String configPort = isHttpEnabled ? getConfigValue("kc.http-port").getValue() : httpsPort ; String scheme = isHttpEnabled ? "http://" : "https://"; @@ -285,15 +289,15 @@ public final class DefaultHostnameProvider implements HostnameProvider, Hostname } defaultPath = config.get("path", frontEndBaseUri == null ? null : frontEndBaseUri.getPath()); - noProxy = Configuration.getConfigValue("kc.proxy").getValue().equals("false"); - defaultTlsPort = Integer.parseInt(httpPort); + noProxy = Mode.none.equals(ProxyOptions.Mode.valueOf(getKcConfigValue(PROXY.getKey()).getValue())); + defaultTlsPort = Integer.parseInt(httpsPort); if (defaultTlsPort == DEFAULT_HTTPS_PORT_VALUE) { defaultTlsPort = RESTEASY_DEFAULT_PORT_VALUE; } if (frontEndBaseUri == null) { - hostnamePort = Integer.parseInt(Configuration.getConfigValue("kc.hostname-port").getValue()); + hostnamePort = Integer.parseInt(getConfigValue("kc.hostname-port").getValue()); } else { hostnamePort = frontEndBaseUri.getPort(); } diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java index a32741dbc5..a14077df2a 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java @@ -208,6 +208,20 @@ public class ConfigurationTest { assertEquals("http://c.jwk.url", initConfig("client-registration", "openid-connect").get("static-jwk-url")); } + @Test + public void testResolveTransformedValue() { + System.setProperty(CLI_ARGS, ""); + assertEquals("none", createConfig().getConfigValue("kc.proxy").getValue()); + System.setProperty(CLI_ARGS, "--proxy=none"); + assertEquals("none", createConfig().getConfigValue("kc.proxy").getValue()); + System.setProperty(CLI_ARGS, ""); + assertEquals("none", createConfig().getConfigValue("kc.proxy").getValue()); + System.setProperty(CLI_ARGS, "--proxy=none" + ARG_SEPARATOR + "--http-enabled=false"); + assertEquals("false", createConfig().getConfigValue("kc.http-enabled").getValue()); + System.setProperty(CLI_ARGS, "--proxy=none" + ARG_SEPARATOR + "--http-enabled=true"); + assertEquals("true", createConfig().getConfigValue("kc.http-enabled").getValue()); + } + @Test public void testPropertyNamesFromConfig() { System.setProperty(CLI_ARGS, "--spi-client-registration-openid-connect-static-jwk-url=http://c.jwk.url");