From b05453c0e3298c4ac67760df4d9f87303a03d24d Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Thu, 10 Nov 2022 14:52:09 -0800 Subject: [PATCH 01/12] Update inputs --- airbyte-api/src/main/openapi/config.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/airbyte-api/src/main/openapi/config.yaml b/airbyte-api/src/main/openapi/config.yaml index e9691ae1b4e01..7350b2eb6e6ce 100644 --- a/airbyte-api/src/main/openapi/config.yaml +++ b/airbyte-api/src/main/openapi/config.yaml @@ -4649,6 +4649,8 @@ components: type: string oAuthInputConfiguration: $ref: "#/components/schemas/OAuthInputConfiguration" + sourceId: + $ref: "#/components/schemas/SourceId" DestinationOauthConsentRequest: type: object required: @@ -4665,6 +4667,8 @@ components: type: string oAuthInputConfiguration: $ref: "#/components/schemas/OAuthInputConfiguration" + destinationId: + $ref: "#/components/schemas/SourceId" OAuthConsentRead: type: object required: From 06d6c4d72f340b9e3c7bbbf3b10ac5fb23de1234 Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Thu, 10 Nov 2022 16:13:18 -0800 Subject: [PATCH 02/12] PR comments and autogen files --- airbyte-api/src/main/openapi/config.yaml | 2 +- docs/reference/api/generated-api-html/index.html | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/airbyte-api/src/main/openapi/config.yaml b/airbyte-api/src/main/openapi/config.yaml index 7350b2eb6e6ce..3921372c8ca1c 100644 --- a/airbyte-api/src/main/openapi/config.yaml +++ b/airbyte-api/src/main/openapi/config.yaml @@ -4668,7 +4668,7 @@ components: oAuthInputConfiguration: $ref: "#/components/schemas/OAuthInputConfiguration" destinationId: - $ref: "#/components/schemas/SourceId" + $ref: "#/components/schemas/DestinationId" OAuthConsentRead: type: object required: diff --git a/docs/reference/api/generated-api-html/index.html b/docs/reference/api/generated-api-html/index.html index aa977cc558121..831bf9923b861 100644 --- a/docs/reference/api/generated-api-html/index.html +++ b/docs/reference/api/generated-api-html/index.html @@ -11126,6 +11126,7 @@

DestinationOauthConsentReques
workspaceId
UUID format: uuid
redirectUrl
String The url to redirect to after getting the user consent
oAuthInputConfiguration (optional)
OAuthConfiguration
+
destinationId (optional)
UUID format: uuid
@@ -11841,6 +11842,7 @@

SourceOauthConsentRequest -
workspaceId
UUID format: uuid
redirectUrl
String The url to redirect to after getting the user consent
oAuthInputConfiguration (optional)
OAuthConfiguration
+
sourceId (optional)
UUID format: uuid

