Skip to content

Commit

Permalink
Fixes smallrye#269. Support secret keys names.
Browse files Browse the repository at this point in the history
  • Loading branch information
radcortez committed Apr 17, 2020
1 parent b21b84e commit 0608a13
Show file tree
Hide file tree
Showing 12 changed files with 394 additions and 45 deletions.
12 changes: 12 additions & 0 deletions implementation/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,18 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jboss.weld</groupId>
<artifactId>weld-junit4</artifactId>
<version>2.0.1.Final</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>javax.enterprise</groupId>
<artifactId>cdi-api</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package io.smallrye.config;

import static io.smallrye.config.SecretKeys.doLocked;

import org.jboss.logging.Logger;

public class LoggingConfigSourceInterceptor implements ConfigSourceInterceptor {
private static final Logger LOG = Logger.getLogger("io.smallrye.config");

@Override
public ConfigValue getValue(final ConfigSourceInterceptorContext context, final String name) {
try {
// Unlocked keys will run here.
ConfigValue configValue = doLocked(() -> context.proceed(name));
if (configValue != null)
lookup(configValue);
else
notFound(name);
return configValue;
} catch (SecurityException e) {
// Handled next, to omit the values to log from the secret.
}

// Locked keys here.
final ConfigValue secret = context.proceed(name);
if (secret != null)
lookup(secret.from().withValue("secret").withConfigSourceName("secret").withLineNumber(-1).build());
else
notFound(name);
return secret;
}

private void notFound(final String name) {
if (LOG.isDebugEnabled()) {
LOG.debugv("The config {0} was not found", name);
}
}

private void lookup(final ConfigValue configValue) {
if (LOG.isDebugEnabled()) {
LOG.debugv("The config {0} was loaded from {1} with the value {2}",
configValue.getName(), getLocation(configValue), configValue.getValue());
}
}

private String getLocation(final ConfigValue configValue) {
return configValue.getLineNumber() != -1 ? configValue.getConfigSourceName() + ":" + configValue.getLineNumber()
: configValue.getConfigSourceName();
}
}
52 changes: 52 additions & 0 deletions implementation/src/main/java/io/smallrye/config/SecretKeys.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package io.smallrye.config;

import java.io.Serializable;
import java.util.function.Supplier;

