Skip to content

fix: removing EnvConfigSource keys that are not intended #51291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shawkins wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: removing EnvConfigSource keys that are not intended #51291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/documentation/upgrading/topics/changes/changes-26_8_0.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ When calling {project_name}'s experimental AuthZEN endpoints, the caller now nee

The Admin REST API endpoints for listing client sessions (`GET /admin/realms/\{realm}/clients/\{id}/user-sessions` and `GET /admin/realms/\{realm}/clients/\{id}/offline-sessions`) now filter out sessions belonging to users that the caller does not have permission to view. Previously, any caller with the `view-clients` role could see all user sessions for a client, potentially exposing user identities to callers without the `view-users` role. After upgrading, callers without `view-users` permission will see fewer results from these endpoints.

=== `show-config` will mask all SPI option values

The `show-config` command will mask all SPI option values. The command is not currently aware of provider factory configuration property metadata, so it cannot determine if a property is marked as `isSecret`.

=== Redesigned identity provider buttons on the login page

The social identity provider section on the login page has been redesigned with updated icons and layout.
Expand Down
11 changes: 9 additions & 2 deletions quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/ShowConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,15 @@ protected void runCommand() {
private void printProperty(String property, PropertyMapper<?> mapper, ConfigValue configValue) {
String sourceName = configValue.getConfigSourceName();
String value = configValue.getValue();

value = maskValue(value, sourceName, mapper);

if (property.startsWith(MicroProfileConfigProvider.SPI_PREFIX)) {
// could be marked as ProviderConfigProperty.isSecret, so the simplest option for now
// is to just mask all direct usage of spi options.
// the most straight-forward alternative is to move show-config to be run after the quarkus start

@Pepo48 Pepo48 Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shawkins, correct. To be specific, if we decide to revert the changes in KcEnvConfigSource.java and Configuration.java, I would adjust:

Suggested change
// the most straight-forward alternative is to move show-config to be run after the quarkus start
if (property.startsWith(MicroProfileConfigProvider.SPI_PREFIX)
|| property.startsWith(MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX + "spi.")) {

During the review yesterday, I ran the added tests with this change on top and everything seemed to work.

@shawkins shawkins Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, I'm not saying that we have to take this approach, just that it's an option if anyone feels the changes to the Configuration creation seem too risky.

value = PropertyMappers.VALUE_MASK;
} else {
value = maskValue(value, sourceName, mapper);
}

spec.commandLine().getOut().printf("\t%s = %s (%s)%n", property, value, KeycloakConfigSourceProvider.getConfigSourceDisplayName(sourceName));
}
Expand Down
11 changes: 10 additions & 1 deletion quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/Configuration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@

import io.quarkus.runtime.configuration.ConfigUtils;
import io.smallrye.config.ConfigValue;
import io.smallrye.config.DotEnvConfigSourceProvider;
import io.smallrye.config.SmallRyeConfig;
import io.smallrye.config.SysPropConfigSource;

import static org.keycloak.quarkus.runtime.cli.Picocli.ARG_PREFIX;
import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX;
Expand Down Expand Up @@ -101,7 +103,14 @@ public static boolean isInitialized() {

public static synchronized SmallRyeConfig getConfig() {
if (config == null) {
config = ConfigUtils.emptyConfigBuilder().addDiscoveredSources().withCustomizers(new ConfigBuilderCustomizer()).build();
// we're manually adding the default sources to have control over the EnvConfigSource
config = ConfigUtils.emptyConfigBuilder().setAddDefaultSources(false).setAddPropertiesSources(true)
.addDiscoveredSources()
.withCustomizers(new ConfigBuilderCustomizer())
.withSources(new SysPropConfigSource())
.withSources(new DotEnvConfigSourceProvider()
.getConfigSources(Thread.currentThread().getContextClassLoader()))
Comment on lines +111 to +112

@shawkins shawkins Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not an issue for two reasons. 1. all spi options are now masked. 2. .env files are not a documented configsource, it is not expected that they will be used.

.build();
}
return config;
}
Expand Down
35 changes: 21 additions & 14 deletions ...s/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/KcEnvConfigSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

Expand All @@ -32,16 +33,16 @@

import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX;

import static io.smallrye.config.common.utils.StringUtil.replaceNonAlphanumericByUnderscores;

// Not extending EnvConfigSource as it's too smart for our own good. It does unnecessary mapping of provided keys
// leading to e.g. duplicate entries (like kc.db-password and kc.db.password), or incorrectly handling getters due to
// how equals() is implemented. We don't need that here as we do our own mapping.
// Instead an EnvConfigSource will be created for just the entries this logic is not concerned with
public class KcEnvConfigSource extends PropertiesConfigSource {

public static final String NAME = "KcEnvVarConfigSource";
public static final String KCKEY_PREFIX = "KCKEY_";
public static final String KCRAW_PREFIX = "KCRAW_";
public static final String KC_PREFIX = "KC_";

static final Map<String, String> ENV_OVERRIDE = new HashMap<String, String>();

Expand All @@ -50,14 +51,13 @@ public KcEnvConfigSource(Map<String, String> env) {
}

private static Map<String, String> buildProperties(Map<String, String> env) {
Map<String, String> properties = new HashMap<>();
String kcPrefix = replaceNonAlphanumericByUnderscores(NS_KEYCLOAK_PREFIX.toUpperCase());
Map<String, String> properties = new HashMap<>(env);

for (Map.Entry<String, String> entry : env.entrySet()) {
String key = entry.getKey();
String value = entry.getValue();

if (!(key.startsWith(kcPrefix) || key.startsWith(KCRAW_PREFIX))) {
if (!(key.startsWith(KC_PREFIX) || key.startsWith(KCRAW_PREFIX))) {
continue;
}

Expand All @@ -68,13 +68,13 @@ private static Map<String, String> buildProperties(Map<String, String> env) {
baseKey = key.substring(KCRAW_PREFIX.length());

// Fail fast if both KC_ and KCRAW_ are set for the same base key
if (env.containsKey(kcPrefix + baseKey)) {
if (env.containsKey(KC_PREFIX + baseKey)) {
throw new IllegalArgumentException(
"Both " + kcPrefix + baseKey + " and " + KCRAW_PREFIX + baseKey
"Both " + KC_PREFIX + baseKey + " and " + KCRAW_PREFIX + baseKey
+ " are set. Use only one.");
}
} else {
baseKey = key.substring(kcPrefix.length());
baseKey = key.substring(KC_PREFIX.length());
}

// Resolve the transformed key
Expand Down Expand Up @@ -108,14 +108,21 @@ private static Map<String, String> buildProperties(Map<String, String> env) {

public static Collection<ConfigSource> getConfigSources() {
Map<String, String> env = System.getenv();

if (ENV_OVERRIDE.isEmpty()) {
return List.of(new KcEnvConfigSource(env));
}

env = new HashMap<String, String>(env);
env.putAll(ENV_OVERRIDE);

// create the quarkus env from anything not applicable to the KcEnvConfigSource
Map<String, String> filteredEnv = new HashMap<>();
for (Iterator<Map.Entry<String, String>> iterator = env.entrySet().iterator(); iterator.hasNext();) {
var entry = iterator.next();
String key = entry.getKey();
if (!key.startsWith(KC_PREFIX) && !key.startsWith(KCRAW_PREFIX) && !key.startsWith(KCKEY_PREFIX)) {
iterator.remove();
filteredEnv.put(key, entry.getValue());
}
}
EnvConfigSource quarkusEnv = new EnvConfigSource(filteredEnv, EnvConfigSource.ORDINAL);

return List.of(new KcEnvConfigSource(env), new EnvConfigSource(ENV_OVERRIDE, EnvConfigSource.ORDINAL + 1));
return List.of(new KcEnvConfigSource(env), quarkusEnv);
}
}
20 changes: 20 additions & 0 deletions quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/cli/PicocliTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,26 @@ public void testShowConfigHidesSystemProperties() {
assertThat(nonRunningPicocli.getOutString(), not(containsString("kc.something")));
});
}

@Test
public void testShowConfigCommandHidesSecondaryMappedVaultPassword() {
putEnvVar("KC_SPI_VAULT__KEYSTORE__PASS", "mapped-vault-secret");
NonRunningPicocli nonRunningPicocli = pseudoLaunch("show-config");
assertThat(nonRunningPicocli.getOutString(), not(containsString("mapped-vault-secret")));
}

@Test
public void testUnknownSpiOptionMasked() {
putEnvVar("KC_SPI_CUSTOM__ID__SECRET", "custom-secret");
NonRunningPicocli nonRunningPicocli = pseudoLaunch("show-config");
assertThat(nonRunningPicocli.getOutString(), not(containsString("custom-secret")));
}

@Test
public void testShowConfigCommandHidesSecondaryMappedVaultPasswordLegacyFormat() {
NonRunningPicocli nonRunningPicocli = pseudoLaunch("show-config", "--spi-vault-keystore-pass=mapped-vault-secret");
assertThat(nonRunningPicocli.getOutString(), not(containsString("mapped-vault-secret")));
}

@Test
public void testShowConfigDisplaysPrimaryValue() {
Expand Down
Loading