From ce7627fd99aae5de492a15c30b57dfe55259054e Mon Sep 17 00:00:00 2001 From: Baptiste Arnaud Date: Fri, 25 Jul 2025 11:41:52 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=B8=20(gmail)=20Improve=20OAuth=20cred?= =?UTF-8?q?ential=20popup=20handling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .cursor/rules/general.mdc | 3 +- .../CreateForgedOAuthCredentialsModal.tsx | 77 +++--- ...dateForgedOAuthCredentialsModalContent.tsx | 60 ++--- .../components/credentials/useOAuthPopup.ts | 225 ++++++++++++++++++ .../blocks/gmail/src/actions/sendEmail.ts | 57 ++--- .../gmail/src/helpers/createGmailClient.ts | 15 ++ 6 files changed, 324 insertions(+), 113 deletions(-) create mode 100644 apps/builder/src/features/forge/components/credentials/useOAuthPopup.ts create mode 100644 packages/forge/blocks/gmail/src/helpers/createGmailClient.ts diff --git a/.cursor/rules/general.mdc b/.cursor/rules/general.mdc index 9d15db02f..81ee77232 100644 --- a/.cursor/rules/general.mdc +++ b/.cursor/rules/general.mdc @@ -10,7 +10,8 @@ alwaysApply: true - Never use `any` type. Always use proper TypeScript types, interfaces, or union types instead. - Use `satisfies` instead of `as` when possible to make sure we keep strong type-safety. -- Prefer writing long and complex functions that provide good, deep abstraction. These functions should have a TSDoc comment description of what it does in great details. Avoid providing `@param` description, params name should be self-explanatory. In the function itself you can add inline comments only if the code is hard to understand. +- Prefer writing long and complex functions that provide good, deep abstraction. These functions should have a TSDoc comment description of what it does in great details. Avoid providing `@param` description, params name should be self-explanatory. IMPORTANT: Only add inline comments if a piece of logic is hard to understand, it should ideally explain what happens in the next N lines of code, never add a comment to explain a single line. +- Prefer infer the return type of a function instead of declaring it. - Helper functions should be placed at the bottom of the file. ## Testing diff --git a/apps/builder/src/features/forge/components/credentials/CreateForgedOAuthCredentialsModal.tsx b/apps/builder/src/features/forge/components/credentials/CreateForgedOAuthCredentialsModal.tsx index 26a02f4eb..6c392f806 100644 --- a/apps/builder/src/features/forge/components/credentials/CreateForgedOAuthCredentialsModal.tsx +++ b/apps/builder/src/features/forge/components/credentials/CreateForgedOAuthCredentialsModal.tsx @@ -1,4 +1,3 @@ -import { stringify } from "querystring"; import { CopyButton } from "@/components/CopyButton"; import { TextInput } from "@/components/inputs/TextInput"; import { useWorkspace } from "@/features/workspace/WorkspaceProvider"; @@ -21,6 +20,7 @@ import { useMutation } from "@tanstack/react-query"; import type { ForgedBlockDefinition } from "@typebot.io/forge-repository/definitions"; import { Button } from "@typebot.io/ui/components/Button"; import { useState } from "react"; +import { useOAuthPopup } from "./useOAuthPopup"; type Props = { blockDef: ForgedBlockDefinition; @@ -69,7 +69,6 @@ export const CreateForgedOAuthCredentialsModalContent = ({ "blockDef" | "onNewCredentials" | "defaultData" | "editorContext" | "scope" >) => { const { workspace } = useWorkspace(); - const [isAuthorizing, setIsAuthorizing] = useState(false); const [name, setName] = useState(""); const [tab, setTab] = useState<"default" | "your-app">( blockDef.auth && "defaultClientEnvKeys" in blockDef.auth @@ -95,54 +94,42 @@ export const CreateForgedOAuthCredentialsModalContent = ({ }), ); - const openOAuthPopup = async () => { + const handleOAuthSuccess = (code: string) => { if (!workspace) return; - - setIsAuthorizing(true); - - window.open( - `/api/${blockDef.id}/oauth/authorize?${stringify({ - clientId: clientId, - })}`, - "oauthPopup", - "width=500,height=700", - ); - - const handleOAuthResponse = (event: MessageEvent) => { - if (event.data?.type === "oauth") { - window.removeEventListener("message", handleOAuthResponse); - setIsAuthorizing(false); - const { code } = event.data; - const credentials = { - name, - blockType: blockDef.id, - code, - customClient: - tab === "your-app" - ? { - id: clientId, - secret: clientSecret, - } - : undefined, - }; - mutate( - scope === "workspace" - ? { - ...credentials, - scope: "workspace", - workspaceId: workspace.id, - } - : { - ...credentials, - scope: "user", - }, - ); - } + const credentials = { + name: name.trim(), + blockType: blockDef.id, + code, + customClient: + tab === "your-app" + ? { + id: clientId.trim(), + secret: clientSecret.trim(), + } + : undefined, }; - window.addEventListener("message", handleOAuthResponse); + mutate( + scope === "workspace" + ? { + ...credentials, + scope: "workspace", + workspaceId: workspace.id, + } + : { + ...credentials, + scope: "user", + }, + ); }; + const { openOAuthPopup, isAuthorizing } = useOAuthPopup({ + blockId: blockDef.id, + clientId, + workspace: workspace ?? null, + onSuccess: handleOAuthSuccess, + }); + if (!blockDef.auth) return null; return ( diff --git a/apps/builder/src/features/forge/components/credentials/UpdateForgedOAuthCredentialsModalContent.tsx b/apps/builder/src/features/forge/components/credentials/UpdateForgedOAuthCredentialsModalContent.tsx index 13186c8ff..4696fd397 100644 --- a/apps/builder/src/features/forge/components/credentials/UpdateForgedOAuthCredentialsModalContent.tsx +++ b/apps/builder/src/features/forge/components/credentials/UpdateForgedOAuthCredentialsModalContent.tsx @@ -1,4 +1,3 @@ -import { stringify } from "querystring"; import { CopyButton } from "@/components/CopyButton"; import { TextInput } from "@/components/inputs/TextInput"; import { useWorkspace } from "@/features/workspace/WorkspaceProvider"; @@ -19,6 +18,7 @@ import { useMutation, useQuery } from "@tanstack/react-query"; import type { ForgedBlockDefinition } from "@typebot.io/forge-repository/definitions"; import { Button } from "@typebot.io/ui/components/Button"; import { useEffect, useState } from "react"; +import { useOAuthPopup } from "./useOAuthPopup"; type Props = { credentialsId: string; @@ -96,42 +96,31 @@ export const UpdateForgedOAuthCredentialsModalContent = ({ }), ); - const openOAuthPopup = async () => { + const handleOAuthSuccess = (code: string) => { if (!workspace) return; - - window.open( - `/api/${blockDef.id}/oauth/authorize?${stringify({ - clientId: clientId, - })}`, - "oauthPopup", - "width=500,height=700", - ); - - const handleOAuthResponse = (event: MessageEvent) => { - if (event.data?.type === "oauth") { - window.removeEventListener("message", handleOAuthResponse); - const { code } = event.data; - mutate({ - name, - blockType: blockDef.id, - workspaceId: workspace.id, - credentialsId, - code, - customClient: - tab === "your-app" - ? { - id: clientId, - secret: clientSecret, - } - : undefined, - }); - } - }; - - window.removeEventListener("message", handleOAuthResponse); - window.addEventListener("message", handleOAuthResponse); + mutate({ + name, + blockType: blockDef.id, + workspaceId: workspace.id, + credentialsId, + code, + customClient: + tab === "your-app" + ? { + id: clientId, + secret: clientSecret, + } + : undefined, + }); }; + const { openOAuthPopup, isAuthorizing } = useOAuthPopup({ + blockId: blockDef.id, + clientId, + workspace: workspace ?? null, + onSuccess: handleOAuthSuccess, + }); + if (!blockDef.auth) return null; return ( @@ -196,7 +185,8 @@ export const UpdateForgedOAuthCredentialsModalContent = ({ disabled={ !name || isPending || - (tab === "your-app" && (!clientId || !clientSecret)) + (tab === "your-app" && (!clientId || !clientSecret)) || + isAuthorizing } > diff --git a/apps/builder/src/features/forge/components/credentials/useOAuthPopup.ts b/apps/builder/src/features/forge/components/credentials/useOAuthPopup.ts new file mode 100644 index 000000000..e2df03ef4 --- /dev/null +++ b/apps/builder/src/features/forge/components/credentials/useOAuthPopup.ts @@ -0,0 +1,225 @@ +import { stringify } from "querystring"; +import { toast } from "@/lib/toast"; +import { useEffect, useRef, useState } from "react"; + +interface UseOAuthPopupOptions { + /** The block definition ID for the OAuth provider */ + blockId: string; + /** Client ID for the OAuth application. If undefined default clientId will be used */ + clientId?: string; + /** Current workspace information */ + workspace: { id: string } | null; + /** Timeout duration in milliseconds (default: 30000) */ + timeout?: number; + /** Custom popup window features */ + popupFeatures?: string; + /** Callback function when OAuth succeeds with authorization code */ + onSuccess: (code: string) => void; + /** Callback function when OAuth fails with error */ + onError?: (error: string) => void; +} + +interface UseOAuthPopupReturn { + /** Function to open the OAuth popup and start the flow */ + openOAuthPopup: () => void; + /** Whether the OAuth authorization is currently in progress */ + isAuthorizing: boolean; + /** Function to manually cleanup and cancel the OAuth flow */ + cleanup: () => void; +} + +/** + * Custom hook for managing OAuth popup authentication flow. + * + * This hook encapsulates all the complexity of OAuth popup management including: + * - Resource cleanup (event listeners, timeouts, popup windows) + * - Security validations (origin checking) + * - Error handling and user feedback + * - Timeout management + * - Memory leak prevention + * + */ +export const useOAuthPopup = ({ + blockId, + clientId, + workspace, + timeout = 120000, + popupFeatures = "width=500,height=700,scrollbars=yes,resizable=yes,status=yes,location=yes", + onSuccess, + onError, +}: UseOAuthPopupOptions): UseOAuthPopupReturn => { + const [isAuthorizing, _setIsAuthorizing] = useState(false); + + const oauthWindowRef = useRef(null); + const timeoutRef = useRef(null); + const messageHandlerRef = useRef<((event: MessageEvent) => void) | null>( + null, + ); + const popupCheckIntervalRef = useRef(null); + const isAuthorizingRef = useRef(false); + + const setIsAuthorizing = (value: boolean) => { + isAuthorizingRef.current = value; + _setIsAuthorizing(value); + }; + + /** + * Cleans up OAuth popup resources and resets authorization state. + * This function handles proper cleanup of event listeners, timeouts, and popup window references. + */ + const cleanup = () => { + if (messageHandlerRef.current) { + window.removeEventListener("message", messageHandlerRef.current); + messageHandlerRef.current = null; + } + + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + timeoutRef.current = null; + } + + if (popupCheckIntervalRef.current) { + clearInterval(popupCheckIntervalRef.current); + popupCheckIntervalRef.current = null; + } + + if (oauthWindowRef.current && !oauthWindowRef.current.closed) { + oauthWindowRef.current.close(); + } + oauthWindowRef.current = null; + + setIsAuthorizing(false); + }; + + /** + * Opens OAuth popup and handles the complete authentication flow with proper error handling, + * timeout management, and security validations. This function ensures robust OAuth integration + * by managing popup lifecycle, validating message origins, and providing comprehensive error handling. + */ + const openOAuthPopup = () => { + if (!workspace) { + const errorMessage = + "Workspace not available. Please refresh and try again."; + toast({ + description: errorMessage, + status: "error", + }); + onError?.(errorMessage); + return; + } + + if (isAuthorizing) { + return; + } + + setIsAuthorizing(true); + + try { + const popupUrl = `/api/${blockId}/oauth/authorize?${stringify({ + clientId: clientId, + })}`; + + const popup = window.open(popupUrl, "oauthPopup", popupFeatures); + + if (!popup || popup.closed) { + throw new Error( + "Popup was blocked by browser. Please allow popups for this site and try again.", + ); + } + + oauthWindowRef.current = popup; + + timeoutRef.current = setTimeout(() => { + cleanup(); + const errorMessage = + "OAuth authentication timed out. Please try again."; + toast({ + description: errorMessage, + status: "error", + }); + onError?.(errorMessage); + }, timeout); + + const handleOAuthResponse = (event: MessageEvent) => { + const allowedOrigins = [window.location.origin]; + if (!allowedOrigins.includes(event.origin)) { + console.warn( + "Received OAuth message from unauthorized origin:", + event.origin, + ); + return; + } + + if (event.data?.type !== "oauth") { + return; + } + + try { + cleanup(); + + const { code, error } = event.data; + + if (error) { + throw new Error(`OAuth failed: ${error}`); + } + + if (!code) { + throw new Error( + "No authorization code received from OAuth provider", + ); + } + + onSuccess(code); + } catch (err) { + const errorMessage = + err instanceof Error ? err.message : "OAuth authentication failed"; + toast({ + description: errorMessage, + status: "error", + }); + onError?.(errorMessage); + } + }; + + messageHandlerRef.current = handleOAuthResponse; + window.addEventListener("message", handleOAuthResponse); + + popupCheckIntervalRef.current = setInterval(() => { + if (popup.closed) { + if (popupCheckIntervalRef.current) { + clearInterval(popupCheckIntervalRef.current); + popupCheckIntervalRef.current = null; + } + if (isAuthorizingRef.current) { + cleanup(); + } + } + }, 1000); + + popup.focus(); + } catch (error) { + cleanup(); + const errorMessage = + error instanceof Error + ? error.message + : "Failed to start OAuth authentication"; + toast({ + description: errorMessage, + status: "error", + }); + onError?.(errorMessage); + } + }; + + useEffect(() => { + return () => { + cleanup(); + }; + }, []); + + return { + openOAuthPopup, + isAuthorizing, + cleanup, + }; +}; diff --git a/packages/forge/blocks/gmail/src/actions/sendEmail.ts b/packages/forge/blocks/gmail/src/actions/sendEmail.ts index 1da054ea6..85859d595 100644 --- a/packages/forge/blocks/gmail/src/actions/sendEmail.ts +++ b/packages/forge/blocks/gmail/src/actions/sendEmail.ts @@ -1,10 +1,9 @@ -import { gmail } from "@googleapis/gmail"; import { createAction, option } from "@typebot.io/forge"; import { parseUnknownError } from "@typebot.io/lib/parseUnknownError"; import { isDefined, isNotEmpty } from "@typebot.io/lib/utils"; -import { OAuth2Client } from "google-auth-library"; import { auth } from "../auth"; import { buildEmail } from "../helpers/buildEmail"; +import { createGmailClient } from "../helpers/createGmailClient"; import { parseFrom } from "../helpers/parseFrom"; export const sendEmail = createAction({ @@ -66,27 +65,31 @@ export const sendEmail = createAction({ data: [], }; - const client = new OAuth2Client({ - credentials: { - access_token: credentials.accessToken, - }, - }); + try { + const gmailClient = createGmailClient(credentials.accessToken); - const gmailClient = gmail({ - version: "v1", - auth: client, - }); + const response = await gmailClient.users.labels.list({ + userId: "me", + }); - const response = await gmailClient.users.labels.list({ userId: "me" }); - return { - data: - response.data.labels - ?.filter((label) => label.type === "user") - .map((label) => ({ - value: label.id ?? "", - label: label.name ?? "", - })) ?? [], - }; + return { + data: + response.data.labels + ?.filter((label) => label.type === "user") + .map((label) => ({ + value: label.id ?? "", + label: label.name ?? "", + })) ?? [], + }; + } catch (err) { + const parsedError = await parseUnknownError({ + err, + context: "While fetching Gmail labels", + }); + return { + error: parsedError, + }; + } }, }, ], @@ -98,17 +101,7 @@ export const sendEmail = createAction({ logs.add("No access token available"); return; } - - const client = new OAuth2Client({ - credentials: { - access_token: credentials.accessToken, - }, - }); - - const gmailClient = gmail({ - version: "v1", - auth: client, - }); + const gmailClient = createGmailClient(credentials.accessToken); const attachmentsVariableValue = options.attachments ? variables.get(options.attachments) diff --git a/packages/forge/blocks/gmail/src/helpers/createGmailClient.ts b/packages/forge/blocks/gmail/src/helpers/createGmailClient.ts new file mode 100644 index 000000000..baea4673a --- /dev/null +++ b/packages/forge/blocks/gmail/src/helpers/createGmailClient.ts @@ -0,0 +1,15 @@ +import { gmail } from "@googleapis/gmail"; +import { OAuth2Client } from "google-auth-library"; + +export const createGmailClient = (accessToken: string) => { + const oauthClient = new OAuth2Client({ + credentials: { + access_token: accessToken, + }, + }); + + return gmail({ + version: "v1", + auth: oauthClient, + }); +};