Apply ConfigValueConverter to Mirroring configuration temporarily (#895)

Motivation: Before we support content encryption within CentralDogma, we need a way to secure sensitive information in mirroring configuration. We can do this using `ConfigValueConverter` that is introduced via #890 as a temporary workaround. Modifications: - Apply `ConfigValueConverter` to mirroring configuration. Result: - You can temporarily hide sensitive information in mirroring configuration using `ConfigValueConverter`. Please note that this feature will be deprecated after we implement #755.
line · Nov 23, 2023 · c0a6025 · c0a6025
1 parent 9f5ce68
commit c0a6025
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 38 deletions.
diff --git a/server/src/main/java/com/linecorp/centraldogma/server/DefaultConfigValueConverter.java b/server/src/main/java/com/linecorp/centraldogma/server/DefaultConfigValueConverter.java
@@ -19,6 +19,7 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Paths;
+import java.util.Base64;
 import java.util.List;
 
 import com.google.common.collect.ImmutableList;
@@ -28,9 +29,10 @@ enum DefaultConfigValueConverter implements ConfigValueConverter {
 
     private static final String PLAINTEXT = "plaintext";
     private static final String FILE = "file";
+    private static final String BASE64 = "base64";
 
     // TODO(minwoox): Add more prefixes such as classpath, url, etc.
-    private static final List<String> SUPPORTED_PREFIXES = ImmutableList.of(PLAINTEXT, FILE);
+    private static final List<String> SUPPORTED_PREFIXES = ImmutableList.of(PLAINTEXT, FILE, BASE64);
 
     @Override
     public List<String> supportedPrefixes() {
@@ -48,6 +50,8 @@ public String convert(String prefix, String value) {
                 } catch (IOException e) {
                     throw new RuntimeException("failed to read a file: " + value, e);
                 }
+            case BASE64:
+                return new String(Base64.getDecoder().decode(value), StandardCharsets.UTF_8).trim();
             default:
                 // Should never reach here.
                 throw new Error();

diff --git a/.../linecorp/centraldogma/server/internal/mirror/credential/AccessTokenMirrorCredential.java b/.../linecorp/centraldogma/server/internal/mirror/credential/AccessTokenMirrorCredential.java
@@ -16,19 +16,25 @@
 
 package com.linecorp.centraldogma.server.internal.mirror.credential;
 
+import static com.linecorp.centraldogma.server.CentralDogmaConfig.convertValue;
 import static com.linecorp.centraldogma.server.internal.mirror.credential.MirrorCredentialUtil.requireNonEmpty;
 
 import java.util.regex.Pattern;
 
 import javax.annotation.Nullable;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.google.common.base.MoreObjects.ToStringHelper;
 
 public final class AccessTokenMirrorCredential extends AbstractMirrorCredential {
 
+    private static final Logger logger = LoggerFactory.getLogger(AccessTokenMirrorCredential.class);
+
     private final String accessToken;
 
     @JsonCreator
@@ -38,12 +44,17 @@ public AccessTokenMirrorCredential(@JsonProperty("id") @Nullable String id,
                                        Iterable<Pattern> hostnamePatterns,
                                        @JsonProperty("accessToken") String accessToken) {
         super(id, hostnamePatterns);
-
         this.accessToken = requireNonEmpty(accessToken, "accessToken");
     }
 
     public String accessToken() {
-        return accessToken;
+        try {
+            return convertValue(accessToken, "credentials.accessToken");
+        } catch (Throwable t) {
+            // The accessToken probably has `:` without prefix. Just return it as is for backward compatibility.
+            logger.debug("Failed to convert the access token of the credential: {}", id(), t);
+            return accessToken;
+        }
     }
 
     @Override

diff --git a/...ava/com/linecorp/centraldogma/server/internal/mirror/credential/MirrorCredentialUtil.java b/...ava/com/linecorp/centraldogma/server/internal/mirror/credential/MirrorCredentialUtil.java
@@ -18,30 +18,7 @@
 
 import static java.util.Objects.requireNonNull;
 
-import java.nio.charset.StandardCharsets;
-import java.util.Base64;
-
-import javax.annotation.Nullable;
-
 final class MirrorCredentialUtil {
-    private static final String BASE64_PREFIX = "base64:";
-
-    static byte[] decodeBase64(String value, String name) {
-        requireNonNull(value, name);
-        return Base64.getDecoder().decode(value);
-    }
-
-    @Nullable
-    static String maybeDecodeBase64(@Nullable String value, String name) {
-        if (value == null) {
-            return null;
-        }
-        if (value.startsWith(BASE64_PREFIX)) {
-            return new String(decodeBase64(value.substring(BASE64_PREFIX.length()), name),
-                              StandardCharsets.UTF_8);
-        }
-        return value;
-    }
 
     static String requireNonEmpty(String value, String name) {
         requireNonNull(value, name);

diff --git a/...com/linecorp/centraldogma/server/internal/mirror/credential/PasswordMirrorCredential.java b/...com/linecorp/centraldogma/server/internal/mirror/credential/PasswordMirrorCredential.java
@@ -16,20 +16,26 @@
 
 package com.linecorp.centraldogma.server.internal.mirror.credential;
 
+import static com.linecorp.centraldogma.server.CentralDogmaConfig.convertValue;
 import static com.linecorp.centraldogma.server.internal.mirror.credential.MirrorCredentialUtil.requireNonEmpty;
 import static java.util.Objects.requireNonNull;
 
 import java.util.regex.Pattern;
 
 import javax.annotation.Nullable;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.google.common.base.MoreObjects.ToStringHelper;
 
 public final class PasswordMirrorCredential extends AbstractMirrorCredential {
 
+    private static final Logger logger = LoggerFactory.getLogger(PasswordMirrorCredential.class);
+
     private final String username;
     private final String password;
 
@@ -51,7 +57,14 @@ public String username() {
     }
 
     public String password() {
-        return password;
+        try {
+            return convertValue(password, "credentials.password");
+        } catch (Throwable t) {
+            // The password probably has `:` without prefix. Just return it as is for backward compatibility.
+            logger.debug("Failed to convert the password of the credential. username: {}, id: {}",
+                         username, id(), t);
+            return password;
+        }
     }
 
     @Override

diff --git a/...om/linecorp/centraldogma/server/internal/mirror/credential/PublicKeyMirrorCredential.java b/...om/linecorp/centraldogma/server/internal/mirror/credential/PublicKeyMirrorCredential.java
@@ -16,7 +16,7 @@
 
 package com.linecorp.centraldogma.server.internal.mirror.credential;
 
-import static com.linecorp.centraldogma.server.internal.mirror.credential.MirrorCredentialUtil.maybeDecodeBase64;
+import static com.linecorp.centraldogma.server.CentralDogmaConfig.convertValue;
 import static com.linecorp.centraldogma.server.internal.mirror.credential.MirrorCredentialUtil.requireNonEmpty;
 
 import java.util.List;
@@ -25,6 +25,9 @@
 
 import javax.annotation.Nullable;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
@@ -35,6 +38,8 @@
 
 public final class PublicKeyMirrorCredential extends AbstractMirrorCredential {
 
+    private static final Logger logger = LoggerFactory.getLogger(PublicKeyMirrorCredential.class);
+
     private static final Splitter NEWLINE_SPLITTER = Splitter.on(CharMatcher.anyOf("\n\r"))
                                                              .omitEmptyStrings()
                                                              .trimResults();
@@ -43,7 +48,7 @@ public final class PublicKeyMirrorCredential extends AbstractMirrorCredential {
 
     private final String username;
     private final String publicKey;
-    private final List<String> privateKey;
+    private final String privateKey;
     @Nullable
     private final String passphrase;
 
@@ -61,11 +66,8 @@ public PublicKeyMirrorCredential(@JsonProperty("id") @Nullable String id,
 
         this.username = requireNonEmpty(username, "username");
         this.publicKey = requireNonEmpty(publicKey, "publicKey");
-        requireNonEmpty(privateKey, "privateKey");
-        // privateKey is converted into a list of Strings that will be used as an input of
-        // KeyPairResourceLoader.loadKeyPairs(...)
-        this.privateKey = ImmutableList.copyOf(NEWLINE_SPLITTER.splitToList(privateKey));
-        this.passphrase = maybeDecodeBase64(passphrase, "passphrase");
+        this.privateKey = requireNonEmpty(privateKey, "privateKey");
+        this.passphrase = passphrase;
     }
 
     public String username() {
@@ -77,12 +79,31 @@ public String publicKey() {
     }
 
     public List<String> privateKey() {
-        return privateKey;
+        String converted;
+        try {
+            converted = convertValue(privateKey, "credentials.privateKey");
+        } catch (Throwable t) {
+            // Just use it as is for backward compatibility.
+            logger.debug("Failed to convert the key of the credential. username: {}, id: {}",
+                         username, id(), t);
+            converted = privateKey;
+        }
+        assert converted != null;
+        // privateKey is converted into a list of Strings that will be used as an input of
+        // KeyPairResourceLoader.loadKeyPairs(...)
+        return ImmutableList.copyOf(NEWLINE_SPLITTER.splitToList(converted));
     }
 
     @Nullable
     public String passphrase() {
-        return passphrase;
+        try {
+            return convertValue(passphrase, "credentials.passphrase");
+        } catch (Throwable t) {
+            // The passphrase probably has `:` without prefix. Just return it as is for backward compatibility.
+            logger.debug("Failed to convert the passphrase of the credential. username: {}, id: {}",
+                         username, id(), t);
+            return passphrase;
+        }
     }
 
     @Override

diff --git a/...inecorp/centraldogma/server/internal/mirror/credential/PublicKeyMirrorCredentialTest.java b/...inecorp/centraldogma/server/internal/mirror/credential/PublicKeyMirrorCredentialTest.java
@@ -20,11 +20,15 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import java.util.List;
+
 import org.junit.jupiter.api.Test;
 
 import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableList;
 
 import com.linecorp.centraldogma.internal.Jackson;
+import com.linecorp.centraldogma.server.ConfigValueConverter;
 import com.linecorp.centraldogma.server.mirror.MirrorCredential;
 
 class PublicKeyMirrorCredentialTest {
@@ -129,6 +133,9 @@ void testDeserialization() throws Exception {
                                                          PUBLIC_KEY, PRIVATE_KEY, PASSPHRASE));
 
         // base64 passphrase
+        final PublicKeyMirrorCredential base64Expected =
+                new PublicKeyMirrorCredential(null, HOSTNAME_PATTERNS, USERNAME,
+                                              PUBLIC_KEY, PRIVATE_KEY, PASSPHRASE_BASE64);
         assertThat(Jackson.readValue('{' +
                                      "  \"type\": \"public_key\"," +
                                      "  \"hostnamePatterns\": [" +
@@ -139,8 +146,8 @@ void testDeserialization() throws Exception {
                                      "  \"privateKey\": \"" + Jackson.escapeText(PRIVATE_KEY) + "\"," +
                                      "  \"passphrase\": \"" + Jackson.escapeText(PASSPHRASE_BASE64) + '"' +
                                      '}', MirrorCredential.class))
-                .isEqualTo(new PublicKeyMirrorCredential(null, HOSTNAME_PATTERNS, USERNAME,
-                                                         PUBLIC_KEY, PRIVATE_KEY, PASSPHRASE));
+                .isEqualTo(base64Expected);
+        assertThat(base64Expected.passphrase()).isEqualTo(PASSPHRASE);
 
         // ID
         assertThat(Jackson.readValue('{' +
@@ -153,5 +160,35 @@ void testDeserialization() throws Exception {
                                      '}', MirrorCredential.class))
                 .isEqualTo(new PublicKeyMirrorCredential("foo", null, USERNAME,
                                                          PUBLIC_KEY, PRIVATE_KEY, PASSPHRASE));
+
+        final PublicKeyMirrorCredential conveterExpected =
+                new PublicKeyMirrorCredential("foo", null, USERNAME,
+                                              PUBLIC_KEY, PRIVATE_KEY, "mirror_encryption:foo");
+        assertThat(Jackson.readValue('{' +
+                                     "  \"type\": \"public_key\"," +
+                                     "  \"id\": \"foo\"," +
+                                     "  \"username\": \"trustin\"," +
+                                     "  \"publicKey\": \"" + Jackson.escapeText(PUBLIC_KEY) + "\"," +
+                                     "  \"privateKey\": \"" + Jackson.escapeText(PRIVATE_KEY) + "\"," +
+                                     "  \"passphrase\": \"mirror_encryption:foo\"" +
+                                     '}', MirrorCredential.class))
+                .isEqualTo(conveterExpected);
+        assertThat(conveterExpected.passphrase()).isEqualTo("bar");
+    }
+
+    public static class PasswordConfigValueConverter implements ConfigValueConverter {
+
+        @Override
+        public List<String> supportedPrefixes() {
+            return ImmutableList.of("mirror_encryption");
+        }
+
+        @Override
+        public String convert(String prefix, String value) {
+            if ("foo".equals(value)) {
+                return "bar";
+            }
+            throw new IllegalArgumentException("unsupported prefix: " + prefix + ", value: " + value);
+        }
     }
 }
diff --git a/...rc/test/resources/META-INF/services/com.linecorp.centraldogma.server.ConfigValueConverter b/...rc/test/resources/META-INF/services/com.linecorp.centraldogma.server.ConfigValueConverter
@@ -1 +1,2 @@
 com.linecorp.centraldogma.server.ConfigDeserializationTest$KeyConfigValueConverter
+com.linecorp.centraldogma.server.internal.mirror.credential.PublicKeyMirrorCredentialTest$PasswordConfigValueConverter