Skip to content

Commit

Permalink
Do not run value transformation if a option is being requested by its…
Browse files Browse the repository at this point in the history 
… name and there is no dependency on other options

Closes keycloak#24757

Co-authored-by: Steven Hawkins <shawkins@redhat.com>
Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
  • Loading branch information
@pedroigor @shawkins
pedroigor and shawkins committed Nov 21, 2023
1 parent cd1e0e0 commit 4c8724e
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 43 deletions.
18 changes: 16 additions & 2 deletions quarkus/config-api/src/main/java/org/keycloak/config/ProxyOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,24 @@
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<Mode> PROXY = new OptionBuilder<>("proxy", Mode.class)
Expand Down
10 changes: 9 additions & 1 deletion quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/Configuration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,15 @@ public static Optional<String> getBuildTimeProperty(String name) {
Optional<String> 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()) {
Expand Down
23 changes: 9 additions & 14 deletions ...time/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,10 @@ ConfigValue getConfigValue(String name, ConfigSourceInterceptorContext context)
}
}

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;
Expand All @@ -124,19 +124,13 @@ ConfigValue getConfigValue(String name, ConfigSourceInterceptorContext context)
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<String> 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) {
Expand Down Expand Up @@ -196,13 +190,14 @@ boolean isMask() {
return mask;
}

private ConfigValue transformValue(Optional<String> value, ConfigSourceInterceptorContext context) {
private ConfigValue transformValue(String name, Optional<String> 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<String> mappedValue = mapper.apply(value, context);
Expand All @@ -211,7 +206,7 @@ private ConfigValue transformValue(Optional<String> value, ConfigSourceIntercept
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) {
Expand Down
28 changes: 10 additions & 18 deletions ...rc/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ProxyPropertyMappers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

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;

Expand All @@ -19,40 +18,33 @@ public static PropertyMapper[] getProxyPropertyMappers() {
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<String> getValidProxyModeValue(Optional<String> 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<String> isProxyHeadersEnabled(Optional<String> 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());
}
}

Expand Down
20 changes: 12 additions & 8 deletions .../runtime/src/main/java/org/keycloak/quarkus/runtime/hostname/DefaultHostnameProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,9 +39,10 @@
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;
Expand Down Expand Up @@ -237,15 +241,15 @@ public HostnameProvider create(KeycloakSession session) {

@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://";

Expand Down Expand Up @@ -285,15 +289,15 @@ public void init(Config.Scope config) {
}

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();
}
Expand Down
14 changes: 14 additions & 0 deletions ...time/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,20 @@ public void testSpiConfigurationUsingCommandLineArguments() {
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");
Expand Down

0 comments on commit 4c8724e

Please sign in to comment.