diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 0d909e8f6..cfd69b94d 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -3021,6 +3021,7 @@ export const createExecutor = secretsSet(input), connectionsCreate: (input) => connectionsCreate(input), + connectionsGet: (id) => connectionsGet(id), httpClientLayer: config.httpClientLayer, endpointUrlPolicy: config.oauthEndpointUrlPolicy, }); diff --git a/packages/core/sdk/src/oauth-service.ts b/packages/core/sdk/src/oauth-service.ts index 4357ea698..cac870744 100644 --- a/packages/core/sdk/src/oauth-service.ts +++ b/packages/core/sdk/src/oauth-service.ts @@ -76,6 +76,10 @@ import { beginDynamicAuthorization, discoverAuthorizationServerMetadata, discoverProtectedResourceMetadata, + type BeginDynamicAuthorizationInput, + type OAuthAuthorizationServerMetadata, + type OAuthClientInformation, + type OAuthProtectedResourceMetadata, } from "./oauth-discovery"; import { buildAuthorizationUrl, @@ -111,6 +115,14 @@ const DynamicDcrSessionPayload = Schema.Struct({ resource: Schema.NullOr(Schema.String).pipe(Schema.withDecodingDefaultType(Effect.succeed(null))), }); +const PendingDynamicDcrSessionRows = Schema.Array( + Schema.Struct({ + payload: Schema.Unknown, + expires_at: Schema.Union([Schema.Number, Schema.BigInt, Schema.String]), + created_at: Schema.Union([Schema.Date, Schema.String, Schema.Number]), + }), +); + const AuthorizationCodeSessionPayload = Schema.Struct({ kind: Schema.Literal("authorization-code"), identityLabel: Schema.NullOr(Schema.String), @@ -141,14 +153,17 @@ const OAuthSessionPayload = Schema.Union([ AuthorizationCodeSessionPayload, ]); type OAuthSessionPayload = typeof OAuthSessionPayload.Type; +type PreviousDynamicAuthorizationState = BeginDynamicAuthorizationInput["previousState"]; const decodeSessionPayload = Schema.decodeUnknownSync(OAuthSessionPayload); const encodeSessionPayload = Schema.encodeSync(OAuthSessionPayload); +const isPendingDynamicDcrSessionRows = Schema.is(PendingDynamicDcrSessionRows); const UnknownFromJsonString = Schema.fromJsonString(Schema.Unknown); const decodeUnknownJsonOption = Schema.decodeUnknownOption(UnknownFromJsonString); const decodeProviderStateSync = Schema.decodeUnknownSync(OAuthProviderStateSchema); +const decodeProviderStateOption = Schema.decodeUnknownOption(OAuthProviderStateSchema); const encodeProviderStateSync = Schema.encodeSync(OAuthProviderStateSchema); const coerceJson = (value: unknown): unknown => { @@ -187,6 +202,10 @@ export interface OAuthServiceDeps { readonly connectionsCreate: ( input: CreateConnectionInput, ) => Effect.Effect; + /** Reads an existing Connection so dynamic-DCR retries can reuse the + * registered OAuth client instead of registering a new client every + * time the user restarts a browser flow. */ + readonly connectionsGet?: (id: string) => Effect.Effect; /** Random session id generator. Tests override to make outputs * deterministic. */ readonly newSessionId?: () => string; @@ -239,6 +258,7 @@ export const makeOAuth2Service = ( const newSessionId = deps.newSessionId ?? defaultSessionId; const httpClientLayer = deps.httpClientLayer; const endpointUrlPolicy = deps.endpointUrlPolicy; + const connectionsGet = deps.connectionsGet ?? (() => Effect.succeed(null)); const secretsGetResolved = deps.secretsGetResolved ?? ((id: string) => @@ -372,17 +392,119 @@ export const makeOAuth2Service = ( // ------------------------------------------------------------------- // start — branches on strategy.kind // ------------------------------------------------------------------- + + const dynamicClientAuthMethod = ( + state: Extract, + ): "none" | "client_secret_basic" | "client_secret_post" => + state.clientSecretSecretId + ? state.clientAuth === "basic" + ? "client_secret_basic" + : "client_secret_post" + : "none"; + + const timestampMillis = (value: unknown): number => { + if (value instanceof Date) return value.getTime(); + if (typeof value === "string" || typeof value === "number") return new Date(value).getTime(); + return 0; + }; + + const previousDynamicStateFromConnection = ( + connectionId: string, + ): Effect.Effect => + Effect.gen(function* () { + const existing = yield* connectionsGet(connectionId); + const state = existing?.providerState + ? Option.getOrNull(decodeProviderStateOption(coerceJson(existing.providerState))) + : null; + if (!state || state.kind !== "dynamic-dcr") return undefined; + + const clientSecret = + state.clientSecretSecretId !== null + ? yield* getSecretFromRecordedScope({ + secretId: state.clientSecretSecretId, + scopeId: state.clientSecretSecretScopeId ?? null, + }) + : null; + if (state.clientSecretSecretId !== null && !clientSecret) return undefined; + + return { + authorizationServerUrl: state.authorizationServerUrl ?? null, + authorizationServerMetadataUrl: state.authorizationServerMetadataUrl, + clientInformation: { + client_id: state.clientId, + token_endpoint_auth_method: dynamicClientAuthMethod(state), + ...(clientSecret ? { client_secret: clientSecret } : {}), + }, + }; + }); + + const previousDynamicStateFromPendingSession = (input: { + readonly connectionId: string; + readonly tokenScope: string; + }): Effect.Effect => + Effect.gen(function* () { + const rowsRaw = yield* deps.fuma.use("oauth2_session.findReusableDynamicDcr", (db) => + db.findMany("oauth2_session", { + where: (b) => + b.and( + b("connection_id", "=", input.connectionId), + b("token_scope", "=", input.tokenScope), + b("strategy", "=", "dynamic-dcr"), + ), + }), + ); + const rows = isPendingDynamicDcrSessionRows(rowsRaw) ? rowsRaw : []; + + const reusable = rows + .filter((row) => Number(row.expires_at) > now()) + .sort((a, b) => { + const aTime = timestampMillis(a.created_at); + const bTime = timestampMillis(b.created_at); + return bTime - aTime; + }); + + for (const row of reusable) { + const payload = decodeSessionPayload(row.payload); + if (payload.kind !== "dynamic-dcr") continue; + return { + authorizationServerUrl: payload.authorizationServerUrl, + authorizationServerMetadataUrl: payload.authorizationServerMetadataUrl, + authorizationServerMetadata: + payload.authorizationServerMetadata as OAuthAuthorizationServerMetadata, + resourceMetadata: payload.resourceMetadata as OAuthProtectedResourceMetadata | null, + resourceMetadataUrl: payload.resourceMetadataUrl, + clientInformation: payload.clientInformation as OAuthClientInformation, + }; + } + return undefined; + }); + + const previousDynamicState = (input: { + readonly connectionId: string; + readonly tokenScope: string; + }) => + previousDynamicStateFromPendingSession(input).pipe( + Effect.flatMap((pending) => + pending ? Effect.succeed(pending) : previousDynamicStateFromConnection(input.connectionId), + ), + ); + const startDynamicDcr = ( input: OAuthStartInput, strategy: OAuthDynamicDcrStrategy, ): Effect.Effect => Effect.gen(function* () { + const previousState = yield* previousDynamicState({ + connectionId: input.connectionId, + tokenScope: input.tokenScope, + }); const started = yield* beginDynamicAuthorization( { endpoint: input.endpoint, redirectUrl: input.redirectUrl, state: "", scopes: strategy.scopes, + previousState, }, { httpClientLayer, diff --git a/packages/core/sdk/src/testing.test.ts b/packages/core/sdk/src/testing.test.ts index 8ceb0aca3..16b4c3e8b 100644 --- a/packages/core/sdk/src/testing.test.ts +++ b/packages/core/sdk/src/testing.test.ts @@ -115,6 +115,78 @@ layer(TestLayer, { timeout: "15 seconds" })("testing fixtures", (it) => { }), ); + it.effect("dynamic client registration is reused across OAuth start retries", () => + Effect.gen(function* () { + const workspace = yield* TestWorkspace.current(); + const oauth = yield* OAuthTestServer; + const scope = workspace.scopes[0]!; + yield* oauth.clearRequests; + + const start = () => + workspace.executor.oauth.start({ + endpoint: oauth.mcpResourceUrl, + connectionId: "test-oauth-dcr-retry", + tokenScope: String(scope.id), + redirectUrl: "http://127.0.0.1/callback", + pluginId: "test", + identityLabel: "MCP OAuth Test", + strategy: { kind: "dynamic-dcr", scopes: ["read"] }, + }); + + const startedA = yield* start(); + const startedB = yield* start(); + + expect(startedA.authorizationUrl).not.toBeNull(); + expect(startedB.authorizationUrl).not.toBeNull(); + expect( + (yield* oauth.requests).filter((request) => request.path === "/register"), + ).toHaveLength(1); + expect(new URL(startedB.authorizationUrl ?? "").searchParams.get("client_id")).toBe( + new URL(startedA.authorizationUrl ?? "").searchParams.get("client_id"), + ); + }), + ); + + it.effect("dynamic client registration is reused after a completed connection reconnect", () => + Effect.gen(function* () { + const workspace = yield* TestWorkspace.current(); + const oauth = yield* OAuthTestServer; + const scope = workspace.scopes[0]!; + yield* oauth.clearRequests; + + const start = () => + workspace.executor.oauth.start({ + endpoint: oauth.mcpResourceUrl, + connectionId: "test-oauth-dcr-reconnect", + tokenScope: String(scope.id), + redirectUrl: "http://127.0.0.1/callback", + pluginId: "test", + identityLabel: "MCP OAuth Test", + strategy: { kind: "dynamic-dcr", scopes: ["read"] }, + }); + + const startedA = yield* start(); + const callback = yield* oauth.completeAuthorizationCodeFlow({ + authorizationUrl: startedA.authorizationUrl ?? "", + }); + yield* workspace.executor.oauth.complete({ + state: callback.state, + code: callback.code, + tokenScope: String(scope.id), + }); + + const startedB = yield* start(); + + expect(startedB.authorizationUrl).not.toBeNull(); + expect( + (yield* oauth.requests).filter((request) => request.path === "/register"), + ).toHaveLength(1); + expect(new URL(startedB.authorizationUrl ?? "").searchParams.get("client_id")).toBe( + new URL(startedA.authorizationUrl ?? "").searchParams.get("client_id"), + ); + }), + ); + it.effect( "OAuthTestServer can mint a bearer token through the full authorization-code flow", () => diff --git a/packages/react/src/api/oauth-popup.test.ts b/packages/react/src/api/oauth-popup.test.ts index 396d1b276..d27ec973c 100644 --- a/packages/react/src/api/oauth-popup.test.ts +++ b/packages/react/src/api/oauth-popup.test.ts @@ -42,7 +42,7 @@ describe("openOAuthPopup", () => { expect(openFailed).toBe(true); }); - it("opens supported OAuth URLs through a reserved popup", () => { + it("opens supported OAuth URLs through a reserved popup with opener available", () => { let features = ""; let opened = ""; const popup: FakePopup = { closed: false, close: () => {}, location: { href: "" } }; @@ -81,7 +81,7 @@ describe("openOAuthPopup", () => { writable: true, }); expect(opened).toBe("about:blank"); - expect(popup.opener).toBe(null); + expect(popup.opener).toBeUndefined(); expect(popup.location.href).toBe("https://auth.example/authorize"); expect(features).toContain("popup=1"); expect(features).not.toContain("noopener"); @@ -128,7 +128,7 @@ describe("openOAuthPopup", () => { }); expect(opened).toBe("about:blank"); expect(reservedPopup).not.toBeNull(); - expect(popup.opener).toBe(null); + expect(popup.opener).toBeUndefined(); expect(popup.location.href).toBe("https://auth.example/authorize"); }); diff --git a/packages/react/src/api/oauth-popup.ts b/packages/react/src/api/oauth-popup.ts index ec51be81d..4bede00b3 100644 --- a/packages/react/src/api/oauth-popup.ts +++ b/packages/react/src/api/oauth-popup.ts @@ -72,15 +72,9 @@ export const reserveOAuthPopup = (input: { }): ReservedOAuthPopup | null => { const popup = window.open("about:blank", input.popupName, oauthPopupFeatures(input)); if (!popup) return null; - // The app keeps a WindowProxy for navigation/closed polling, but the - // provider should not receive opener access after the reserved window - // is navigated cross-origin. The callback uses BroadcastChannel. - // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: popup opener access can throw in browser-specific states - try { - popup.opener = null; - } catch { - // Best-effort hardening; the popup handle is still usable for the flow. - } + // Keep opener available for the same-origin callback page. Browser + // BroadcastChannel delivery can be partitioned in popup flows, so the + // callback's postMessage path is the primary completion signal. return { popup }; };