Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement connector config dependency for OAuth consent URL #7983

Merged
merged 8 commits into from
Nov 17, 2021
Merged

Implement connector config dependency for OAuth consent URL #7983

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2a50ead
Refactor OAuth consent flow with new API
ChristopheDuong Nov 15, 2021
b381cfe
Use input connector configuration in getConsentURL for OAuth flow
ChristopheDuong Nov 15, 2021
2d699cb
Format template
ChristopheDuong Nov 15, 2021
cf0c8db
Merge remote-tracking branch 'origin/master' into oauth-consent-url
ChristopheDuong Nov 16, 2021
f6f457a
Remove deprecated methods for consent url
ChristopheDuong Nov 16, 2021
4ff0684
Tweaks tests to handle input configs
ChristopheDuong Nov 16, 2021
dcf9524
Add json validation tests on oauth flow
ChristopheDuong Nov 16, 2021
2a153bc
Instance-wide params should only be injected if a connector will be u…
ChristopheDuong Nov 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...e-integrations/connector-templates/source-python-http-api/integration_tests/acceptance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

@pytest.fixture(scope="session", autouse=True)
def connector_setup():
""" This fixture is a placeholder for external resources that acceptance test might require."""
"""This fixture is a placeholder for external resources that acceptance test might require."""
# TODO: setup test dependencies if needed. otherwise remove the TODO comments
yield
# TODO: clean up test dependencies
2 changes: 1 addition & 1 deletion airbyte-integrations/connector-templates/source-python/integration_tests/acceptance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

@pytest.fixture(scope="session", autouse=True)
def connector_setup():
""" This fixture is a placeholder for external resources that acceptance test might require."""
"""This fixture is a placeholder for external resources that acceptance test might require."""
# TODO: setup test dependencies
yield
# TODO: clean up test dependencies
2 changes: 1 addition & 1 deletion airbyte-integrations/connector-templates/source-singer/integration_tests/acceptance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

@pytest.fixture(scope="session", autouse=True)
def connector_setup():
""" This fixture is a placeholder for external resources that acceptance test might require."""
"""This fixture is a placeholder for external resources that acceptance test might require."""
# TODO: setup test dependencies
yield
# TODO: clean up test dependencies
58 changes: 53 additions & 5 deletions airbyte-oauth/src/main/java/io/airbyte/oauth/BaseOAuth2Flow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import io.airbyte.config.persistence.ConfigNotFoundException;
import io.airbyte.config.persistence.ConfigRepository;
import io.airbyte.protocol.models.OAuthConfigSpecification;
import io.airbyte.validation.json.JsonSchemaValidator;
import io.airbyte.validation.json.JsonValidationException;
import java.io.IOException;
import java.lang.reflect.Type;
import java.net.URI;
Expand Down Expand Up @@ -75,25 +77,61 @@ public BaseOAuth2Flow(final ConfigRepository configRepository,
}

@Override
@Deprecated
public String getSourceConsentUrl(final UUID workspaceId, final UUID sourceDefinitionId, final String redirectUrl)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to keep this method, or should it be removed in this PR? (not sure what depends on it so just asking, if easy to remove here probably better)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it and added test to make sure the ones that need to be backward compatible can still be used without authConfigSpecification

throws IOException, ConfigNotFoundException {
final JsonNode oAuthParamConfig = getSourceOAuthParamConfig(workspaceId, sourceDefinitionId);
return formatConsentUrl(sourceDefinitionId, getClientIdUnsafe(oAuthParamConfig), redirectUrl);
return formatConsentUrl(sourceDefinitionId, getClientIdUnsafe(oAuthParamConfig), redirectUrl, Jsons.emptyObject());
}

@Override
public String getSourceConsentUrl(final UUID workspaceId,
final UUID sourceDefinitionId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration,
final OAuthConfigSpecification oAuthConfigSpecification)
throws IOException, ConfigNotFoundException, JsonValidationException {
validateInputOAuthConfiguration(oAuthConfigSpecification, inputOAuthConfiguration);
final JsonNode oAuthParamConfig = getSourceOAuthParamConfig(workspaceId, sourceDefinitionId);
return formatConsentUrl(sourceDefinitionId, getClientIdUnsafe(oAuthParamConfig), redirectUrl, inputOAuthConfiguration);
}

@Override
@Deprecated
public String getDestinationConsentUrl(final UUID workspaceId, final UUID destinationDefinitionId, final String redirectUrl)
throws IOException, ConfigNotFoundException {
final JsonNode oAuthParamConfig = getDestinationOAuthParamConfig(workspaceId, destinationDefinitionId);
return formatConsentUrl(destinationDefinitionId, getClientIdUnsafe(oAuthParamConfig), redirectUrl);
return formatConsentUrl(destinationDefinitionId, getClientIdUnsafe(oAuthParamConfig), redirectUrl, Jsons.emptyObject());
}

