diff --git a/clients/web/src/test/core/auth/runner-interactive-oauth.test.ts b/clients/web/src/test/core/auth/runner-interactive-oauth.test.ts index 05d3f810..eac480ee 100644 --- a/clients/web/src/test/core/auth/runner-interactive-oauth.test.ts +++ b/clients/web/src/test/core/auth/runner-interactive-oauth.test.ts @@ -84,6 +84,34 @@ describe("runRunnerInteractiveOAuth", () => { expect(client.completeOAuthFlow).not.toHaveBeenCalled(); }); + it("clears the callback timeout on already_authorized", async () => { + vi.useFakeTimers(); + try { + const client = mockClient({ + authenticate: vi.fn(async () => undefined), + }); + const redirectUrlProvider = { redirectUrl: "" }; + + const result = await runRunnerInteractiveOAuth({ + client, + redirectUrlProvider, + callbackListen: { + hostname: "127.0.0.1", + port: 6276, + pathname: "/oauth/callback", + }, + callbackTimeoutMs: 60_000, + createCallbackServer: () => createMockCallbackServer(handlers), + }); + + expect(result).toEqual({ kind: "already_authorized" }); + expect(vi.getTimerCount()).toBe(0); + await vi.advanceTimersByTimeAsync(60_000); + } finally { + vi.useRealTimers(); + } + }); + it("completes connect-time OAuth via authenticate and callback", async () => { const redirectUrlProvider = { redirectUrl: "" }; const client = mockClient({ diff --git a/clients/web/src/test/core/mcp/oauthManager.test.ts b/clients/web/src/test/core/mcp/oauthManager.test.ts index 328ceca8..13fe1bc4 100644 --- a/clients/web/src/test/core/mcp/oauthManager.test.ts +++ b/clients/web/src/test/core/mcp/oauthManager.test.ts @@ -1215,9 +1215,6 @@ describe("OAuthManager", () => { }); it("returns satisfied for EMA insufficient_scope after user confirms", async () => { - const silentSpy = vi - .spyOn(emaFlow, "trySilentEmaAuth") - .mockResolvedValue({ status: "success" }); const params = createMockParams({ enterpriseManagedAuth: { idp: { @@ -1237,6 +1234,14 @@ describe("OAuthManager", () => { token_type: "Bearer", scope: "mcp", }); + const silentSpy = vi.spyOn(emaFlow, "trySilentEmaAuth").mockImplementation(async () => { + storageOf(params).getTokens.mockResolvedValue({ + access_token: "tok", + token_type: "Bearer", + scope: "mcp weather:read", + }); + return { status: "success" }; + }); const manager = new OAuthManager(params); manager.setOAuthConfig({ enterpriseManaged: true }); @@ -1257,6 +1262,51 @@ describe("OAuthManager", () => { silentSpy.mockRestore(); }); + it("does not persist union scope or return satisfied when silent EMA mint is down-scoped", async () => { + const silentSpy = vi + .spyOn(emaFlow, "trySilentEmaAuth") + .mockResolvedValue({ status: "success" }); + const authUrl = new URL("https://idp.example.com/authorize?state=ema"); + const startSpy = vi + .spyOn(emaFlow, "startEmaIdpAuthorization") + .mockResolvedValue(authUrl); + const params = createMockParams({ + enterpriseManagedAuth: { + idp: { + issuer: "https://idp.example.com", + clientId: "app-client", + clientSecret: "secret", + }, + }, + }); + storageOf(params).getScope.mockReturnValue("mcp"); + storageOf(params).getTokens.mockResolvedValue({ + access_token: "tok", + token_type: "Bearer", + scope: "mcp", + }); + const manager = new OAuthManager(params); + manager.setOAuthConfig({ enterpriseManaged: true }); + + const outcome = await manager.handleAuthChallenge( + { + reason: "insufficient_scope", + requiredScopes: ["weather:read"], + }, + { confirmedStepUp: true }, + ); + + expect(outcome).toEqual( + expect.objectContaining({ + kind: "interactive", + authorizationUrl: authUrl, + }), + ); + expect(storageOf(params).saveScope).not.toHaveBeenCalled(); + silentSpy.mockRestore(); + startSpy.mockRestore(); + }); + it("completeOAuthFlow mints EMA tokens with pending step-up union scope", async () => { const silentSpy = vi .spyOn(emaFlow, "trySilentEmaAuth") diff --git a/core/auth/node/runner-interactive-oauth.ts b/core/auth/node/runner-interactive-oauth.ts index ab6a9aad..77de2b17 100644 --- a/core/auth/node/runner-interactive-oauth.ts +++ b/core/auth/node/runner-interactive-oauth.ts @@ -58,6 +58,8 @@ export async function runRunnerInteractiveOAuth( flowReject = reject; }); + let timeoutId: ReturnType | undefined; + try { const { redirectUrl } = await server.start({ hostname: options.callbackListen.hostname, @@ -85,7 +87,6 @@ export async function runRunnerInteractiveOAuth( const timeoutMs = options.callbackTimeoutMs ?? DEFAULT_RUNNER_INTERACTIVE_OAUTH_TIMEOUT_MS; - let timeoutId: ReturnType | undefined; const waitForCallback = Promise.race([ flowDone.finally(() => { if (timeoutId !== undefined) { @@ -131,6 +132,9 @@ export async function runRunnerInteractiveOAuth( return { kind: "success" }; } finally { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } await server.stop().catch(() => {}); } } diff --git a/core/mcp/oauthManager.ts b/core/mcp/oauthManager.ts index 8d38362d..96c95e19 100644 --- a/core/mcp/oauthManager.ts +++ b/core/mcp/oauthManager.ts @@ -602,13 +602,13 @@ export class OAuthManager { if (enriched.reason === "insufficient_scope") { const silent = await trySilentEmaAuth(config); if (silent.status === "success") { - if (enriched.authorizationScopes?.length) { - await this.oauthConfig.storage!.saveScope( - this.getServerUrl(), - enriched.authorizationScopes.join(" "), - ); - } if (await this.checkAuthChallengeSatisfied(enriched)) { + if (enriched.authorizationScopes?.length) { + await this.oauthConfig.storage!.saveScope( + this.getServerUrl(), + enriched.authorizationScopes.join(" "), + ); + } return { kind: "satisfied" }; } }