From 0e49686b2fa9c02308404b7ac1adc506c2d6b626 Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Mon, 14 Nov 2022 11:32:16 -0800 Subject: [PATCH 03/12] Fix oAuth retry --- airbyte-commons/build.gradle | 1 + .../java/io/airbyte/commons/json/Jsons.java | 4 + .../java/io/airbyte/server/ServerApp.java | 2 +- .../airbyte/server/handlers/OAuthHandler.java | 73 +++++++++++++++++- .../server/handlers/OAuthHandlerTest.java | 77 ++++++++++++++++++- 5 files changed, 152 insertions(+), 5 deletions(-) diff --git a/airbyte-commons/build.gradle b/airbyte-commons/build.gradle index 3c4c0f9e19617..92a81add77dde 100644 --- a/airbyte-commons/build.gradle +++ b/airbyte-commons/build.gradle @@ -7,6 +7,7 @@ dependencies { // this dependency is an exception to the above rule because it is only used INTERNALLY to the commons library. implementation 'com.jayway.jsonpath:json-path:2.7.0' + // implementation("com.fasterxml.jackson.core:jackson-core:2.13.0") } Task publishArtifactsTask = getPublishArtifactsTask("$rootProject.ext.version", project) diff --git a/airbyte-commons/src/main/java/io/airbyte/commons/json/Jsons.java b/airbyte-commons/src/main/java/io/airbyte/commons/json/Jsons.java index a92e2c49985cd..d461175ba1b78 100644 --- a/airbyte-commons/src/main/java/io/airbyte/commons/json/Jsons.java +++ b/airbyte-commons/src/main/java/io/airbyte/commons/json/Jsons.java @@ -281,6 +281,10 @@ public static void mergeMaps(final Map originalMap, final String Entry::getValue))); } + public static Map deserializeToStringMap(JsonNode json) { + return OBJECT_MAPPER.convertValue(json, new TypeReference<>() {}); + } + /** * By the Jackson DefaultPrettyPrinter prints objects with an extra space as follows: {"name" : * "airbyte"}. We prefer {"name": "airbyte"}. diff --git a/airbyte-server/src/main/java/io/airbyte/server/ServerApp.java b/airbyte-server/src/main/java/io/airbyte/server/ServerApp.java index 9936fa19ee272..ab60c482473e2 100644 --- a/airbyte-server/src/main/java/io/airbyte/server/ServerApp.java +++ b/airbyte-server/src/main/java/io/airbyte/server/ServerApp.java @@ -295,7 +295,7 @@ public static ServerRunnable getServer(final ServerFactory apiFactory, final HealthCheckHandler healthCheckHandler = new HealthCheckHandler(configRepository); - final OAuthHandler oAuthHandler = new OAuthHandler(configRepository, httpClient, trackingClient); + final OAuthHandler oAuthHandler = new OAuthHandler(configRepository, httpClient, trackingClient, secretsRepositoryReader); final SourceHandler sourceHandler = new SourceHandler( configRepository, 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 index 92241095c4a68..12fb8e3d0d264 100644 --- a/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java +++ b/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java @@ -4,6 +4,9 @@ package io.airbyte.server.handlers; +import com.fasterxml.jackson.databind.JsonNode; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Iterables; import io.airbyte.analytics.TrackingClient; import io.airbyte.api.model.generated.CompleteDestinationOAuthRequest; import io.airbyte.api.model.generated.CompleteSourceOauthRequest; @@ -12,13 +15,17 @@ import io.airbyte.api.model.generated.SetInstancewideDestinationOauthParamsRequestBody; import io.airbyte.api.model.generated.SetInstancewideSourceOauthParamsRequestBody; import io.airbyte.api.model.generated.SourceOauthConsentRequest; +import io.airbyte.commons.constants.AirbyteSecretConstants; +import io.airbyte.commons.json.JsonPaths; import io.airbyte.commons.json.Jsons; import io.airbyte.config.DestinationOAuthParameter; +import io.airbyte.config.SourceConnection; import io.airbyte.config.SourceOAuthParameter; import io.airbyte.config.StandardDestinationDefinition; import io.airbyte.config.StandardSourceDefinition; import io.airbyte.config.persistence.ConfigNotFoundException; import io.airbyte.config.persistence.ConfigRepository; +import io.airbyte.config.persistence.SecretsRepositoryReader; import io.airbyte.oauth.OAuthFlowImplementation; import io.airbyte.oauth.OAuthImplementationFactory; import io.airbyte.persistence.job.factory.OAuthConfigSupplier; @@ -27,8 +34,11 @@ 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.UUID; +import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,13 +50,16 @@ public class OAuthHandler { private final ConfigRepository configRepository; private final OAuthImplementationFactory oAuthImplementationFactory; private final TrackingClient trackingClient; + private final SecretsRepositoryReader secretsRepositoryReader; public OAuthHandler(final ConfigRepository configRepository, final HttpClient httpClient, - final TrackingClient trackingClient) { + final TrackingClient trackingClient, + final SecretsRepositoryReader secretsRepositoryReader) { this.configRepository = configRepository; this.oAuthImplementationFactory = new OAuthImplementationFactory(configRepository, httpClient); this.trackingClient = trackingClient; + this.secretsRepositoryReader = secretsRepositoryReader; } public OAuthConsentRead getSourceOAuthConsent(final SourceOauthConsentRequest sourceDefinitionIdRequestBody) @@ -58,11 +71,22 @@ public OAuthConsentRead getSourceOAuthConsent(final SourceOauthConsentRequest so final Map metadata = generateSourceMetadata(sourceDefinitionIdRequestBody.getSourceDefinitionId()); final OAuthConsentRead result; if (OAuthConfigSupplier.hasOAuthConfigSpecification(spec)) { + final SourceConnection hydratedSourceConnection = + secretsRepositoryReader.getSourceConnectionWithSecrets(sourceDefinitionIdRequestBody.getSourceId()); + + List fieldsToGet = + buildJsonPathFromOAuthFlowInitParameters(spec.getAuthSpecification().getOauth2Specification().getOauthFlowInitParameters()); + + JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); + + JsonNode oAuthInputConfigurationForConsent = getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, + sourceDefinitionIdRequestBody.getoAuthInputConfiguration()); + result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getSourceConsentUrl( sourceDefinitionIdRequestBody.getWorkspaceId(), sourceDefinitionIdRequestBody.getSourceDefinitionId(), sourceDefinitionIdRequestBody.getRedirectUrl(), - sourceDefinitionIdRequestBody.getoAuthInputConfiguration(), + oAuthInputConfigurationForConsent, spec.getAdvancedAuth().getOauthConfigSpecification())); } else { result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getSourceConsentUrl( @@ -87,11 +111,22 @@ public OAuthConsentRead getDestinationOAuthConsent(final DestinationOauthConsent final Map metadata = generateDestinationMetadata(destinationDefinitionIdRequestBody.getDestinationDefinitionId()); final OAuthConsentRead result; if (OAuthConfigSupplier.hasOAuthConfigSpecification(spec)) { + final SourceConnection hydratedSourceConnection = + secretsRepositoryReader.getSourceConnectionWithSecrets(destinationDefinitionIdRequestBody.getDestinationId()); + + List fieldsToGet = + buildJsonPathFromOAuthFlowInitParameters(spec.getAuthSpecification().getOauth2Specification().getOauthFlowInitParameters()); + + JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); + + JsonNode oAuthInputConfigurationForConsent = getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, + destinationDefinitionIdRequestBody.getoAuthInputConfiguration()); + result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getDestinationConsentUrl( destinationDefinitionIdRequestBody.getWorkspaceId(), destinationDefinitionIdRequestBody.getDestinationDefinitionId(), destinationDefinitionIdRequestBody.getRedirectUrl(), - destinationDefinitionIdRequestBody.getoAuthInputConfiguration(), + oAuthInputConfigurationForConsent, spec.getAdvancedAuth().getOauthConfigSpecification())); } else { result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getDestinationConsentUrl( @@ -207,4 +242,36 @@ private Map generateDestinationMetadata(final UUID destinationDe return TrackingMetadata.generateDestinationDefinitionMetadata(destinationDefinition); } + @VisibleForTesting + List buildJsonPathFromOAuthFlowInitParameters(List> oAuthFlowInitParameters) { + return oAuthFlowInitParameters.stream() + .map(path -> "$." + String.join(".", path)) + .toList(); + } + + @VisibleForTesting + JsonNode getOauthFromDBIfNeeded(JsonNode oAuthInputConfigurationFromDB, JsonNode oAuthInputConfigurationFromInput) { + Map result = new HashMap<>(); + + Jsons.deserializeToStringMap(oAuthInputConfigurationFromInput) + .forEach((k, v) -> { + if (AirbyteSecretConstants.SECRETS_MASK.equals(v)) { + result.put(k, oAuthInputConfigurationFromDB.get(k).textValue()); + } else { + result.put(k, v); + } + }); + + return Jsons.jsonNode(result); + } + + @VisibleForTesting + JsonNode getOAuthInputConfiguration(JsonNode hydratedSourceConnectionConfiguration, List pathsToGet) { + return Jsons.jsonNode(pathsToGet.stream().map(path -> Map.entry(path, + JsonPaths.getSingleValue(hydratedSourceConnectionConfiguration, path))) + .collect(Collectors.toMap( + entry -> Iterables.getLast(List.of(entry.getKey().split("\\."))), + entry -> entry.getValue().get()))); + } + } 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 index 0ea12a1e0594e..a5f0d38bda4fc 100644 --- a/airbyte-server/src/test/java/io/airbyte/server/handlers/OAuthHandlerTest.java +++ b/airbyte-server/src/test/java/io/airbyte/server/handlers/OAuthHandlerTest.java @@ -8,6 +8,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import com.fasterxml.jackson.databind.JsonNode; import io.airbyte.analytics.TrackingClient; import io.airbyte.api.model.generated.SetInstancewideDestinationOauthParamsRequestBody; import io.airbyte.api.model.generated.SetInstancewideSourceOauthParamsRequestBody; @@ -15,6 +16,7 @@ import io.airbyte.config.DestinationOAuthParameter; import io.airbyte.config.SourceOAuthParameter; import io.airbyte.config.persistence.ConfigRepository; +import io.airbyte.config.persistence.SecretsRepositoryReader; import io.airbyte.validation.json.JsonValidationException; import java.io.IOException; import java.net.http.HttpClient; @@ -34,6 +36,7 @@ class OAuthHandlerTest { private OAuthHandler handler; private TrackingClient trackingClient; private HttpClient httpClient; + private SecretsRepositoryReader secretsRepositoryReader; private static final String CLIENT_ID = "123"; private static final String CLIENT_ID_KEY = "client_id"; private static final String CLIENT_SECRET_KEY = "client_secret"; @@ -44,7 +47,8 @@ public void init() { configRepository = Mockito.mock(ConfigRepository.class); trackingClient = mock(TrackingClient.class); httpClient = Mockito.mock(HttpClient.class); - handler = new OAuthHandler(configRepository, httpClient, trackingClient); + secretsRepositoryReader = mock(SecretsRepositoryReader.class); + handler = new OAuthHandler(configRepository, httpClient, trackingClient, secretsRepositoryReader); } @Test @@ -151,4 +155,75 @@ void resetDestinationInstancewideOauthParams() throws JsonValidationException, I assertEquals(oauthParameterId, capturedValues.get(1).getOauthParameterId()); } + @Test + void testBuildJsonPathFromOAuthFlowInitParameters() { + List> input = List.of( + List.of("1"), + List.of("2", "3")); + + List expected = List.of("$.1", "$.2.3"); + + assertEquals(expected, handler.buildJsonPathFromOAuthFlowInitParameters(input)); + } + + @Test + void testGetOAuthInputConfiguration() { + JsonNode hydratedConfig = Jsons.deserialize( + """ + { + "field1": "1", + "field2": "2", + "field3": { + "field3_1": "3_1", + "field3_2": "3_2" + } + } + """); + + List pathsToGet = List.of( + "$.field1", + "$.field3.field3_1", + "$.field3.field3_2"); + + JsonNode expected = Jsons.deserialize( + """ + { + "field1": "1", + "field3_1": "3_1", + "field3_2": "3_2" + } + """); + + assertEquals(expected, handler.getOAuthInputConfiguration(hydratedConfig, pathsToGet)); + } + + @Test + void testGetOauthFromDBIfNeeded() { + JsonNode fromInput = Jsons.deserialize( + """ + { + "testMask": "**********", + "testNotMask": "this" + } + """); + + JsonNode fromDb = Jsons.deserialize( + """ + { + "testMask": "mask", + "testNotMask": "notThis" + } + """); + + JsonNode expected = Jsons.deserialize( + """ + { + "testMask": "mask", + "testNotMask": "this" + } + """); + + assertEquals(expected, handler.getOauthFromDBIfNeeded(fromDb, fromInput)); + } + } From 7dff0bf1b76a4017114fa9c4fe5f0b696976e92c Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Mon, 14 Nov 2022 11:47:10 -0800 Subject: [PATCH 04/12] Handle null --- .../airbyte/server/handlers/OAuthHandler.java | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 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 index 12fb8e3d0d264..9a429d1389bb8 100644 --- a/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java +++ b/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java @@ -71,16 +71,22 @@ public OAuthConsentRead getSourceOAuthConsent(final SourceOauthConsentRequest so final Map metadata = generateSourceMetadata(sourceDefinitionIdRequestBody.getSourceDefinitionId()); final OAuthConsentRead result; if (OAuthConfigSupplier.hasOAuthConfigSpecification(spec)) { - final SourceConnection hydratedSourceConnection = - secretsRepositoryReader.getSourceConnectionWithSecrets(sourceDefinitionIdRequestBody.getSourceId()); + JsonNode oAuthInputConfigurationForConsent; - List fieldsToGet = - buildJsonPathFromOAuthFlowInitParameters(spec.getAuthSpecification().getOauth2Specification().getOauthFlowInitParameters()); + if (sourceDefinitionIdRequestBody.getSourceId() == null) { + oAuthInputConfigurationForConsent = sourceDefinitionIdRequestBody.getoAuthInputConfiguration() + } else { + final SourceConnection hydratedSourceConnection = + secretsRepositoryReader.getSourceConnectionWithSecrets(sourceDefinitionIdRequestBody.getSourceId()); - JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); + List fieldsToGet = + buildJsonPathFromOAuthFlowInitParameters(spec.getAuthSpecification().getOauth2Specification().getOauthFlowInitParameters()); - JsonNode oAuthInputConfigurationForConsent = getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, - sourceDefinitionIdRequestBody.getoAuthInputConfiguration()); + JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); + + oAuthInputConfigurationForConsent = getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, + sourceDefinitionIdRequestBody.getoAuthInputConfiguration()); + } result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getSourceConsentUrl( sourceDefinitionIdRequestBody.getWorkspaceId(), @@ -111,16 +117,22 @@ public OAuthConsentRead getDestinationOAuthConsent(final DestinationOauthConsent final Map metadata = generateDestinationMetadata(destinationDefinitionIdRequestBody.getDestinationDefinitionId()); final OAuthConsentRead result; if (OAuthConfigSupplier.hasOAuthConfigSpecification(spec)) { - final SourceConnection hydratedSourceConnection = - secretsRepositoryReader.getSourceConnectionWithSecrets(destinationDefinitionIdRequestBody.getDestinationId()); + JsonNode oAuthInputConfigurationForConsent; + + if (destinationDefinitionIdRequestBody.getDestinationId() == null) { + oAuthInputConfigurationForConsent = destinationDefinitionIdRequestBody.getoAuthInputConfiguration(); + } else { + final SourceConnection hydratedSourceConnection = + secretsRepositoryReader.getSourceConnectionWithSecrets(destinationDefinitionIdRequestBody.getDestinationId()); - List fieldsToGet = - buildJsonPathFromOAuthFlowInitParameters(spec.getAuthSpecification().getOauth2Specification().getOauthFlowInitParameters()); + List fieldsToGet = + buildJsonPathFromOAuthFlowInitParameters(spec.getAuthSpecification().getOauth2Specification().getOauthFlowInitParameters()); - JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); + JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); - JsonNode oAuthInputConfigurationForConsent = getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, - destinationDefinitionIdRequestBody.getoAuthInputConfiguration()); + oAuthInputConfigurationForConsent = getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, + destinationDefinitionIdRequestBody.getoAuthInputConfiguration()); + } result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getDestinationConsentUrl( destinationDefinitionIdRequestBody.getWorkspaceId(), From 3d29990f20c6068c2301b6bd6bf5e6db6bdd170a Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Mon, 14 Nov 2022 11:48:44 -0800 Subject: [PATCH 05/12] Add missing ; --- .../src/main/java/io/airbyte/server/handlers/OAuthHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 9a429d1389bb8..c5045bd0f157a 100644 --- a/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java +++ b/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java @@ -74,7 +74,7 @@ public OAuthConsentRead getSourceOAuthConsent(final SourceOauthConsentRequest so JsonNode oAuthInputConfigurationForConsent; if (sourceDefinitionIdRequestBody.getSourceId() == null) { - oAuthInputConfigurationForConsent = sourceDefinitionIdRequestBody.getoAuthInputConfiguration() + oAuthInputConfigurationForConsent = sourceDefinitionIdRequestBody.getoAuthInputConfiguration(); } else { final SourceConnection hydratedSourceConnection = secretsRepositoryReader.getSourceConnectionWithSecrets(sourceDefinitionIdRequestBody.getSourceId()); From 796c3ecdc5cd695b43025df273dc8649873f4af1 Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Mon, 14 Nov 2022 14:45:30 -0800 Subject: [PATCH 06/12] PR comments --- airbyte-commons/build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/airbyte-commons/build.gradle b/airbyte-commons/build.gradle index 92a81add77dde..3c4c0f9e19617 100644 --- a/airbyte-commons/build.gradle +++ b/airbyte-commons/build.gradle @@ -7,7 +7,6 @@ dependencies { // this dependency is an exception to the above rule because it is only used INTERNALLY to the commons library. implementation 'com.jayway.jsonpath:json-path:2.7.0' - // implementation("com.fasterxml.jackson.core:jackson-core:2.13.0") } Task publishArtifactsTask = getPublishArtifactsTask("$rootProject.ext.version", project) From ddac0ece7f04d7f402a7daeaaa521badf3f2b4ac Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Mon, 14 Nov 2022 15:45:06 -0800 Subject: [PATCH 07/12] use advanced oAuth --- .../airbyte/server/handlers/OAuthHandler.java | 61 ++++++++++--------- .../handlers/helpers/OAuthPathExtractor.java | 38 ++++++++++++ .../helper/OAuthPathExtractorTest.java | 43 +++++++++++++ 3 files changed, 114 insertions(+), 28 deletions(-) create mode 100644 airbyte-server/src/main/java/io/airbyte/server/handlers/helpers/OAuthPathExtractor.java create mode 100644 airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java 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 index c5045bd0f157a..876d3119250d4 100644 --- a/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java +++ b/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java @@ -18,6 +18,7 @@ import io.airbyte.commons.constants.AirbyteSecretConstants; import io.airbyte.commons.json.JsonPaths; import io.airbyte.commons.json.Jsons; +import io.airbyte.config.DestinationConnection; import io.airbyte.config.DestinationOAuthParameter; import io.airbyte.config.SourceConnection; import io.airbyte.config.SourceOAuthParameter; @@ -31,6 +32,7 @@ import io.airbyte.persistence.job.factory.OAuthConfigSupplier; import io.airbyte.persistence.job.tracker.TrackingMetadata; import io.airbyte.protocol.models.ConnectorSpecification; +import io.airbyte.server.handlers.helpers.OAuthPathExtractor; import io.airbyte.validation.json.JsonValidationException; import java.io.IOException; import java.net.http.HttpClient; @@ -62,46 +64,48 @@ public OAuthHandler(final ConfigRepository configRepository, this.secretsRepositoryReader = secretsRepositoryReader; } - public OAuthConsentRead getSourceOAuthConsent(final SourceOauthConsentRequest sourceDefinitionIdRequestBody) + public OAuthConsentRead getSourceOAuthConsent(final SourceOauthConsentRequest sourceOauthConsentRequest) throws JsonValidationException, ConfigNotFoundException, IOException { final StandardSourceDefinition sourceDefinition = - configRepository.getStandardSourceDefinition(sourceDefinitionIdRequestBody.getSourceDefinitionId()); + configRepository.getStandardSourceDefinition(sourceOauthConsentRequest.getSourceDefinitionId()); final OAuthFlowImplementation oAuthFlowImplementation = oAuthImplementationFactory.create(sourceDefinition); final ConnectorSpecification spec = sourceDefinition.getSpec(); - final Map metadata = generateSourceMetadata(sourceDefinitionIdRequestBody.getSourceDefinitionId()); + final Map metadata = generateSourceMetadata(sourceOauthConsentRequest.getSourceDefinitionId()); final OAuthConsentRead result; if (OAuthConfigSupplier.hasOAuthConfigSpecification(spec)) { - JsonNode oAuthInputConfigurationForConsent; + final JsonNode oAuthInputConfigurationForConsent; - if (sourceDefinitionIdRequestBody.getSourceId() == null) { - oAuthInputConfigurationForConsent = sourceDefinitionIdRequestBody.getoAuthInputConfiguration(); + if (sourceOauthConsentRequest.getSourceId() == null) { + oAuthInputConfigurationForConsent = sourceOauthConsentRequest.getoAuthInputConfiguration(); } else { final SourceConnection hydratedSourceConnection = - secretsRepositoryReader.getSourceConnectionWithSecrets(sourceDefinitionIdRequestBody.getSourceId()); + secretsRepositoryReader.getSourceConnectionWithSecrets(sourceOauthConsentRequest.getSourceId()); - List fieldsToGet = - buildJsonPathFromOAuthFlowInitParameters(spec.getAuthSpecification().getOauth2Specification().getOauthFlowInitParameters()); + final List fieldsToGet = + buildJsonPathFromOAuthFlowInitParameters( + OAuthPathExtractor.extractOauthConfigurationPaths( + spec.getAdvancedAuth().getOauthConfigSpecification().getOauthUserInputFromConnectorConfigSpecification())); - JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); + final JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); oAuthInputConfigurationForConsent = getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, - sourceDefinitionIdRequestBody.getoAuthInputConfiguration()); + sourceOauthConsentRequest.getoAuthInputConfiguration()); } result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getSourceConsentUrl( - sourceDefinitionIdRequestBody.getWorkspaceId(), - sourceDefinitionIdRequestBody.getSourceDefinitionId(), - sourceDefinitionIdRequestBody.getRedirectUrl(), + sourceOauthConsentRequest.getWorkspaceId(), + sourceOauthConsentRequest.getSourceDefinitionId(), + sourceOauthConsentRequest.getRedirectUrl(), oAuthInputConfigurationForConsent, spec.getAdvancedAuth().getOauthConfigSpecification())); } else { result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getSourceConsentUrl( - sourceDefinitionIdRequestBody.getWorkspaceId(), - sourceDefinitionIdRequestBody.getSourceDefinitionId(), - sourceDefinitionIdRequestBody.getRedirectUrl(), Jsons.emptyObject(), null)); + sourceOauthConsentRequest.getWorkspaceId(), + sourceOauthConsentRequest.getSourceDefinitionId(), + sourceOauthConsentRequest.getRedirectUrl(), Jsons.emptyObject(), null)); } try { - trackingClient.track(sourceDefinitionIdRequestBody.getWorkspaceId(), "Get Oauth Consent URL - Backend", metadata); + trackingClient.track(sourceOauthConsentRequest.getWorkspaceId(), "Get Oauth Consent URL - Backend", metadata); } catch (final Exception e) { LOGGER.error(ERROR_MESSAGE, e); } @@ -117,18 +121,19 @@ public OAuthConsentRead getDestinationOAuthConsent(final DestinationOauthConsent final Map metadata = generateDestinationMetadata(destinationDefinitionIdRequestBody.getDestinationDefinitionId()); final OAuthConsentRead result; if (OAuthConfigSupplier.hasOAuthConfigSpecification(spec)) { - JsonNode oAuthInputConfigurationForConsent; + final JsonNode oAuthInputConfigurationForConsent; if (destinationDefinitionIdRequestBody.getDestinationId() == null) { oAuthInputConfigurationForConsent = destinationDefinitionIdRequestBody.getoAuthInputConfiguration(); } else { - final SourceConnection hydratedSourceConnection = - secretsRepositoryReader.getSourceConnectionWithSecrets(destinationDefinitionIdRequestBody.getDestinationId()); + final DestinationConnection hydratedSourceConnection = + secretsRepositoryReader.getDestinationConnectionWithSecrets(destinationDefinitionIdRequestBody.getDestinationId()); - List fieldsToGet = - buildJsonPathFromOAuthFlowInitParameters(spec.getAuthSpecification().getOauth2Specification().getOauthFlowInitParameters()); + final List fieldsToGet = + buildJsonPathFromOAuthFlowInitParameters(OAuthPathExtractor.extractOauthConfigurationPaths( + spec.getAdvancedAuth().getOauthConfigSpecification().getOauthUserInputFromConnectorConfigSpecification())); - JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); + final JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); oAuthInputConfigurationForConsent = getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, destinationDefinitionIdRequestBody.getoAuthInputConfiguration()); @@ -255,15 +260,15 @@ private Map generateDestinationMetadata(final UUID destinationDe } @VisibleForTesting - List buildJsonPathFromOAuthFlowInitParameters(List> oAuthFlowInitParameters) { + List buildJsonPathFromOAuthFlowInitParameters(final List> oAuthFlowInitParameters) { return oAuthFlowInitParameters.stream() .map(path -> "$." + String.join(".", path)) .toList(); } @VisibleForTesting - JsonNode getOauthFromDBIfNeeded(JsonNode oAuthInputConfigurationFromDB, JsonNode oAuthInputConfigurationFromInput) { - Map result = new HashMap<>(); + JsonNode getOauthFromDBIfNeeded(final JsonNode oAuthInputConfigurationFromDB, final JsonNode oAuthInputConfigurationFromInput) { + final Map result = new HashMap<>(); Jsons.deserializeToStringMap(oAuthInputConfigurationFromInput) .forEach((k, v) -> { @@ -278,7 +283,7 @@ JsonNode getOauthFromDBIfNeeded(JsonNode oAuthInputConfigurationFromDB, JsonNode } @VisibleForTesting - JsonNode getOAuthInputConfiguration(JsonNode hydratedSourceConnectionConfiguration, List pathsToGet) { + JsonNode getOAuthInputConfiguration(final JsonNode hydratedSourceConnectionConfiguration, final List pathsToGet) { return Jsons.jsonNode(pathsToGet.stream().map(path -> Map.entry(path, JsonPaths.getSingleValue(hydratedSourceConnectionConfiguration, path))) .collect(Collectors.toMap( diff --git a/airbyte-server/src/main/java/io/airbyte/server/handlers/helpers/OAuthPathExtractor.java b/airbyte-server/src/main/java/io/airbyte/server/handlers/helpers/OAuthPathExtractor.java new file mode 100644 index 0000000000000..c54d2a3fdb829 --- /dev/null +++ b/airbyte-server/src/main/java/io/airbyte/server/handlers/helpers/OAuthPathExtractor.java @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2022 Airbyte, Inc., all rights reserved. + */ + +package io.airbyte.server.handlers.helpers; + +import com.fasterxml.jackson.databind.JsonNode; +import java.util.ArrayList; +import java.util.List; + +public class OAuthPathExtractor { + + private static final String PROPERTIES = "properties"; + private static final String PATH_IN_CONNECTOR_CONFIG = "path_in_connector_config"; + + public static List> extractOauthConfigurationPaths(final JsonNode configuration) { + + if (configuration.has(PROPERTIES) && configuration.get(PROPERTIES).isObject()) { + final List> result = new ArrayList<>(); + + configuration.get(PROPERTIES).fields().forEachRemaining(entry -> { + final JsonNode value = entry.getValue(); + if (value.isObject() && value.has(PATH_IN_CONNECTOR_CONFIG) && value.get(PATH_IN_CONNECTOR_CONFIG).isArray()) { + final List path = new ArrayList<>(); + for (final JsonNode pathPart : value.get(PATH_IN_CONNECTOR_CONFIG)) { + path.add(pathPart.textValue()); + } + result.add(path); + } + }); + + return result; + } else { + return new ArrayList<>(); + } + } + +} diff --git a/airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java b/airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java new file mode 100644 index 0000000000000..0ef027cb5cf15 --- /dev/null +++ b/airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2022 Airbyte, Inc., all rights reserved. + */ + +package io.airbyte.server.handlers.helper; + +import com.fasterxml.jackson.databind.JsonNode; +import io.airbyte.commons.json.Jsons; +import io.airbyte.server.handlers.helpers.OAuthPathExtractor; +import java.util.List; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; + +class OAuthPathExtractorTest { + + @Test + void testExtract() { + final JsonNode input = Jsons.deserialize(""" + { + "type": "object", + "additionalProperties": false, + "properties": { + "tenant_id": { + "type": "string", + "path_in_connector_config": ["tenant_id"] + }, + "another_property": { + "type": "string", + "path_in_connector_config": ["another", "property"] + } + } + } + """); + + final List> expected = List.of( + List.of("tenant_id"), + List.of("another", "property")); + + Assertions.assertThat(OAuthPathExtractor.extractOauthConfigurationPaths(input)) + .containsAll(expected); + } + +} From 35f500a8ed0ebb9066033b4299bbbcb89e8f1ab6 Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Mon, 14 Nov 2022 15:56:39 -0800 Subject: [PATCH 08/12] PR comments --- .../airbyte/server/handlers/OAuthHandler.java | 61 ++++++++++--------- 1 file changed, 32 insertions(+), 29 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 index 876d3119250d4..f90caf61d1573 100644 --- a/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java +++ b/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java @@ -81,15 +81,9 @@ public OAuthConsentRead getSourceOAuthConsent(final SourceOauthConsentRequest so final SourceConnection hydratedSourceConnection = secretsRepositoryReader.getSourceConnectionWithSecrets(sourceOauthConsentRequest.getSourceId()); - final List fieldsToGet = - buildJsonPathFromOAuthFlowInitParameters( - OAuthPathExtractor.extractOauthConfigurationPaths( - spec.getAdvancedAuth().getOauthConfigSpecification().getOauthUserInputFromConnectorConfigSpecification())); - - final JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); - - oAuthInputConfigurationForConsent = getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, - sourceOauthConsentRequest.getoAuthInputConfiguration()); + oAuthInputConfigurationForConsent = getOAuthInputConfigurationForConsent(spec, + hydratedSourceConnection.getConfiguration(), + sourceOauthConsentRequest.getoAuthInputConfiguration() ); } result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getSourceConsentUrl( @@ -112,47 +106,43 @@ public OAuthConsentRead getSourceOAuthConsent(final SourceOauthConsentRequest so return result; } - public OAuthConsentRead getDestinationOAuthConsent(final DestinationOauthConsentRequest destinationDefinitionIdRequestBody) + public OAuthConsentRead getDestinationOAuthConsent(final DestinationOauthConsentRequest destinationOauthConsentRequest) throws JsonValidationException, ConfigNotFoundException, IOException { final StandardDestinationDefinition destinationDefinition = - configRepository.getStandardDestinationDefinition(destinationDefinitionIdRequestBody.getDestinationDefinitionId()); + configRepository.getStandardDestinationDefinition(destinationOauthConsentRequest.getDestinationDefinitionId()); final OAuthFlowImplementation oAuthFlowImplementation = oAuthImplementationFactory.create(destinationDefinition); final ConnectorSpecification spec = destinationDefinition.getSpec(); - final Map metadata = generateDestinationMetadata(destinationDefinitionIdRequestBody.getDestinationDefinitionId()); + final Map metadata = generateDestinationMetadata(destinationOauthConsentRequest.getDestinationDefinitionId()); final OAuthConsentRead result; if (OAuthConfigSupplier.hasOAuthConfigSpecification(spec)) { final JsonNode oAuthInputConfigurationForConsent; - if (destinationDefinitionIdRequestBody.getDestinationId() == null) { - oAuthInputConfigurationForConsent = destinationDefinitionIdRequestBody.getoAuthInputConfiguration(); + if (destinationOauthConsentRequest.getDestinationId() == null) { + oAuthInputConfigurationForConsent = destinationOauthConsentRequest.getoAuthInputConfiguration(); } else { final DestinationConnection hydratedSourceConnection = - secretsRepositoryReader.getDestinationConnectionWithSecrets(destinationDefinitionIdRequestBody.getDestinationId()); + secretsRepositoryReader.getDestinationConnectionWithSecrets(destinationOauthConsentRequest.getDestinationId()); - final List fieldsToGet = - buildJsonPathFromOAuthFlowInitParameters(OAuthPathExtractor.extractOauthConfigurationPaths( - spec.getAdvancedAuth().getOauthConfigSpecification().getOauthUserInputFromConnectorConfigSpecification())); + oAuthInputConfigurationForConsent = getOAuthInputConfigurationForConsent(spec, + hydratedSourceConnection.getConfiguration(), + destinationOauthConsentRequest.getoAuthInputConfiguration() ); - final JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnection.getConfiguration(), fieldsToGet); - - oAuthInputConfigurationForConsent = getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, - destinationDefinitionIdRequestBody.getoAuthInputConfiguration()); } result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getDestinationConsentUrl( - destinationDefinitionIdRequestBody.getWorkspaceId(), - destinationDefinitionIdRequestBody.getDestinationDefinitionId(), - destinationDefinitionIdRequestBody.getRedirectUrl(), + destinationOauthConsentRequest.getWorkspaceId(), + destinationOauthConsentRequest.getDestinationDefinitionId(), + destinationOauthConsentRequest.getRedirectUrl(), oAuthInputConfigurationForConsent, spec.getAdvancedAuth().getOauthConfigSpecification())); } else { result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getDestinationConsentUrl( - destinationDefinitionIdRequestBody.getWorkspaceId(), - destinationDefinitionIdRequestBody.getDestinationDefinitionId(), - destinationDefinitionIdRequestBody.getRedirectUrl(), Jsons.emptyObject(), null)); + destinationOauthConsentRequest.getWorkspaceId(), + destinationOauthConsentRequest.getDestinationDefinitionId(), + destinationOauthConsentRequest.getRedirectUrl(), Jsons.emptyObject(), null)); } try { - trackingClient.track(destinationDefinitionIdRequestBody.getWorkspaceId(), "Get Oauth Consent URL - Backend", metadata); + trackingClient.track(destinationOauthConsentRequest.getWorkspaceId(), "Get Oauth Consent URL - Backend", metadata); } catch (final Exception e) { LOGGER.error(ERROR_MESSAGE, e); } @@ -247,6 +237,19 @@ public void setDestinationInstancewideOauthParams(final SetInstancewideDestinati configRepository.writeDestinationOAuthParam(param); } + private JsonNode getOAuthInputConfigurationForConsent(final ConnectorSpecification spec, + final JsonNode hydratedSourceConnectionConfiguration, + final JsonNode destinationDefinitionIdRequestBody) { + final List fieldsToGet = + buildJsonPathFromOAuthFlowInitParameters(OAuthPathExtractor.extractOauthConfigurationPaths( + spec.getAdvancedAuth().getOauthConfigSpecification().getOauthUserInputFromConnectorConfigSpecification())); + + final JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnectionConfiguration, fieldsToGet); + + return getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, + destinationDefinitionIdRequestBody); + } + private Map generateSourceMetadata(final UUID sourceDefinitionId) throws JsonValidationException, ConfigNotFoundException, IOException { final StandardSourceDefinition sourceDefinition = configRepository.getStandardSourceDefinition(sourceDefinitionId); From 1788b34a47bc668a065988d0256f17da6a6ac11c Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Mon, 14 Nov 2022 15:59:52 -0800 Subject: [PATCH 09/12] More PR comments --- .../main/java/io/airbyte/server/handlers/OAuthHandler.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 index f90caf61d1573..380c351d3f643 100644 --- a/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java +++ b/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java @@ -276,7 +276,12 @@ JsonNode getOauthFromDBIfNeeded(final JsonNode oAuthInputConfigurationFromDB, fi Jsons.deserializeToStringMap(oAuthInputConfigurationFromInput) .forEach((k, v) -> { if (AirbyteSecretConstants.SECRETS_MASK.equals(v)) { - result.put(k, oAuthInputConfigurationFromDB.get(k).textValue()); + if (oAuthInputConfigurationFromDB.has(k)) { + result.put(k, oAuthInputConfigurationFromDB.get(k).textValue()); + } else { + LOGGER.warn("Missing the k {} in the config store in DB", k); + } + } else { result.put(k, v); } From fce3c2b07d2093cdd361c9fe77379a312870bb61 Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Tue, 15 Nov 2022 08:52:46 -0800 Subject: [PATCH 10/12] Format --- .../java/io/airbyte/server/handlers/OAuthHandler.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 index 380c351d3f643..53b239ada8b81 100644 --- a/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java +++ b/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java @@ -83,7 +83,7 @@ public OAuthConsentRead getSourceOAuthConsent(final SourceOauthConsentRequest so oAuthInputConfigurationForConsent = getOAuthInputConfigurationForConsent(spec, hydratedSourceConnection.getConfiguration(), - sourceOauthConsentRequest.getoAuthInputConfiguration() ); + sourceOauthConsentRequest.getoAuthInputConfiguration()); } result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getSourceConsentUrl( @@ -125,7 +125,7 @@ public OAuthConsentRead getDestinationOAuthConsent(final DestinationOauthConsent oAuthInputConfigurationForConsent = getOAuthInputConfigurationForConsent(spec, hydratedSourceConnection.getConfiguration(), - destinationOauthConsentRequest.getoAuthInputConfiguration() ); + destinationOauthConsentRequest.getoAuthInputConfiguration()); } @@ -238,8 +238,8 @@ public void setDestinationInstancewideOauthParams(final SetInstancewideDestinati } private JsonNode getOAuthInputConfigurationForConsent(final ConnectorSpecification spec, - final JsonNode hydratedSourceConnectionConfiguration, - final JsonNode destinationDefinitionIdRequestBody) { + final JsonNode hydratedSourceConnectionConfiguration, + final JsonNode destinationDefinitionIdRequestBody) { final List fieldsToGet = buildJsonPathFromOAuthFlowInitParameters(OAuthPathExtractor.extractOauthConfigurationPaths( spec.getAdvancedAuth().getOauthConfigSpecification().getOauthUserInputFromConnectorConfigSpecification())); From 2dc776aa982ae73cbbfe33af01b1bbbfd8191112 Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Tue, 15 Nov 2022 11:35:38 -0800 Subject: [PATCH 11/12] Avoid conflict in json extract --- .../airbyte/server/handlers/OAuthHandler.java | 25 ++++++++--------- .../handlers/helpers/OAuthPathExtractor.java | 10 ++++--- .../server/handlers/OAuthHandlerTest.java | 28 ++++++++++--------- .../helper/OAuthPathExtractorTest.java | 9 +++--- 4 files changed, 38 insertions(+), 34 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 index 53b239ada8b81..31454d2b11703 100644 --- a/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java +++ b/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java @@ -239,15 +239,15 @@ public void setDestinationInstancewideOauthParams(final SetInstancewideDestinati private JsonNode getOAuthInputConfigurationForConsent(final ConnectorSpecification spec, final JsonNode hydratedSourceConnectionConfiguration, - final JsonNode destinationDefinitionIdRequestBody) { - final List fieldsToGet = + final JsonNode oAuthInputConfiguration) { + final Map fieldsToGet = buildJsonPathFromOAuthFlowInitParameters(OAuthPathExtractor.extractOauthConfigurationPaths( spec.getAdvancedAuth().getOauthConfigSpecification().getOauthUserInputFromConnectorConfigSpecification())); final JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnectionConfiguration, fieldsToGet); return getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, - destinationDefinitionIdRequestBody); + oAuthInputConfiguration); } private Map generateSourceMetadata(final UUID sourceDefinitionId) @@ -263,10 +263,10 @@ private Map generateDestinationMetadata(final UUID destinationDe } @VisibleForTesting - List buildJsonPathFromOAuthFlowInitParameters(final List> oAuthFlowInitParameters) { - return oAuthFlowInitParameters.stream() - .map(path -> "$." + String.join(".", path)) - .toList(); + Map buildJsonPathFromOAuthFlowInitParameters(final Map> oAuthFlowInitParameters) { + return oAuthFlowInitParameters.entrySet().stream() + .map(entry -> Map.entry(entry.getKey(), "$." + String.join(".", entry.getValue()))) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); } @VisibleForTesting @@ -279,7 +279,7 @@ JsonNode getOauthFromDBIfNeeded(final JsonNode oAuthInputConfigurationFromDB, fi if (oAuthInputConfigurationFromDB.has(k)) { result.put(k, oAuthInputConfigurationFromDB.get(k).textValue()); } else { - LOGGER.warn("Missing the k {} in the config store in DB", k); + LOGGER.warn("Missing the key {} in the config store in DB", k); } } else { @@ -291,12 +291,11 @@ JsonNode getOauthFromDBIfNeeded(final JsonNode oAuthInputConfigurationFromDB, fi } @VisibleForTesting - JsonNode getOAuthInputConfiguration(final JsonNode hydratedSourceConnectionConfiguration, final List pathsToGet) { - return Jsons.jsonNode(pathsToGet.stream().map(path -> Map.entry(path, - JsonPaths.getSingleValue(hydratedSourceConnectionConfiguration, path))) + JsonNode getOAuthInputConfiguration(final JsonNode hydratedSourceConnectionConfiguration, final Map pathsToGet) { + return Jsons.jsonNode(pathsToGet.entrySet().stream() .collect(Collectors.toMap( - entry -> Iterables.getLast(List.of(entry.getKey().split("\\."))), - entry -> entry.getValue().get()))); + Map.Entry::getKey, + entry -> JsonPaths.getSingleValue(hydratedSourceConnectionConfiguration, entry.getValue()).get()))); } } diff --git a/airbyte-server/src/main/java/io/airbyte/server/handlers/helpers/OAuthPathExtractor.java b/airbyte-server/src/main/java/io/airbyte/server/handlers/helpers/OAuthPathExtractor.java index c54d2a3fdb829..ddf74bc4767eb 100644 --- a/airbyte-server/src/main/java/io/airbyte/server/handlers/helpers/OAuthPathExtractor.java +++ b/airbyte-server/src/main/java/io/airbyte/server/handlers/helpers/OAuthPathExtractor.java @@ -6,17 +6,19 @@ import com.fasterxml.jackson.databind.JsonNode; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; public class OAuthPathExtractor { private static final String PROPERTIES = "properties"; private static final String PATH_IN_CONNECTOR_CONFIG = "path_in_connector_config"; - public static List> extractOauthConfigurationPaths(final JsonNode configuration) { + public static Map> extractOauthConfigurationPaths(final JsonNode configuration) { if (configuration.has(PROPERTIES) && configuration.get(PROPERTIES).isObject()) { - final List> result = new ArrayList<>(); + final Map> result = new HashMap<>(); configuration.get(PROPERTIES).fields().forEachRemaining(entry -> { final JsonNode value = entry.getValue(); @@ -25,13 +27,13 @@ public static List> extractOauthConfigurationPaths(final JsonNode c for (final JsonNode pathPart : value.get(PATH_IN_CONNECTOR_CONFIG)) { path.add(pathPart.textValue()); } - result.add(path); + result.put(entry.getKey(), path); } }); return result; } else { - return new ArrayList<>(); + return new HashMap<>(); } } 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 index a5f0d38bda4fc..4b6ad812cac08 100644 --- a/airbyte-server/src/test/java/io/airbyte/server/handlers/OAuthHandlerTest.java +++ b/airbyte-server/src/test/java/io/airbyte/server/handlers/OAuthHandlerTest.java @@ -157,18 +157,20 @@ void resetDestinationInstancewideOauthParams() throws JsonValidationException, I @Test void testBuildJsonPathFromOAuthFlowInitParameters() { - List> input = List.of( - List.of("1"), - List.of("2", "3")); + final Map> input = Map.ofEntries( + Map.entry("field1", List.of("1")), + Map.entry("field2", List.of("2", "3"))); - List expected = List.of("$.1", "$.2.3"); + final Map expected = Map.ofEntries( + Map.entry("field1", "$.1"), + Map.entry("field2", "$.2.3")); assertEquals(expected, handler.buildJsonPathFromOAuthFlowInitParameters(input)); } @Test void testGetOAuthInputConfiguration() { - JsonNode hydratedConfig = Jsons.deserialize( + final JsonNode hydratedConfig = Jsons.deserialize( """ { "field1": "1", @@ -180,12 +182,12 @@ void testGetOAuthInputConfiguration() { } """); - List pathsToGet = List.of( - "$.field1", - "$.field3.field3_1", - "$.field3.field3_2"); + final Map pathsToGet = Map.ofEntries( + Map.entry("field1", "$.field1"), + Map.entry("field3_1", "$.field3.field3_1"), + Map.entry("field3_2", "$.field3.field3_2")); - JsonNode expected = Jsons.deserialize( + final JsonNode expected = Jsons.deserialize( """ { "field1": "1", @@ -199,7 +201,7 @@ void testGetOAuthInputConfiguration() { @Test void testGetOauthFromDBIfNeeded() { - JsonNode fromInput = Jsons.deserialize( + final JsonNode fromInput = Jsons.deserialize( """ { "testMask": "**********", @@ -207,7 +209,7 @@ void testGetOauthFromDBIfNeeded() { } """); - JsonNode fromDb = Jsons.deserialize( + final JsonNode fromDb = Jsons.deserialize( """ { "testMask": "mask", @@ -215,7 +217,7 @@ void testGetOauthFromDBIfNeeded() { } """); - JsonNode expected = Jsons.deserialize( + final JsonNode expected = Jsons.deserialize( """ { "testMask": "mask", diff --git a/airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java b/airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java index 0ef027cb5cf15..05c7615f40817 100644 --- a/airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java +++ b/airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java @@ -9,6 +9,7 @@ import io.airbyte.server.handlers.helpers.OAuthPathExtractor; import java.util.List; import org.assertj.core.api.Assertions; +import java.util.Map; import org.junit.jupiter.api.Test; class OAuthPathExtractorTest { @@ -32,12 +33,12 @@ void testExtract() { } """); - final List> expected = List.of( - List.of("tenant_id"), - List.of("another", "property")); + final Map> expected = Map.ofEntries( + Map.entry("tenant_id", List.of("tenant_id")), + Map.entry("another_property", List.of("another", "property"))); Assertions.assertThat(OAuthPathExtractor.extractOauthConfigurationPaths(input)) - .containsAll(expected); + .containsExactlyInAnyOrderEntriesOf(expected); } } From 46f00547a2e14b4a8adf384e0002c60836762a2d Mon Sep 17 00:00:00 2001 From: Benoit Moriceau Date: Tue, 15 Nov 2022 14:25:41 -0800 Subject: [PATCH 12/12] Format --- .../src/main/java/io/airbyte/server/handlers/OAuthHandler.java | 1 - .../airbyte/server/handlers/helper/OAuthPathExtractorTest.java | 2 +- 2 files changed, 1 insertion(+), 2 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 index 31454d2b11703..8dbe83913034a 100644 --- a/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java +++ b/airbyte-server/src/main/java/io/airbyte/server/handlers/OAuthHandler.java @@ -6,7 +6,6 @@ import com.fasterxml.jackson.databind.JsonNode; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.Iterables; import io.airbyte.analytics.TrackingClient; import io.airbyte.api.model.generated.CompleteDestinationOAuthRequest; import io.airbyte.api.model.generated.CompleteSourceOauthRequest; diff --git a/airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java b/airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java index 05c7615f40817..e7c7d95f83080 100644 --- a/airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java +++ b/airbyte-server/src/test/java/io/airbyte/server/handlers/helper/OAuthPathExtractorTest.java @@ -8,8 +8,8 @@ import io.airbyte.commons.json.Jsons; import io.airbyte.server.handlers.helpers.OAuthPathExtractor; import java.util.List; -import org.assertj.core.api.Assertions; import java.util.Map; +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; class OAuthPathExtractorTest {