Skip to content

Commit f087f9f

Browse files
benmoriceauschlattk
benmoriceau
authored and
schlattk
committed
make sure that all the secrets are remove (airbytehq#8881)
explore all the nodes to mask secrets, even if they are arrays or nested at a given deep level. It makes what has been made in airbytehq#8859 more generic
1 parent bceea45 commit f087f9f

File tree

10 files changed

+214
-177
lines changed

10 files changed

+214
-177
lines changed

‎airbyte-config/persistence/src/main/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessor.java

+82-22
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,19 @@
66

77
import com.fasterxml.jackson.databind.JsonNode;
88
import com.fasterxml.jackson.databind.node.ArrayNode;
9+
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
910
import com.fasterxml.jackson.databind.node.ObjectNode;
1011
import com.google.common.annotations.VisibleForTesting;
1112
import com.google.common.base.Preconditions;
1213
import io.airbyte.commons.json.Jsons;
1314
import io.airbyte.validation.json.JsonSchemaValidator;
15+
import java.util.HashSet;
16+
import java.util.LinkedList;
1417
import java.util.List;
1518
import java.util.Optional;
19+
import java.util.Queue;
20+
import java.util.Set;
21+
import lombok.Value;
1622
import org.slf4j.Logger;
1723
import org.slf4j.LoggerFactory;
1824

@@ -22,6 +28,9 @@ public class JsonSecretsProcessor {
2228

2329
public static String AIRBYTE_SECRET_FIELD = "airbyte_secret";
2430
public static final String PROPERTIES_FIELD = "properties";
31+
public static String TYPE_FIELD = "type";
32+
public static String ARRAY_TYPE_FIELD = "array";
33+
public static String ITEMS_FIELD = "items";
2534

2635
private static final JsonSchemaValidator VALIDATOR = new JsonSchemaValidator();
2736

@@ -46,36 +55,80 @@ public JsonNode maskSecrets(final JsonNode obj, final JsonNode schema) {
4655
return obj;
4756
}
4857
Preconditions.checkArgument(schema.isObject());
49-
// get the properties field
50-
final ObjectNode properties = (ObjectNode) schema.get(PROPERTIES_FIELD);
51-
final JsonNode copy = obj.deepCopy();
52-
// for the property keys
53-
for (final String key : Jsons.keys(properties)) {
54-
final JsonNode fieldSchema = properties.get(key);
55-
// if the json schema field is an obj and has the airbyte secret field
56-
if (isSecret(fieldSchema) && copy.has(key)) {
57-
// mask and set it
58-
if (copy.has(key)) {
59-
((ObjectNode) copy).put(key, SECRETS_MASK);
58+
59+
final SecretKeys secretKeys = getAllSecretKeys(schema);
60+
return maskAllSecrets(obj, secretKeys);
61+
}
62+
63+
private JsonNode maskAllSecrets(final JsonNode obj, final SecretKeys secretKeys) {
64+
final JsonNode copiedObj = obj.deepCopy();
65+
final Queue<JsonNode> toProcess = new LinkedList<>();
66+
toProcess.add(copiedObj);
67+
68+
while (!toProcess.isEmpty()) {
69+
final JsonNode currentNode = toProcess.remove();
70+
for (final String key : Jsons.keys(currentNode)) {
71+
if (secretKeys.fieldSecretKey.contains(key)) {
72+
((ObjectNode) currentNode).put(key, SECRETS_MASK);
73+
} else if (currentNode.get(key).isObject()) {
74+
toProcess.add(currentNode.get(key));
75+
} else if (currentNode.get(key).isArray()) {
76+
if (secretKeys.arraySecretKey.contains(key)) {
77+
final ArrayNode sanitizedArrayNode = new ArrayNode(JsonNodeFactory.instance);
78+
currentNode.get(key).forEach((secret) -> sanitizedArrayNode.add(SECRETS_MASK));
79+
((ObjectNode) currentNode).put(key, sanitizedArrayNode);
80+
} else {
81+
final ArrayNode arrayNode = (ArrayNode) currentNode.get(key);
82+
arrayNode.forEach((node) -> {
83+
toProcess.add(node);
84+
});
85+
}
6086
}
61-
} else if (canBeProcessed(fieldSchema) && copy.has(key)) {
62-
((ObjectNode) copy).set(key, maskSecrets(copy.get(key), fieldSchema));
6387
}
88+
}
6489

65-
final var combinationKey = findJsonCombinationNode(fieldSchema);
90+
return copiedObj;
91+
}
92+
93+
@Value
94+
private class SecretKeys {
95+
96+
private final Set<String> fieldSecretKey;
97+
private final Set<String> arraySecretKey;
98+
99+
}
66100

67-
if (combinationKey.isPresent() && copy.has(key)) {
68-
var combinationCopy = copy.get(key);
69-
final var arrayNode = (ArrayNode) fieldSchema.get(combinationKey.get());
70-
for (int i = 0; i < arrayNode.size(); i++) {
71-
// Mask field values if any of the combination option is declaring it as secrets
72-
combinationCopy = maskSecrets(combinationCopy, arrayNode.get(i));
101+
private SecretKeys getAllSecretKeys(final JsonNode schema) {
102+
final Set<String> fieldSecretKeys = new HashSet<>();
103+
final Set<String> arraySecretKeys = new HashSet<>();
104+
105+
final Queue<JsonNode> toProcess = new LinkedList<>();
106+
toProcess.add(schema);
107+
108+
while (!toProcess.isEmpty()) {
109+
final JsonNode currentNode = toProcess.remove();
110+
for (final String key : Jsons.keys(currentNode)) {
111+
if (isArrayDefinition(currentNode.get(key))) {
112+
final JsonNode arrayItems = currentNode.get(key).get(ITEMS_FIELD);
113+
if (arrayItems.has(AIRBYTE_SECRET_FIELD) && arrayItems.get(AIRBYTE_SECRET_FIELD).asBoolean()) {
114+
arraySecretKeys.add(key);
115+
} else {
116+
toProcess.add(arrayItems);
117+
}
118+
} else if (isSecret(currentNode.get(key))) {
119+
fieldSecretKeys.add(key);
120+
} else if (currentNode.get(key).isObject()) {
121+
toProcess.add(currentNode.get(key));
122+
} else if (currentNode.get(key).isArray()) {
123+
final ArrayNode arrayNode = (ArrayNode) currentNode.get(key);
124+
arrayNode.forEach((node) -> {
125+
toProcess.add(node);
126+
});
73127
}
74-
((ObjectNode) copy).set(key, combinationCopy);
75128
}
76129
}
77130

78-
return copy;
131+
return new SecretKeys(fieldSecretKeys, arraySecretKeys);
79132
}
80133

81134
public static Optional<String> findJsonCombinationNode(final JsonNode node) {
@@ -152,4 +205,11 @@ public static boolean canBeProcessed(final JsonNode schema) {
152205
return schema.isObject() && schema.has(PROPERTIES_FIELD) && schema.get(PROPERTIES_FIELD).isObject();
153206
}
154207

208+
public static boolean isArrayDefinition(final JsonNode obj) {
209+
return obj.isObject()
210+
&& obj.has(TYPE_FIELD)
211+
&& obj.get(TYPE_FIELD).asText().equals(ARRAY_TYPE_FIELD)
212+
&& obj.has(ITEMS_FIELD);
213+
}
214+
155215
}

‎airbyte-config/persistence/src/test/java/io/airbyte/config/persistence/split_secrets/JsonSecretsProcessorTest.java

+48-155
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,16 @@
77
import static org.junit.jupiter.api.Assertions.assertEquals;
88

99
import com.fasterxml.jackson.databind.JsonNode;
10+
import com.fasterxml.jackson.databind.ObjectMapper;
1011
import com.google.common.collect.ImmutableMap;
1112
import io.airbyte.commons.json.Jsons;
13+
import java.io.IOException;
14+
import java.io.InputStream;
15+
import java.util.stream.Stream;
1216
import org.junit.jupiter.api.Test;
17+
import org.junit.jupiter.params.ParameterizedTest;
18+
import org.junit.jupiter.params.provider.Arguments;
19+
import org.junit.jupiter.params.provider.MethodSource;
1320

1421
public class JsonSecretsProcessorTest {
1522

@@ -153,163 +160,8 @@ public class JsonSecretsProcessorTest {
153160
+ " }\n"
154161
+ " }");
155162

156-
private final static String test = """
157-
{
158-
"provider": {
159-
"bucket": "bucket",
160-
"endpoint": "",
161-
"path_prefix": "",
162-
"aws_access_key_id": "nothingtosee",
163-
"aws_secret_access_key": "same"
164-
}
165-
}
166-
""";
167-
168-
private final static String testSpecs =
169-
"""
170-
{
171-
"type": "object",
172-
"title": "S3 Source Spec",
173-
"required": [
174-
"dataset",
175-
"path_pattern",
176-
"provider"
177-
],
178-
"properties": {
179-
"provider": {
180-
"type": "object",
181-
"title": "S3: Amazon Web Services",
182-
"required": [
183-
"bucket"
184-
],
185-
"properties": {
186-
"bucket": {
187-
"type": "string",
188-
"title": "Bucket",
189-
"description": "Name of the S3 bucket where the file(s) exist."
190-
},
191-
"use_ssl": {
192-
"type": "boolean",
193-
"title": "Use Ssl",
194-
"description": "Is remote server using secure SSL/TLS connection"
195-
},
196-
"endpoint": {
197-
"type": "string",
198-
"title": "Endpoint",
199-
"default": "",
200-
"description": "Endpoint to an S3 compatible service. Leave empty to use AWS."
201-
},
202-
"path_prefix": {
203-
"type": "string",
204-
"title": "Path Prefix",
205-
"default": "",
206-
"description": "By providing a path-like prefix (e.g. myFolder/thisTable/) under which all the relevant files sit, we can optimise finding these in S3. This is optional but recommended if your bucket contains many folders/files."
207-
},
208-
"verify_ssl_cert": {
209-
"type": "boolean",
210-
"title": "Verify Ssl Cert",
211-
"description": "Allow self signed certificates"
212-
},
213-
"aws_access_key_id": {
214-
"type": "string",
215-
"title": "Aws Access Key Id",
216-
"description": "In order to access private Buckets stored on AWS S3, this connector requires credentials with the proper permissions. If accessing publicly available data, this field is not necessary.",
217-
"airbyte_secret": true
218-
},
219-
"aws_secret_access_key": {
220-
"type": "string",
221-
"title": "Aws Secret Access Key",
222-
"description": "In order to access private Buckets stored on AWS S3, this connector requires credentials with the proper permissions. If accessing publicly available data, this field is not necessary.",
223-
"airbyte_secret": true
224-
}
225-
}
226-
}
227-
}
228-
}
229-
""";
230-
231163
JsonSecretsProcessor processor = new JsonSecretsProcessor();
232164

233-
@Test
234-
public void testNestedSecrets() {
235-
final JsonNode obj = Jsons.deserialize(test);
236-
final JsonNode specObj = Jsons.deserialize(testSpecs);
237-
final JsonNode sanitized = processor.maskSecrets(obj, specObj);
238-
239-
final JsonNode expected = Jsons.jsonNode(ImmutableMap.builder()
240-
.put("bucket", "bucket")
241-
.put("endpoint", "")
242-
.put("path_prefix", "")
243-
.put("aws_access_key_id", JsonSecretsProcessor.SECRETS_MASK)
244-
.put("aws_secret_access_key", JsonSecretsProcessor.SECRETS_MASK).build());
245-
assertEquals(expected, sanitized.get("provider"));
246-
}
247-
248-
@Test
249-
public void testMaskSecrets() {
250-
final JsonNode obj = Jsons.jsonNode(ImmutableMap.builder()
251-
.put("field1", "value1")
252-
.put("field2", 2)
253-
.put("secret1", "donttellanyone")
254-
.put("secret2", "verysecret").build());
255-
256-
final JsonNode sanitized = processor.maskSecrets(obj, SCHEMA_ONE_LAYER);
257-
258-
final JsonNode expected = Jsons.jsonNode(ImmutableMap.builder()
259-
.put("field1", "value1")
260-
.put("field2", 2)
261-
.put("secret1", JsonSecretsProcessor.SECRETS_MASK)
262-
.put("secret2", JsonSecretsProcessor.SECRETS_MASK).build());
263-
assertEquals(expected, sanitized);
264-
}
265-
266-
@Test
267-
public void testMaskSecretsNotInObj() {
268-
final JsonNode obj = Jsons.jsonNode(ImmutableMap.builder()
269-
.put("field1", "value1")
270-
.put("field2", 2).build());
271-
272-
final JsonNode actual = processor.maskSecrets(obj, SCHEMA_ONE_LAYER);
273-
274-
// Didn't have secrets, no fields should have been impacted.
275-
assertEquals(obj, actual);
276-
}
277-
278-
@Test
279-
public void testMaskSecretInnerObject() {
280-
final JsonNode oneOf = Jsons.jsonNode(ImmutableMap.builder()
281-
.put("s3_bucket_name", "name")
282-
.put("secret_access_key", "secret").build());
283-
final JsonNode base = Jsons.jsonNode(ImmutableMap.builder()
284-
.put("warehouse", "house")
285-
.put("loading_method", oneOf).build());
286-
287-
final JsonNode actual = processor.maskSecrets(base, SCHEMA_INNER_OBJECT);
288-
289-
final JsonNode expectedOneOf = Jsons.jsonNode(ImmutableMap.builder()
290-
.put("s3_bucket_name", "name")
291-
.put("secret_access_key", JsonSecretsProcessor.SECRETS_MASK)
292-
.build());
293-
final JsonNode expected = Jsons.jsonNode(ImmutableMap.builder()
294-
.put("warehouse", "house")
295-
.put("loading_method", expectedOneOf).build());
296-
297-
assertEquals(expected, actual);
298-
}
299-
300-
@Test
301-
public void testMaskSecretNotInInnerObject() {
302-
final JsonNode base = Jsons.jsonNode(ImmutableMap.builder()
303-
.put("warehouse", "house").build());
304-
305-
final JsonNode actual = processor.maskSecrets(base, SCHEMA_INNER_OBJECT);
306-
307-
final JsonNode expected = Jsons.jsonNode(ImmutableMap.builder()
308-
.put("warehouse", "house").build());
309-
310-
assertEquals(expected, actual);
311-
}
312-
313165
@Test
314166
public void testCopySecrets() {
315167
final JsonNode src = Jsons.jsonNode(ImmutableMap.builder()
@@ -431,4 +283,45 @@ void testHandlesSameKeyInOneOf() {
431283
final JsonNode actual = new JsonSecretsProcessor().copySecrets(src, dst, ONE_OF_WITH_SAME_KEY_IN_SUB_SCHEMAS);
432284
}
433285

286+
private static Stream<Arguments> scenarioProvider() {
287+
return Stream.of(
288+
Arguments.of("array", true),
289+
Arguments.of("array", false),
290+
Arguments.of("array_of_oneof", true),
291+
Arguments.of("array_of_oneof", false),
292+
Arguments.of("nested_object", true),
293+
Arguments.of("nested_object", false),
294+
Arguments.of("nested_oneof", true),
295+
Arguments.of("nested_oneof", false),
296+
Arguments.of("oneof", true),
297+
Arguments.of("oneof", false),
298+
Arguments.of("optional_password", true),
299+
Arguments.of("optional_password", false),
300+
Arguments.of("postgres_ssh_key", true),
301+
Arguments.of("postgres_ssh_key", false),
302+
Arguments.of("simple", true),
303+
Arguments.of("simple", false));
304+
}
305+
306+
@ParameterizedTest
307+
@MethodSource("scenarioProvider")
308+
void testSecretScenario(final String folder, final boolean partial) throws IOException {
309+
final ObjectMapper objectMapper = new ObjectMapper();
310+
311+
final InputStream specIs = getClass().getClassLoader().getResourceAsStream(folder + "/spec.json");
312+
final JsonNode specs = objectMapper.readTree(specIs);
313+
314+
final String inputFilePath = folder + (partial ? "/partial_config.json" : "/full_config.json");
315+
final InputStream inputIs = getClass().getClassLoader().getResourceAsStream(inputFilePath);
316+
final JsonNode input = objectMapper.readTree(inputIs);
317+
318+
final String expectedFilePath = folder + "/expected.json";
319+
final InputStream expectedIs = getClass().getClassLoader().getResourceAsStream(expectedFilePath);
320+
final JsonNode expected = objectMapper.readTree(expectedIs);
321+
322+
final JsonNode actual = processor.maskSecrets(input, specs);
323+
324+
assertEquals(expected, actual);
325+
}
326+
434327
}

‎airbyte-config/persistence/src/test/resources/array/expected.json

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"username": "charles",
3+
"rotating_key_strings": ["**********", "**********", "**********"],
4+
"rotating_key_objects": [
5+
{
6+
"a": "**********"
7+
},
8+
{
9+
"a": "**********"
10+
},
11+
{
12+
"a": "**********"
13+
}
14+
]
15+
}

0 commit comments

Comments
 (0)