public class SecretKeys implements Serializable {
private static final ThreadLocal<Boolean> LOCKED = ThreadLocal.withInitial(() -> Boolean.TRUE);

public static boolean isLocked() {
return LOCKED.get();
}

public static void doUnlocked(Runnable runnable) {
doUnlocked(() -> {
runnable.run();
return null;
});
}

public static <T> T doUnlocked(Supplier<T> supplier) {
if (isLocked()) {
LOCKED.set(false);
try {
return supplier.get();
} finally {
LOCKED.set(true);
}
} else {
return supplier.get();
}
}

public static void doLocked(Runnable runnable) {
doLocked(() -> {
runnable.run();
return null;
});
}

public static <T> T doLocked(Supplier<T> supplier) {
if (!isLocked()) {
LOCKED.set(true);
try {
return supplier.get();
} finally {
LOCKED.set(false);
}
} else {
return supplier.get();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import java.util.function.IntFunction;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;

import org.eclipse.microprofile.config.Config;
Expand Down Expand Up @@ -68,18 +69,21 @@ public int compare(ConfigSource o1, ConfigSource o2) {
private final Map<Type, Converter<?>> converters;
private final Map<Type, Converter<Optional<?>>> optionalConverters = new ConcurrentHashMap<>();
private final ConfigSourceInterceptorContext interceptorChain;
private final Set<String> secretKeys;

SmallRyeConfig(SmallRyeConfigBuilder builder) {
this.configSourcesRef = buildConfigSources(builder);
this.interceptorChain = buildInterceptorChain(builder);
this.converters = buildConverters(builder);
this.secretKeys = buildSecretKeys(builder);
this.interceptorChain = buildInterceptorChain(builder);
}

@Deprecated
protected SmallRyeConfig(List<ConfigSource> configSources, Map<Type, Converter<?>> converters) {
this.configSourcesRef = new AtomicReference<>(Collections.unmodifiableList(configSources));
this.converters = new ConcurrentHashMap<>(Converters.ALL_CONVERTERS);
this.converters.putAll(converters);
this.secretKeys = Collections.emptySet();
this.interceptorChain = buildInterceptorChain(new SmallRyeConfigBuilder());
}

Expand Down Expand Up @@ -124,6 +128,10 @@ private Map<Type, Converter<?>> buildConverters(final SmallRyeConfigBuilder buil
return converters;
}

private Set<String> buildSecretKeys(final SmallRyeConfigBuilder builder) {
return Collections.unmodifiableSet(builder.getSecretKeys());
}

private ConfigSourceInterceptorContext buildInterceptorChain(final SmallRyeConfigBuilder builder) {
final List<InterceptorWithPriority> interceptors = new ArrayList<>(builder.getInterceptors());
if (builder.isAddDiscoveredInterceptors()) {
Expand All @@ -135,7 +143,10 @@ private ConfigSourceInterceptorContext buildInterceptorChain(final SmallRyeConfi

interceptors.sort(Comparator.comparingInt(InterceptorWithPriority::getPriority).reversed());

SmallRyeConfigSourceInterceptorContext current = new SmallRyeConfigSourceInterceptorContext(
SmallRyeConfigSourceInterceptorContext current;

// Last interceptor to lookup the key
current = new SmallRyeConfigSourceInterceptorContext(
(ConfigSourceInterceptor) (context, name) -> {
for (ConfigSource configSource : getConfigSources()) {
if (configSource instanceof ConfigValueConfigSource) {
Expand All @@ -159,6 +170,14 @@ private ConfigSourceInterceptorContext buildInterceptorChain(final SmallRyeConfi
return null;
}, null);

// Security interceptor to prevent access to secret keys
current = new SmallRyeConfigSourceInterceptorContext((ConfigSourceInterceptor) (context, name) -> {
if (SecretKeys.isLocked() && isSecret(name)) {
throw new SecurityException("Not allowed to access secret key " + name);
}
return context.proceed(name);
}, current);

for (int i = interceptors.size() - 1; i >= 0; i--) {
current = new SmallRyeConfigSourceInterceptorContext(interceptors.get(i).getInterceptor(current), current);
}
Expand Down Expand Up @@ -254,6 +273,22 @@ public Iterable<ConfigSource> getConfigSources() {
return configSourcesRef.get();
}

public void unlockSecrets(Runnable runnable) {
SecretKeys.doUnlocked(runnable);
}

public <T> T unlockSecrets(Supplier<T> supplier) {
return SecretKeys.doUnlocked(supplier);
}

private boolean isSecret(String name) {
return secretKeys.contains(name);
}

private boolean isSecretAccessible(String name) {
return !isSecret(name) || !SecretKeys.isLocked();
}

/**
* Add a configuration source to the configuration object. The list of configuration sources is re-sorted
* to insert the new source into the correct position. Configuration source wrappers configured with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.OptionalInt;
import java.util.ServiceLoader;
import java.util.Set;
import java.util.function.Function;
import java.util.function.UnaryOperator;
import java.util.stream.Collectors;
Expand All @@ -50,6 +52,7 @@ public class SmallRyeConfigBuilder implements ConfigBuilder {
private List<ConfigSource> sources = new ArrayList<>();
private Function<ConfigSource, ConfigSource> sourceWrappers = UnaryOperator.identity();
private Map<Type, ConverterWithPriority> converters = new HashMap<>();
private Set<String> secretKeys = new HashSet<>();
private List<InterceptorWithPriority> interceptors = new ArrayList<>();
private ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
private boolean addDefaultSources = false;
Expand Down Expand Up @@ -185,6 +188,11 @@ public SmallRyeConfigBuilder withInterceptorFactories(ConfigSourceInterceptorFac
return this;
}

public SmallRyeConfigBuilder withSecretKeys(String... keys) {
secretKeys.addAll(Stream.of(keys).collect(Collectors.toSet()));
return this;
}

@Override
public SmallRyeConfigBuilder withConverters(Converter<?>[] converters) {
for (Converter<?> converter : converters) {
Expand Down Expand Up @@ -246,6 +254,10 @@ Map<Type, ConverterWithPriority> getConverters() {
return converters;
}

Set<String> getSecretKeys() {
return secretKeys;
}

List<InterceptorWithPriority> getInterceptors() {
return interceptors;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public static <T> T getValue(InjectionPoint injectionPoint, Config config) {
}
final SmallRyeConfig src = (SmallRyeConfig) config;
Converter<T> converter = resolveConverter(injectionPoint, src);
String rawValue = src.getRawValue(name);
String rawValue = getRawValue(src, name);
if (rawValue == null) {
rawValue = getDefaultValue(injectionPoint);
}
Expand All @@ -59,6 +59,10 @@ public static <T> T getValue(InjectionPoint injectionPoint, Config config) {
return converted;
}

private static String getRawValue(SmallRyeConfig config, String name) {
return config.unlockSecrets(() -> config.getRawValue(name));
}

private static NoSuchElementException propertyNotFound(final String name) {
return new NoSuchElementException("Required property " + name + " not found");
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package io.smallrye.config;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;

import java.util.NoSuchElementException;

import org.eclipse.microprofile.config.Config;
import org.junit.Test;

public class LoggingConfigSourceInterceptorTest {
@Test
public void interceptor() throws Exception {
SmallRyeConfig config = (SmallRyeConfig) buildConfig();

assertEquals("abc", config.getValue("my.prop", String.class));
assertThrows(SecurityException.class, () -> config.getValue("secret", String.class));
assertThrows(NoSuchElementException.class, () -> config.getValue("not.found", String.class));

// This should not log the secret value:
assertEquals("12345678", config.unlockSecrets(() -> config.getRawValue("secret")));
}

private static Config buildConfig() throws Exception {
return new SmallRyeConfigBuilder()
.addDefaultSources()
.withSources(new ConfigValuePropertiesConfigSource(
LoggingConfigSourceInterceptorTest.class.getResource("/config-values.properties")))
.withInterceptors(new LoggingConfigSourceInterceptor())
.withSecretKeys("secret")
.build();
}
}
Loading

0 comments on commit 0608a13

Please sign in to comment.