From 866ae3180ac16edfff63f2dbed09482aed344738 Mon Sep 17 00:00:00 2001 From: Roberto Cortez Date: Wed, 2 Oct 2024 13:58:00 +0100 Subject: [PATCH] Secret type to represent secrets and integrate with the secrets handling --- .../config/ConfigMappingGenerator.java | 43 +++++++++++++ .../config/ConfigMappingInterface.java | 9 +++ .../smallrye/config/ConfigMappingLoader.java | 19 ++++++ .../java/io/smallrye/config/PropertyName.java | 2 +- .../main/java/io/smallrye/config/Secret.java | 26 ++++++++ .../SecretKeysConfigSourceInterceptor.java | 10 +-- .../io/smallrye/config/SmallRyeConfig.java | 6 ++ .../config/SmallRyeConfigBuilder.java | 10 ++- .../config/ConfigMappingSecretsTest.java | 63 +++++++++++++++++++ 9 files changed, 180 insertions(+), 8 deletions(-) create mode 100644 implementation/src/main/java/io/smallrye/config/Secret.java create mode 100644 implementation/src/test/java/io/smallrye/config/ConfigMappingSecretsTest.java 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..cde544024 --- /dev/null +++ b/implementation/src/main/java/io/smallrye/config/Secret.java @@ -0,0 +1,26 @@ +package io.smallrye.config; + +import org.eclipse.microprofile.config.spi.Converter; + +public interface Secret { + T get(); + + class SecretConverter implements Converter> { + private static final long serialVersionUID = -4624156385855243648L; + private final Converter delegate; + + public SecretConverter(final Converter delegate) { + this.delegate = delegate; + } + + @Override + public Secret convert(final String value) throws IllegalArgumentException, NullPointerException { + return new Secret() { + @Override + public T get() { + return delegate.convert(value); + } + }; + } + } +} 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/SmallRyeConfig.java b/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java index 1a4b2a56a..4ead4318c 100644 --- a/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java +++ b/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java @@ -129,6 +129,8 @@ public ConfigMappingContext get() { return new ConfigMappingContext(SmallRyeConfig.this, mappingsBuilder.getMappings()); } }); + // We clear the properties because they may contain secrets + this.configSources.getPropertyNames().clear(); if (getOptionalValue(SMALLRYE_CONFIG_MAPPING_VALIDATE_UNKNOWN, boolean.class).orElse(true)) { context.reportUnknown(mappingsBuilder.getIgnoredPaths()); @@ -1059,6 +1061,10 @@ Iterable latest() { names.remove(ConfigSource.CONFIG_ORDINAL); return Collections.unmodifiableSet(names); } + + void clear() { + names.clear(); + } } } 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..636839fa7 --- /dev/null +++ b/implementation/src/test/java/io/smallrye/config/ConfigMappingSecretsTest.java @@ -0,0 +1,63 @@ +package io.smallrye.config; + +import static io.smallrye.config.Converters.STRING_CONVERTER; +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.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +import org.junit.jupiter.api.Test; + +class ConfigMappingSecretsTest { + @Test + void secrets() throws Exception { + SmallRyeConfig config = new SmallRyeConfigBuilder() + .addDefaultInterceptors() + .withConverter(Secret.class, 100, new Secret.SecretConverter(STRING_CONVERTER)) + .withMapping(MappingSecrets.class) + .withSources(config( + "secrets.secret", "secret", + "secrets.list[0]", "secret", + "secrets.map.key", "secret", + "secrets.optional", "secret")) + .withSecretKeys() + .build(); + + MappingSecrets mapping = config.getConfigMapping(MappingSecrets.class); + assertEquals("secret", mapping.secret().get()); + assertEquals("secret", mapping.list().get(0).get()); + assertEquals("secret", mapping.map().get("key").get()); + assertTrue(mapping.optional().isPresent()); + assertEquals("secret", mapping.optional().get().get()); + + assertThrows(SecurityException.class, () -> config.getRawValue("secrets.secret")); + assertThrows(SecurityException.class, () -> config.getRawValue("secrets.list[0]")); + assertThrows(SecurityException.class, () -> config.getRawValue("secrets.map.key")); + assertThrows(SecurityException.class, () -> config.getRawValue("secrets.optional")); + + Set properties = stream(config.getPropertyNames().spliterator(), false).collect(toSet()); + assertFalse(properties.contains("secrets.secret")); + assertFalse(properties.contains("secrets.secrets[0]")); + assertFalse(properties.contains("secrets.secret-map.key")); + assertFalse(properties.contains("secrets.optional")); + } + + @ConfigMapping(prefix = "secrets") + interface MappingSecrets { + Secret secret(); + + List> list(); + + Map> map(); + + Optional> optional(); + } +}