Skip to content

Commit 15689ad

Browse files
authored
fix(oauth): dont convert config values to string when building consent url (#20932)
* fix: don't convert config values to strings * tests * add note
1 parent c7f8e67 commit 15689ad

File tree

2 files changed

+25
-20
lines changed

2 files changed

+25
-20
lines changed

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

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import static io.airbyte.metrics.lib.ApmTraceConstants.Tags.WORKSPACE_ID_KEY;
1010

1111
import com.fasterxml.jackson.databind.JsonNode;
12+
import com.fasterxml.jackson.databind.node.ObjectNode;
1213
import com.google.common.annotations.VisibleForTesting;
1314
import io.airbyte.analytics.TrackingClient;
1415
import io.airbyte.api.model.generated.CompleteDestinationOAuthRequest;
@@ -40,7 +41,6 @@
4041
import io.airbyte.validation.json.JsonValidationException;
4142
import java.io.IOException;
4243
import java.net.http.HttpClient;
43-
import java.util.HashMap;
4444
import java.util.List;
4545
import java.util.Map;
4646
import java.util.Optional;
@@ -322,23 +322,25 @@ Map<String, String> buildJsonPathFromOAuthFlowInitParameters(final Map<String, L
322322

323323
@VisibleForTesting
324324
JsonNode getOauthFromDBIfNeeded(final JsonNode oAuthInputConfigurationFromDB, final JsonNode oAuthInputConfigurationFromInput) {
325-
final Map<String, String> result = new HashMap<>();
326-
327-
Jsons.deserializeToStringMap(oAuthInputConfigurationFromInput)
328-
.forEach((k, v) -> {
329-
if (AirbyteSecretConstants.SECRETS_MASK.equals(v)) {
330-
if (oAuthInputConfigurationFromDB.has(k)) {
331-
result.put(k, oAuthInputConfigurationFromDB.get(k).textValue());
332-
} else {
333-
LOGGER.warn("Missing the key {} in the config store in DB", k);
334-
}
335-
336-
} else {
337-
result.put(k, v);
338-
}
339-
});
325+
final ObjectNode result = (ObjectNode) Jsons.emptyObject();
326+
327+
oAuthInputConfigurationFromInput.fields().forEachRemaining(entry -> {
328+
final String k = entry.getKey();
329+
final JsonNode v = entry.getValue();
330+
331+
// Note: This does not currently handle replacing masked secrets within nested objects.
332+
if (AirbyteSecretConstants.SECRETS_MASK.equals(v.textValue())) {
333+
if (oAuthInputConfigurationFromDB.has(k)) {
334+
result.set(k, oAuthInputConfigurationFromDB.get(k));
335+
} else {
336+
LOGGER.warn("Missing the key {} in the config store in DB", k);
337+
}
338+
} else {
339+
result.set(k, v);
340+
}
341+
});
340342

341-
return Jsons.jsonNode(result);
343+
return result;
342344
}
343345

344346
@VisibleForTesting

airbyte-server/src/test/java/io/airbyte/server/handlers/OAuthHandlerTest.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,23 +206,26 @@ void testGetOauthFromDBIfNeeded() {
206206
"""
207207
{
208208
"testMask": "**********",
209-
"testNotMask": "this"
209+
"testNotMask": "this",
210+
"testOtherType": true
210211
}
211212
""");
212213

213214
final JsonNode fromDb = Jsons.deserialize(
214215
"""
215216
{
216217
"testMask": "mask",
217-
"testNotMask": "notThis"
218+
"testNotMask": "notThis",
219+
"testOtherType": true
218220
}
219221
""");
220222

221223
final JsonNode expected = Jsons.deserialize(
222224
"""
223225
{
224226
"testMask": "mask",
225-
"testNotMask": "this"
227+
"testNotMask": "this",
228+
"testOtherType": true
226229
}
227230
""");
228231

0 commit comments

Comments
 (0)