fix: throw exception when processing secrets in complex json (#267)

Co-authored-by: Bernd Ruecker <[email protected]>
camunda · Jul 31, 2023 · bbf4882 · bbf4882
1 parent 0df7e41
commit bbf4882
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 52 deletions.
diff --git a/core/src/main/java/io/camunda/connector/impl/secret/SecretHandler.java b/core/src/main/java/io/camunda/connector/impl/secret/SecretHandler.java
@@ -60,6 +60,10 @@ public void handleSecretElement(
     handleSecretElement(value, failureMessage, type, setValueHandler, this);
   }
 
+  public void handleSecretElement(Object value, String type, Consumer<String> setValueHandler) {
+    handleSecretElement(value, null, type, setValueHandler, this);
+  }
+
   @Override
   public void handleSecretContainer(Object input, SecretElementHandler elementHandler) {
     if (input == null) {
@@ -103,39 +107,23 @@ protected void handleSecretsArray(Object[] input) {
     final var array = input;
     for (int i = 0; i < array.length; i++) {
       int index = i;
-      handleSecretElement(
-          array[index],
-          "Element at index " + index + " in array has no nested properties and is no String!",
-          "Array",
-          value -> array[index] = value);
+      handleSecretElement(array[index], "Array", value -> array[index] = value);
     }
   }
 
   protected void handleSecretsMap(final Map<Object, Object> input) {
-    input.forEach(
-        (k, v) ->
-            handleSecretElement(
-                v,
-                "Element at key '" + k + "' in map has no nested properties and is no String!",
-                "Map",
-                value -> input.put(k, value)));
+    input.forEach((k, v) -> handleSecretElement(v, "Map", value -> input.put(k, value)));
   }
 
   protected void handleSecretsList(List<Object> input) {
     for (ListIterator<Object> iterator = input.listIterator(); iterator.hasNext(); ) {
-      handleSecretElement(
-          iterator.next(),
-          "Element at index "
-              + iterator.previousIndex()
-              + " in list has no nested properties and is no String!",
-          "List",
-          iterator::set);
+      handleSecretElement(iterator.next(), "List", iterator::set);
     }
   }
 
   protected void handleSecretsIterable(Iterable<?> input) {
     for (Object o : input) {
-      handleSecretElement(o, "Element in iterable has no nested properties!", "Set", null);
+      handleSecretElement(o, "Set", null);
     }
   }
 
@@ -188,7 +176,9 @@ protected void handleSecretElement(
                       type + " is immutable but contains String secrets to replace!");
                 }
               } else {
-                throw new IllegalStateException(failureMessage);
+                if (failureMessage != null) {
+                  throw new IllegalStateException(failureMessage);
+                }
               }
             });
   }

diff --git a/core/src/test/java/io/camunda/connector/impl/outbound/OutboundConnectorContextTest.java b/core/src/test/java/io/camunda/connector/impl/outbound/OutboundConnectorContextTest.java
@@ -201,17 +201,15 @@ void shouldReplaceSecretsInObjectList() {
     }
 
     @Test
