Skip to content

Commit

Permalink
Secret type to represent secrets and integrate with the secrets handling
Browse files Browse the repository at this point in the history
  • Loading branch information
radcortez committed Oct 2, 2024
1 parent 5c88f7b commit 0122acb
Show file tree
Hide file tree
Showing 8 changed files with 212 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ static byte[] generate(final ConfigMappingInterface mapping) {
generateToString(visitor, mapping);
generateNames(visitor, mapping);
generateDefaults(visitor, mapping);
generateSecrets(visitor, mapping);

return writer.toByteArray();
}
Expand Down Expand Up @@ -1021,6 +1022,48 @@ private static void generateDefaults(final ClassVisitor classVisitor, final Conf
mv.visitEnd();
}

private static void generateSecrets(final ClassVisitor classVisitor, final ConfigMappingInterface mapping) {
MethodVisitor mv = classVisitor.visitMethod(ACC_PUBLIC | ACC_STATIC, "getSecrets", "()Ljava/util/Map;",
"()Ljava/util/Map<Ljava/lang/String;Ljava/lang/String;>;",
null);

mv.visitTypeInsn(NEW, "java/util/HashSet");
mv.visitInsn(DUP);
mv.visitMethodInsn(INVOKESPECIAL, "java/util/HashSet", "<init>", "()V", false);
mv.visitVarInsn(ASTORE, 0);

for (Map.Entry<String, Property> entry : ConfigMappingInterface.getProperties(mapping)
.get(mapping.getInterfaceType())
.get("").entrySet()) {
if (entry.getValue().isLeaf()) {
LeafProperty property = entry.getValue().asLeaf();
if (property.isSecret()) {
mv.visitVarInsn(ALOAD, 0);
mv.visitLdcInsn(entry.getKey());
mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Set", "add", "(Ljava/lang/Object;)Z", true);
mv.visitInsn(POP);
}
}

if (entry.getValue().isMap()) {
if (entry.getValue().asMap().getValueProperty().isLeaf()) {
LeafProperty property = entry.getValue().asMap().getValueProperty().asLeaf();
if (property.isSecret()) {
mv.visitVarInsn(ALOAD, 0);
mv.visitLdcInsn(entry.getKey());
mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Set", "add", "(Ljava/lang/Object;)Z", true);
mv.visitInsn(POP);
}
}
}
}

mv.visitVarInsn(ALOAD, 0);
mv.visitInsn(ARETURN);
mv.visitMaxs(0, 0);
mv.visitEnd();
}

private static boolean hasDefaultValue(final Class<?> klass, final Object value) {
if (value == null) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,10 @@ public Class<?> getValueRawType() {
return rawType;
}

public boolean isSecret() {
return Secret.class.isAssignableFrom(rawType);
}

@Override
public boolean isLeaf() {
return true;
Expand Down Expand Up @@ -772,6 +776,11 @@ private static ConfigMappingInterface createConfigurationInterface(Class<?> inte
return null;
}

// TODO - What should we do with this? We want to avoid having to use @WithConverter all the time, but the mapping metadata does not know which converters are available.
if (Secret.class.isAssignableFrom(interfaceType)) {
return null;
}

// first, find any supertypes
ConfigMappingInterface[] superTypes = getSuperTypes(interfaceType.getInterfaces(), 0, 0);
// now find any properties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,25 @@ static <T> Map<String, String> configMappingDefaults(final Class<T> interfaceTyp
}
}

static <T> Set<String> configMappingSecrets(final Class<T> interfaceType) {
try {
Method getSecrets = CACHE.get(interfaceType).getImplementationClass().getDeclaredMethod("getSecrets");
return (Set<String>) getSecrets.invoke(null);
} catch (NoSuchMethodException e) {
throw new NoSuchMethodError(e.getMessage());
} catch (IllegalAccessException e) {
throw new IllegalAccessError(e.getMessage());
} catch (InvocationTargetException e) {
try {
throw e.getCause();
} catch (RuntimeException | Error e2) {
throw e2;
} catch (Throwable t) {
throw new UndeclaredThrowableException(t);
}
}
}

