Don't return secrets in the API & Only update credentials when requested

airbyte-api/src/main/openapi/config.yaml

@@ -1135,6 +1135,8 @@ components:
           $ref: "#/components/schemas/SourceId"
         connectionConfiguration:
           $ref: "#/components/schemas/SourceConfiguration"
+        updateConfigurationSecrets:
+          type: boolean
         name:
           type: string
     SourceRead:
@@ -1302,6 +1304,8 @@ components:
           $ref: "#/components/schemas/DestinationConfiguration"
         name:
           type: string
+        updateConfigurationSecrets:
+          type: boolean
     DestinationRead:
       type: object
       required:

airbyte-commons/src/main/java/io/airbyte/commons/json/JsonSecretsProcessor.java

@@ -0,0 +1,100 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2020 Airbyte
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package io.airbyte.commons.json;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.base.Preconditions;
+
+public class JsonSecretsProcessor {
+
+  public static String AIRBYTE_SECRET_FIELD = "airbyte_secret";
+  private static String PROPERTIES_FIELD = "properties";
+
+  /**
+   * Returns a copy of the input object wherein any fields annotated with "airbyte_secret" in the
+   * input schema are removed.
+   * <p>
+   * TODO this method only removes secrets at the top level of the configuration object. It does not support the keywords anyOf, allOf, oneOf, not, and
+   * dependencies. This will be fixed in the future.
+   *
+   * @param schema Schema containing secret annotations
+   * @param obj    Object containing potentially secret fields
+   * @return
+   */
+  public JsonNode removeSecrets(JsonNode obj, JsonNode schema) {
+    assertValidSchema(schema);
+    Preconditions.checkArgument(schema.isObject());
+
+    ObjectNode properties = (ObjectNode) schema.get(PROPERTIES_FIELD);
+    JsonNode copy = obj.deepCopy();
+    for (String key : Jsons.keys(properties)) {
+      if (isSecret(properties.get(key))) {
+        ((ObjectNode) copy).remove(key);
+      }
+    }
+
+    return copy;
+  }
+
+  /**
+   * Returns a copy of the destination object in which any secret fields (as denoted by the input
+   * schema) found in the source object are added.
+   * <p>
+   * TODO this method only absorbs secrets at the top level of the configuration object. It does not support the keywords anyOf, allOf, oneOf, not, and
+   * dependencies. This will be fixed in the future.
+   *
+   * @param src    The object potentially containing secrets
+   * @param dst    The object to absorb secrets into
+   * @param schema
+   * @return
+   */
+  public JsonNode copySecrets(JsonNode src, JsonNode dst, JsonNode schema) {
+    assertValidSchema(schema);
+    Preconditions.checkArgument(dst.isObject());
+    Preconditions.checkArgument(src.isObject());
+
+    ObjectNode dstCopy = dst.deepCopy();
+
+    ObjectNode properties = (ObjectNode) schema.get(PROPERTIES_FIELD);
+    for (String key : Jsons.keys(properties)) {
+      if (isSecret(properties.get(key)) && src.has(key)) {
+        dstCopy.set(key, src.get(key));
+      }
+    }
+
+    return dstCopy;
+  }
+
+  private static boolean isSecret(JsonNode obj) {
+    return obj.isObject() && obj.has(AIRBYTE_SECRET_FIELD) && obj.get(AIRBYTE_SECRET_FIELD).asBoolean();
+  }
+
+  private static void assertValidSchema(JsonNode node) {
+    Preconditions.checkArgument(node.isObject());
+    Preconditions.checkArgument(node.has(PROPERTIES_FIELD), "Schema object must have a properties field");
+    Preconditions.checkArgument(node.get(PROPERTIES_FIELD).isObject(), "Properties field must be a JSON object");
+  }
+}

airbyte-commons/src/test/java/io/airbyte/commons/json/JsonSecretsProcessorTest.java