@Override
public String getDestinationConsentUrl(final UUID workspaceId,
final UUID destinationDefinitionId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration,
final OAuthConfigSpecification oAuthConfigSpecification)
throws IOException, ConfigNotFoundException, JsonValidationException {
validateInputOAuthConfiguration(oAuthConfigSpecification, inputOAuthConfiguration);
final JsonNode oAuthParamConfig = getDestinationOAuthParamConfig(workspaceId, destinationDefinitionId);
return formatConsentUrl(destinationDefinitionId, getClientIdUnsafe(oAuthParamConfig), redirectUrl, inputOAuthConfiguration);
}

/**
* Depending on the OAuth flow implementation, the URL to grant user's consent may differ,
* especially in the query parameters to be provided. This function should generate such consent URL
* accordingly.
*
* @param definitionId The configured definition ID of this client
* @param clientId The configured client ID
* @param redirectUrl the redirect URL
* @param inputOAuthConfiguration any configuration property from connector necessary for this OAuth
* Flow
*/
protected abstract String formatConsentUrl(UUID definitionId, String clientId, String redirectUrl) throws IOException;
protected abstract String formatConsentUrl(final UUID definitionId,
final String clientId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration)
throws IOException;

private static String generateRandomState() {
return RandomStringUtils.randomAlphanumeric(7);
Expand Down Expand Up @@ -151,7 +189,8 @@ public Map<String, Object> completeSourceOAuth(final UUID workspaceId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration,
final OAuthConfigSpecification oAuthConfigSpecification)
throws IOException, ConfigNotFoundException {
throws IOException, ConfigNotFoundException, JsonValidationException {
validateInputOAuthConfiguration(oAuthConfigSpecification, inputOAuthConfiguration);
final JsonNode oAuthParamConfig = getDestinationOAuthParamConfig(workspaceId, sourceDefinitionId);
return formatOAuthOutput(
oAuthParamConfig,
Expand All @@ -171,7 +210,8 @@ public Map<String, Object> completeDestinationOAuth(final UUID workspaceId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration,
final OAuthConfigSpecification oAuthConfigSpecification)
throws IOException, ConfigNotFoundException {
throws IOException, ConfigNotFoundException, JsonValidationException {
validateInputOAuthConfiguration(oAuthConfigSpecification, inputOAuthConfiguration);
final JsonNode oAuthParamConfig = getDestinationOAuthParamConfig(workspaceId, destinationDefinitionId);
return formatOAuthOutput(
oAuthParamConfig,
Expand Down Expand Up @@ -260,6 +300,14 @@ public List<String> getDefaultOAuthOutputPath() {
return List.of("credentials");
}

private static void validateInputOAuthConfiguration(final OAuthConfigSpecification oauthConfigSpecification, final JsonNode inputOAuthConfiguration)
throws JsonValidationException {
if (oauthConfigSpecification.getOauthUserInputFromConnectorConfigSpecification() != null) {
final JsonSchemaValidator validator = new JsonSchemaValidator();
validator.ensure(oauthConfigSpecification.getOauthUserInputFromConnectorConfigSpecification(), inputOAuthConfiguration);
}
}

private static String urlEncode(final String s) {
try {
return URLEncoder.encode(s, StandardCharsets.UTF_8);
Expand Down
24 changes: 17 additions & 7 deletions airbyte-oauth/src/main/java/io/airbyte/oauth/BaseOAuthFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMap.Builder;
import io.airbyte.commons.json.Jsons;
import io.airbyte.commons.map.MoreMaps;
import io.airbyte.config.ConfigSchema;
import io.airbyte.config.DestinationOAuthParameter;
import io.airbyte.config.SourceOAuthParameter;
import io.airbyte.config.persistence.ConfigNotFoundException;
import io.airbyte.config.persistence.ConfigRepository;
import io.airbyte.protocol.models.OAuthConfigSpecification;
import io.airbyte.validation.json.JsonSchemaValidator;
import io.airbyte.validation.json.JsonValidationException;
import java.io.IOException;
import java.util.HashMap;
Expand Down Expand Up @@ -124,21 +126,29 @@ protected Map<String, Object> formatOAuthOutput(final JsonNode oAuthParamConfig,
*/
protected Map<String, Object> formatOAuthOutput(final JsonNode oAuthParamConfig,
final Map<String, Object> completeOAuthFlow,
final OAuthConfigSpecification oAuthConfigSpecification) {
final Builder<String, Object> outputs = ImmutableMap.builder();
// inject masked params outputs
final OAuthConfigSpecification oAuthConfigSpecification)
throws JsonValidationException {
final JsonSchemaValidator validator = new JsonSchemaValidator();

final Builder<String, Object> oAuthServerOutputBuilder = ImmutableMap.builder();
for (final String key : Jsons.keys(oAuthParamConfig)) {
if (oAuthConfigSpecification.getCompleteOauthServerOutputSpecification().has(key)) {
outputs.put(key, MoreOAuthParameters.SECRET_MASK);
oAuthServerOutputBuilder.put(key, MoreOAuthParameters.SECRET_MASK);
}
}
// collect oauth result outputs
final Map<String, Object> oAuthServerOutputs = oAuthServerOutputBuilder.build();
validator.ensure(oAuthConfigSpecification.getCompleteOauthServerOutputSpecification(), Jsons.jsonNode(oAuthServerOutputs));

final Builder<String, Object> oAuthBuilder = ImmutableMap.builder();
for (final String key : completeOAuthFlow.keySet()) {
if (oAuthConfigSpecification.getCompleteOauthOutputSpecification().has(key)) {
outputs.put(key, completeOAuthFlow.get(key));
oAuthBuilder.put(key, completeOAuthFlow.get(key));
}
}
return outputs.build();
final Map<String, Object> oAuthOutputs = oAuthBuilder.build();
validator.ensure(oAuthConfigSpecification.getCompleteOauthOutputSpecification(), Jsons.jsonNode(oAuthOutputs));

return MoreMaps.merge(oAuthServerOutputs, oAuthOutputs);
}

/**
Expand Down
21 changes: 19 additions & 2 deletions airbyte-oauth/src/main/java/io/airbyte/oauth/OAuthFlowImplementation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,33 @@
import com.fasterxml.jackson.databind.JsonNode;
import io.airbyte.config.persistence.ConfigNotFoundException;
import io.airbyte.protocol.models.OAuthConfigSpecification;
import io.airbyte.validation.json.JsonValidationException;
import java.io.IOException;
import java.util.Map;
import java.util.UUID;

public interface OAuthFlowImplementation {

@Deprecated
String getSourceConsentUrl(UUID workspaceId, UUID sourceDefinitionId, String redirectUrl) throws IOException, ConfigNotFoundException;

String getSourceConsentUrl(UUID workspaceId,
UUID sourceDefinitionId,
String redirectUrl,
JsonNode inputOAuthConfiguration,
OAuthConfigSpecification oauthConfigSpecification)
throws IOException, ConfigNotFoundException, JsonValidationException;

@Deprecated
String getDestinationConsentUrl(UUID workspaceId, UUID destinationDefinitionId, String redirectUrl) throws IOException, ConfigNotFoundException;

String getDestinationConsentUrl(UUID workspaceId,
UUID destinationDefinitionId,
String redirectUrl,
JsonNode inputOAuthConfiguration,
OAuthConfigSpecification oauthConfigSpecification)
throws IOException, ConfigNotFoundException, JsonValidationException;

@Deprecated
Map<String, Object> completeSourceOAuth(UUID workspaceId, UUID sourceDefinitionId, Map<String, Object> queryParams, String redirectUrl)
throws IOException, ConfigNotFoundException;
Expand All @@ -27,7 +44,7 @@ Map<String, Object> completeSourceOAuth(UUID workspaceId,
String redirectUrl,
JsonNode inputOAuthConfiguration,
OAuthConfigSpecification oauthConfigSpecification)
throws IOException, ConfigNotFoundException;
throws IOException, ConfigNotFoundException, JsonValidationException;

@Deprecated
Map<String, Object> completeDestinationOAuth(UUID workspaceId, UUID destinationDefinitionId, Map<String, Object> queryParams, String redirectUrl)
Expand All @@ -39,6 +56,6 @@ Map<String, Object> completeDestinationOAuth(UUID workspaceId,
String redirectUrl,
JsonNode inputOAuthConfiguration,
OAuthConfigSpecification oAuthConfigSpecification)
throws IOException, ConfigNotFoundException;
throws IOException, ConfigNotFoundException, JsonValidationException;

}
7 changes: 6 additions & 1 deletion airbyte-oauth/src/main/java/io/airbyte/oauth/flows/AsanaOAuthFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package io.airbyte.oauth.flows;

import com.fasterxml.jackson.databind.JsonNode;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import io.airbyte.config.persistence.ConfigRepository;
Expand Down Expand Up @@ -34,7 +35,11 @@ public AsanaOAuthFlow(final ConfigRepository configRepository, final HttpClient
}

@Override
protected String formatConsentUrl(final UUID definitionId, final String clientId, final String redirectUrl) throws IOException {
protected String formatConsentUrl(final UUID definitionId,
final String clientId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration)
throws IOException {
try {
return new URIBuilder(AUTHORIZE_URL)
.addParameter("client_id", clientId)
Expand Down
6 changes: 5 additions & 1 deletion airbyte-oauth/src/main/java/io/airbyte/oauth/flows/GithubOAuthFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ public GithubOAuthFlow(final ConfigRepository configRepository, final HttpClient
}

@Override
protected String formatConsentUrl(final UUID definitionId, final String clientId, final String redirectUrl) throws IOException {
protected String formatConsentUrl(final UUID definitionId,
final String clientId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration)
throws IOException {
try {
// No scope means read-only access to public information
// https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps#available-scopes
Expand Down
16 changes: 6 additions & 10 deletions airbyte-oauth/src/main/java/io/airbyte/oauth/flows/HubspotOAuthFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package io.airbyte.oauth.flows;

import com.fasterxml.jackson.databind.JsonNode;
import com.google.common.collect.ImmutableMap;
import io.airbyte.config.persistence.ConfigRepository;
import io.airbyte.oauth.BaseOAuth2Flow;
Expand All @@ -27,17 +28,12 @@ public HubspotOAuthFlow(final ConfigRepository configRepository, final HttpClien
super(configRepository, httpClient, stateSupplier, TOKEN_REQUEST_CONTENT_TYPE.JSON);
}

/**
* Depending on the OAuth flow implementation, the URL to grant user's consent may differ,
* especially in the query parameters to be provided. This function should generate such consent URL
* accordingly.
*
* @param definitionId The configured definition ID of this client
* @param clientId The configured client ID
* @param redirectUrl the redirect URL
*/
@Override
protected String formatConsentUrl(final UUID definitionId, final String clientId, final String redirectUrl) throws IOException {
protected String formatConsentUrl(final UUID definitionId,
final String clientId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration)
throws IOException {
try {
return new URIBuilder(AUTHORIZE_URL)
.addParameter("client_id", clientId)
Expand Down
6 changes: 5 additions & 1 deletion airbyte-oauth/src/main/java/io/airbyte/oauth/flows/IntercomOAuthFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ public IntercomOAuthFlow(final ConfigRepository configRepository, final HttpClie
}

@Override
protected String formatConsentUrl(final UUID definitionId, final String clientId, final String redirectUrl) throws IOException {
protected String formatConsentUrl(final UUID definitionId,
final String clientId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration)
throws IOException {
try {
return new URIBuilder(AUTHORIZE_URL)
.addParameter("client_id", clientId)
Expand Down
7 changes: 6 additions & 1 deletion airbyte-oauth/src/main/java/io/airbyte/oauth/flows/QuickbooksOAuthFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package io.airbyte.oauth.flows;

import com.fasterxml.jackson.databind.JsonNode;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import io.airbyte.config.persistence.ConfigRepository;
Expand Down Expand Up @@ -36,7 +37,11 @@ public String getScopes() {
}

@Override
protected String formatConsentUrl(final UUID definitionId, final String clientId, final String redirectUrl) throws IOException {
protected String formatConsentUrl(final UUID definitionId,
final String clientId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration)
throws IOException {
try {

return (new URIBuilder(CONSENT_URL)
Expand Down
7 changes: 6 additions & 1 deletion airbyte-oauth/src/main/java/io/airbyte/oauth/flows/SalesforceOAuthFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package io.airbyte.oauth.flows;

import com.fasterxml.jackson.databind.JsonNode;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import io.airbyte.config.persistence.ConfigRepository;
Expand Down Expand Up @@ -38,7 +39,11 @@ public SalesforceOAuthFlow(final ConfigRepository configRepository, final HttpCl
}

@Override
protected String formatConsentUrl(final UUID definitionId, final String clientId, final String redirectUrl) throws IOException {
protected String formatConsentUrl(final UUID definitionId,
final String clientId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration)
throws IOException {
try {
return new URIBuilder(AUTHORIZE_URL)
.addParameter("client_id", clientId)
Expand Down
7 changes: 6 additions & 1 deletion airbyte-oauth/src/main/java/io/airbyte/oauth/flows/SlackOAuthFlow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package io.airbyte.oauth.flows;

import com.fasterxml.jackson.databind.JsonNode;
import com.google.common.annotations.VisibleForTesting;
import io.airbyte.config.persistence.ConfigRepository;
import io.airbyte.oauth.BaseOAuth2Flow;
Expand Down Expand Up @@ -34,7 +35,11 @@ public SlackOAuthFlow(final ConfigRepository configRepository, final HttpClient
* accordingly.
*/
@Override
protected String formatConsentUrl(final UUID definitionId, final String clientId, final String redirectUrl) throws IOException {
protected String formatConsentUrl(final UUID definitionId,
final String clientId,
final String redirectUrl,
final JsonNode inputOAuthConfiguration)
throws IOException {
try {
return new URIBuilder(SLACK_CONSENT_URL_BASE)
.addParameter("client_id", clientId)
Expand Down
Loading