fix(oauth): dont convert config values to string when building consen…

…t url (#20932) * fix: don't convert config values to strings * tests * add note
airbytehq · Jan 3, 2023 · 15689ad · 15689ad
1 parent c7f8e67
commit 15689ad
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 20 deletions.
diff --git a/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java b/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java
@@ -9,6 +9,7 @@
 import static io.airbyte.metrics.lib.ApmTraceConstants.Tags.WORKSPACE_ID_KEY;
 
 import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.annotations.VisibleForTesting;
 import io.airbyte.analytics.TrackingClient;
 import io.airbyte.api.model.generated.CompleteDestinationOAuthRequest;
@@ -40,7 +41,6 @@
 import io.airbyte.validation.json.JsonValidationException;
 import java.io.IOException;
 import java.net.http.HttpClient;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -322,23 +322,25 @@ Map<String, String> buildJsonPathFromOAuthFlowInitParameters(final Map<String, L
 
   @VisibleForTesting
   JsonNode getOauthFromDBIfNeeded(final JsonNode oAuthInputConfigurationFromDB, final JsonNode oAuthInputConfigurationFromInput) {
-    final Map<String, String> result = new HashMap<>();
-
-    Jsons.deserializeToStringMap(oAuthInputConfigurationFromInput)
-        .forEach((k, v) -> {
-          if (AirbyteSecretConstants.SECRETS_MASK.equals(v)) {
-            if (oAuthInputConfigurationFromDB.has(k)) {
-              result.put(k, oAuthInputConfigurationFromDB.get(k).textValue());
-            } else {
-              LOGGER.warn("Missing the key {} in the config store in DB", k);
-            }
-
-          } else {
-            result.put(k, v);
-          }
-        });
+    final ObjectNode result = (ObjectNode) Jsons.emptyObject();
+
+    oAuthInputConfigurationFromInput.fields().forEachRemaining(entry -> {
+      final String k = entry.getKey();
+      final JsonNode v = entry.getValue();
+
+      // Note: This does not currently handle replacing masked secrets within nested objects.
+      if (AirbyteSecretConstants.SECRETS_MASK.equals(v.textValue())) {
+        if (oAuthInputConfigurationFromDB.has(k)) {
+          result.set(k, oAuthInputConfigurationFromDB.get(k));
+        } else {
+          LOGGER.warn("Missing the key {} in the config store in DB", k);
+        }
+      } else {
+        result.set(k, v);
+      }
+    });
 
-    return Jsons.jsonNode(result);
+    return result;
   }
 
   @VisibleForTesting

diff --git a/airbyte-server/src/test/java/io/airbyte/server/handlers/OAuthHandlerTest.java b/airbyte-server/src/test/java/io/airbyte/server/handlers/OAuthHandlerTest.java
@@ -206,23 +206,26 @@ void testGetOauthFromDBIfNeeded() {
         """
         {
           "testMask": "**********",
-          "testNotMask": "this"
+          "testNotMask": "this",
+          "testOtherType": true
         }
         """);
 
     final JsonNode fromDb = Jsons.deserialize(
         """
         {
           "testMask": "mask",
-          "testNotMask": "notThis"
+          "testNotMask": "notThis",
+          "testOtherType": true
         }
         """);
 
     final JsonNode expected = Jsons.deserialize(
         """
         {
           "testMask": "mask",
-          "testNotMask": "this"
+          "testNotMask": "this",
+          "testOtherType": true
         }
         """);