Skip to content

Commit

Permalink
feat: allow end user to provide oauth app if server creds aren't avai…
Browse files Browse the repository at this point in the history
…lable (#14775)
  • Loading branch information
erohmensing committed Dec 14, 2024
1 parent 9b9b93c commit e3b1859
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 46 deletions.
4 changes: 2 additions & 2 deletions airbyte-api/server-api/src/main/openapi/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9317,7 +9317,7 @@ components:
$ref: "#/components/schemas/SourceDefinitionSpecification"
advancedAuth:
$ref: "#/components/schemas/AdvancedAuth"
advancedAuthCredentialsAvailable:
advancedAuthGlobalCredentialsAvailable:
type: boolean
jobInfo:
$ref: "#/components/schemas/SynchronousJobRead"
Expand Down Expand Up @@ -9762,7 +9762,7 @@ components:
$ref: "#/components/schemas/DestinationDefinitionSpecification"
advancedAuth:
$ref: "#/components/schemas/AdvancedAuth"
advancedAuthCredentialsAvailable:
advancedAuthGlobalCredentialsAvailable:
type: boolean
jobInfo:
$ref: "#/components/schemas/SynchronousJobRead"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ SourceDefinitionSpecificationRead getSourceSpecificationRead(final StandardSourc
if (advancedAuth.isPresent()) {
final Optional<SourceOAuthParameter> sourceOAuthParameter =
oAuthService.getSourceOAuthParameterOptional(workspaceId, sourceDefinition.getSourceDefinitionId());
specRead.setAdvancedAuthCredentialsAvailable(sourceOAuthParameter.isPresent());
specRead.setAdvancedAuthGlobalCredentialsAvailable(sourceOAuthParameter.isPresent());
}

return specRead;
Expand All @@ -190,7 +190,7 @@ DestinationDefinitionSpecificationRead getDestinationSpecificationRead(final Sta
if (advancedAuth.isPresent()) {
final Optional<DestinationOAuthParameter> destinationOAuthParameter =
oAuthService.getDestinationOAuthParameterOptional(workspaceId, destinationDefinition.getDestinationDefinitionId());
specRead.setAdvancedAuthCredentialsAvailable(destinationOAuthParameter.isPresent());
specRead.setAdvancedAuthGlobalCredentialsAvailable(destinationOAuthParameter.isPresent());
}

return specRead;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import io.airbyte.persistence.job.factory.OAuthConfigSupplier;
import io.airbyte.persistence.job.tracker.TrackingMetadata;
import io.airbyte.protocol.models.ConnectorSpecification;
import io.airbyte.protocol.models.OAuthConfigSpecification;
import io.airbyte.validation.json.JsonValidationException;
import jakarta.inject.Named;
import jakarta.inject.Singleton;
Expand Down Expand Up @@ -147,10 +148,16 @@ public OAuthConsentRead getSourceOAuthConsent(final SourceOauthConsentRequest so
final Map<String, Object> metadata = TrackingMetadata.generateSourceDefinitionMetadata(sourceDefinition, sourceVersion);
final OAuthConsentRead result;

final JsonNode sourceOAuthParamConfig =
getSourceOAuthParamConfig(sourceOauthConsentRequest.getWorkspaceId(), sourceDefinition.getSourceDefinitionId());
final Optional<SourceOAuthParameter> paramOptional = oAuthService
.getSourceOAuthParameterWithSecretsOptional(sourceOauthConsentRequest.getWorkspaceId(), sourceOauthConsentRequest.getSourceDefinitionId());
final JsonNode sourceOAuthParamConfig = paramOptional.isPresent()
? MoreOAuthParameters.flattenOAuthConfig(paramOptional.get().getConfiguration())
: Jsons.emptyObject();

if (OAuthConfigSupplier.hasOAuthConfigSpecification(spec)) {
final OAuthConfigSpecification oauthConfigSpecification = spec.getAdvancedAuth().getOauthConfigSpecification();
OAuthHelper.updateOauthConfigToAcceptAdditionalUserInputProperties(oauthConfigSpecification);

final JsonNode oAuthInputConfigurationForConsent;

if (sourceOauthConsentRequest.getSourceId() == null) {
Expand All @@ -168,12 +175,14 @@ public OAuthConsentRead getSourceOAuthConsent(final SourceOauthConsentRequest so
sourceOauthConsentRequest.getoAuthInputConfiguration());
}

final JsonNode oAuthInputConfigValues = Jsons.mergeNodes(sourceOAuthParamConfig, oAuthInputConfigurationForConsent);

result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getSourceConsentUrl(
sourceOauthConsentRequest.getWorkspaceId(),
sourceOauthConsentRequest.getSourceDefinitionId(),
sourceOauthConsentRequest.getRedirectUrl(),
oAuthInputConfigurationForConsent,
spec.getAdvancedAuth().getOauthConfigSpecification(), sourceOAuthParamConfig));
oAuthInputConfigValues,
oauthConfigSpecification, oAuthInputConfigValues));
} else {
result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getSourceConsentUrl(
sourceOauthConsentRequest.getWorkspaceId(),
Expand Down Expand Up @@ -213,10 +222,16 @@ public OAuthConsentRead getDestinationOAuthConsent(final DestinationOauthConsent
final Map<String, Object> metadata = TrackingMetadata.generateDestinationDefinitionMetadata(destinationDefinition, destinationVersion);
final OAuthConsentRead result;

final JsonNode destinationOAuthParamConfig = getDestinationOAuthParamConfig(
final Optional<DestinationOAuthParameter> paramOptional = oAuthService.getDestinationOAuthParameterWithSecretsOptional(
destinationOauthConsentRequest.getWorkspaceId(), destinationOauthConsentRequest.getDestinationDefinitionId());
final JsonNode destinationOAuthParamConfig = paramOptional.isPresent()
? MoreOAuthParameters.flattenOAuthConfig(paramOptional.get().getConfiguration())
: Jsons.emptyObject();

if (OAuthConfigSupplier.hasOAuthConfigSpecification(spec)) {
final OAuthConfigSpecification oauthConfigSpecification = spec.getAdvancedAuth().getOauthConfigSpecification();
OAuthHelper.updateOauthConfigToAcceptAdditionalUserInputProperties(oauthConfigSpecification);

final JsonNode oAuthInputConfigurationForConsent;

if (destinationOauthConsentRequest.getDestinationId() == null) {
Expand All @@ -235,12 +250,14 @@ public OAuthConsentRead getDestinationOAuthConsent(final DestinationOauthConsent

}

final JsonNode oAuthInputConfigValues = Jsons.mergeNodes(destinationOAuthParamConfig, oAuthInputConfigurationForConsent);

result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getDestinationConsentUrl(
destinationOauthConsentRequest.getWorkspaceId(),
destinationOauthConsentRequest.getDestinationDefinitionId(),
destinationOauthConsentRequest.getRedirectUrl(),
oAuthInputConfigurationForConsent,
spec.getAdvancedAuth().getOauthConfigSpecification(), destinationOAuthParamConfig));
oAuthInputConfigValues,
spec.getAdvancedAuth().getOauthConfigSpecification(), oAuthInputConfigValues));
} else {
result = new OAuthConsentRead().consentUrl(oAuthFlowImplementation.getDestinationConsentUrl(
destinationOauthConsentRequest.getWorkspaceId(),
Expand Down Expand Up @@ -283,9 +300,16 @@ public CompleteOAuthResponse completeSourceOAuth(final CompleteSourceOauthReques
final Map<String, Object> metadata = TrackingMetadata.generateSourceDefinitionMetadata(sourceDefinition, sourceVersion);
final Map<String, Object> result;

final JsonNode sourceOAuthParamConfig =
getSourceOAuthParamConfig(completeSourceOauthRequest.getWorkspaceId(), sourceDefinition.getSourceDefinitionId());
final Optional<SourceOAuthParameter> paramOptional = oAuthService
.getSourceOAuthParameterWithSecretsOptional(completeSourceOauthRequest.getWorkspaceId(), completeSourceOauthRequest.getSourceDefinitionId());
final JsonNode sourceOAuthParamConfig = paramOptional.isPresent()
? MoreOAuthParameters.flattenOAuthConfig(paramOptional.get().getConfiguration())
: Jsons.emptyObject();

if (OAuthConfigSupplier.hasOAuthConfigSpecification(spec)) {
final OAuthConfigSpecification oauthConfigSpecification = spec.getAdvancedAuth().getOauthConfigSpecification();
OAuthHelper.updateOauthConfigToAcceptAdditionalUserInputProperties(oauthConfigSpecification);

final JsonNode oAuthInputConfigurationForConsent;

if (completeSourceOauthRequest.getSourceId() == null) {
Expand All @@ -303,14 +327,16 @@ public CompleteOAuthResponse completeSourceOAuth(final CompleteSourceOauthReques
completeSourceOauthRequest.getoAuthInputConfiguration());
}

final JsonNode oAuthInputConfigValues = Jsons.mergeNodes(sourceOAuthParamConfig, oAuthInputConfigurationForConsent);

result = oAuthFlowImplementation.completeSourceOAuth(
completeSourceOauthRequest.getWorkspaceId(),
completeSourceOauthRequest.getSourceDefinitionId(),
completeSourceOauthRequest.getQueryParams(),
completeSourceOauthRequest.getRedirectUrl(),
oAuthInputConfigurationForConsent,
spec.getAdvancedAuth().getOauthConfigSpecification(),
sourceOAuthParamConfig);
oAuthInputConfigValues,
oauthConfigSpecification,
oAuthInputConfigValues);
} else {
// deprecated but this path is kept for connectors that don't define OAuth Spec yet
result = oAuthFlowImplementation.completeSourceOAuth(
Expand Down Expand Up @@ -344,10 +370,16 @@ public CompleteOAuthResponse completeDestinationOAuth(final CompleteDestinationO
final Map<String, Object> metadata = TrackingMetadata.generateDestinationDefinitionMetadata(destinationDefinition, destinationVersion);
final Map<String, Object> result;

final JsonNode destinationOAuthParamConfig = getDestinationOAuthParamConfig(
final Optional<DestinationOAuthParameter> paramOptional = oAuthService.getDestinationOAuthParameterWithSecretsOptional(
completeDestinationOAuthRequest.getWorkspaceId(), completeDestinationOAuthRequest.getDestinationDefinitionId());
final JsonNode destinationOAuthParamConfig = paramOptional.isPresent()
? MoreOAuthParameters.flattenOAuthConfig(paramOptional.get().getConfiguration())
: Jsons.emptyObject();

if (OAuthConfigSupplier.hasOAuthConfigSpecification(spec)) {
final OAuthConfigSpecification oauthConfigSpecification = spec.getAdvancedAuth().getOauthConfigSpecification();
OAuthHelper.updateOauthConfigToAcceptAdditionalUserInputProperties(oauthConfigSpecification);

final JsonNode oAuthInputConfigurationForConsent;

if (completeDestinationOAuthRequest.getDestinationId() == null) {
Expand All @@ -366,13 +398,15 @@ public CompleteOAuthResponse completeDestinationOAuth(final CompleteDestinationO

}

final JsonNode oAuthInputConfigValues = Jsons.mergeNodes(destinationOAuthParamConfig, oAuthInputConfigurationForConsent);

result = oAuthFlowImplementation.completeDestinationOAuth(
completeDestinationOAuthRequest.getWorkspaceId(),
completeDestinationOAuthRequest.getDestinationDefinitionId(),
completeDestinationOAuthRequest.getQueryParams(),
completeDestinationOAuthRequest.getRedirectUrl(),
oAuthInputConfigurationForConsent,
spec.getAdvancedAuth().getOauthConfigSpecification(), destinationOAuthParamConfig);
oAuthInputConfigValues,
oauthConfigSpecification, oAuthInputConfigValues);
} else {
// deprecated but this path is kept for connectors that don't define OAuth Spec yet
result = oAuthFlowImplementation.completeDestinationOAuth(
Expand Down Expand Up @@ -441,11 +475,15 @@ public void setDestinationInstancewideOauthParams(final SetInstancewideDestinati
private JsonNode getOAuthInputConfigurationForConsent(final ConnectorSpecification spec,
final JsonNode hydratedSourceConnectionConfiguration,
final JsonNode oAuthInputConfiguration) {
final Map<String, String> fieldsToGet =
final Map<String, String> configOauthFields =
buildJsonPathFromOAuthFlowInitParameters(OAuthHelper.extractOauthConfigurationPaths(
spec.getAdvancedAuth().getOauthConfigSpecification().getOauthUserInputFromConnectorConfigSpecification()));
final Map<String, String> serverOrConfigOauthFields = buildJsonPathFromOAuthFlowInitParameters(OAuthHelper.extractOauthConfigurationPaths(
spec.getAdvancedAuth().getOauthConfigSpecification().getCompleteOauthServerOutputSpecification()));
configOauthFields.putAll(serverOrConfigOauthFields);

final JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnectionConfiguration, fieldsToGet);
final JsonNode oAuthInputConfigurationFromDB = getOAuthInputConfiguration(hydratedSourceConnectionConfiguration, configOauthFields);
LOGGER.warn("oAuthInputConfigurationFromDB: {}", oAuthInputConfigurationFromDB);

return getOauthFromDBIfNeeded(oAuthInputConfigurationFromDB, oAuthInputConfiguration);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,11 @@ void testDestinationSyncModeEnrichmentWithoutOverwrite(final boolean supportsRef

@ValueSource(booleans = {true, false})
@ParameterizedTest
void getDestinationSpecificationReadAdvancedAuth(final boolean advancedAuthCredentialsAvailable) throws IOException {
void getDestinationSpecificationReadAdvancedAuth(final boolean advancedAuthGlobalCredentialsAvailable) throws IOException {
final UUID workspaceId = UUID.randomUUID();
final UUID destinationDefinitionId = UUID.randomUUID();
when(oAuthService.getDestinationOAuthParameterOptional(workspaceId, destinationDefinitionId))
.thenReturn(advancedAuthCredentialsAvailable ? Optional.of(new DestinationOAuthParameter()) : Optional.empty());
.thenReturn(advancedAuthGlobalCredentialsAvailable ? Optional.of(new DestinationOAuthParameter()) : Optional.empty());

final DestinationDefinitionIdWithWorkspaceId destinationDefinitionIdWithWorkspaceId =
new DestinationDefinitionIdWithWorkspaceId().destinationDefinitionId(destinationDefinitionId).workspaceId(workspaceId);
Expand All @@ -325,16 +325,16 @@ void getDestinationSpecificationReadAdvancedAuth(final boolean advancedAuthCrede
connectorDefinitionSpecificationHandler.getDestinationSpecificationRead(destinationDefinition, connectorSpecification, true, workspaceId);

verify(oAuthService).getDestinationOAuthParameterOptional(workspaceId, destinationDefinitionId);
assertEquals(advancedAuthCredentialsAvailable, response.getAdvancedAuthCredentialsAvailable());
assertEquals(advancedAuthGlobalCredentialsAvailable, response.getAdvancedAuthGlobalCredentialsAvailable());
}

@ValueSource(booleans = {true, false})
@ParameterizedTest
void getSourceSpecificationReadAdvancedAuth(final boolean advancedAuthCredentialsAvailable) throws IOException {
void getSourceSpecificationReadAdvancedAuth(final boolean advancedAuthGlobalCredentialsAvailable) throws IOException {
final UUID workspaceId = UUID.randomUUID();
final UUID sourceDefinitionId = UUID.randomUUID();
when(oAuthService.getSourceOAuthParameterOptional(workspaceId, sourceDefinitionId))
.thenReturn(advancedAuthCredentialsAvailable ? Optional.of(new SourceOAuthParameter()) : Optional.empty());
.thenReturn(advancedAuthGlobalCredentialsAvailable ? Optional.of(new SourceOAuthParameter()) : Optional.empty());

final SourceDefinitionIdWithWorkspaceId sourceDefinitionIdWithWorkspaceId =
new SourceDefinitionIdWithWorkspaceId().sourceDefinitionId(sourceDefinitionId).workspaceId(workspaceId);
Expand All @@ -352,7 +352,7 @@ void getSourceSpecificationReadAdvancedAuth(final boolean advancedAuthCredential
connectorDefinitionSpecificationHandler.getSourceSpecificationRead(sourceDefinition, connectorSpecification, workspaceId);

verify(oAuthService).getSourceOAuthParameterOptional(workspaceId, sourceDefinitionId);
assertEquals(advancedAuthCredentialsAvailable, response.getAdvancedAuthCredentialsAvailable());
assertEquals(advancedAuthGlobalCredentialsAvailable, response.getAdvancedAuthGlobalCredentialsAvailable());
}

}
2 changes: 1 addition & 1 deletion airbyte-webapp/src/core/domain/connector/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export type ConnectorDefinitionOrEnterpriseStub = ConnectorDefinition | Enterpri

export type SourceDefinitionSpecificationDraft = Pick<
SourceDefinitionSpecificationRead,
"documentationUrl" | "connectionSpecification" | "advancedAuth"
"documentationUrl" | "connectionSpecification" | "advancedAuth" | "advancedAuthGlobalCredentialsAvailable"
>;

export type ConnectorDefinitionSpecificationRead =
Expand Down
2 changes: 2 additions & 0 deletions airbyte-webapp/src/core/services/features/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { FeatureItem } from "./types";

export const defaultOssFeatures = [
FeatureItem.AllowAutoDetectSchema,
FeatureItem.AllowOAuthConnector,
FeatureItem.AllowUpdateConnectors,
FeatureItem.AllowUploadCustomImage,
FeatureItem.EnterpriseUpsell,
Expand All @@ -10,6 +11,7 @@ export const defaultOssFeatures = [
export const defaultEnterpriseFeatures = [
...defaultOssFeatures,
FeatureItem.AllowAllRBACRoles,
FeatureItem.AllowOAuthConnector,
FeatureItem.ConnectionHistoryGraphs,
FeatureItem.DiagnosticsExport,
FeatureItem.DisplayOrganizationUsers,
Expand Down
2 changes: 1 addition & 1 deletion airbyte-webapp/src/core/services/features/types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export enum FeatureItem {
AllowUploadCustomImage = "ALLOW_UPLOAD_CUSTOM_IMAGE",
AllowDBTCloudIntegration = "ALLOW_DBT_CLOUD_INTEGRATION",
AllowUpdateConnectors = "ALLOW_UPDATE_CONNECTORS",
AllowOAuthConnector = "ALLOW_OAUTH_CONNECTOR",
AllowOAuthConnector = "ALLOW_OAUTH_CONNECTOR", // TODO (ella) : remove this feature flag
AllowChangeDataGeographies = "ALLOW_CHANGE_DATA_GEOGRAPHIES",
CloudForTeamsBranding = "CLOUD_FOR_TEAMS_BRANDING",
CloudForTeamsUpsell = "CLOUD_FOR_TEAMS_UPSELLING",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1007,11 +1007,6 @@ describe("Connector form", () => {
},
});
}
it("should render regular inputs for auth fields", async () => {
const container = await renderNewOAuthForm({ disableOAuth: true });
expect(getInputByName(container, "connectionConfiguration.credentials.access_token")).toBeInTheDocument();
expect(getOAuthButton(container)).not.toBeInTheDocument();
});

it("should render the oauth button", async () => {
const container = await renderNewOAuthForm();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { useAuthentication } from "views/Connector/ConnectorForm/useAuthenticati
import { useNotificationService } from "../../../../../../hooks/services/Notification";
import { useConnectorForm } from "../../../connectorFormContext";
import { ConnectorFormValues } from "../../../types";
import { makeConnectionConfigurationPath, serverProvidedOauthPaths } from "../../../utils";
import { makeConnectionConfigurationPath, serverProvidedOauthPaths, userProvidedOauthInputPaths } from "../../../utils";

interface Credentials {
credentials: AdvancedAuth;
Expand Down Expand Up @@ -74,12 +74,7 @@ function useFormOauthAdapter(
done: done || hasAuthFieldValues,
hasRun,
run: async () => {
const oauthInputProperties =
(
connector?.advancedAuth?.oauthConfigSpecification?.oauthUserInputFromConnectorConfigSpecification as {
properties: Array<{ path_in_connector_config: string[] }>;
}
)?.properties ?? {};
const oauthInputProperties = userProvidedOauthInputPaths(connector);

if (!isEmpty(oauthInputProperties)) {
const oauthInputFields =
Expand Down
Loading

0 comments on commit e3b1859

Please sign in to comment.