diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMappingGenerator.java b/implementation/src/main/java/io/smallrye/config/ConfigMappingGenerator.java index 7992604d4..a5fa954f9 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigMappingGenerator.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigMappingGenerator.java @@ -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(); } @@ -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;", + null); + + mv.visitTypeInsn(NEW, "java/util/HashSet"); + mv.visitInsn(DUP); + mv.visitMethodInsn(INVOKESPECIAL, "java/util/HashSet", "", "()V", false); + mv.visitVarInsn(ASTORE, 0); + + for (Map.Entry 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; diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMappingInterface.java b/implementation/src/main/java/io/smallrye/config/ConfigMappingInterface.java index 7edc1a368..16050b13c 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigMappingInterface.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigMappingInterface.java @@ -581,6 +581,10 @@ public Class getValueRawType() { return rawType; } + public boolean isSecret() { + return Secret.class.isAssignableFrom(rawType); + } + @Override public boolean isLeaf() { return true; @@ -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 diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMappingLoader.java b/implementation/src/main/java/io/smallrye/config/ConfigMappingLoader.java index da2cd1425..bf451868b 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigMappingLoader.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigMappingLoader.java @@ -102,6 +102,25 @@ static Map configMappingDefaults(final Class interfaceTyp } } + static Set configMappingSecrets(final Class interfaceType) { + try { + Method getSecrets = CACHE.get(interfaceType).getImplementationClass().getDeclaredMethod("getSecrets"); + return (Set) 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 configMappingObject(final Class interfaceType, final ConfigMappingContext configMappingContext) { ConfigMappingObject instance; try { diff --git a/implementation/src/main/java/io/smallrye/config/PropertyName.java b/implementation/src/main/java/io/smallrye/config/PropertyName.java index f51a3d5df..a7aafee2a 100644 --- a/implementation/src/main/java/io/smallrye/config/PropertyName.java +++ b/implementation/src/main/java/io/smallrye/config/PropertyName.java @@ -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; } diff --git a/implementation/src/main/java/io/smallrye/config/Secret.java b/implementation/src/main/java/io/smallrye/config/Secret.java new file mode 100644 index 000000000..7b1800ea7 --- /dev/null +++ b/implementation/src/main/java/io/smallrye/config/Secret.java @@ -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 { + private static final long serialVersionUID = 3586679829566742841L; + + @Override + public Secret convert(final String value) throws IllegalArgumentException, NullPointerException { + return new PlainSecret(value.toCharArray()); + } + } +} diff --git a/implementation/src/main/java/io/smallrye/config/SecretKeysConfigSourceInterceptor.java b/implementation/src/main/java/io/smallrye/config/SecretKeysConfigSourceInterceptor.java index b2e2e69db..e1800ebc7 100644 --- a/implementation/src/main/java/io/smallrye/config/SecretKeysConfigSourceInterceptor.java +++ b/implementation/src/main/java/io/smallrye/config/SecretKeysConfigSourceInterceptor.java @@ -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; @@ -12,15 +14,15 @@ public class SecretKeysConfigSourceInterceptor implements ConfigSourceInterceptor { private static final long serialVersionUID = 7291982039729980590L; - private final Set secrets; + private final Set secrets; public SecretKeysConfigSourceInterceptor(final Set 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); @@ -33,7 +35,7 @@ public Iterator iterateNames(final ConfigSourceInterceptorContext contex Iterator namesIterator = context.iterateNames(); while (namesIterator.hasNext()) { String name = namesIterator.next(); - if (!secrets.contains(name)) { + if (!secrets.contains(new PropertyName(name))) { names.add(name); } } diff --git a/implementation/src/main/java/io/smallrye/config/SmallRyeConfigBuilder.java b/implementation/src/main/java/io/smallrye/config/SmallRyeConfigBuilder.java index a155c1a19..6a25b8000 100644 --- a/implementation/src/main/java/io/smallrye/config/SmallRyeConfigBuilder.java +++ b/implementation/src/main/java/io/smallrye/config/SmallRyeConfigBuilder.java @@ -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; } @@ -765,7 +765,7 @@ final class MappingBuilder { private final Map, Set> mappings = new HashMap<>(); private final List 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); @@ -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 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; } diff --git a/implementation/src/test/java/io/smallrye/config/ConfigMappingSecretsTest.java b/implementation/src/test/java/io/smallrye/config/ConfigMappingSecretsTest.java new file mode 100644 index 000000000..a354e986b --- /dev/null +++ b/implementation/src/test/java/io/smallrye/config/ConfigMappingSecretsTest.java @@ -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 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 secrets(); + + Map secretMap(); + } +}