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.

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

🪟 🎉 Show an error when OAuth credentials are missing #19466

Merged
merged 1 commit into from
Nov 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
64 changes: 49 additions & 15 deletions airbyte-webapp/src/hooks/services/useConnectorAuth.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useCallback, useMemo, useRef } from "react";
import { useIntl } from "react-intl";
import { useAsyncFn, useEffectOnce, useEvent } from "react-use";

import { useConfig } from "config";
Expand All @@ -7,10 +8,13 @@ import { DestinationAuthService } from "core/domain/connector/DestinationAuthSer
import { isSourceDefinitionSpecification } from "core/domain/connector/source";
import { SourceAuthService } from "core/domain/connector/SourceAuthService";
import { DestinationOauthConsentRequest, SourceOauthConsentRequest } from "core/request/AirbyteClient";
import { isCommonRequestError } from "core/request/CommonRequestError";
import { useConnectorForm } from "views/Connector/ConnectorForm/connectorFormContext";

import { useDefaultRequestMiddlewares } from "../../services/useDefaultRequestMiddlewares";
import { useQuery } from "../useQuery";
import { useAppMonitoringService } from "./AppMonitoringService";
import { useNotificationService } from "./Notification";
import { useCurrentWorkspace } from "./useWorkspace";

let windowObjectReference: Window | null = null; // global variable
Expand Down Expand Up @@ -46,8 +50,11 @@ export function useConnectorAuth(): {
queryParams: Record<string, unknown>
) => Promise<Record<string, unknown>>;
} {
const { formatMessage } = useIntl();
const { trackError } = useAppMonitoringService();
const { workspaceId } = useCurrentWorkspace();
const { apiUrl, oauthRedirectUrl } = useConfig();
const notificationService = useNotificationService();
const { connectorId } = useConnectorForm();

// TODO: move to separate initFacade and use refs instead
Expand All @@ -70,28 +77,55 @@ export function useConnectorAuth(): {
payload: SourceOauthConsentRequest | DestinationOauthConsentRequest;
consentUrl: string;
}> => {
if (isSourceDefinitionSpecification(connector)) {
const payload: SourceOauthConsentRequest = {
try {
if (isSourceDefinitionSpecification(connector)) {
const payload: SourceOauthConsentRequest = {
workspaceId,
sourceDefinitionId: ConnectorSpecification.id(connector),
redirectUrl: `${oauthRedirectUrl}/auth_flow`,
oAuthInputConfiguration,
sourceId: connectorId,
};
const response = await sourceAuthService.getConsentUrl(payload);

return { consentUrl: response.consentUrl, payload };
}
const payload: DestinationOauthConsentRequest = {
workspaceId,
sourceDefinitionId: ConnectorSpecification.id(connector),
destinationDefinitionId: ConnectorSpecification.id(connector),
redirectUrl: `${oauthRedirectUrl}/auth_flow`,
oAuthInputConfiguration,
sourceId: connectorId,
destinationId: connectorId,
};
const response = await sourceAuthService.getConsentUrl(payload);
const response = await destinationAuthService.getConsentUrl(payload);

return { consentUrl: response.consentUrl, payload };
} catch (e) {
// If this API returns a 404 the OAuth credentials have not been added to the database.
if (isCommonRequestError(e) && e.status === 404) {
if (process.env.NODE_ENV === "development") {
notificationService.registerNotification({
id: "oauthConnector.credentialsMissing",
// Since it's dev only we don't need i18n on this string
title: "OAuth is not enabled for this connector on this environment.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Just out of curiosity - do we have any OAuth connectors enabled in dev mode? From this message it sounds like some are enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you can find the (internal) documentation to what we have enabled on frontend-dev here: https://www.notion.so/Testing-OAuth-Locally-fea17aeb14c74cacb5f3ed856daae753

});
} else {
// Log error to our monitoring, this should never happen and means OAuth credentials
// where missed
trackError(e, {
id: "oauthConnector.credentialsMissing",
connectorSpecId: ConnectorSpecification.id(connector),
workspaceId,
});
notificationService.registerNotification({
id: "oauthConnector.credentialsMissing",
title: formatMessage({ id: "connector.oauthCredentialsMissing" }),
isError: true,
});
}
}
throw e;
}
const payload: DestinationOauthConsentRequest = {
workspaceId,
destinationDefinitionId: ConnectorSpecification.id(connector),
redirectUrl: `${oauthRedirectUrl}/auth_flow`,
oAuthInputConfiguration,
destinationId: connectorId,
};
const response = await destinationAuthService.getConsentUrl(payload);

return { consentUrl: response.consentUrl, payload };
},
completeOauthRequest: async (
params: SourceOauthConsentRequest | DestinationOauthConsentRequest,
Expand Down
1 change: 1 addition & 0 deletions airbyte-webapp/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@
"connector.setupGuide": "Setup Guide",
"connector.setupGuide.notFound": "No Setup Guide found for this connector.",
"connector.exampleValues": "Example {count, plural, one {value} other {values}}",
"connector.oauthCredentialsMissing": "OAuth login is temporarily unavailable for this connector. Please try again later.",

"credits.credits": "Credits",
"credits.whatAreCredits": "What are credits?",
Expand Down