@@ -0,0 +1,93 @@
+package io.airbyte.commons.json;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.ImmutableMap;
+import io.airbyte.commons.resources.MoreResources;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+public class JsonSecretsProcessorTest {
+
+  JsonSecretsProcessor processor = new JsonSecretsProcessor();
+
+  @Test
+  public void testRemoveSecrets() throws IOException {
+    JsonNode obj = Jsons.jsonNode(ImmutableMap.builder()
+        .put("field1", "value1")
+        .put("field2", 2)
+        .put("secret1", "donttellanyone")
+        .put("secret2", 12345).build());
+    JsonNode schema = Jsons.deserialize(MoreResources.readResource("secrets_json_schema.json"));
+
+    JsonNode sanitized = processor.removeSecrets(obj, schema);
+
+    JsonNode expected = Jsons.jsonNode(ImmutableMap.of("field1", "value1", "field2", 2));
+    assertEquals(expected, sanitized);
+  }
+
+  @Test
+  public void testRemoveSecretsNotInObj() throws IOException {
+    JsonNode schema = Jsons.deserialize(MoreResources.readResource("secrets_json_schema.json"));
+    JsonNode obj = Jsons.jsonNode(ImmutableMap.builder()
+        .put("field1", "value1")
+        .put("field2", 2).build());
+
+    JsonNode actual = processor.removeSecrets(obj, schema);
+
+    // Didn't have secrets, no fields should have been impacted.
+    assertEquals(obj, actual);
+  }
+
+  @Test
+  public void testCopySecrets() throws IOException {
+    JsonNode src = Jsons.jsonNode(ImmutableMap.builder()
+        .put("field1", "value1")
+        .put("field2", 2)
+        .put("additional_field", "dont_copy_me")
+        .put("secret1", "donttellanyone")
+        .put("secret2", 12345)
+        .build());
+
+    JsonNode dst = Jsons.jsonNode(ImmutableMap.builder()
+        .put("field1", "value1")
+        .put("field2", 2)
+        .build());
+
+    JsonNode schema = Jsons.deserialize(MoreResources.readResource("secrets_json_schema.json"));
+
+    JsonNode actual = processor.copySecrets(src, dst, schema);
+
+    JsonNode expected = Jsons.jsonNode(ImmutableMap.builder()
+        .put("field1", "value1")
+        .put("field2", 2)
+        .put("secret1", "donttellanyone")
+        .put("secret2", 12345)
+        .build());
+
+    assertEquals(expected, actual);
+  }
+
+  @Test
+  public void testCopySecretsNotInSrc() throws IOException {
+    JsonNode schema = Jsons.deserialize(MoreResources.readResource("secrets_json_schema.json"));
+    JsonNode src = Jsons.jsonNode(ImmutableMap.builder()
+        .put("field1", "value1")
+        .put("field2", 2)
+        .put("additional_field", "dont_copy_me")
+        .build());
+
+    JsonNode dst = Jsons.jsonNode(ImmutableMap.builder()
+        .put("field1", "value1")
+        .put("field2", 2)
+        .build());
+
+    JsonNode expected = dst.deepCopy();
+    JsonNode actual = processor.copySecrets(src, dst, schema);
+
+    assertEquals(expected, actual);
+  }
+
+}

airbyte-commons/src/test/resources/secrets_json_schema.json

@@ -0,0 +1,18 @@
+{
+  "properties": {
+    "secret1": {
+      "type": "string",
+      "airbyte_secret": true
+    },
+    "secret2": {
+      "type": "number",
+      "airbyte_secret": "true"
+    },
+    "field1": {
+      "type": "string"
+    },
+    "field2": {
+      "type": "number"
+    }
+  }
+}

airbyte-integrations/connectors/source-shopify-singer/unit_tests/unit_test.py

