diff --git a/packages/types/src/oauth.ts b/packages/types/src/oauth.ts index fdf22b38998..c261819de10 100644 --- a/packages/types/src/oauth.ts +++ b/packages/types/src/oauth.ts @@ -6,11 +6,23 @@ export const OAUTH_POPUP_MESSAGE_TYPE = "oauth-popup-message"; +export type OAuthPopupPayload = + | { + nonce: string; + code: string; + redirectPath: string; + scopes?: string[]; + authuser?: string; + error?: undefined; + } + | { + nonce: string; + error: string; + }; + export type OAuthPopupMessage = { type: typeof OAUTH_POPUP_MESSAGE_TYPE; - nonce: string; - grantResponse: GrantResponse; -}; +} & OAuthPopupPayload; export type GrantResponse = | { error: string } diff --git a/packages/unified-server/src/connection/api/grant.ts b/packages/unified-server/src/connection/api/grant.ts index 2aa730c6bc3..9a83104316a 100644 --- a/packages/unified-server/src/connection/api/grant.ts +++ b/packages/unified-server/src/connection/api/grant.ts @@ -31,13 +31,17 @@ interface GrantRequest { * API which performs first-time authorization for a connection. */ export async function grant( - req: IncomingMessage, + req: IncomingMessage & { body?: unknown }, res: ServerResponse, config: ServerConfig ): Promise { - const params = Object.fromEntries( + const queryParams = Object.fromEntries( new URL(req.url ?? "", "http://example.com").searchParams.entries() - ) as object as GrantRequest; + ); + const params = { + ...queryParams, + ...(typeof req.body === "object" && req.body !== null ? req.body : {}), + } as unknown as GrantRequest; if (!params.code) { return badRequestJson(res, { error: "missing code" }); } diff --git a/packages/unified-server/src/connection/server.ts b/packages/unified-server/src/connection/server.ts index ba3dab5f483..b0c646b8a6f 100644 --- a/packages/unified-server/src/connection/server.ts +++ b/packages/unified-server/src/connection/server.ts @@ -40,11 +40,12 @@ export function createServer(config: ServerConfig): Express { }) ); + server.use(express.json()); server.use(cookieParser()); // TODO: #3172 - Common error handling - server.get("/grant", async (req: Request, res: Response) => + server.post("/grant", async (req: Request, res: Response) => grant(req, res, config) ); diff --git a/packages/visual-editor/src/ui/elements/connection/connection-broker.ts b/packages/visual-editor/src/ui/elements/connection/connection-broker.ts index f01c3684ded..ee22cb01c8e 100644 --- a/packages/visual-editor/src/ui/elements/connection/connection-broker.ts +++ b/packages/visual-editor/src/ui/elements/connection/connection-broker.ts @@ -6,7 +6,7 @@ import { OAUTH_POPUP_MESSAGE_TYPE, - type GrantResponse, + type OAuthPopupPayload, } from "@breadboard-ai/types/oauth.js"; import { sendToAllowedEmbedderIfPresent } from "../../utils/embedder.js"; import { type OAuthStateParameter } from "./connection-common.js"; @@ -62,9 +62,9 @@ export class ConnectionBroker extends HTMLElement { return; } - function sendToOpener(grantResponse: GrantResponse): void { + function sendToOpener(msg: OAuthPopupPayload): void { window.opener.postMessage( - { type: OAUTH_POPUP_MESSAGE_TYPE, nonce, grantResponse }, + { type: OAUTH_POPUP_MESSAGE_TYPE, ...msg }, window.location.origin ); } @@ -73,7 +73,7 @@ export class ConnectionBroker extends HTMLElement { // user clicks "Cancel" during the OAuth flow. const error = thisUrl.searchParams.get("error"); if (error) { - sendToOpener({ error }); + sendToOpener({ nonce, error }); window.close(); return; } @@ -85,35 +85,13 @@ export class ConnectionBroker extends HTMLElement { return; } - // TODO(aomarks) Would it be better to send the code directly back to the - // opener, so that it can check the nonce, and only then do this grant RPC - // itself? - const grantUrl = new URL("/connection/grant/", window.location.origin); - grantUrl.searchParams.set("code", code); - grantUrl.searchParams.set( - "redirect_path", - new URL(window.location.href).pathname - ); - const response = await fetch(grantUrl, { credentials: "include" }); - let grantResponse: GrantResponse; - try { - grantResponse = await response.json(); - } catch { - grantResponse = { - error: "Invalid response from connection server", - }; - } - - // Add the actual scopes the user selected. - if (grantResponse.error === undefined) { - grantResponse.scopes = - thisUrl.searchParams.get("scope")?.trim().split(/ +/) ?? []; - grantResponse.authuser = - thisUrl.searchParams.get("authuser") ?? undefined; - } + // Send the authorization code and nonce back to the originating tab so it can + // verify the nonce before making the grant request. + const scopes = thisUrl.searchParams.get("scope")?.trim().split(/ +/) ?? []; + const authuser = thisUrl.searchParams.get("authuser") ?? undefined; + const redirectPath = new URL(window.location.href).pathname; - // Send the grant response back to the originating tab and close up shop. - sendToOpener(grantResponse); + sendToOpener({ nonce, code, redirectPath, scopes, authuser }); sendToAllowedEmbedderIfPresent({ type: "oauth_redirect", success: true, diff --git a/packages/visual-editor/src/ui/utils/oauth-based-opal-shell.ts b/packages/visual-editor/src/ui/utils/oauth-based-opal-shell.ts index 4740860e6d7..bcaca6455d6 100644 --- a/packages/visual-editor/src/ui/utils/oauth-based-opal-shell.ts +++ b/packages/visual-editor/src/ui/utils/oauth-based-opal-shell.ts @@ -11,6 +11,7 @@ import * as Comlink from "comlink"; import type { BreadboardMessage } from "@breadboard-ai/types/embedder.js"; import { OAUTH_POPUP_MESSAGE_TYPE, + type GrantResponse, type MissingScopesTokenResult, type OAuthPopupMessage, type SignedOutTokenResult, @@ -461,14 +462,56 @@ export class OAuthBasedOpalShell implements OpalShellHostProtocol { error: { code: "other", userMessage: "Verification failed" }, }; } - const { grantResponse } = popupMessage; - const issueTime = Date.now(); - console.info(`[shell host] Received grant response`); - if (grantResponse.error !== undefined) { - if (grantResponse.error === "access_denied") { + if (popupMessage.error !== undefined) { + if (popupMessage.error === "access_denied") { console.info(`[shell host] User cancelled sign-in`); return { ok: false, error: { code: "user-cancelled" } }; } + console.error(`[shell host] Unknown grant error`, popupMessage.error); + return { + ok: false, + error: { + code: "other", + userMessage: `Unknown grant error ${JSON.stringify(popupMessage.error)}`, + }, + }; + } + if (!popupMessage.code) { + console.error(`[shell host] Missing authorization code`, popupMessage); + return { + ok: false, + error: { code: "other", userMessage: "Missing authorization code" }, + }; + } + + // Now that the nonce is verified, call the token grant API. + const grantUrl = new URL("/connection/grant/", window.location.origin); + const response = await fetch(grantUrl, { + method: "POST", + headers: { "Content-Type": "application/json" }, + credentials: "include", + body: JSON.stringify({ + code: popupMessage.code, + redirect_path: popupMessage.redirectPath ?? "/oauth/", + }), + }); + let grantResponse: GrantResponse; + try { + grantResponse = await response.json(); + } catch { + grantResponse = { + error: "Invalid response from connection server", + }; + } + + if (grantResponse.error === undefined) { + grantResponse.scopes = popupMessage.scopes ?? []; + grantResponse.authuser = popupMessage.authuser; + } + + const issueTime = Date.now(); + console.info(`[shell host] Received grant response`); + if (grantResponse.error !== undefined) { console.error(`[shell host] Unknown grant error`, grantResponse.error); return { ok: false,