From a50073e3e78b013a16deeab4e0562681cc155f28 Mon Sep 17 00:00:00 2001 From: "Sherif A. Nada" Date: Mon, 13 Sep 2021 08:36:39 -0700 Subject: [PATCH] GAds oauth demo (#5996) --- airbyte-api/src/main/openapi/config.yaml | 92 +++++++------- airbyte-oauth/build.gradle | 1 - .../oauth/OAuthImplementationFactory.java | 5 +- .../oauth/google/GoogleAdsOauthFlow.java | 48 +++++++ .../google/GoogleAnalyticsOauthFlow.java | 35 ++++++ .../oauth/{ => google}/GoogleOAuthFlow.java | 119 +++++++++++------- .../GoogleOAuthFlowIntegrationTest.java | 4 +- .../{ => google}/GoogleOAuthFlowTest.java | 13 +- .../job_factory/OAuthConfigSupplier.java | 32 ++++- .../job_factory/OAuthConfigSupplierTest.java | 100 ++++++++++++++- 10 files changed, 342 insertions(+), 107 deletions(-) create mode 100644 airbyte-oauth/src/main/java/io/airbyte/oauth/google/GoogleAdsOauthFlow.java create mode 100644 airbyte-oauth/src/main/java/io/airbyte/oauth/google/GoogleAnalyticsOauthFlow.java rename airbyte-oauth/src/main/java/io/airbyte/oauth/{ => google}/GoogleOAuthFlow.java (68%) rename airbyte-oauth/src/test/java/io/airbyte/oauth/{ => google}/GoogleOAuthFlowTest.java (94%) diff --git a/airbyte-api/src/main/openapi/config.yaml b/airbyte-api/src/main/openapi/config.yaml index 084f2ddb3ae0..583a14909088 100644 --- a/airbyte-api/src/main/openapi/config.yaml +++ b/airbyte-api/src/main/openapi/config.yaml @@ -1184,6 +1184,29 @@ paths: $ref: "#/components/responses/NotFoundResponse" "422": $ref: "#/components/responses/InvalidInputResponse" + /v1/source_oauths/oauth_params/create: + post: + tags: + - oauth + summary: > + Sets instancewide variables to be used for the oauth flow when creating this source. When set, these variables will be injected + into a connector's configuration before any interaction with the connector image itself. This enables running oauth flows with + consistent variables e.g: the company's Google Ads developer_token, client_id, and client_secret without the user having to know + about these variables. + operationId: setInstancewideSourceOauthParams + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/SetInstancewideSourceOauthParamsRequestBody" + required: true + responses: + "200": + description: Successful + "400": + $ref: "#/components/responses/ExceptionResponse" + "404": + $ref: "#/components/responses/NotFoundResponse" /v1/source_oauths/get_consent_url: post: tags: @@ -1276,6 +1299,29 @@ paths: $ref: "#/components/responses/NotFoundResponse" "422": $ref: "#/components/responses/InvalidInputResponse" + /v1/destination_oauths/oauth_params/create: + post: + tags: + - oauth + summary: > + Sets instancewide variables to be used for the oauth flow when creating this destination. When set, these variables will be injected + into a connector's configuration before any interaction with the connector image itself. This enables running oauth flows with + consistent variables e.g: the company's Google Ads developer_token, client_id, and client_secret without the user having to know + about these variables. + operationId: setInstancewideDestinationOauthParams + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/SetInstancewideDestinationOauthParamsRequestBody" + required: true + responses: + "200": + description: Successful + "400": + $ref: "#/components/responses/ExceptionResponse" + "404": + $ref: "#/components/responses/NotFoundResponse" /v1/web_backend/connections/list: post: tags: @@ -1620,52 +1666,6 @@ paths: $ref: "#/components/schemas/ImportRead" "404": $ref: "#/components/responses/NotFoundResponse" - /v1/source_oauths/oauth_params/create: - post: - tags: - - oauth - summary: > - Sets instancewide variables to be used for the oauth flow when creating this source. When set, these variables will be injected - into a connector's configuration before any interaction with the connector image itself. This enables running oauth flows with - consistent variables e.g: the company's Google Ads developer_token, client_id, and client_secret without the user having to know - about these variables. - operationId: setInstancewideSourceOauthParams - requestBody: - content: - application/json: - schema: - $ref: "#/components/schemas/SetInstancewideSourceOauthParamsRequestBody" - required: true - responses: - "200": - description: Successful - "400": - $ref: "#/components/responses/ExceptionResponse" - "404": - $ref: "#/components/responses/NotFoundResponse" - /v1/destination_oauths/oauth_params/create: - post: - tags: - - oauth - summary: > - Sets instancewide variables to be used for the oauth flow when creating this destination. When set, these variables will be injected - into a connector's configuration before any interaction with the connector image itself. This enables running oauth flows with - consistent variables e.g: the company's Google Ads developer_token, client_id, and client_secret without the user having to know - about these variables. - operationId: setInstancewideDestinationOauthParams - requestBody: - content: - application/json: - schema: - $ref: "#/components/schemas/SetInstancewideDestinationOauthParamsRequestBody" - required: true - responses: - "200": - description: Successful - "400": - $ref: "#/components/responses/ExceptionResponse" - "404": - $ref: "#/components/responses/NotFoundResponse" components: securitySchemes: bearerAuth: diff --git a/airbyte-oauth/build.gradle b/airbyte-oauth/build.gradle index 51d9c0b24bbc..a5b74ae07a95 100644 --- a/airbyte-oauth/build.gradle +++ b/airbyte-oauth/build.gradle @@ -7,6 +7,5 @@ dependencies { implementation project(':airbyte-config:models') implementation project(':airbyte-config:persistence') implementation project(':airbyte-json-validation') - testImplementation project(':airbyte-oauth') } diff --git a/airbyte-oauth/src/main/java/io/airbyte/oauth/OAuthImplementationFactory.java b/airbyte-oauth/src/main/java/io/airbyte/oauth/OAuthImplementationFactory.java index 9ceea20c566a..645b5296b243 100644 --- a/airbyte-oauth/src/main/java/io/airbyte/oauth/OAuthImplementationFactory.java +++ b/airbyte-oauth/src/main/java/io/airbyte/oauth/OAuthImplementationFactory.java @@ -26,6 +26,8 @@ import com.google.common.collect.ImmutableMap; import io.airbyte.config.persistence.ConfigRepository; +import io.airbyte.oauth.google.GoogleAdsOauthFlow; +import io.airbyte.oauth.google.GoogleAnalyticsOauthFlow; import java.util.Map; public class OAuthImplementationFactory { @@ -34,7 +36,8 @@ public class OAuthImplementationFactory { public OAuthImplementationFactory(ConfigRepository configRepository) { OAUTH_FLOW_MAPPING = ImmutableMap.builder() - .put("airbyte/source-google-analytics-v4", new GoogleOAuthFlow(configRepository)) + .put("airbyte/source-google-analytics-v4", new GoogleAnalyticsOauthFlow(configRepository)) + .put("airbyte/source-google-ads", new GoogleAdsOauthFlow(configRepository)) .build(); } diff --git a/airbyte-oauth/src/main/java/io/airbyte/oauth/google/GoogleAdsOauthFlow.java b/airbyte-oauth/src/main/java/io/airbyte/oauth/google/GoogleAdsOauthFlow.java new file mode 100644 index 000000000000..28c79a3f8f70 --- /dev/null +++ b/airbyte-oauth/src/main/java/io/airbyte/oauth/google/GoogleAdsOauthFlow.java @@ -0,0 +1,48 @@ +/* + * MIT License + * + * Copyright (c) 2020 Airbyte + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package io.airbyte.oauth.google; + +import com.fasterxml.jackson.databind.JsonNode; +import io.airbyte.config.persistence.ConfigRepository; + +public class GoogleAdsOauthFlow extends GoogleOAuthFlow { + + public GoogleAdsOauthFlow(ConfigRepository configRepository) { + super(configRepository, "https://www.googleapis.com/auth/adwords"); + } + + @Override + protected String getClientIdUnsafe(JsonNode config) { + // the config object containing client ID and secret is nested inside the "credentials" object + return super.getClientIdUnsafe(config.get("credentials")); + } + + @Override + protected String getClientSecretUnsafe(JsonNode config) { + // the config object containing client ID and secret is nested inside the "credentials" object + return super.getClientSecretUnsafe(config.get("credentials")); + } + +} diff --git a/airbyte-oauth/src/main/java/io/airbyte/oauth/google/GoogleAnalyticsOauthFlow.java b/airbyte-oauth/src/main/java/io/airbyte/oauth/google/GoogleAnalyticsOauthFlow.java new file mode 100644 index 000000000000..7f7ba6ce9727 --- /dev/null +++ b/airbyte-oauth/src/main/java/io/airbyte/oauth/google/GoogleAnalyticsOauthFlow.java @@ -0,0 +1,35 @@ +/* + * MIT License + * + * Copyright (c) 2020 Airbyte + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package io.airbyte.oauth.google; + +import io.airbyte.config.persistence.ConfigRepository; + +public class GoogleAnalyticsOauthFlow extends GoogleOAuthFlow { + + public GoogleAnalyticsOauthFlow(ConfigRepository configRepository) { + super(configRepository, "https://www.googleapis.com/auth/analytics.readonly"); + } + +} diff --git a/airbyte-oauth/src/main/java/io/airbyte/oauth/GoogleOAuthFlow.java b/airbyte-oauth/src/main/java/io/airbyte/oauth/google/GoogleOAuthFlow.java similarity index 68% rename from airbyte-oauth/src/main/java/io/airbyte/oauth/GoogleOAuthFlow.java rename to airbyte-oauth/src/main/java/io/airbyte/oauth/google/GoogleOAuthFlow.java index ef40b7e92dc4..ad3a6c06b8d4 100644 --- a/airbyte-oauth/src/main/java/io/airbyte/oauth/GoogleOAuthFlow.java +++ b/airbyte-oauth/src/main/java/io/airbyte/oauth/google/GoogleOAuthFlow.java @@ -22,7 +22,7 @@ * SOFTWARE. */ -package io.airbyte.oauth; +package io.airbyte.oauth.google; import com.fasterxml.jackson.databind.JsonNode; import com.google.common.annotations.VisibleForTesting; @@ -34,13 +34,17 @@ import io.airbyte.config.SourceOAuthParameter; import io.airbyte.config.persistence.ConfigNotFoundException; import io.airbyte.config.persistence.ConfigRepository; +import io.airbyte.oauth.MoreOAuthParameters; +import io.airbyte.oauth.OAuthFlowImplementation; import io.airbyte.validation.json.JsonValidationException; import java.io.IOException; import java.net.URI; +import java.net.URLEncoder; import java.net.http.HttpClient; import java.net.http.HttpClient.Version; import java.net.http.HttpRequest; import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; import java.util.Optional; @@ -53,59 +57,53 @@ public class GoogleOAuthFlow implements OAuthFlowImplementation { private final HttpClient httpClient; - private static final String GOOGLE_ANALYTICS_CONSENT_URL = "https://accounts.google.com/o/oauth2/v2/auth"; - private static final String GOOGLE_ANALYTICS_ACCESS_TOKEN_URL = "https://oauth2.googleapis.com/token"; - @VisibleForTesting - static final String GOOGLE_ANALYTICS_SCOPE = "https%3A//www.googleapis.com/auth/analytics.readonly"; - private static final List GOOGLE_QUERY_PARAMETERS = List.of( - String.format("scope=%s", GOOGLE_ANALYTICS_SCOPE), - "access_type=offline", - "include_granted_scopes=true", - "response_type=code", - "prompt=consent"); + private final static String CONSENT_URL = "https://accounts.google.com/o/oauth2/v2/auth"; + private final static String ACCESS_TOKEN_URL = "https://oauth2.googleapis.com/token"; + + private final String scope; + private final List googleQueryParameters; private final ConfigRepository configRepository; - public GoogleOAuthFlow(ConfigRepository configRepository) { - this(configRepository, HttpClient.newBuilder().version(Version.HTTP_1_1).build()); + public GoogleOAuthFlow(ConfigRepository configRepository, String scope) { + this(configRepository, scope, HttpClient.newBuilder().version(Version.HTTP_1_1).build()); } @VisibleForTesting - GoogleOAuthFlow(ConfigRepository configRepository, HttpClient httpClient) { + GoogleOAuthFlow(ConfigRepository configRepository, String scope, HttpClient httpClient) { this.configRepository = configRepository; this.httpClient = httpClient; + this.scope = UrlEncode(scope); + this.googleQueryParameters = List.of( + String.format("scope=%s", this.scope), + "access_type=offline", + "include_granted_scopes=true", + "response_type=code", + "prompt=consent"); } @Override public String getSourceConsentUrl(UUID workspaceId, UUID sourceDefinitionId, String redirectUrl) throws IOException, ConfigNotFoundException { final JsonNode oAuthParamConfig = getSourceOAuthParamConfig(workspaceId, sourceDefinitionId); - if (oAuthParamConfig.has("client_id")) { - final String clientId = oAuthParamConfig.get("client_id").asText(); - return getConsentUrl(sourceDefinitionId, clientId, redirectUrl); - } else { - throw new IOException("Undefined parameter 'client_id' for Google OAuth Flow."); - } + return getConsentUrl(sourceDefinitionId, getClientIdUnsafe(oAuthParamConfig), redirectUrl); } @Override public String getDestinationConsentUrl(UUID workspaceId, UUID destinationDefinitionId, String redirectUrl) throws IOException, ConfigNotFoundException { final JsonNode oAuthParamConfig = getDestinationOAuthParamConfig(workspaceId, destinationDefinitionId); - if (oAuthParamConfig.has("client_id")) { - final String clientId = oAuthParamConfig.get("client_id").asText(); - return getConsentUrl(destinationDefinitionId, clientId, redirectUrl); - } else { - throw new IOException("Undefined parameter 'client_id' for Google OAuth Flow."); - } + return getConsentUrl(destinationDefinitionId, getClientIdUnsafe(oAuthParamConfig), redirectUrl); } - private static String getConsentUrl(UUID definitionId, String clientId, String redirectUrl) { - final StringBuilder result = new StringBuilder(GOOGLE_ANALYTICS_CONSENT_URL) + private String getConsentUrl(UUID definitionId, String clientId, String redirectUrl) { + final StringBuilder result = new StringBuilder(CONSENT_URL) .append("?"); - for (String queryParameter : GOOGLE_QUERY_PARAMETERS) { + for (String queryParameter : googleQueryParameters) { result.append(queryParameter).append("&"); } return result + // TODO state should be randomly generated, and the 2nd step of oauth should verify its value + // matches the initially generated state value .append("state=").append(definitionId.toString()).append("&") .append("client_id=").append(clientId).append("&") .append("redirect_uri=").append(redirectUrl) @@ -118,13 +116,9 @@ public Map completeSourceOAuth(UUID workspaceId, UUID sourceDefi if (queryParams.containsKey("code")) { final String code = (String) queryParams.get("code"); final JsonNode oAuthParamConfig = getSourceOAuthParamConfig(workspaceId, sourceDefinitionId); - if (oAuthParamConfig.has("client_id") && oAuthParamConfig.has("client_secret")) { - final String clientId = oAuthParamConfig.get("client_id").asText(); - final String clientSecret = oAuthParamConfig.get("client_secret").asText(); - return completeOAuthFlow(clientId, clientSecret, code, redirectUrl); - } else { - throw new IOException("Undefined parameter 'client_id' and 'client_secret' for Google OAuth Flow."); - } + final String clientId = getClientIdUnsafe(oAuthParamConfig); + final String clientSecret = getClientSecretUnsafe(oAuthParamConfig); + return completeOAuthFlow(clientId, clientSecret, code, redirectUrl); } else { throw new IOException("Undefined 'code' from consent redirected url."); } @@ -139,13 +133,9 @@ public Map completeDestinationOAuth(UUID workspaceId, if (queryParams.containsKey("code")) { final String code = (String) queryParams.get("code"); final JsonNode oAuthParamConfig = getDestinationOAuthParamConfig(workspaceId, destinationDefinitionId); - if (oAuthParamConfig.has("client_id") && oAuthParamConfig.has("client_secret")) { - final String clientId = oAuthParamConfig.get("client_id").asText(); - final String clientSecret = oAuthParamConfig.get("client_secret").asText(); - return completeOAuthFlow(clientId, clientSecret, code, redirectUrl); - } else { - throw new IOException("Undefined parameter 'client_id' and 'client_secret' for Google OAuth Flow."); - } + final String clientId = getClientIdUnsafe(oAuthParamConfig); + final String clientSecret = getClientSecretUnsafe(oAuthParamConfig); + return completeOAuthFlow(clientId, clientSecret, code, redirectUrl); } else { throw new IOException("Undefined 'code' from consent redirected url."); } @@ -161,7 +151,7 @@ private Map completeOAuthFlow(String clientId, String clientSecr .build(); final HttpRequest request = HttpRequest.newBuilder() .POST(HttpRequest.BodyPublishers.ofString(toUrlEncodedString(body))) - .uri(URI.create(GOOGLE_ANALYTICS_ACCESS_TOKEN_URL)) + .uri(URI.create(ACCESS_TOKEN_URL)) .header("Content-Type", "application/x-www-form-urlencoded") .build(); final HttpResponse response; @@ -171,7 +161,10 @@ private Map completeOAuthFlow(String clientId, String clientSecr if (data.has("refresh_token")) { return Map.of("refresh_token", data.get("refresh_token").asText()); } else { - throw new IOException(String.format("Missing 'refresh_token' in query params from %s", GOOGLE_ANALYTICS_ACCESS_TOKEN_URL)); + // TODO This means the response from Google did not have a refresh token and is probably a + // programming error + // handle this better + throw new IOException(String.format("Missing 'refresh_token' in query params from %s. Response: %s", ACCESS_TOKEN_URL, data)); } } catch (InterruptedException e) { throw new IOException("Failed to complete Google OAuth flow", e); @@ -217,4 +210,40 @@ private JsonNode getDestinationOAuthParamConfig(UUID workspaceId, UUID destinati } } + private String UrlEncode(String s) { + try { + return URLEncoder.encode(s, StandardCharsets.UTF_8); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + /** + * Throws an exception if the client ID cannot be extracted. Subclasses should override this to + * parse the config differently. + * + * @return + */ + protected String getClientIdUnsafe(JsonNode oauthConfig) { + if (oauthConfig.get("client_id") != null) { + return oauthConfig.get("client_id").asText(); + } else { + throw new IllegalArgumentException("Undefined parameter 'client_id' for Google OAuth Flow."); + } + } + + /** + * Throws an exception if the client secret cannot be extracted. Subclasses should override this to + * parse the config differently. + * + * @return + */ + protected String getClientSecretUnsafe(JsonNode oauthConfig) { + if (oauthConfig.get("client_secret") != null) { + return oauthConfig.get("client_secret").asText(); + } else { + throw new IllegalArgumentException("Undefined parameter 'client_secret' for Google OAuth Flow."); + } + } + } diff --git a/airbyte-oauth/src/test-integration/java/io.airbyte.oauth/GoogleOAuthFlowIntegrationTest.java b/airbyte-oauth/src/test-integration/java/io.airbyte.oauth/GoogleOAuthFlowIntegrationTest.java index 4b843866ddae..55ee5f99e91d 100644 --- a/airbyte-oauth/src/test-integration/java/io.airbyte.oauth/GoogleOAuthFlowIntegrationTest.java +++ b/airbyte-oauth/src/test-integration/java/io.airbyte.oauth/GoogleOAuthFlowIntegrationTest.java @@ -37,6 +37,8 @@ import io.airbyte.config.SourceOAuthParameter; import io.airbyte.config.persistence.ConfigNotFoundException; import io.airbyte.config.persistence.ConfigRepository; +import io.airbyte.oauth.google.GoogleAnalyticsOauthFlow; +import io.airbyte.oauth.google.GoogleOAuthFlow; import io.airbyte.validation.json.JsonValidationException; import java.io.IOException; import java.io.OutputStream; @@ -71,7 +73,7 @@ public void setup() throws IOException { "Must provide path to a oauth credentials file."); } configRepository = mock(ConfigRepository.class); - googleOAuthFlow = new GoogleOAuthFlow(configRepository); + googleOAuthFlow = new GoogleAnalyticsOauthFlow(configRepository); server = HttpServer.create(new InetSocketAddress(80), 0); server.setExecutor(null); // creates a default executor diff --git a/airbyte-oauth/src/test/java/io/airbyte/oauth/GoogleOAuthFlowTest.java b/airbyte-oauth/src/test/java/io/airbyte/oauth/google/GoogleOAuthFlowTest.java similarity index 94% rename from airbyte-oauth/src/test/java/io/airbyte/oauth/GoogleOAuthFlowTest.java rename to airbyte-oauth/src/test/java/io/airbyte/oauth/google/GoogleOAuthFlowTest.java index 9a7aac41ef24..50d5319d5372 100644 --- a/airbyte-oauth/src/test/java/io/airbyte/oauth/GoogleOAuthFlowTest.java +++ b/airbyte-oauth/src/test/java/io/airbyte/oauth/google/GoogleOAuthFlowTest.java @@ -22,7 +22,7 @@ * SOFTWARE. */ -package io.airbyte.oauth; +package io.airbyte.oauth.google; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -56,6 +56,7 @@ public class GoogleOAuthFlowTest { private static final Logger LOGGER = LoggerFactory.getLogger(GoogleOAuthFlowTest.class); private static final Path CREDENTIALS_PATH = Path.of("secrets/credentials.json"); private static final String REDIRECT_URL = "https%3A//airbyte.io"; + private static final String SCOPE = "https%3A//www.googleapis.com/auth/analytics.readonly"; private HttpClient httpClient; private ConfigRepository configRepository; @@ -68,7 +69,7 @@ public class GoogleOAuthFlowTest { public void setup() { httpClient = mock(HttpClient.class); configRepository = mock(ConfigRepository.class); - googleOAuthFlow = new GoogleOAuthFlow(configRepository, httpClient); + googleOAuthFlow = new GoogleOAuthFlow(configRepository, SCOPE, httpClient); workspaceId = UUID.randomUUID(); definitionId = UUID.randomUUID(); @@ -92,8 +93,8 @@ public void testGetConsentUrlIncompleteOAuthParameters() throws IOException, Jso .withDestinationDefinitionId(definitionId) .withWorkspaceId(workspaceId) .withConfiguration(Jsons.emptyObject()))); - assertThrows(IOException.class, () -> googleOAuthFlow.getSourceConsentUrl(workspaceId, definitionId, REDIRECT_URL)); - assertThrows(IOException.class, () -> googleOAuthFlow.getDestinationConsentUrl(workspaceId, definitionId, REDIRECT_URL)); + assertThrows(IllegalArgumentException.class, () -> googleOAuthFlow.getSourceConsentUrl(workspaceId, definitionId, REDIRECT_URL)); + assertThrows(IllegalArgumentException.class, () -> googleOAuthFlow.getDestinationConsentUrl(workspaceId, definitionId, REDIRECT_URL)); } @Test @@ -108,7 +109,7 @@ public void testGetSourceConsentUrl() throws IOException, ConfigNotFoundExceptio final String actualSourceUrl = googleOAuthFlow.getSourceConsentUrl(workspaceId, definitionId, REDIRECT_URL); final String expectedSourceUrl = String.format( "https://accounts.google.com/o/oauth2/v2/auth?scope=%s&access_type=offline&include_granted_scopes=true&response_type=code&prompt=consent&state=%s&client_id=%s&redirect_uri=%s", - GoogleOAuthFlow.GOOGLE_ANALYTICS_SCOPE, + SCOPE, definitionId, getClientId(), REDIRECT_URL); @@ -128,7 +129,7 @@ public void testGetDestinationConsentUrl() throws IOException, ConfigNotFoundExc final String actualDestinationUrl = googleOAuthFlow.getDestinationConsentUrl(workspaceId, definitionId, REDIRECT_URL); final String expectedDestinationUrl = String.format( "https://accounts.google.com/o/oauth2/v2/auth?scope=%s&access_type=offline&include_granted_scopes=true&response_type=code&prompt=consent&state=%s&client_id=%s&redirect_uri=%s", - GoogleOAuthFlow.GOOGLE_ANALYTICS_SCOPE, + SCOPE, definitionId, getClientId(), REDIRECT_URL); diff --git a/airbyte-scheduler/persistence/src/main/java/io/airbyte/scheduler/persistence/job_factory/OAuthConfigSupplier.java b/airbyte-scheduler/persistence/src/main/java/io/airbyte/scheduler/persistence/job_factory/OAuthConfigSupplier.java index 2c4e0bb1e6a3..6f47d1f75a4a 100644 --- a/airbyte-scheduler/persistence/src/main/java/io/airbyte/scheduler/persistence/job_factory/OAuthConfigSupplier.java +++ b/airbyte-scheduler/persistence/src/main/java/io/airbyte/scheduler/persistence/job_factory/OAuthConfigSupplier.java @@ -24,8 +24,11 @@ package io.airbyte.scheduler.persistence.job_factory; +import static com.fasterxml.jackson.databind.node.JsonNodeType.OBJECT; + import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import io.airbyte.commons.json.Jsons; import io.airbyte.config.persistence.ConfigRepository; @@ -48,6 +51,8 @@ public OAuthConfigSupplier(ConfigRepository configRepository, boolean maskSecret public JsonNode injectSourceOAuthParameters(UUID sourceDefinitionId, UUID workspaceId, JsonNode sourceConnectorConfig) throws IOException { try { + // TODO there will be cases where we shouldn't write oauth params. See + // https://github.com/airbytehq/airbyte/issues/5989 MoreOAuthParameters.getSourceOAuthParameter(configRepository.listSourceOAuthParam().stream(), workspaceId, sourceDefinitionId) .ifPresent( sourceOAuthParameter -> injectJsonNode((ObjectNode) sourceConnectorConfig, (ObjectNode) sourceOAuthParameter.getConfiguration())); @@ -69,15 +74,32 @@ public JsonNode injectDestinationOAuthParameters(UUID destinationDefinitionId, U } } - private void injectJsonNode(ObjectNode config, ObjectNode fromConfig) { + @VisibleForTesting + void injectJsonNode(ObjectNode mainConfig, ObjectNode fromConfig) { + // TODO this method might make sense to have as a general utility in Jsons for (String key : Jsons.keys(fromConfig)) { - if (maskSecrets) { - config.set(key, Jsons.jsonNode(SECRET_MASK)); + if (fromConfig.get(key).getNodeType() == OBJECT) { + // nested objects are merged rather than overwrite the contents of the equivalent object in config + if (mainConfig.get(key) == null) { + injectJsonNode(mainConfig.putObject(key), (ObjectNode) fromConfig.get(key)); + } else if (mainConfig.get(key).getNodeType() == OBJECT) { + injectJsonNode((ObjectNode) mainConfig.get(key), (ObjectNode) fromConfig.get(key)); + } else { + throw new IllegalStateException("Can't merge an object node into a non-object node!"); + } } else { - if (!config.has(key) || isSecretMask(config.get(key).asText())) { - config.set(key, fromConfig.get(key)); + if (maskSecrets) { + // TODO secrets should be masked with the correct type + // https://github.com/airbytehq/airbyte/issues/5990 + // In the short-term this is not world-ending as all secret fields are currently strings + mainConfig.set(key, Jsons.jsonNode(SECRET_MASK)); + } else { + if (!mainConfig.has(key) || isSecretMask(mainConfig.get(key).asText())) { + mainConfig.set(key, fromConfig.get(key)); + } } } + } } diff --git a/airbyte-scheduler/persistence/src/test/java/io/airbyte/scheduler/persistence/job_factory/OAuthConfigSupplierTest.java b/airbyte-scheduler/persistence/src/test/java/io/airbyte/scheduler/persistence/job_factory/OAuthConfigSupplierTest.java index 1e87cea74538..e82ec1e7a328 100644 --- a/airbyte-scheduler/persistence/src/test/java/io/airbyte/scheduler/persistence/job_factory/OAuthConfigSupplierTest.java +++ b/airbyte-scheduler/persistence/src/test/java/io/airbyte/scheduler/persistence/job_factory/OAuthConfigSupplierTest.java @@ -29,6 +29,7 @@ import static org.mockito.Mockito.when; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.JsonNodeType; import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.collect.ImmutableMap; import io.airbyte.commons.json.Jsons; @@ -40,6 +41,7 @@ import java.util.Map; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; public class OAuthConfigSupplierTest { @@ -142,8 +144,8 @@ void testInjectMaskedOAuthParameters() throws JsonValidationException, IOExcepti assertEquals(expectedConfig, actualConfig); } - private JsonNode generateJsonConfig() { - return Jsons.jsonNode(ImmutableMap.builder() + private ObjectNode generateJsonConfig() { + return (ObjectNode) Jsons.jsonNode(ImmutableMap.builder() .put("apiSecret", "123") .put("client", "testing") .build()); @@ -156,4 +158,98 @@ private Map generateOAuthParameters() { .build(); } + private void maskAllValues(ObjectNode node) { + for (String key : Jsons.keys(node)) { + if (node.get(key).getNodeType() == JsonNodeType.OBJECT) { + maskAllValues((ObjectNode) node.get(key)); + } else { + node.set(key, Jsons.jsonNode(OAuthConfigSupplier.SECRET_MASK)); + } + } + } + + @Test + void testInjectUnnestedNode_Masked() { + OAuthConfigSupplier supplier = new OAuthConfigSupplier(configRepository, true); + ObjectNode oauthParams = (ObjectNode) Jsons.jsonNode(generateOAuthParameters()); + ObjectNode maskedOauthParams = Jsons.clone(oauthParams); + maskAllValues(maskedOauthParams); + ObjectNode actual = generateJsonConfig(); + ObjectNode expected = Jsons.clone(actual); + expected.setAll(maskedOauthParams); + + supplier.injectJsonNode(actual, oauthParams); + assertEquals(expected, actual); + } + + @Test + void testInjectUnnestedNode_Unmasked() { + OAuthConfigSupplier supplier = new OAuthConfigSupplier(configRepository, false); + ObjectNode oauthParams = (ObjectNode) Jsons.jsonNode(generateOAuthParameters()); + + ObjectNode actual = generateJsonConfig(); + ObjectNode expected = Jsons.clone(actual); + expected.setAll(oauthParams); + + supplier.injectJsonNode(actual, oauthParams); + + assertEquals(expected, actual); + } + + @Test + void testInjectNewNestedNode_Masked() { + OAuthConfigSupplier supplier = new OAuthConfigSupplier(configRepository, true); + ObjectNode oauthParams = (ObjectNode) Jsons.jsonNode(generateOAuthParameters()); + ObjectNode maskedOauthParams = Jsons.clone(oauthParams); + maskAllValues(maskedOauthParams); + ObjectNode nestedConfig = (ObjectNode) Jsons.jsonNode(ImmutableMap.builder() + .put("oauth_credentials", oauthParams) + .build()); + + // nested node does not exist in actual object + ObjectNode actual = generateJsonConfig(); + ObjectNode expected = Jsons.clone(actual); + expected.putObject("oauth_credentials").setAll(maskedOauthParams); + + supplier.injectJsonNode(actual, nestedConfig); + assertEquals(expected, actual); + } + + @Test + @DisplayName("A nested config should be inserted with the same nesting structure") + void testInjectNewNestedNode_Unmasked() { + OAuthConfigSupplier supplier = new OAuthConfigSupplier(configRepository, false); + ObjectNode oauthParams = (ObjectNode) Jsons.jsonNode(generateOAuthParameters()); + ObjectNode nestedConfig = (ObjectNode) Jsons.jsonNode(ImmutableMap.builder() + .put("oauth_credentials", oauthParams) + .build()); + + // nested node does not exist in actual object + ObjectNode actual = generateJsonConfig(); + ObjectNode expected = Jsons.clone(actual); + expected.putObject("oauth_credentials").setAll(oauthParams); + + supplier.injectJsonNode(actual, nestedConfig); + assertEquals(expected, actual); + } + + @Test + @DisplayName("A nested node which partially exists in the main config should be merged into the main config, not overwrite the whole nested object") + void testInjectedPartiallyExistingNestedNode_Unmasked() { + OAuthConfigSupplier supplier = new OAuthConfigSupplier(configRepository, false); + ObjectNode oauthParams = (ObjectNode) Jsons.jsonNode(generateOAuthParameters()); + ObjectNode nestedConfig = (ObjectNode) Jsons.jsonNode(ImmutableMap.builder() + .put("oauth_credentials", oauthParams) + .build()); + + // nested node partially exists in actual object + ObjectNode actual = generateJsonConfig(); + actual.putObject("oauth_credentials").put("irrelevant_field", "_"); + ObjectNode expected = Jsons.clone(actual); + ((ObjectNode) expected.get("oauth_credentials")).setAll(oauthParams); + + supplier.injectJsonNode(actual, nestedConfig); + assertEquals(expected, actual); + } + }