Skip to content

Commit

Permalink
Fix secret (#3851)
Browse files Browse the repository at this point in the history 
Co-authored-by: Davin Chia <davinchia@gmail.com>
  • Loading branch information
@ChristopheDuong @davinchia
ChristopheDuong and davinchia committed Jun 4, 2021
1 parent de5d623 commit 900b522
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 17 deletions.
68 changes: 56 additions & 12 deletions airbyte-server/src/main/java/io/airbyte/server/converters/JsonSecretsProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,56 +25,86 @@
package io.airbyte.server.converters;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import io.airbyte.commons.json.Jsons;
import java.util.List;
import java.util.Optional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class JsonSecretsProcessor {

public static String AIRBYTE_SECRET_FIELD = "airbyte_secret";
private static final Logger LOGGER = LoggerFactory.getLogger(JsonSecretsProcessor.class);

@VisibleForTesting
static String SECRETS_MASK = "**********";

private static String PROPERTIES_FIELD = "properties";
private static final String PROPERTIES_FIELD = "properties";

/**
* Returns a copy of the input object wherein any fields annotated with "airbyte_secret" in the
* input schema are masked.
* <p>
* TODO this method only masks 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.
* This method masks secrets both at the top level of the configuration object and in nested
* properties in a oneOf.
*
* @param schema Schema containing secret annotations
* @param obj Object containing potentially secret fields
* @return
*/
public JsonNode maskSecrets(JsonNode obj, JsonNode schema) {
// if schema is an object and has a properties field
if (!canBeProcessed(schema)) {
return obj;
}
Preconditions.checkArgument(schema.isObject());

// get the properties field
ObjectNode properties = (ObjectNode) schema.get(PROPERTIES_FIELD);
JsonNode copy = obj.deepCopy();
// for the property keys
for (String key : Jsons.keys(properties)) {
if (isSecret(properties.get(key)) && copy.has(key)) {
((ObjectNode) copy).put(key, SECRETS_MASK);
JsonNode fieldSchema = properties.get(key);
// if the json schema field is an obj and has the airbyte secret field
if (isSecret(fieldSchema) && copy.has(key)) {
// mask and set it
if (copy.has(key)) {
((ObjectNode) copy).put(key, SECRETS_MASK);
}
}

var combinationKey = findJsonCombinationNode(fieldSchema);
if (combinationKey.isPresent() && copy.has(key)) {
var combinationCopy = copy.get(key);
var arrayNode = (ArrayNode) fieldSchema.get(combinationKey.get());
for (int i = 0; i < arrayNode.size(); i++) {
// Mask field values if any of the combination option is declaring it as secrets
combinationCopy = maskSecrets(combinationCopy, arrayNode.get(i));
}
((ObjectNode) copy).set(key, combinationCopy);
}
}

return copy;
}

private static Optional<String> findJsonCombinationNode(JsonNode node) {
for (String combinationNode : List.of("allOf", "anyOf", "oneOf")) {
if (node.has(combinationNode) && node.get(combinationNode).isArray()) {
return Optional.of(combinationNode);
}
}
return Optional.empty();
}

/**
* 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.
* This method absorbs secrets both at the top level of the configuration object and in nested
* properties in a oneOf.
*
* @param src The object potentially containing secrets
* @param dst The object to absorb secrets into
Expand All @@ -92,12 +122,26 @@ public JsonNode copySecrets(JsonNode src, JsonNode dst, JsonNode schema) {

ObjectNode properties = (ObjectNode) schema.get(PROPERTIES_FIELD);
for (String key : Jsons.keys(properties)) {
JsonNode fieldSchema = properties.get(key);
// We only copy the original secret if the destination object isn't attempting to overwrite it
// i.e: if the value of the secret isn't set to the mask
if (isSecret(properties.get(key)) && src.has(key)) {
if (isSecret(fieldSchema) && src.has(key)) {
if (dst.has(key) && dst.get(key).asText().equals(SECRETS_MASK))
dstCopy.set(key, src.get(key));
}

var combinationKey = findJsonCombinationNode(fieldSchema);
if (combinationKey.isPresent() && dstCopy.has(key)) {
var combinationCopy = dstCopy.get(key);
if (src.has(key)) {
var arrayNode = (ArrayNode) fieldSchema.get(combinationKey.get());
for (int i = 0; i < arrayNode.size(); i++) {
// Absorb field values if any of the combination option is declaring it as secrets
combinationCopy = copySecrets(src.get(key), combinationCopy, arrayNode.get(i));
}
}
dstCopy.set(key, combinationCopy);
}
}

return dstCopy;
Expand Down
4 changes: 4 additions & 0 deletions airbyte-server/src/main/java/io/airbyte/server/handlers/DestinationHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,13 @@
import java.util.List;
import java.util.UUID;
import java.util.function.Supplier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class DestinationHandler {

private static final Logger LOGGER = LoggerFactory.getLogger(DestinationHandler.class);

private final ConnectionsHandler connectionsHandler;
private final SpecFetcher specFetcher;
private final Supplier<UUID> uuidGenerator;
Expand Down
125 changes: 120 additions & 5 deletions airbyte-server/src/test/java/io/airbyte/server/converters/JsonSecretsProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

public class JsonSecretsProcessorTest {

private static final JsonNode SCHEMA = Jsons.deserialize(
private static final JsonNode SCHEMA_ONE_LAYER = Jsons.deserialize(
"{\n"
+ " \"properties\": {\n"
+ " \"secret1\": {\n"
Expand All @@ -53,6 +53,35 @@ public class JsonSecretsProcessorTest {
+ " }\n"
+ "}\n");

private static final JsonNode SCHEMA_INNER_OBJECT = Jsons.deserialize(
"{\n"
+ " \"type\": \"object\",\n"
+ " \"properties\": {\n"
+ " \"warehouse\": {\n"
+ " \"type\": \"string\"\n"
+ " },\n"
+ " \"loading_method\": {\n"
+ " \"type\": \"object\",\n"
+ " \"oneOf\": [\n"
+ " {\n"
+ " \"properties\": {}\n"
+ " },\n"
+ " {\n"
+ " \"properties\": {\n"
+ " \"s3_bucket_name\": {\n"
+ " \"type\": \"string\"\n"
+ " },\n"
+ " \"secret_access_key\": {\n"
+ " \"type\": \"string\",\n"
+ " \"airbyte_secret\": true\n"
+ " }\n"
+ " }\n"
+ " }\n"
+ " ]\n"
+ " }\n"
+ " }\n"
+ " }");

JsonSecretsProcessor processor = new JsonSecretsProcessor();

@Test
Expand All @@ -63,7 +92,7 @@ public void testMaskSecrets() {
.put("secret1", "donttellanyone")
.put("secret2", "verysecret").build());

JsonNode sanitized = processor.maskSecrets(obj, SCHEMA);
JsonNode sanitized = processor.maskSecrets(obj, SCHEMA_ONE_LAYER);

JsonNode expected = Jsons.jsonNode(ImmutableMap.builder()
.put("field1", "value1")
Expand All @@ -79,12 +108,47 @@ public void testMaskSecretsNotInObj() {
.put("field1", "value1")
.put("field2", 2).build());

JsonNode actual = processor.maskSecrets(obj, SCHEMA);
JsonNode actual = processor.maskSecrets(obj, SCHEMA_ONE_LAYER);

// Didn't have secrets, no fields should have been impacted.
assertEquals(obj, actual);
}

@Test
public void testMaskSecretInnerObject() {
JsonNode oneOf = Jsons.jsonNode(ImmutableMap.builder()
.put("s3_bucket_name", "name")
.put("secret_access_key", "secret").build());
JsonNode base = Jsons.jsonNode(ImmutableMap.builder()
.put("warehouse", "house")
.put("loading_method", oneOf).build());

JsonNode actual = processor.maskSecrets(base, SCHEMA_INNER_OBJECT);

JsonNode expectedOneOf = Jsons.jsonNode(ImmutableMap.builder()
.put("s3_bucket_name", "name")
.put("secret_access_key", JsonSecretsProcessor.SECRETS_MASK)
.build());
JsonNode expected = Jsons.jsonNode(ImmutableMap.builder()
.put("warehouse", "house")
.put("loading_method", expectedOneOf).build());

assertEquals(expected, actual);
}

@Test
public void testMaskSecretNotInInnerObject() {
JsonNode base = Jsons.jsonNode(ImmutableMap.builder()
.put("warehouse", "house").build());

JsonNode actual = processor.maskSecrets(base, SCHEMA_INNER_OBJECT);

JsonNode expected = Jsons.jsonNode(ImmutableMap.builder()
.put("warehouse", "house").build());

assertEquals(expected, actual);
}

@Test
public void testCopySecrets() {
JsonNode src = Jsons.jsonNode(ImmutableMap.builder()
Expand All @@ -102,7 +166,7 @@ public void testCopySecrets() {
.put("secret2", "newvalue")
.build());

JsonNode actual = processor.copySecrets(src, dst, SCHEMA);
JsonNode actual = processor.copySecrets(src, dst, SCHEMA_ONE_LAYER);

JsonNode expected = Jsons.jsonNode(ImmutableMap.builder()
.put("field1", "value1")
Expand All @@ -125,10 +189,61 @@ public void testCopySecretsNotInSrc() {
JsonNode dst = Jsons.jsonNode(ImmutableMap.builder()
.put("field1", "value1")
.put("field2", 2)
.put("secret1", JsonSecretsProcessor.SECRETS_MASK)
.build());

JsonNode expected = dst.deepCopy();
JsonNode actual = processor.copySecrets(src, dst, SCHEMA_ONE_LAYER);

assertEquals(expected, actual);
}

@Test
public void testCopySecretInnerObject() {
JsonNode srcOneOf = Jsons.jsonNode(ImmutableMap.builder()
.put("s3_bucket_name", "name")
.put("secret_access_key", "secret")
.put("additional_field", "dont_copy_me")
.build());
JsonNode src = Jsons.jsonNode(ImmutableMap.builder()
.put("warehouse", "house")
.put("loading_method", srcOneOf).build());

JsonNode dstOneOf = Jsons.jsonNode(ImmutableMap.builder()
.put("s3_bucket_name", "name")
.put("secret_access_key", JsonSecretsProcessor.SECRETS_MASK)
.build());
JsonNode dst = Jsons.jsonNode(ImmutableMap.builder()
.put("warehouse", "house")
.put("loading_method", dstOneOf).build());

JsonNode actual = processor.copySecrets(src, dst, SCHEMA_INNER_OBJECT);

JsonNode expectedOneOf = Jsons.jsonNode(ImmutableMap.builder()
.put("s3_bucket_name", "name")
.put("secret_access_key", "secret").build());
JsonNode expected = Jsons.jsonNode(ImmutableMap.builder()
.put("warehouse", "house")
.put("loading_method", expectedOneOf).build());

assertEquals(expected, actual);
}

@Test
public void testCopySecretNotInSrcInnerObject() {
JsonNode src = Jsons.jsonNode(ImmutableMap.builder()
.put("warehouse", "house").build());

JsonNode dstOneOf = Jsons.jsonNode(ImmutableMap.builder()
.put("s3_bucket_name", "name")
.put("secret_access_key", JsonSecretsProcessor.SECRETS_MASK)
.build());
JsonNode dst = Jsons.jsonNode(ImmutableMap.builder()
.put("warehouse", "house")
.put("loading_method", dstOneOf).build());

JsonNode actual = processor.copySecrets(src, dst, SCHEMA_INNER_OBJECT);
JsonNode expected = dst.deepCopy();
JsonNode actual = processor.copySecrets(src, dst, SCHEMA);

assertEquals(expected, actual);
}
Expand Down

0 comments on commit 900b522

Please sign in to comment.