static <T> T configMappingObject(final Class<T> interfaceType, final ConfigMappingContext configMappingContext) {
ConfigMappingObject instance;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ static boolean equals(final String name, final String other) {
return true;
}

if (name.equals("*") && (other.equals("") || other.equals("\"\""))) {
if (name.equals("*") && (other.isEmpty() || other.equals("\"\""))) {
return false;
}

Expand Down
53 changes: 53 additions & 0 deletions implementation/src/main/java/io/smallrye/config/Secret.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package io.smallrye.config;

import java.util.Arrays;

import javax.security.auth.Destroyable;

import org.eclipse.microprofile.config.spi.Converter;

public interface Secret extends Destroyable {

char[] get();

// TODO - Does it make sense to have a method like this?
default char[] getAndDestroy() {
throw new UnsupportedOperationException();
}

class PlainSecret implements Secret {
private char[] secret;

public PlainSecret(final char[] secret) {
this.secret = secret;
}

@Override
public char[] get() {
return secret.clone();
}

@Override
public void destroy() {
final char[] secret = this.secret;
this.secret = null;
if (secret != null) {
Arrays.fill(secret, '\0');
}
}

@Override
public boolean isDestroyed() {
return secret == null;
}
}

final class SecretConverter implements Converter<Secret> {
private static final long serialVersionUID = 3586679829566742841L;

@Override
public Secret convert(final String value) throws IllegalArgumentException, NullPointerException {
return new PlainSecret(value.toCharArray());
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.smallrye.config;

import static java.util.stream.Collectors.toSet;

import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
Expand All @@ -12,15 +14,15 @@
public class SecretKeysConfigSourceInterceptor implements ConfigSourceInterceptor {
private static final long serialVersionUID = 7291982039729980590L;

private final Set<String> secrets;
private final Set<PropertyName> secrets;

public SecretKeysConfigSourceInterceptor(final Set<String> secrets) {
this.secrets = secrets;
this.secrets = secrets.stream().map(PropertyName::new).collect(toSet());
}

@Override
public ConfigValue getValue(final ConfigSourceInterceptorContext context, final String name) {
if (SecretKeys.isLocked() && secrets.contains(name)) {
if (SecretKeys.isLocked() && secrets.contains(new PropertyName(name))) {
throw ConfigMessages.msg.notAllowed(name);
}
return context.proceed(name);
Expand All @@ -33,7 +35,7 @@ public Iterator<String> iterateNames(final ConfigSourceInterceptorContext contex
Iterator<String> namesIterator = context.iterateNames();
while (namesIterator.hasNext()) {
String name = namesIterator.next();
if (!secrets.contains(name)) {
if (!secrets.contains(new PropertyName(name))) {
names.add(name);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ public SmallRyeConfigBuilder withMapping(Class<?> klass) {
}

public SmallRyeConfigBuilder withMapping(Class<?> klass, String prefix) {
mappingsBuilder.mapping(klass, prefix);
mappingsBuilder.mapping(klass, prefix, this);
return this;
}

Expand Down Expand Up @@ -765,7 +765,7 @@ final class MappingBuilder {
private final Map<Class<?>, Set<String>> mappings = new HashMap<>();
private final List<String> ignoredPaths = new ArrayList<>();

MappingBuilder mapping(Class<?> type, String prefix) {
MappingBuilder mapping(Class<?> type, String prefix, final SmallRyeConfigBuilder configBuilder) {
Assert.checkNotNullParam("type", type);
Assert.checkNotNullParam("path", prefix);
Class<?> mappingClass = getConfigMappingClass(type);
Expand All @@ -774,12 +774,16 @@ MappingBuilder mapping(Class<?> type, String prefix) {
ignoredPaths.add(prefix.isEmpty() ? "*" : prefix + ".**");
}
mappings.computeIfAbsent(mappingClass, k -> new HashSet<>(4)).add(prefix);
// Load the mapping defaults, to make the defaults available to all config sources

// Load the mapping defaults, to make the defaults available to all config sources
for (Map.Entry<String, String> defaultEntry : ConfigMappingLoader.configMappingDefaults(mappingClass).entrySet()) {
// Do not override builder defaults with mapping defaults
defaultValues.putIfAbsent(prefix(prefix, defaultEntry.getKey()), defaultEntry.getValue());
}
// Load secret names
for (String secret : ConfigMappingLoader.configMappingSecrets(mappingClass)) {
configBuilder.withSecretKeys(prefix(prefix, secret));
}
return this;
}

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

import static io.smallrye.config.KeyValuesConfigSource.config;
import static java.util.stream.Collectors.toSet;
import static java.util.stream.StreamSupport.stream;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.util.List;
import java.util.Map;
import java.util.Set;

import org.eclipse.microprofile.config.spi.Converter;
import org.junit.jupiter.api.Test;

import io.smallrye.config.Secret.PlainSecret;

class ConfigMappingSecretsTest {
@Test
void secrets() throws Exception {
SmallRyeConfig config = new SmallRyeConfigBuilder()
.addDefaultInterceptors()
// TODO - Converter must be available by default
.withConverter(Secret.class, 100, Converters.newEmptyValueConverter(new Secret.SecretConverter()))
.withConverter(PlainSecret.class, 100, Converters.newEmptyValueConverter(new Converter<>() {
final Secret.SecretConverter secretConverter = new PlainSecret.SecretConverter();

@Override
public PlainSecret convert(final String value) throws IllegalArgumentException, NullPointerException {
return (PlainSecret) secretConverter.convert(value);
}
}))
.withMapping(MappingSecrets.class)
.withSources(config(
"secrets.secret", "secret",
"secrets.plain-secret", "plain",
"secrets.secrets[0]", "secret",
"secrets.secret-map.key", "secret"))
.withSecretKeys()
.build();

MappingSecrets mapping = config.getConfigMapping(MappingSecrets.class);
assertArrayEquals("secret".toCharArray(), mapping.secret().get());
assertArrayEquals("plain".toCharArray(), mapping.plainSecret().get());
assertArrayEquals("secret".toCharArray(), mapping.secrets().get(0).get());
assertArrayEquals("secret".toCharArray(), mapping.secretMap().get("key").get());

assertThrows(SecurityException.class, () -> config.getRawValue("secrets.secret"));
assertThrows(SecurityException.class, () -> config.getRawValue("secrets.plain-secret"));
assertThrows(SecurityException.class, () -> config.getRawValue("secrets.secrets[0]"));
assertThrows(SecurityException.class, () -> config.getRawValue("secrets.secret-map.key"));

// TODO - getPropertyNames is cached and is first accessed by mappings with secrets to populate. We may have to clear the cache after mapping is done
Set<String> properties = stream(config.getPropertyNames().spliterator(), false).collect(toSet());

properties = stream(config.getLatestPropertyNames().spliterator(), false).collect(toSet());
assertFalse(properties.contains("secrets.secret"));
assertFalse(properties.contains("secrets.plain-secret"));
assertFalse(properties.contains("secrets.secrets[0]"));
assertFalse(properties.contains("secrets.secret-map.key"));
}

@ConfigMapping(prefix = "secrets")
interface MappingSecrets {
Secret secret();

PlainSecret plainSecret();

List<Secret> secrets();

Map<String, Secret> secretMap();
}
}

0 comments on commit 0122acb

Please sign in to comment.