Secret type to represent secrets and integrate with the secrets handling

smallrye · Oct 4, 2024 · 866ae31 · 866ae31
1 parent 5c88f7b
commit 866ae31
Show file tree

Hide file tree

Showing 9 changed files with 180 additions and 8 deletions.
diff --git 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<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;

diff --git 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
@@ -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 {

diff --git 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
@@ -0,0 +1,26 @@
+package io.smallrye.config;
+
+import org.eclipse.microprofile.config.spi.Converter;
+
+public interface Secret<T> {
+    T get();
+
+    class SecretConverter<T> implements Converter<Secret<T>> {
+        private static final long serialVersionUID = -4624156385855243648L;
+        private final Converter<T> delegate;
+
+        public SecretConverter(final Converter<T> delegate) {
+            this.delegate = delegate;
+        }
+
+        @Override
+        public Secret<T> convert(final String value) throws IllegalArgumentException, NullPointerException {
+            return new Secret<T>() {
+                @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
@@ -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<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);
@@ -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);
                 }
             }

diff --git 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<String> 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
@@ -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<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);
@@ -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;
         }
 

diff --git a/implementation/src/test/java/io/smallrye/config/ConfigMappingSecretsTest.java 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<String> 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<String> secret();
+
+        List<Secret<String>> list();
+
+        Map<String, Secret<String>> map();
+
+        Optional<Secret<String>> optional();
+    }
+}