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 <shawkins@redhat.com>
Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
Pedro Igor 2023-11-16 18:46:34 -03:00
parent cd1e0e06c6
commit 4c8724e7b1
6 changed files with 70 additions and 43 deletions

View file

@ -3,10 +3,24 @@ package org.keycloak.config;
public class ProxyOptions { public class ProxyOptions {
public enum Mode { public enum Mode {
none, none(false),
edge, edge,
reencrypt, 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<Mode> PROXY = new OptionBuilder<>("proxy", Mode.class) public static final Option<Mode> PROXY = new OptionBuilder<>("proxy", Mode.class)

View file

@ -55,7 +55,15 @@ public final class Configuration {
Optional<String> value = getRawPersistedProperty(name); Optional<String> value = getRawPersistedProperty(name);
if (value.isEmpty()) { 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()) { if (value.isEmpty()) {

View file

@ -111,10 +111,10 @@ public class PropertyMapper<T> {
} }
} }
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) { if (defaultValue != null) {
return defaultValue; return defaultValue;
@ -124,19 +124,13 @@ public class PropertyMapper<T> {
ConfigValue current = context.proceed(name); ConfigValue current = context.proceed(name);
if (current != null) { if (current != null) {
return transformValue(ofNullable(current.getValue()), context); return transformValue(name, ofNullable(current.getValue()), context).withConfigSourceName(current.getConfigSourceName());
} }
return current; return current;
} }
Optional<String> configValue = ofNullable(config.getValue()); ConfigValue transformedValue = transformValue(name, ofNullable(config.getValue()), context).withConfigSourceName(config.getConfigSourceName());
if (config.getName().equals(name)) {
return config;
}
ConfigValue transformedValue = transformValue(configValue, context);
// we always fallback to the current value from the property we are mapping // we always fallback to the current value from the property we are mapping
if (transformedValue == null) { if (transformedValue == null) {
@ -196,13 +190,14 @@ public class PropertyMapper<T> {
return mask; return mask;
} }
private ConfigValue transformValue(Optional<String> value, ConfigSourceInterceptorContext context) { private ConfigValue transformValue(String name, Optional<String> value, ConfigSourceInterceptorContext context) {
if (value == null) { if (value == null) {
return null; return null;
} }
if (mapper == null) { if (mapper == null || (mapFrom == null && name.equals(getFrom()))) {
return ConfigValue.builder().withName(to).withValue(value.orElse(null)).build(); // 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<String> mappedValue = mapper.apply(value, context); Optional<String> mappedValue = mapper.apply(value, context);
@ -211,7 +206,7 @@ public class PropertyMapper<T> {
return null; 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) { private ConfigValue convertValue(ConfigValue configValue) {

View file

@ -4,7 +4,6 @@ import io.smallrye.config.ConfigSourceInterceptorContext;
import java.util.Optional; 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.configuration.mappers.PropertyMapper.fromOption;
import static org.keycloak.quarkus.runtime.integration.QuarkusPlatform.addInitializationException; import static org.keycloak.quarkus.runtime.integration.QuarkusPlatform.addInitializationException;
@ -19,40 +18,33 @@ final class ProxyPropertyMappers {
return new PropertyMapper[] { return new PropertyMapper[] {
fromOption(ProxyOptions.PROXY) fromOption(ProxyOptions.PROXY)
.to("quarkus.http.proxy.proxy-address-forwarding") .to("quarkus.http.proxy.proxy-address-forwarding")
.transformer(ProxyPropertyMappers::getValidProxyModeValue) .transformer(ProxyPropertyMappers::isProxyHeadersEnabled)
.paramLabel("mode") .paramLabel("mode")
.build(), .build(),
fromOption(ProxyOptions.PROXY_FORWARDED_HOST) fromOption(ProxyOptions.PROXY_FORWARDED_HOST)
.to("quarkus.http.proxy.enable-forwarded-host") .to("quarkus.http.proxy.enable-forwarded-host")
.mapFrom("proxy") .mapFrom("proxy")
.transformer(ProxyPropertyMappers::getValidProxyModeValue) .transformer(ProxyPropertyMappers::isProxyHeadersEnabled)
.build(), .build(),
fromOption(ProxyOptions.PROXY_FORWARDED_HEADER_ENABLED) fromOption(ProxyOptions.PROXY_FORWARDED_HEADER_ENABLED)
.to("quarkus.http.proxy.allow-forwarded") .to("quarkus.http.proxy.allow-forwarded")
.mapFrom("proxy") .mapFrom("proxy")
.transformer(ProxyPropertyMappers::getValidProxyModeValue) .transformer(ProxyPropertyMappers::isProxyHeadersEnabled)
.build(), .build(),
fromOption(ProxyOptions.PROXY_X_FORWARDED_HEADER_ENABLED) fromOption(ProxyOptions.PROXY_X_FORWARDED_HEADER_ENABLED)
.to("quarkus.http.proxy.allow-x-forwarded") .to("quarkus.http.proxy.allow-x-forwarded")
.mapFrom("proxy") .mapFrom("proxy")
.transformer(ProxyPropertyMappers::getValidProxyModeValue) .transformer(ProxyPropertyMappers::isProxyHeadersEnabled)
.build() .build()
}; };
} }
private static Optional<String> getValidProxyModeValue(Optional<String> value, ConfigSourceInterceptorContext context) { private static Optional<String> isProxyHeadersEnabled(Optional<String> value, ConfigSourceInterceptorContext context) {
String mode = value.get(); try {
return Optional.of(String.valueOf(ProxyOptions.Mode.valueOf(value.get()).isProxyHeadersEnabled()));
switch (mode) { } catch (IllegalArgumentException iae) {
case "none": addInitializationException(Messages.invalidProxyMode(value.get()));
case "passthrough": return Optional.of(Boolean.FALSE.toString());
return of(Boolean.FALSE.toString());
case "edge":
case "reencrypt":
return of(Boolean.TRUE.toString());
default:
addInitializationException(Messages.invalidProxyMode(mode));
return of(Boolean.FALSE.toString());
} }
} }

View file

@ -18,6 +18,9 @@
package org.keycloak.quarkus.runtime.hostname; package org.keycloak.quarkus.runtime.hostname;
import static org.keycloak.common.util.UriUtils.checkUrl; 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.ADMIN;
import static org.keycloak.urls.UrlType.LOCAL_ADMIN; import static org.keycloak.urls.UrlType.LOCAL_ADMIN;
import static org.keycloak.urls.UrlType.BACKEND; 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.enums.SslRequired;
import org.keycloak.common.util.Resteasy; import org.keycloak.common.util.Resteasy;
import org.keycloak.config.HostnameOptions; 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.KeycloakSession;
import org.keycloak.models.RealmModel; import org.keycloak.models.RealmModel;
import org.keycloak.quarkus.runtime.configuration.Configuration;
import org.keycloak.urls.HostnameProvider; import org.keycloak.urls.HostnameProvider;
import org.keycloak.urls.HostnameProviderFactory; import org.keycloak.urls.HostnameProviderFactory;
import org.keycloak.urls.UrlType; import org.keycloak.urls.UrlType;
@ -237,15 +241,15 @@ public final class DefaultHostnameProvider implements HostnameProvider, Hostname
@Override @Override
public void init(Config.Scope config) { public void init(Config.Scope config) {
boolean isHttpEnabled = Boolean.parseBoolean(Configuration.getConfigValue("kc.http-enabled").getValue()); boolean isHttpEnabled = Boolean.parseBoolean(getConfigValue("kc.http-enabled").getValue());
String configPath = Configuration.getConfigValue("kc.http-relative-path").getValue(); String configPath = getConfigValue("kc.http-relative-path").getValue();
if (!configPath.startsWith("/")) { if (!configPath.startsWith("/")) {
configPath = "/" + configPath; configPath = "/" + configPath;
} }
String httpPort = Configuration.getConfigValue("kc.https-port").getValue(); String httpsPort = getConfigValue("kc.https-port").getValue();
String configPort = isHttpEnabled ? Configuration.getConfigValue("kc.http-port").getValue() : httpPort ; String configPort = isHttpEnabled ? getConfigValue("kc.http-port").getValue() : httpsPort ;
String scheme = isHttpEnabled ? "http://" : "https://"; 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()); defaultPath = config.get("path", frontEndBaseUri == null ? null : frontEndBaseUri.getPath());
noProxy = Configuration.getConfigValue("kc.proxy").getValue().equals("false"); noProxy = Mode.none.equals(ProxyOptions.Mode.valueOf(getKcConfigValue(PROXY.getKey()).getValue()));
defaultTlsPort = Integer.parseInt(httpPort); defaultTlsPort = Integer.parseInt(httpsPort);
if (defaultTlsPort == DEFAULT_HTTPS_PORT_VALUE) { if (defaultTlsPort == DEFAULT_HTTPS_PORT_VALUE) {
defaultTlsPort = RESTEASY_DEFAULT_PORT_VALUE; defaultTlsPort = RESTEASY_DEFAULT_PORT_VALUE;
} }
if (frontEndBaseUri == null) { if (frontEndBaseUri == null) {
hostnamePort = Integer.parseInt(Configuration.getConfigValue("kc.hostname-port").getValue()); hostnamePort = Integer.parseInt(getConfigValue("kc.hostname-port").getValue());
} else { } else {
hostnamePort = frontEndBaseUri.getPort(); hostnamePort = frontEndBaseUri.getPort();
} }

View file

@ -208,6 +208,20 @@ public class ConfigurationTest {
assertEquals("http://c.jwk.url", initConfig("client-registration", "openid-connect").get("static-jwk-url")); 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 @Test
public void testPropertyNamesFromConfig() { public void testPropertyNamesFromConfig() {
System.setProperty(CLI_ARGS, "--spi-client-registration-openid-connect-static-jwk-url=http://c.jwk.url"); System.setProperty(CLI_ARGS, "--spi-client-registration-openid-connect-static-jwk-url=http://c.jwk.url");