@@ -22,6 +22,5 @@
 SOFTWARE.
 """
 
-
 def test_example_method():
     assert True

airbyte-server/src/main/java/io/airbyte/server/handlers/DestinationHandler.java

@@ -34,13 +34,16 @@
 import io.airbyte.api.model.DestinationRead;
 import io.airbyte.api.model.DestinationReadList;
 import io.airbyte.api.model.DestinationUpdate;
+import io.airbyte.api.model.SourceDefinitionSpecificationRead;
 import io.airbyte.api.model.WorkspaceIdRequestBody;
+import io.airbyte.commons.json.JsonSecretsProcessor;
 import io.airbyte.config.DestinationConnection;
 import io.airbyte.config.StandardDestinationDefinition;
 import io.airbyte.config.persistence.ConfigNotFoundException;
 import io.airbyte.config.persistence.ConfigRepository;
 import io.airbyte.validation.json.JsonSchemaValidator;
 import io.airbyte.validation.json.JsonValidationException;
+
 import java.io.IOException;
 import java.util.List;
 import java.util.UUID;
@@ -53,6 +56,7 @@ public class DestinationHandler {
   private final Supplier<UUID> uuidGenerator;
   private final ConfigRepository configRepository;
   private final JsonSchemaValidator validator;
+  private final JsonSecretsProcessor secretProcessor;
 
   public DestinationHandler(final ConfigRepository configRepository,
                             final JsonSchemaValidator integrationSchemaValidation,
@@ -64,6 +68,7 @@ public DestinationHandler(final ConfigRepository configRepository,
     this.schedulerHandler = schedulerHandler;
     this.connectionsHandler = connectionsHandler;
     this.uuidGenerator = uuidGenerator;
+    this.secretProcessor = new JsonSecretsProcessor();
   }
 
   public DestinationHandler(final ConfigRepository configRepository,
@@ -76,9 +81,8 @@ public DestinationHandler(final ConfigRepository configRepository,
   public DestinationRead createDestination(final DestinationCreate destinationCreate)
       throws ConfigNotFoundException, IOException, JsonValidationException {
     // validate configuration
-    validateDestination(
-        destinationCreate.getDestinationDefinitionId(),
-        destinationCreate.getConnectionConfiguration());
+    DestinationDefinitionSpecificationRead spec = getSpec(destinationCreate.getDestinationDefinitionId());
+    validateDestination(spec, destinationCreate.getConnectionConfiguration());
 
     // persist
     final UUID destinationId = uuidGenerator.get();
@@ -91,7 +95,7 @@ public DestinationRead createDestination(final DestinationCreate destinationCrea
         false);
 
     // read configuration from db
-    return buildDestinationRead(destinationId);
+    return buildDestinationRead(destinationId, spec);
   }
 
   public void deleteDestination(final DestinationIdRequestBody destinationIdRequestBody)
@@ -127,10 +131,17 @@ public DestinationRead updateDestination(final DestinationUpdate destinationUpda
     final DestinationConnection currentDci =
         configRepository.getDestinationConnection(destinationUpdate.getDestinationId());
 
+    final DestinationDefinitionSpecificationRead spec = getSpec(currentDci.getDestinationDefinitionId());
+
+    // if secrets are not being updated, copy them from the existing configuration
+    if (destinationUpdate.getUpdateConfigurationSecrets() == null || !destinationUpdate.getUpdateConfigurationSecrets()) {
+      JsonNode updateConfigurationWithSecrets = secretProcessor
+          .copySecrets(currentDci.getConfiguration(), destinationUpdate.getConnectionConfiguration(), spec.getConnectionSpecification());
+      destinationUpdate.setConnectionConfiguration(updateConfigurationWithSecrets);
+    }
+
     // validate configuration
-    validateDestination(
-        currentDci.getDestinationDefinitionId(),
-        destinationUpdate.getConnectionConfiguration());
+    validateDestination(spec, destinationUpdate.getConnectionConfiguration());
 
     // persist
     persistDestinationConnection(
@@ -142,7 +153,7 @@ public DestinationRead updateDestination(final DestinationUpdate destinationUpda
         currentDci.getTombstone());
 
     // read configuration from db
-    return buildDestinationRead(destinationUpdate.getDestinationId());
+    return buildDestinationRead(destinationUpdate.getDestinationId(), spec);
   }
 
   public DestinationRead getDestination(DestinationIdRequestBody destinationIdRequestBody)
@@ -169,12 +180,14 @@ public DestinationReadList listDestinationsForWorkspace(WorkspaceIdRequestBody w
     return new DestinationReadList().destinations(reads);
   }
 
-  private void validateDestination(final UUID destinationId,
-                                   final JsonNode implementationJson)
+  private void validateDestination(final DestinationDefinitionSpecificationRead spec,
+                                   final JsonNode configuration) throws JsonValidationException {
+    validator.ensure(spec.getConnectionSpecification(), configuration);
+  }
+
+  private DestinationDefinitionSpecificationRead getSpec(UUID destinationDefinitionId)
       throws JsonValidationException, IOException, ConfigNotFoundException {
-    DestinationDefinitionSpecificationRead dcs =
-        schedulerHandler.getDestinationSpecification(new DestinationDefinitionIdRequestBody().destinationDefinitionId(destinationId));
-    validator.ensure(dcs.getConnectionSpecification(), implementationJson);
+    return schedulerHandler.getDestinationSpecification(new DestinationDefinitionIdRequestBody().destinationDefinitionId(destinationDefinitionId));
   }
 
   private void persistDestinationConnection(final String name,
@@ -195,10 +208,18 @@ private void persistDestinationConnection(final String name,
     configRepository.writeDestinationConnection(destinationConnection);
   }
 
-  private DestinationRead buildDestinationRead(final UUID destinationId)
+  private DestinationRead buildDestinationRead(final UUID destinationId) throws JsonValidationException, IOException, ConfigNotFoundException {
+    DestinationDefinitionSpecificationRead spec = getSpec(configRepository.getDestinationConnection(destinationId).getDestinationDefinitionId());
+    return buildDestinationRead(destinationId, spec);
+  }
+
+  private DestinationRead buildDestinationRead(final UUID destinationId, DestinationDefinitionSpecificationRead spec)
       throws ConfigNotFoundException, IOException, JsonValidationException {
-    // read configuration from db
+
+    // remove secrets from config before returning the read
     final DestinationConnection dci = configRepository.getDestinationConnection(destinationId);
+    dci.setConfiguration(secretProcessor.removeSecrets(dci.getConfiguration(), spec.getConnectionSpecification()));
+
     final StandardDestinationDefinition standardDestinationDefinition =
         configRepository.getStandardDestinationDefinition(dci.getDestinationDefinitionId());
     return buildDestinationRead(dci, standardDestinationDefinition);