-    void shouldFailIfReplaceSecretsInNumberList() {
+    void shouldIgnoreReplaceSecretsInNumberList() {
       // given
       OutboundConnectorContext connectorContext =
           OutboundConnectorContextBuilder.create().secret("s3cr3t", "plain").build();
       final var testInput = new InputNumberList();
       // when
-      Exception expected = catchException(() -> connectorContext.replaceSecrets(testInput));
-      // then
-      assertThat(expected)
-          .isInstanceOf(IllegalStateException.class)
-          .hasMessage("Element at index 0 in list has no nested properties and is no String!");
+      connectorContext.replaceSecrets(testInput);
+      // then expect no any thrown exception, and input data didn't change
+      assertThat(testInput.numberList).isEqualTo(new InputNumberList().numberList);
     }
 
     @Test
@@ -241,31 +239,27 @@ void shouldReplaceSecretsInObjectSet() {
     }
 
     @Test
-    void shouldFailIfReplaceSecretsInStringSet() {
+    void shouldIgnoreReplaceSecretsInStringSet() {
       // given
       OutboundConnectorContext connectorContext =
           OutboundConnectorContextBuilder.create().secret("s3cr3t", "plain").build();
       final var testInput = new InputStringSet();
       // when
-      Exception expected = catchException(() -> connectorContext.replaceSecrets(testInput));
-      // then
-      assertThat(expected)
-          .isInstanceOf(IllegalStateException.class)
-          .hasMessage("Element in iterable has no nested properties!");
+      connectorContext.replaceSecrets(testInput);
+      // then expect no any thrown exception, and input data didn't change
+      assertThat(testInput.stringSet).isEqualTo(new InputStringSet().stringSet);
     }
 
     @Test
-    void shouldFailIfReplaceSecretsInNumberSet() {
+    void shouldIgnoreReplaceSecretsInNumberSet() {
       // given
       OutboundConnectorContext connectorContext =
           OutboundConnectorContextBuilder.create().secret("s3cr3t", "plain").build();
       final var testInput = new InputNumberSet();
       // when
-      Exception expected = catchException(() -> connectorContext.replaceSecrets(testInput));
-      // then
-      assertThat(expected)
-          .isInstanceOf(IllegalStateException.class)
-          .hasMessage("Element in iterable has no nested properties!");
+      connectorContext.replaceSecrets(testInput);
+      // then expect no any thrown exception, and input data didn't change
+      assertThat(testInput.numberSet).isEqualTo(new InputNumberSet().numberSet);
     }
 
     @Test
@@ -293,17 +287,15 @@ void shouldReplaceSecretsInObjectArray() {
     }
 
     @Test
-    void shouldFailIfReplaceSecretsInNumberArray() {
+    void shouldIgnoreReplaceSecretsInNumberArray() {
       // given
       OutboundConnectorContext connectorContext =
           OutboundConnectorContextBuilder.create().secret("s3cr3t", "plain").build();
       final var testInput = new InputNumberArray();
       // when
-      Exception expected = catchException(() -> connectorContext.replaceSecrets(testInput));
-      // then
-      assertThat(expected)
-          .isInstanceOf(IllegalStateException.class)
-          .hasMessage("Element at index 0 in array has no nested properties and is no String!");
+      connectorContext.replaceSecrets(testInput);
+      // then expect no any thrown exception, and input data didn't change
+      assertThat(testInput.numberArray).isEqualTo(new InputNumberArray().numberArray);
     }
 
     @Test
@@ -332,6 +324,19 @@ void shouldReplaceSecretsInObjectMap() {
           .allSatisfy((key, value) -> assertThat(value.getSecretField()).isEqualTo("plain"));
     }
 
+    @Test
+    void shouldReplaceSecretsInMultiplyObjectMap() {
+      // given
+      OutboundConnectorContext connectorContext =
+          OutboundConnectorContextBuilder.create().secret("s3cr3t", "plain").build();
+      final var testInput = new InputMultiplyObjectMap();
+      // when
+      connectorContext.replaceSecrets(testInput);
+      // then expect that secret replaced without exception
+      assertThat(((Map<Object, Object>) testInput.inputMultiplyMap.get("baz")).get("bar"))
+          .isEqualTo("plain");
+    }
+
     @Test
     void shouldFailIfReplaceSecretsInImmutableStringMap() {
       // given
@@ -347,18 +352,15 @@ void shouldFailIfReplaceSecretsInImmutableStringMap() {
     }
 
     @Test
-    void shouldFailIfReplaceSecretsInNumberMap() {
+    void shouldIgnoreReplaceSecretsInNumberMap() {
       // given
       OutboundConnectorContext connectorContext =
           OutboundConnectorContextBuilder.create().secret("s3cr3t", "plain").build();
       final var testInput = new InputNumberMap();
       // when
-      Exception expected = catchException(() -> connectorContext.replaceSecrets(testInput));
-      // then
-      assertThat(expected)
-          .isInstanceOf(IllegalStateException.class)
-          .hasMessageContaining("Element at key")
-          .hasMessageContaining("in map has no nested properties and is no String!");
+      connectorContext.replaceSecrets(testInput);
+      // then expect no any thrown exception, and map didn't change
+      assertThat(testInput.numberMap).isEqualTo(new InputNumberMap().numberMap);
     }
 
     @Test
@@ -568,6 +570,12 @@ public static class InputObjectMap {
     public final Map<String, OutboundTestInput> inputMap = Map.of("bar", new OutboundTestInput());
   }
 
+  public static class InputMultiplyObjectMap {
+    @Secret
+    public final Map<String, Object> inputMultiplyMap =
+        Map.of("bar", 5, "baz", new HashMap<>(Map.of("bar", "secrets.s3cr3t", "baz", "foo")));
+  }
+
   public static class InputObjectSet {
     @Secret
     public final Set<OutboundTestInput> inputSet =