diff --git a/clients/web/src/test/core/auth/challenge.test.ts b/clients/web/src/test/core/auth/challenge.test.ts index 59690a00..bafacc01 100644 --- a/clients/web/src/test/core/auth/challenge.test.ts +++ b/clients/web/src/test/core/auth/challenge.test.ts @@ -146,6 +146,39 @@ describe("parseAuthChallengeFromResponse", () => { const response = new Response(null, { status: 500 }); expect(parseAuthChallengeFromResponse(response)).toBeUndefined(); }); + + it("carries error_description into the challenge message", () => { + const response = new Response(null, { + status: 401, + headers: { + "WWW-Authenticate": + 'Bearer error="invalid_token", error_description="Token expired"', + }, + }); + + expect(parseAuthChallengeFromResponse(response)).toEqual({ + reason: "invalid_token", + message: "Token expired", + raw: { + httpStatus: 401, + wwwAuthenticate: + 'Bearer error="invalid_token", error_description="Token expired"', + }, + }); + }); + + it("maps 403 with a non-scope error to unauthorized", () => { + const response = new Response(null, { + status: 403, + headers: { + "WWW-Authenticate": 'Bearer error="invalid_token"', + }, + }); + + expect(parseAuthChallengeFromResponse(response)?.reason).toBe( + "unauthorized", + ); + }); }); describe("parseAuthChallengeFromError", () => { @@ -174,6 +207,82 @@ describe("parseAuthChallengeFromError", () => { it("returns undefined for bare 401 without auth markers", () => { expect(parseAuthChallengeFromError({ status: 401 })).toBeUndefined(); }); + + it("returns the challenge directly for AuthChallengeError instances", () => { + const err = new AuthChallengeError({ reason: "invalid_token" }, 401); + expect(parseAuthChallengeFromError(err)).toEqual({ + reason: "invalid_token", + }); + }); + + it("returns undefined for non-object and null errors", () => { + expect(parseAuthChallengeFromError("boom")).toBeUndefined(); + expect(parseAuthChallengeFromError(null)).toBeUndefined(); + }); + + it("merges context into an embedded authChallenge", () => { + expect( + parseAuthChallengeFromError( + { + authChallenge: { reason: "token_expired", context: { method: "x" } }, + }, + { toolName: "get_temp" }, + ), + ).toEqual({ + reason: "token_expired", + context: { method: "x", toolName: "get_temp" }, + }); + }); + + it("falls back to the numeric code when status is absent", () => { + expect( + parseAuthChallengeFromError({ + code: 403, + wwwAuthenticate: 'Bearer error="insufficient_scope", scope="admin"', + }), + ).toMatchObject({ + reason: "insufficient_scope", + requiredScopes: ["admin"], + raw: { httpStatus: 403 }, + }); + }); + + it("returns undefined when the status is neither 401 nor 403", () => { + expect(parseAuthChallengeFromError({ status: 500 })).toBeUndefined(); + }); + + it("reads WWW-Authenticate from a headers.get accessor", () => { + expect( + parseAuthChallengeFromError({ + status: 401, + headers: { + get: (name: string) => + name === "WWW-Authenticate" ? 'Bearer error="invalid_token"' : null, + }, + }), + ).toMatchObject({ + reason: "invalid_token", + raw: { httpStatus: 401, wwwAuthenticate: 'Bearer error="invalid_token"' }, + }); + }); + + it("reads WWW-Authenticate from an embedded raw challenge", () => { + expect( + parseAuthChallengeFromError({ + status: 401, + authChallenge: { raw: { wwwAuthenticate: "Bearer realm=mcp" } }, + }), + ).toMatchObject({ + reason: "token_expired", + raw: { httpStatus: 401, wwwAuthenticate: "Bearer realm=mcp" }, + }); + }); + + it("returns undefined for an empty WWW-Authenticate header", () => { + expect( + parseAuthChallengeFromError({ status: 401, wwwAuthenticate: "" }), + ).toBeUndefined(); + }); }); describe("isAuthChallengeError", () => { @@ -206,6 +315,53 @@ describe("isAuthChallengeError", () => { it("does not treat connect-time unauthorized wording as auth challenge", () => { expect(isAuthChallengeError(new Error("network failed"))).toBe(false); }); + + it("returns false for non-object and null errors", () => { + expect(isAuthChallengeError("boom")).toBe(false); + expect(isAuthChallengeError(null)).toBe(false); + }); + + it("detects an embedded authChallenge with a reason", () => { + expect( + isAuthChallengeError({ authChallenge: { reason: "token_expired" } }), + ).toBe(true); + }); + + it("uses the numeric code when status is absent", () => { + expect( + isAuthChallengeError({ + code: 403, + wwwAuthenticate: 'Bearer error="insufficient_scope"', + }), + ).toBe(true); + }); + + it("reads WWW-Authenticate from a headers.get accessor", () => { + expect( + isAuthChallengeError({ + status: 401, + headers: { + get: (name: string) => + name === "WWW-Authenticate" ? "Bearer realm=mcp" : null, + }, + }), + ).toBe(true); + }); + + it("reads WWW-Authenticate from an embedded raw challenge", () => { + expect( + isAuthChallengeError({ + status: 401, + authChallenge: { raw: { wwwAuthenticate: "Bearer realm=mcp" } }, + }), + ).toBe(true); + }); + + it("returns false for an empty WWW-Authenticate header", () => { + expect(isAuthChallengeError({ status: 401, wwwAuthenticate: "" })).toBe( + false, + ); + }); }); describe("isConnectAuthRecoveryError", () => { diff --git a/clients/web/src/test/core/auth/mcpAuth.test.ts b/clients/web/src/test/core/auth/mcpAuth.test.ts index f973574b..79af5d79 100644 --- a/clients/web/src/test/core/auth/mcpAuth.test.ts +++ b/clients/web/src/test/core/auth/mcpAuth.test.ts @@ -6,11 +6,15 @@ const { discoverOAuthServerInfo, startAuthorization, selectResourceURL, + registerClient, + isHttpsUrl, } = vi.hoisted(() => ({ sdkAuth: vi.fn(), discoverOAuthServerInfo: vi.fn(), startAuthorization: vi.fn(), selectResourceURL: vi.fn(), + registerClient: vi.fn(), + isHttpsUrl: vi.fn(), })); vi.mock("@modelcontextprotocol/sdk/client/auth.js", async (importOriginal) => { @@ -24,12 +28,17 @@ vi.mock("@modelcontextprotocol/sdk/client/auth.js", async (importOriginal) => { discoverOAuthServerInfo, selectResourceURL, startAuthorization, + registerClient, + isHttpsUrl, }; }); import { mcpAuth } from "@inspector/core/auth/mcpAuth.js"; +import { InvalidClientMetadataError } from "@modelcontextprotocol/sdk/server/auth/errors.js"; -function makeProvider(): OAuthClientProvider { +function makeProvider( + overrides: Partial = {}, +): OAuthClientProvider { return { redirectUrl: "http://localhost/callback", clientMetadata: { @@ -47,12 +56,38 @@ function makeProvider(): OAuthClientProvider { redirectToAuthorization: vi.fn(), saveCodeVerifier: vi.fn(), codeVerifier: vi.fn().mockReturnValue("verifier"), + ...overrides, }; } +/** + * Arrange the discovery + authorization mocks used by the + * `forceReauthorization` (authorize-without-refresh) path. + */ +function stubForcePath( + discovery: Partial<{ + authorizationServerUrl: string; + authorizationServerMetadata: Record; + resourceMetadata: unknown; + }> = {}, +): void { + discoverOAuthServerInfo.mockResolvedValue({ + authorizationServerUrl: "https://as.example.com", + authorizationServerMetadata: { issuer: "https://as.example.com" }, + resourceMetadata: undefined, + ...discovery, + }); + selectResourceURL.mockResolvedValue(undefined); + startAuthorization.mockResolvedValue({ + authorizationUrl: new URL("https://as.example.com/authorize"), + codeVerifier: "cv", + }); +} + describe("mcpAuth", () => { beforeEach(() => { vi.clearAllMocks(); + isHttpsUrl.mockReturnValue(true); }); it("delegates to SDK auth() when forceReauthorization is not set", async () => { @@ -76,17 +111,24 @@ describe("mcpAuth", () => { expect(discoverOAuthServerInfo).not.toHaveBeenCalled(); }); + it("rejects forceReauthorization combined with authorizationCode", async () => { + const provider = makeProvider(); + + await expect( + mcpAuth(provider, { + serverUrl: "https://mcp.example.com", + authorizationCode: "code", + forceReauthorization: true, + }), + ).rejects.toThrow( + "forceReauthorization cannot be combined with authorizationCode", + ); + expect(sdkAuth).not.toHaveBeenCalled(); + expect(discoverOAuthServerInfo).not.toHaveBeenCalled(); + }); + it("uses discovery + startAuthorization when forceReauthorization is true", async () => { - discoverOAuthServerInfo.mockResolvedValue({ - authorizationServerUrl: "https://as.example.com", - authorizationServerMetadata: { issuer: "https://as.example.com" }, - resourceMetadata: undefined, - }); - selectResourceURL.mockResolvedValue(undefined); - startAuthorization.mockResolvedValue({ - authorizationUrl: new URL("https://as.example.com/authorize"), - codeVerifier: "cv", - }); + stubForcePath(); const provider = makeProvider(); const result = await mcpAuth(provider, { @@ -102,4 +144,172 @@ describe("mcpAuth", () => { expect(provider.saveCodeVerifier).toHaveBeenCalledWith("cv"); expect(provider.redirectToAuthorization).toHaveBeenCalled(); }); + + it("persists discovery state and forwards the selected resource + state", async () => { + stubForcePath({ + resourceMetadata: { resource: "https://mcp.example.com" }, + }); + selectResourceURL.mockResolvedValue(new URL("https://mcp.example.com/r")); + const saveDiscoveryState = vi.fn().mockResolvedValue(undefined); + const state = vi.fn().mockResolvedValue("state-123"); + const provider = makeProvider({ saveDiscoveryState, state }); + + await mcpAuth(provider, { + serverUrl: "https://mcp.example.com", + resourceMetadataUrl: new URL("https://mcp.example.com/.well-known/x"), + forceReauthorization: true, + }); + + expect(saveDiscoveryState).toHaveBeenCalledWith( + expect.objectContaining({ + authorizationServerUrl: "https://as.example.com", + resourceMetadataUrl: "https://mcp.example.com/.well-known/x", + }), + ); + expect(state).toHaveBeenCalled(); + expect(startAuthorization).toHaveBeenCalledWith( + "https://as.example.com", + expect.objectContaining({ + state: "state-123", + resource: new URL("https://mcp.example.com/r"), + }), + ); + }); + + it("derives scope from resourceMetadata.scopes_supported when no scope given", async () => { + stubForcePath({ + resourceMetadata: { scopes_supported: ["mcp", "weather:read"] }, + }); + const provider = makeProvider(); + + await mcpAuth(provider, { + serverUrl: "https://mcp.example.com", + forceReauthorization: true, + }); + + expect(startAuthorization).toHaveBeenCalledWith( + "https://as.example.com", + expect.objectContaining({ scope: "mcp weather:read" }), + ); + }); + + it("falls back to clientMetadata.scope when nothing else provides scope", async () => { + stubForcePath(); + const provider = makeProvider({ + clientMetadata: { + ...makeProvider().clientMetadata, + scope: "fallback:scope", + }, + }); + + await mcpAuth(provider, { + serverUrl: "https://mcp.example.com", + forceReauthorization: true, + }); + + expect(startAuthorization).toHaveBeenCalledWith( + "https://as.example.com", + expect.objectContaining({ scope: "fallback:scope" }), + ); + }); + + it("dynamically registers a client when no client information exists", async () => { + stubForcePath(); + registerClient.mockResolvedValue({ client_id: "registered" }); + const saveClientInformation = vi.fn().mockResolvedValue(undefined); + const provider = makeProvider({ + clientInformation: vi.fn().mockResolvedValue(undefined), + saveClientInformation, + }); + + const result = await mcpAuth(provider, { + serverUrl: "https://mcp.example.com", + scope: "mcp", + forceReauthorization: true, + }); + + expect(result).toBe("REDIRECT"); + expect(registerClient).toHaveBeenCalledWith( + "https://as.example.com", + expect.objectContaining({ scope: "mcp" }), + ); + expect(saveClientInformation).toHaveBeenCalledWith({ + client_id: "registered", + }); + }); + + it("uses a URL-based client id when the AS supports it", async () => { + stubForcePath({ + authorizationServerMetadata: { + issuer: "https://as.example.com", + client_id_metadata_document_supported: true, + }, + }); + isHttpsUrl.mockReturnValue(true); + const saveClientInformation = vi.fn().mockResolvedValue(undefined); + const provider = makeProvider({ + clientInformation: vi.fn().mockResolvedValue(undefined), + clientMetadataUrl: "https://app.example.com/client.json", + saveClientInformation, + }); + + await mcpAuth(provider, { + serverUrl: "https://mcp.example.com", + forceReauthorization: true, + }); + + expect(registerClient).not.toHaveBeenCalled(); + expect(saveClientInformation).toHaveBeenCalledWith({ + client_id: "https://app.example.com/client.json", + }); + }); + + it("rejects a non-HTTPS clientMetadataUrl", async () => { + stubForcePath(); + isHttpsUrl.mockReturnValue(false); + const provider = makeProvider({ + clientInformation: vi.fn().mockResolvedValue(undefined), + clientMetadataUrl: "http://insecure.example.com/client.json", + saveClientInformation: vi.fn(), + }); + + await expect( + mcpAuth(provider, { + serverUrl: "https://mcp.example.com", + forceReauthorization: true, + }), + ).rejects.toBeInstanceOf(InvalidClientMetadataError); + expect(registerClient).not.toHaveBeenCalled(); + }); + + it("requires saveClientInformation for dynamic registration", async () => { + stubForcePath(); + const provider = makeProvider({ + clientInformation: vi.fn().mockResolvedValue(undefined), + saveClientInformation: undefined, + }); + + await expect( + mcpAuth(provider, { + serverUrl: "https://mcp.example.com", + forceReauthorization: true, + }), + ).rejects.toThrow( + "OAuth client information must be saveable for dynamic registration", + ); + expect(registerClient).not.toHaveBeenCalled(); + }); + + it("requires a redirectUrl for the authorization_code flow", async () => { + stubForcePath(); + const provider = makeProvider({ redirectUrl: undefined }); + + await expect( + mcpAuth(provider, { + serverUrl: "https://mcp.example.com", + forceReauthorization: true, + }), + ).rejects.toThrow("redirectUrl is required for authorization_code flow"); + expect(startAuthorization).not.toHaveBeenCalled(); + }); }); diff --git a/clients/web/src/test/core/auth/oauthUx.test.ts b/clients/web/src/test/core/auth/oauthUx.test.ts index 5cbe86d1..f209c9ab 100644 --- a/clients/web/src/test/core/auth/oauthUx.test.ts +++ b/clients/web/src/test/core/auth/oauthUx.test.ts @@ -1,11 +1,22 @@ import { describe, it, expect } from "vitest"; import { + authRecoveryRestoredMessage, + emaStepUpFailureMessage, + emaStepUpInProgressMessage, emaStepUpSuccessMessage, + isActionTriggeredOAuthRecovery, isEmaStepUp, + isReAuthBannerReason, + isStandardOAuthStepUp, isStepUpConfirmation, + oauthPreRedirectToastCopy, + oauthResumeAbandonedMessage, + oauthResumeSuccessMessage, + reAuthBannerMessage, stepUpAdditionalScopes, stepUpConfirmMessage, stepUpFollowUpMessage, + stepUpInsufficientScopeMessage, stepUpModalTitle, stepUpAuthorizeActionLabel, } from "@inspector/core/auth/oauthUx.js"; @@ -34,6 +45,48 @@ describe("oauthUx step-up copy", () => { ).toBe("This operation needs additional scope: weather:read."); }); + it("stepUpConfirmMessage uses plural label and organization language for multiple EMA scopes", () => { + expect( + stepUpConfirmMessage( + { + reason: "insufficient_scope", + requiredScopes: ["weather:read", "weather:write"], + }, + { enterpriseManaged: true }, + ), + ).toBe( + "This operation needs additional organization scopes: weather:read, weather:write.", + ); + }); + + it("stepUpConfirmMessage uses plural label for multiple standard scopes", () => { + expect( + stepUpConfirmMessage({ + reason: "insufficient_scope", + requiredScopes: ["weather:read", "weather:write"], + }), + ).toBe( + "This operation needs additional scopes: weather:read, weather:write.", + ); + }); + + it("stepUpConfirmMessage falls back to generic standard copy with no tool or scopes", () => { + expect(stepUpConfirmMessage({ reason: "insufficient_scope" })).toBe( + "This operation needs additional OAuth scopes before it can continue.", + ); + }); + + it("stepUpConfirmMessage falls back to generic EMA copy with no tool or scopes", () => { + expect( + stepUpConfirmMessage( + { reason: "insufficient_scope", requiredScopes: ["", ""] }, + { enterpriseManaged: true }, + ), + ).toBe( + "This operation needs additional permissions from your organization before it can continue.", + ); + }); + it("stepUpConfirmMessage uses organization language for EMA", () => { expect( stepUpConfirmMessage(challenge, { enterpriseManaged: true }), @@ -41,15 +94,25 @@ describe("oauthUx step-up copy", () => { expect(stepUpFollowUpMessage({ enterpriseManaged: true })).toMatch( /identity provider/i, ); + expect(stepUpFollowUpMessage()).toMatch(/redirected to authorize/i); expect(stepUpModalTitle({ enterpriseManaged: true })).toMatch( /organization/i, ); + expect(stepUpModalTitle()).toBe("Additional permissions required"); expect(stepUpAuthorizeActionLabel({ enterpriseManaged: true })).toBe( "Authorize", ); expect(stepUpAuthorizeActionLabel()).toBe("Authorize (opens browser)"); }); + it("isStandardOAuthStepUp is true only for non-EMA insufficient_scope", () => { + expect(isStandardOAuthStepUp(challenge)).toBe(true); + expect(isStandardOAuthStepUp(challenge, { enterpriseManaged: true })).toBe( + false, + ); + expect(isStandardOAuthStepUp({ reason: "token_expired" })).toBe(false); + }); + it("isStepUpConfirmation covers standard OAuth and EMA insufficient_scope", () => { expect(isStepUpConfirmation(challenge)).toBe(true); expect(isStepUpConfirmation(challenge, { enterpriseManaged: true })).toBe( @@ -67,6 +130,10 @@ describe("oauthUx step-up copy", () => { ).toBe(false); }); + it("emaStepUpInProgressMessage describes requesting organization permissions", () => { + expect(emaStepUpInProgressMessage()).toMatch(/organization/i); + }); + it("emaStepUpSuccessMessage suggests retry only for command-scoped recovery", () => { expect(emaStepUpSuccessMessage()).toBe( "Organization permissions were updated.", @@ -76,7 +143,194 @@ describe("oauthUx step-up copy", () => { ); }); + it("emaStepUpFailureMessage returns detail when present, else generic copy", () => { + expect(emaStepUpFailureMessage("boom")).toBe("boom"); + expect(emaStepUpFailureMessage(" ")).toBe( + "Could not obtain the additional permissions from your organization.", + ); + expect(emaStepUpFailureMessage()).toBe( + "Could not obtain the additional permissions from your organization.", + ); + }); + it("stepUpAdditionalScopes returns requiredScopes only", () => { expect(stepUpAdditionalScopes(challenge)).toEqual(["weather:read"]); }); + + it("stepUpAdditionalScopes returns empty array when requiredScopes undefined", () => { + expect(stepUpAdditionalScopes({ reason: "insufficient_scope" })).toEqual( + [], + ); + }); +}); + +describe("oauthUx recovery-source predicates", () => { + it("isActionTriggeredOAuthRecovery is true for action sources, false otherwise", () => { + for (const source of ["tool", "prompt", "resource", "app"] as const) { + expect(isActionTriggeredOAuthRecovery(source)).toBe(true); + } + expect(isActionTriggeredOAuthRecovery("ambient")).toBe(false); + expect(isActionTriggeredOAuthRecovery(undefined)).toBe(false); + }); +}); + +describe("oauthUx resume/restore copy", () => { + it("oauthResumeSuccessMessage step_up varies with retry", () => { + expect( + oauthResumeSuccessMessage("step_up", { recoverySource: "tool" }), + ).toBe("Step-up authorization succeeded. Retry your action."); + expect(oauthResumeSuccessMessage("step_up")).toBe( + "Step-up authorization succeeded.", + ); + }); + + it("oauthResumeSuccessMessage reauth varies with retry", () => { + expect( + oauthResumeSuccessMessage("reauth", { recoverySource: "prompt" }), + ).toBe("Authentication succeeded. Retry your action."); + expect(oauthResumeSuccessMessage("reauth")).toBe( + "Authentication succeeded.", + ); + }); + + it("authRecoveryRestoredMessage varies with retry", () => { + expect(authRecoveryRestoredMessage({ recoverySource: "resource" })).toBe( + "Session credentials were updated. Retry your action.", + ); + expect(authRecoveryRestoredMessage()).toBe( + "Session credentials were updated.", + ); + }); + + it("oauthResumeAbandonedMessage reauth is retry-agnostic", () => { + expect( + oauthResumeAbandonedMessage("reauth", { recoverySource: "tool" }), + ).toBe("Sign-in was not completed. Re-authenticate to restore access."); + expect(oauthResumeAbandonedMessage("reauth")).toBe( + "Sign-in was not completed. Re-authenticate to restore access.", + ); + }); + + it("oauthResumeAbandonedMessage step_up varies with retry", () => { + expect( + oauthResumeAbandonedMessage("step_up", { recoverySource: "app" }), + ).toBe("Step-up authorization was not completed. Retry your action."); + expect(oauthResumeAbandonedMessage("step_up")).toBe( + "Step-up authorization was not completed.", + ); + }); +}); + +describe("oauthUx insufficient-scope resolution copy", () => { + it("prefers tool context", () => { + expect( + stepUpInsufficientScopeMessage({ + reason: "insufficient_scope", + context: { toolName: "get_temp" }, + }), + ).toMatch(/tool "get_temp"/); + }); + + it("uses authorizationScopes when present and no tool", () => { + expect( + stepUpInsufficientScopeMessage({ + reason: "insufficient_scope", + authorizationScopes: ["a", "b"], + requiredScopes: ["c"], + }), + ).toBe( + "Authorization completed, but required scopes were not granted (a, b). Grant the requested permissions on the authorization server, then retry your action.", + ); + }); + + it("falls back to requiredScopes when authorizationScopes absent", () => { + expect( + stepUpInsufficientScopeMessage({ + reason: "insufficient_scope", + requiredScopes: ["c"], + }), + ).toMatch(/\(c\)/); + }); + + it("uses generic copy when no tool or scopes", () => { + expect( + stepUpInsufficientScopeMessage({ reason: "insufficient_scope" }), + ).toBe( + "Authorization completed, but the required permissions were not granted. Grant the requested scopes on the authorization server, then retry your action.", + ); + }); +}); + +describe("oauthUx pre-redirect toast copy", () => { + it("returns undefined for a fresh connect handshake", () => { + expect( + oauthPreRedirectToastCopy("reauth", { context: "connect" }), + ).toBeUndefined(); + }); + + it("step_up toast includes server name when provided", () => { + expect(oauthPreRedirectToastCopy("step_up", { serverName: "svc" })).toEqual( + { + title: 'Step-up authorization for "svc"', + message: "Redirecting to authorize additional permissions…", + }, + ); + expect(oauthPreRedirectToastCopy("step_up", {})).toEqual({ + title: "Step-up authorization", + message: "Redirecting to authorize additional permissions…", + }); + }); + + it("enterprise-managed reauth toast re-authenticates", () => { + expect( + oauthPreRedirectToastCopy("reauth", { + serverName: "svc", + enterpriseManaged: true, + }), + ).toEqual({ + title: 'Re-authenticating "svc"', + message: "Re-authenticating…", + }); + expect( + oauthPreRedirectToastCopy("reauth", { enterpriseManaged: true }), + ).toEqual({ title: "Re-authenticating", message: "Re-authenticating…" }); + }); + + it("default reauth toast signals an expired session", () => { + expect(oauthPreRedirectToastCopy("reauth", { serverName: "svc" })).toEqual({ + title: 'Session expired for "svc"', + message: "Session expired, re-authenticating…", + }); + expect(oauthPreRedirectToastCopy("reauth", {})).toEqual({ + title: "Session expired", + message: "Session expired, re-authenticating…", + }); + }); +}); + +describe("oauthUx re-auth banner", () => { + it("isReAuthBannerReason is true for degraded-session reasons", () => { + for (const reason of [ + "token_expired", + "unauthorized", + "invalid_token", + ] as const) { + expect(isReAuthBannerReason(reason)).toBe(true); + } + expect(isReAuthBannerReason("insufficient_scope")).toBe(false); + expect(isReAuthBannerReason(undefined)).toBe(false); + }); + + it("reAuthBannerMessage varies with server name and detail", () => { + expect( + reAuthBannerMessage({ serverName: "svc", detail: "Token expired." }), + ).toBe('Authentication for "svc" needs attention. Token expired.'); + expect(reAuthBannerMessage({ serverName: "svc" })).toBe( + 'Authentication for "svc" needs attention.', + ); + expect(reAuthBannerMessage({ detail: "Token expired." })).toBe( + "Authentication needs attention. Token expired.", + ); + expect(reAuthBannerMessage({})).toBe("Authentication needs attention."); + }); }); 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 eac480ee..1dbf740f 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 @@ -3,12 +3,24 @@ import { runRunnerInteractiveOAuth, type RunnerInteractiveOAuthClient, } from "@inspector/core/auth/node/runner-interactive-oauth.js"; -import type { - OAuthCallbackServer, - OAuthCallbackServerStartOptions, +import { + createOAuthCallbackServer, + type OAuthCallbackServer, + type OAuthCallbackServerStartOptions, } from "@inspector/core/auth/node/oauth-callback-server.js"; import type { AuthChallenge } from "@inspector/core/auth/challenge.js"; +vi.mock( + "@inspector/core/auth/node/oauth-callback-server.js", + async (importOriginal) => { + const actual = + await importOriginal< + typeof import("@inspector/core/auth/node/oauth-callback-server.js") + >(); + return { ...actual, createOAuthCallbackServer: vi.fn() }; + }, +); + function mockClient( overrides: Partial = {}, ): RunnerInteractiveOAuthClient { @@ -308,4 +320,123 @@ describe("runRunnerInteractiveOAuth", () => { expect(onCallbackServer).toHaveBeenCalledTimes(1); expect(onCallbackServer).toHaveBeenCalledWith(mockServer); }); + + it("wraps a non-Error completeOAuthFlow rejection", async () => { + const redirectUrlProvider = { redirectUrl: "" }; + const client = mockClient({ + authenticate: vi.fn(async () => { + await simulateCallback(handlers.current).catch(() => {}); + return new URL("https://as.example/authorize"); + }), + completeOAuthFlow: vi.fn(async () => { + throw "string failure"; + }), + }); + + await expect( + runRunnerInteractiveOAuth({ + client, + redirectUrlProvider, + callbackListen: { + hostname: "127.0.0.1", + port: 6276, + pathname: "/oauth/callback", + }, + createCallbackServer: () => createMockCallbackServer(handlers), + }), + ).rejects.toThrow("string failure"); + }); + + it("falls back to params.error when no error_description is present", async () => { + const redirectUrlProvider = { redirectUrl: "" }; + const client = mockClient({ + authenticate: vi.fn(async () => { + handlers.current.onError?.({ error: "access_denied" }); + return new URL("https://as.example/authorize"); + }), + }); + + await expect( + runRunnerInteractiveOAuth({ + client, + redirectUrlProvider, + callbackListen: { + hostname: "127.0.0.1", + port: 6276, + pathname: "/oauth/callback", + }, + createCallbackServer: () => createMockCallbackServer(handlers), + }), + ).rejects.toThrow("access_denied"); + }); + + it("defaults to createOAuthCallbackServer when none is provided", async () => { + const redirectUrlProvider = { redirectUrl: "" }; + vi.mocked(createOAuthCallbackServer).mockReturnValue( + createMockCallbackServer(handlers), + ); + const client = mockClient({ + authenticate: vi.fn(async () => undefined), + }); + + const result = await runRunnerInteractiveOAuth({ + client, + redirectUrlProvider, + callbackListen: { + hostname: "127.0.0.1", + port: 6276, + pathname: "/oauth/callback", + }, + }); + + expect(result).toEqual({ kind: "already_authorized" }); + expect(createOAuthCallbackServer).toHaveBeenCalled(); + }); + + it("swallows a server.stop() rejection during cleanup", async () => { + const redirectUrlProvider = { redirectUrl: "" }; + const mockServer = createMockCallbackServer(handlers); + vi.mocked(mockServer.stop).mockRejectedValue(new Error("stop failed")); + const client = mockClient({ + authenticate: vi.fn(async () => { + await simulateCallback(handlers.current); + return new URL("https://as.example/authorize"); + }), + }); + + const result = await runRunnerInteractiveOAuth({ + client, + redirectUrlProvider, + callbackListen: { + hostname: "127.0.0.1", + port: 6276, + pathname: "/oauth/callback", + }, + createCallbackServer: () => mockServer, + }); + + expect(result).toEqual({ kind: "success" }); + expect(mockServer.stop).toHaveBeenCalled(); + }); + + it("cleans up when the callback server fails to start", async () => { + const redirectUrlProvider = { redirectUrl: "" }; + const mockServer = createMockCallbackServer(handlers); + vi.mocked(mockServer.start).mockRejectedValue(new Error("bind failed")); + const client = mockClient(); + + await expect( + runRunnerInteractiveOAuth({ + client, + redirectUrlProvider, + callbackListen: { + hostname: "127.0.0.1", + port: 6276, + pathname: "/oauth/callback", + }, + createCallbackServer: () => mockServer, + }), + ).rejects.toThrow("bind failed"); + expect(mockServer.stop).toHaveBeenCalled(); + }); }); diff --git a/clients/web/src/test/core/auth/utils.test.ts b/clients/web/src/test/core/auth/utils.test.ts index cbfa7186..d743fa0c 100644 --- a/clients/web/src/test/core/auth/utils.test.ts +++ b/clients/web/src/test/core/auth/utils.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, afterEach, vi } from "vitest"; import { parseHttpUrl, parseOAuthCallbackParams, @@ -6,8 +6,9 @@ import { parseOAuthState, generateOAuthErrorDescription, formatOAuthFailureDetail, + isUnauthorizedError, } from "@inspector/core/auth/utils.js"; -import { ZodError } from "zod"; +import { z, ZodError } from "zod"; describe("parseHttpUrl", () => { it("parses valid URLs", () => { @@ -19,6 +20,12 @@ describe("parseHttpUrl", () => { it("throws on invalid URLs", () => { expect(() => parseHttpUrl("not-a-url", "test")).toThrow(/Invalid test/); }); + + it("includes the trimmed offending value in the thrown message", () => { + expect(() => parseHttpUrl(" bad url ", "Server URL")).toThrow( + /Invalid Server URL: "bad url"/, + ); + }); }); describe("parseOAuthCallbackParams", () => { @@ -122,4 +129,137 @@ describe("formatOAuthFailureDetail", () => { "Network timeout", ); }); + + it("passes through plain strings that are not bracketed JSON", () => { + expect(formatOAuthFailureDetail("something went wrong")).toBe( + "something went wrong", + ); + }); + + it("stringifies non-string/non-error values", () => { + expect(formatOAuthFailureDetail(42)).toBe("42"); + expect(formatOAuthFailureDetail(null)).toBe("null"); + }); + + it("formats a real ZodError instance (non-token path)", () => { + const err = z.string().safeParse(123).error!; + const result = formatOAuthFailureDetail(err); + expect(result).not.toMatch(/valid tokens/i); + // Root-level failure has an empty path → "input" label + expect(result).toMatch(/^input: /); + }); + + it("joins non-token issues as `path: message`", () => { + const err = new ZodError([ + { + code: "invalid_type", + expected: "string", + path: ["client", "id"], + message: "Required", + }, + ]); + expect(formatOAuthFailureDetail(err)).toBe("client.id: Required"); + }); + + it("labels an issue with an empty path as `input` and defaults a missing message to `invalid`", () => { + const err = new ZodError([ + { + code: "custom", + path: [], + } as unknown as z.core.$ZodIssue, + ]); + expect(formatOAuthFailureDetail(err)).toBe("input: invalid"); + }); + + it("formats a bracketed JSON string of non-token zod issues", () => { + const json = `[ { "code": "invalid_type", "path": [ "scope" ], "message": "Required" } ]`; + expect(formatOAuthFailureDetail(json)).toBe("scope: Required"); + }); + + it("returns the raw string when bracketed JSON is invalid", () => { + const bad = "[ not valid json"; + expect(formatOAuthFailureDetail(bad)).toBe(bad); + }); + + it("returns the raw string when bracketed JSON is not a zod-issue array", () => { + // Parses fine, but empty array / first element lacks `code` + expect(formatOAuthFailureDetail("[]")).toBe("[]"); + const notIssues = `[ { "foo": "bar" } ]`; + expect(formatOAuthFailureDetail(notIssues)).toBe(notIssues); + }); + + it("returns the raw string when bracketed JSON is a non-array value", () => { + const obj = `[1, 2, 3]`; + // Array of numbers: first element is not an object → not a zod-issue array + expect(formatOAuthFailureDetail(obj)).toBe(obj); + }); +}); + +describe("generateOAuthState fallback", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("uses the Math.random fallback when crypto is unavailable", () => { + vi.stubGlobal("crypto", undefined); + const state = generateOAuthState(); + expect(state).toMatch(/^[a-f0-9]{64}$/i); + }); +}); + +describe("isUnauthorizedError", () => { + it("returns true for an object with status 401", () => { + expect(isUnauthorizedError({ status: 401 })).toBe(true); + }); + + it("returns true for an object with code 401", () => { + expect(isUnauthorizedError({ code: 401 })).toBe(true); + }); + + it("returns true when an Error message matches the transport wording", () => { + expect( + isUnauthorizedError(new Error("Connection failed for server (401)")), + ).toBe(true); + }); + + it("returns true when a non-error value stringifies to matching wording", () => { + expect(isUnauthorizedError("request failed with status (401)")).toBe(true); + }); + + it("returns false for unrelated (401) mentions without `failed`", () => { + expect(isUnauthorizedError(new Error("error code (401) noted"))).toBe( + false, + ); + }); + + it("returns false for a non-401 object", () => { + expect(isUnauthorizedError({ status: 500 })).toBe(false); + }); + + it("returns false for null", () => { + expect(isUnauthorizedError(null)).toBe(false); + }); +}); + +describe("generateOAuthErrorDescription without description/uri", () => { + it("omits the details and more-info lines when absent", () => { + const message = generateOAuthErrorDescription({ + successful: false, + error: "server_error", + error_description: null, + error_uri: null, + }); + expect(message).toBe("Error: server_error."); + }); +}); + +describe("parseOAuthCallbackParams error without description", () => { + it("returns null description/uri when only error is present", () => { + expect(parseOAuthCallbackParams("?error=access_denied")).toEqual({ + successful: false, + error: "access_denied", + error_description: null, + error_uri: null, + }); + }); }); diff --git a/clients/web/src/test/core/mcp/inspectorClient-authRecovery-branches.test.ts b/clients/web/src/test/core/mcp/inspectorClient-authRecovery-branches.test.ts new file mode 100644 index 00000000..2052a35c --- /dev/null +++ b/clients/web/src/test/core/mcp/inspectorClient-authRecovery-branches.test.ts @@ -0,0 +1,297 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { AuthChallengeError } from "@inspector/core/auth/challenge.js"; +import type { AuthChallenge } from "@inspector/core/auth/challenge.js"; +import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js"; +import { RemoteClientTransport } from "@inspector/core/mcp/remote/remoteClientTransport.js"; + +/** + * Additional branch coverage for the mid-session auth recovery paths + * (`handleAmbientAuthChallenge` / `runAmbientAuthChallenge`, `withDirectAuthRecovery`, + * `pushRemoteAuthState`, `resumeAfterOAuth`). Complements + * inspectorClient-ambient-auth.test.ts and inspectorClient-direct-auth-recovery.test.ts, + * which each cover a single "happy path" through these methods. + * + * Uses the same `Object.create(InspectorClient.prototype)` technique as the + * sibling test files so each test can wire up only the private fields its + * branch needs, without a live transport or a real OAuthManager. + */ + +type FakeOAuthManager = { + handleAuthChallenge: ReturnType; + completeOAuthFlow?: ReturnType; +}; + +type Internals = { + oauthManager: FakeOAuthManager | null; + baseTransport: unknown; + ambientAuthChallengeInFlight: Map>; + directAuthRecovery: boolean; + directAuthRecoveryActive: boolean | null; + activeToolCallAbortController?: AbortController; + status: string; + reconnectAfterAuthRecovery: () => Promise; + withDirectAuthRecovery: ( + operation: () => Promise, + context?: { method?: string; toolName?: string }, + attempt?: number, + ) => Promise; +}; + +function makeClient(): InspectorClient { + const client = Object.create(InspectorClient.prototype) as InspectorClient; + (client as unknown as Internals).ambientAuthChallengeInFlight = new Map(); + return client; +} + +function internalsOf(client: InspectorClient): Internals { + return client as unknown as Internals; +} + +describe("InspectorClient pushRemoteAuthState", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("is a no-op when the base transport is not a RemoteClientTransport", async () => { + const client = makeClient(); + internalsOf(client).baseTransport = null; + + await expect(client.pushRemoteAuthState()).resolves.toBeUndefined(); + }); +}); + +describe("InspectorClient runAmbientAuthChallenge branches", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + const challenge: AuthChallenge = { reason: "token_expired" }; + + it("returns without dispatching a recovery outcome when no oauthManager is configured", async () => { + const client = makeClient(); + internalsOf(client).oauthManager = null; + const dispatch = vi + .spyOn(client, "dispatchTypedEvent") + .mockImplementation(() => {}); + + await client.handleAmbientAuthChallenge(challenge); + + // Only the initial "ambient" announcement fires; the early return means no + // outcome-specific event (recovered/interactive/oauthError) is dispatched. + expect(dispatch).toHaveBeenCalledTimes(1); + expect(dispatch).toHaveBeenCalledWith("authChallengeAmbient", { + challenge, + }); + }); + + it("reconnects (rather than pushing remote auth state) for a non-remote transport on a satisfied outcome", async () => { + const client = makeClient(); + const handleAuthChallenge = vi + .fn() + .mockResolvedValue({ kind: "satisfied" }); + internalsOf(client).oauthManager = { handleAuthChallenge }; + internalsOf(client).baseTransport = {}; // not a RemoteClientTransport instance + const reconnect = vi.fn().mockResolvedValue(undefined); + internalsOf(client).reconnectAfterAuthRecovery = reconnect; + const pushRemoteAuthState = vi + .spyOn(client, "pushRemoteAuthState") + .mockResolvedValue(undefined); + const dispatch = vi + .spyOn(client, "dispatchTypedEvent") + .mockImplementation(() => {}); + + await client.handleAmbientAuthChallenge(challenge); + + expect(reconnect).toHaveBeenCalledTimes(1); + expect(pushRemoteAuthState).not.toHaveBeenCalled(); + expect(dispatch).toHaveBeenCalledWith("authChallengeRecovered", { + challenge, + }); + }); + + it("dispatches an interactive event with the EMA pending URL for a step_up_confirm outcome", async () => { + const client = makeClient(); + const outcomeChallenge: AuthChallenge = { + reason: "insufficient_scope", + requiredScopes: ["weather:read"], + }; + const handleAuthChallenge = vi.fn().mockResolvedValue({ + kind: "step_up_confirm", + challenge: outcomeChallenge, + }); + internalsOf(client).oauthManager = { handleAuthChallenge }; + const dispatch = vi + .spyOn(client, "dispatchTypedEvent") + .mockImplementation(() => {}); + + await client.handleAmbientAuthChallenge(challenge); + + expect(dispatch).toHaveBeenCalledWith( + "authChallengeInteractive", + expect.objectContaining({ challenge: outcomeChallenge }), + ); + }); + + it("dispatches oauthError for a failed outcome", async () => { + const client = makeClient(); + const error = new Error("recovery failed"); + const handleAuthChallenge = vi + .fn() + .mockResolvedValue({ kind: "failed", error }); + internalsOf(client).oauthManager = { handleAuthChallenge }; + const dispatch = vi + .spyOn(client, "dispatchTypedEvent") + .mockImplementation(() => {}); + + await client.handleAmbientAuthChallenge(challenge); + + expect(dispatch).toHaveBeenCalledWith("oauthError", { error }); + }); + + it("wraps a non-Error thrown from handleAuthChallenge before dispatching oauthError", async () => { + const client = makeClient(); + const handleAuthChallenge = vi.fn().mockRejectedValue("boom-string"); + internalsOf(client).oauthManager = { handleAuthChallenge }; + const dispatch = vi + .spyOn(client, "dispatchTypedEvent") + .mockImplementation(() => {}); + + await client.handleAmbientAuthChallenge(challenge); + + const call = dispatch.mock.calls.find(([name]) => name === "oauthError"); + expect(call).toBeDefined(); + const detail = call?.[1] as { error: Error }; + expect(detail.error).toBeInstanceOf(Error); + expect(detail.error.message).toBe("boom-string"); + }); + + it("passes through a real Error thrown from handleAuthChallenge unwrapped", async () => { + const client = makeClient(); + const thrown = new Error("direct failure"); + const handleAuthChallenge = vi.fn().mockRejectedValue(thrown); + internalsOf(client).oauthManager = { handleAuthChallenge }; + const dispatch = vi + .spyOn(client, "dispatchTypedEvent") + .mockImplementation(() => {}); + + await client.handleAmbientAuthChallenge(challenge); + + const call = dispatch.mock.calls.find(([name]) => name === "oauthError"); + expect(call?.[1]).toEqual({ error: thrown }); + }); +}); + +describe("InspectorClient withDirectAuthRecovery branches", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("rethrows immediately without consulting handleAuthChallenge for a non-auth-challenge error", async () => { + const client = makeClient(); + internalsOf(client).directAuthRecovery = true; + internalsOf(client).directAuthRecoveryActive = true; + const operation = vi.fn().mockRejectedValue(new Error("network down")); + const handleAuthChallenge = vi.spyOn(client, "handleAuthChallenge"); + + await expect( + internalsOf(client).withDirectAuthRecovery.call(client, operation), + ).rejects.toThrow("network down"); + + expect(operation).toHaveBeenCalledTimes(1); + expect(handleAuthChallenge).not.toHaveBeenCalled(); + }); + + it("clears an active tool-call abort controller and retries after a satisfied recovery", async () => { + const client = makeClient(); + internalsOf(client).directAuthRecovery = true; + internalsOf(client).directAuthRecoveryActive = true; + const abortController = new AbortController(); + internalsOf(client).activeToolCallAbortController = abortController; + const reconnect = vi.fn().mockResolvedValue(undefined); + internalsOf(client).reconnectAfterAuthRecovery = reconnect; + + const challenge = { reason: "token_expired" as const }; + const operation = vi + .fn() + .mockRejectedValueOnce(new AuthChallengeError(challenge, 401)) + .mockResolvedValueOnce("ok"); + vi.spyOn(client, "handleAuthChallenge").mockResolvedValue({ + kind: "satisfied", + }); + vi.spyOn(client, "dispatchTypedEvent").mockImplementation(() => {}); + + const result = await internalsOf(client).withDirectAuthRecovery.call( + client, + operation, + ); + + expect(result).toBe("ok"); + expect(internalsOf(client).activeToolCallAbortController).toBeUndefined(); + expect(reconnect).toHaveBeenCalledTimes(1); + expect(operation).toHaveBeenCalledTimes(2); + }); + + it("throws AuthRecoveryRequiredError with emaStepUpConfirm for a step_up_confirm outcome", async () => { + const client = makeClient(); + internalsOf(client).directAuthRecovery = true; + internalsOf(client).directAuthRecoveryActive = true; + const challenge = { + reason: "insufficient_scope" as const, + requiredScopes: ["weather:read"], + }; + const operation = vi + .fn() + .mockRejectedValue(new AuthChallengeError(challenge, 403)); + vi.spyOn(client, "handleAuthChallenge").mockResolvedValue({ + kind: "step_up_confirm", + challenge, + }); + vi.spyOn(client, "dispatchTypedEvent").mockImplementation(() => {}); + + await expect( + internalsOf(client).withDirectAuthRecovery.call(client, operation), + ).rejects.toMatchObject({ + name: "AuthRecoveryRequiredError", + emaStepUpConfirm: true, + }); + expect(operation).toHaveBeenCalledTimes(1); + }); +}); + +describe("InspectorClient resumeAfterOAuth branches", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("reattaches a remote session, pushes auth state, and reconnects when not connected", async () => { + const client = makeClient(); + internalsOf(client).oauthManager = { + handleAuthChallenge: vi.fn(), + completeOAuthFlow: vi.fn().mockResolvedValue(undefined), + }; + internalsOf(client).directAuthRecovery = false; + internalsOf(client).directAuthRecoveryActive = null; + internalsOf(client).status = "disconnected"; + + const transport = Object.create( + RemoteClientTransport.prototype, + ) as RemoteClientTransport; + const attachToSession = vi.fn().mockResolvedValue(undefined); + const pushAuthState = vi.fn().mockResolvedValue(undefined); + (transport as unknown as { attachToSession: unknown }).attachToSession = + attachToSession; + (transport as unknown as { pushAuthState: unknown }).pushAuthState = + pushAuthState; + internalsOf(client).baseTransport = transport; + + const connectSpy = vi.spyOn(client, "connect").mockResolvedValue(undefined); + + await client.resumeAfterOAuth("auth-code", { + remoteSessionId: "session-1", + }); + + expect(attachToSession).toHaveBeenCalledWith("session-1"); + expect(pushAuthState).toHaveBeenCalledTimes(1); + expect(connectSpy).toHaveBeenCalledTimes(1); + }); +}); diff --git a/clients/web/src/test/core/mcp/node/authChallengeFetch.test.ts b/clients/web/src/test/core/mcp/node/authChallengeFetch.test.ts index b512d59a..f94b7241 100644 --- a/clients/web/src/test/core/mcp/node/authChallengeFetch.test.ts +++ b/clients/web/src/test/core/mcp/node/authChallengeFetch.test.ts @@ -49,4 +49,38 @@ describe("createAuthChallengeInterceptFetch", () => { ); } }); + + it("cancels a present response body before throwing", async () => { + const response = new Response("challenge body", { + status: 401, + headers: { "WWW-Authenticate": 'Bearer error="invalid_token"' }, + }); + const cancelSpy = vi.spyOn(response.body!, "cancel"); + const baseFetch = vi.fn(async () => response); + const fetchFn = createAuthChallengeInterceptFetch(baseFetch); + + await expect(fetchFn("https://example.com/mcp")).rejects.toBeInstanceOf( + AuthChallengeError, + ); + expect(cancelSpy).toHaveBeenCalled(); + }); + + it("swallows a body.cancel() rejection and still throws the challenge", async () => { + const response = new Response("challenge body", { + status: 403, + headers: { + "WWW-Authenticate": + 'Bearer error="insufficient_scope", scope="weather:read"', + }, + }); + vi.spyOn(response.body!, "cancel").mockRejectedValue( + new Error("cancel failed"), + ); + const baseFetch = vi.fn(async () => response); + const fetchFn = createAuthChallengeInterceptFetch(baseFetch); + + await expect(fetchFn("https://example.com/mcp")).rejects.toBeInstanceOf( + AuthChallengeError, + ); + }); }); diff --git a/clients/web/src/test/core/mcp/oauthManager.test.ts b/clients/web/src/test/core/mcp/oauthManager.test.ts index 1ace33db..02d764f2 100644 --- a/clients/web/src/test/core/mcp/oauthManager.test.ts +++ b/clients/web/src/test/core/mcp/oauthManager.test.ts @@ -244,6 +244,24 @@ describe("OAuthManager", () => { const manager = new OAuthManager(params); expect(await manager.getOAuthTokens()).toBeUndefined(); }); + + it("returns tokens from in-memory flow state without querying storage", async () => { + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + const params = createMockParams(); + const tokens = { access_token: "cached", token_type: "Bearer" }; + storageOf(params).getTokens.mockResolvedValue(tokens); + storageOf(params).getClientInformation.mockResolvedValue({ + client_id: "cid", + }); + const manager = new OAuthManager(params); + await manager.completeOAuthFlow("code"); + storageOf(params).getTokens.mockClear(); + + const result = await manager.getOAuthTokens(); + + expect(result).toEqual(tokens); + expect(storageOf(params).getTokens).not.toHaveBeenCalled(); + }); }); describe("isOAuthAuthorized", () => { @@ -804,6 +822,65 @@ describe("OAuthManager", () => { startSpy.mockRestore(); parseSpy.mockRestore(); }); + + it("skips onBeforeOAuthRedirect when none is configured", async () => { + const silentSpy = vi + .spyOn(emaFlow, "trySilentEmaAuth") + .mockResolvedValue({ status: "no_idp_session" }); + const authUrl = new URL( + "https://idp.example.com/authorize?state=no-callback", + ); + const startSpy = vi + .spyOn(emaFlow, "startEmaIdpAuthorization") + .mockResolvedValue(authUrl); + const params = createMockParams({ + enterpriseManagedAuth: { + idp: { + issuer: "https://idp.example.com", + clientId: "app-client", + clientSecret: "secret", + }, + }, + }); + const manager = new OAuthManager(params); + manager.setOAuthConfig({ enterpriseManaged: true }); + + const result = await manager.authenticate(); + + expect(result).toEqual(authUrl); + expect( + params.initialConfig.navigation!.navigateToAuthorization, + ).toHaveBeenCalledWith(authUrl); + + silentSpy.mockRestore(); + startSpy.mockRestore(); + }); + + it("skips onBeforeOAuthRedirect when the authorization state has no authId", async () => { + const silentSpy = vi + .spyOn(emaFlow, "trySilentEmaAuth") + .mockResolvedValue({ status: "no_idp_session" }); + const authUrl = new URL( + "https://idp.example.com/authorize?state=no-authid", + ); + const startSpy = vi + .spyOn(emaFlow, "startEmaIdpAuthorization") + .mockResolvedValue(authUrl); + const parseSpy = vi + .spyOn(await import("@inspector/core/auth/utils.js"), "parseOAuthState") + .mockReturnValue(null); + const params = emaParams(); + const manager = new OAuthManager(params); + manager.setOAuthConfig({ enterpriseManaged: true }); + + await manager.authenticate(); + + expect(params.onBeforeOAuthRedirect).not.toHaveBeenCalled(); + + silentSpy.mockRestore(); + startSpy.mockRestore(); + parseSpy.mockRestore(); + }); }); describe("refreshEnterpriseManagedTokens", () => { @@ -1505,4 +1582,643 @@ describe("OAuthManager", () => { expect(state?.serverUrl).toBe(SERVER_URL); }); }); + + describe("getOAuthState (storage not configured)", () => { + it("returns undefined when storage is not configured", async () => { + const params = createMockParams({ + initialConfig: { + redirectUrlProvider: { + getRedirectUrl: vi + .fn() + .mockReturnValue("http://localhost/callback"), + }, + navigation: { navigateToAuthorization: vi.fn() }, + } as OAuthManagerConfig, + }); + const manager = new OAuthManager(params); + await expect(manager.getOAuthState()).resolves.toBeUndefined(); + }); + }); + + describe("checkAuthChallengeSatisfied (storage not configured)", () => { + it("returns false when storage is not configured", async () => { + const params = createMockParams({ + initialConfig: { + redirectUrlProvider: { + getRedirectUrl: vi + .fn() + .mockReturnValue("http://localhost/callback"), + }, + navigation: { navigateToAuthorization: vi.fn() }, + } as OAuthManagerConfig, + }); + const manager = new OAuthManager(params); + expect( + await manager.checkAuthChallengeSatisfied({ + reason: "insufficient_scope", + }), + ).toBe(false); + }); + }); + + describe("handleAuthChallenge (additional branch coverage)", () => { + it("resolves via the second satisfaction check inside the mutex", async () => { + const params = createMockParams(); + const insufficientTokens = { + access_token: "a1", + token_type: "Bearer", + scope: "mcp tools:read", + }; + const sufficientTokens = { + access_token: "a2", + token_type: "Bearer", + scope: "mcp tools:read weather:read", + }; + storageOf(params).getScope.mockReturnValue("mcp tools:read"); + storageOf(params) + .getTokens.mockResolvedValueOnce(insufficientTokens) + .mockResolvedValue(sufficientTokens); + const manager = new OAuthManager(params); + + const outcome = await manager.handleAuthChallenge({ + reason: "insufficient_scope", + requiredScopes: ["weather:read"], + }); + + expect(outcome).toEqual({ kind: "satisfied" }); + expect(mockedMcpAuth).not.toHaveBeenCalled(); + }); + + it("persists broadened scope when silent refresh already satisfies the step-up scope", async () => { + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + const params = createMockParams(); + const insufficientTokens = { + access_token: "a1", + refresh_token: "r1", + token_type: "Bearer", + scope: "mcp tools:read", + }; + const sufficientTokens = { + access_token: "a2", + refresh_token: "r2", + token_type: "Bearer", + scope: "mcp tools:read weather:read", + }; + storageOf(params).getScope.mockReturnValue("mcp tools:read"); + storageOf(params) + .getTokens.mockResolvedValueOnce(insufficientTokens) + .mockResolvedValueOnce(insufficientTokens) + .mockResolvedValueOnce(insufficientTokens) + .mockResolvedValue(sufficientTokens); + const manager = new OAuthManager(params); + + const outcome = await manager.handleAuthChallenge({ + reason: "insufficient_scope", + requiredScopes: ["weather:read"], + }); + + expect(outcome).toEqual({ kind: "satisfied" }); + expect(storageOf(params).saveScope).toHaveBeenCalledWith( + SERVER_URL, + "mcp tools:read weather:read", + ); + }); + + it("returns satisfied without persisting scope when the fresh grant has no scope to record", async () => { + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + const params = createMockParams(); + // scope: "" (not omitted) avoids enrichChallengeWithAuthorizationScopes' + // internal extra getTokens() re-fetch, which only triggers when the + // passed grantedTokenScope is `undefined`. + const noScopeTokens = { + access_token: "a", + token_type: "Bearer", + scope: "", + }; + const midScopeTokens = { + access_token: "b", + token_type: "Bearer", + scope: "newscope", + }; + const finalNoScopeTokens = { + access_token: "c", + token_type: "Bearer", + scope: "", + }; + storageOf(params) + .getTokens.mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(noScopeTokens) + .mockResolvedValueOnce(midScopeTokens) + .mockResolvedValueOnce(finalNoScopeTokens); + const manager = new OAuthManager(params); + + const outcome = await manager.handleAuthChallenge({ + reason: "insufficient_scope", + }); + + expect(outcome).toEqual({ kind: "satisfied" }); + expect(storageOf(params).saveScope).not.toHaveBeenCalled(); + }); + + it("forces reauthorization and persists the granted scope when the retry succeeds", async () => { + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + const params = createMockParams(); + const noScopeTokens = { + access_token: "a", + token_type: "Bearer", + scope: "", + }; + const midScopeTokens = { + access_token: "b", + token_type: "Bearer", + scope: "newscope", + }; + const grantedTokens = { + access_token: "d", + token_type: "Bearer", + scope: "granted:scope", + }; + storageOf(params) + .getTokens.mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(noScopeTokens) + .mockResolvedValueOnce(noScopeTokens) + .mockResolvedValueOnce(midScopeTokens) + .mockResolvedValueOnce(grantedTokens); + const manager = new OAuthManager(params); + + const outcome = await manager.handleAuthChallenge({ + reason: "insufficient_scope", + }); + + expect(outcome).toEqual({ kind: "satisfied" }); + expect(mockedMcpAuth).toHaveBeenCalledTimes(2); + expect(mockedMcpAuth).toHaveBeenNthCalledWith( + 2, + expect.anything(), + expect.objectContaining({ forceReauthorization: true }), + ); + expect(storageOf(params).saveScope).toHaveBeenCalledWith( + SERVER_URL, + "granted:scope", + ); + }); + + it("forces reauthorization without persisting scope when the retry grants no explicit scope", async () => { + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + const params = createMockParams(); + const noScopeTokens = { + access_token: "a", + token_type: "Bearer", + scope: "", + }; + const midScopeTokens = { + access_token: "b", + token_type: "Bearer", + scope: "newscope", + }; + storageOf(params) + .getTokens.mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(noScopeTokens) + .mockResolvedValueOnce(noScopeTokens) + .mockResolvedValueOnce(midScopeTokens) + .mockResolvedValueOnce(noScopeTokens); + const manager = new OAuthManager(params); + + const outcome = await manager.handleAuthChallenge({ + reason: "insufficient_scope", + }); + + expect(outcome).toEqual({ kind: "satisfied" }); + expect(storageOf(params).saveScope).not.toHaveBeenCalled(); + }); + + it("returns failed when the forced reauthorization retry does not complete", async () => { + mockedMcpAuth + .mockResolvedValueOnce("AUTHORIZED") + .mockResolvedValueOnce("REDIRECT"); + const params = createMockParams(); + storageOf(params).getScope.mockReturnValue("mcp tools:read"); + storageOf(params).getTokens.mockResolvedValue({ + access_token: "access", + token_type: "Bearer", + scope: "mcp tools:read", + }); + const manager = new OAuthManager(params); + + const outcome = await manager.handleAuthChallenge({ + reason: "insufficient_scope", + requiredScopes: ["weather:read"], + }); + + expect(outcome.kind).toBe("failed"); + if (outcome.kind === "failed") { + expect(outcome.error.message).toMatch(/weather:read/); + } + }); + + it("falls back to challenge requiredScopes when no catalog scope is configured", async () => { + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + const params = createMockParams(); + const manager = new OAuthManager(params); + + const outcome = await manager.handleAuthChallenge({ + reason: "unauthorized", + requiredScopes: ["fallback:scope"], + }); + + expect(outcome).toEqual({ kind: "satisfied" }); + expect(mockedMcpAuth).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ scope: "fallback:scope" }), + ); + }); + }); + + describe("completeOAuthFlow (non-Error rejection)", () => { + it("wraps non-Error throw values in the dispatched error", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + mockedMcpAuth.mockRejectedValue("plain-string-failure"); + const params = createMockParams(); + const manager = new OAuthManager(params); + + await expect(manager.completeOAuthFlow("code")).rejects.toBe( + "plain-string-failure", + ); + expect(params.dispatchOAuthError).toHaveBeenCalledWith( + expect.objectContaining({ error: expect.any(Error) }), + ); + const dispatchedError = ( + params.dispatchOAuthError as ReturnType + ).mock.calls[0][0].error as Error; + expect(dispatchedError.message).toBe("plain-string-failure"); + errorSpy.mockRestore(); + }); + }); + + describe("handleEnterpriseManagedAuthChallenge (additional branch coverage)", () => { + it("returns failed when EMA silent re-mint fails during confirmed step-up", async () => { + const mintError = new Error("mint failed during step-up"); + const silentSpy = vi + .spyOn(emaFlow, "trySilentEmaAuth") + .mockResolvedValue({ status: "mint_failed", error: mintError }); + const params = createMockParams({ + enterpriseManagedAuth: { + idp: { + issuer: "https://idp.example.com", + clientId: "app-client", + clientSecret: "secret", + }, + }, + }); + 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({ kind: "failed", error: mintError }); + silentSpy.mockRestore(); + }); + + it("returns satisfied for EMA token_expired when refreshEmaResourceTokens succeeds, using the configured fallback scope", async () => { + const refreshSpy = vi + .spyOn(emaFlow, "refreshEmaResourceTokens") + .mockResolvedValue({ access_token: "R", token_type: "Bearer" }); + const params = createMockParams({ + enterpriseManagedAuth: { + idp: { + issuer: "https://idp.example.com", + clientId: "app-client", + clientSecret: "secret", + }, + }, + }); + const manager = new OAuthManager(params); + manager.setOAuthConfig({ + enterpriseManaged: true, + scope: "fallback:scope", + }); + + const outcome = await manager.handleAuthChallenge({ + reason: "token_expired", + }); + + expect(outcome).toEqual({ kind: "satisfied" }); + expect(refreshSpy).toHaveBeenCalledWith( + expect.objectContaining({ scope: "fallback:scope" }), + ); + refreshSpy.mockRestore(); + }); + + it("returns failed when starting the EMA IdP authorization throws an Error", async () => { + const refreshSpy = vi + .spyOn(emaFlow, "refreshEmaResourceTokens") + .mockResolvedValue(undefined); + const startSpy = vi + .spyOn(emaFlow, "startEmaIdpAuthorization") + .mockRejectedValue(new Error("idp unreachable")); + const params = createMockParams({ + enterpriseManagedAuth: { + idp: { + issuer: "https://idp.example.com", + clientId: "app-client", + clientSecret: "secret", + }, + }, + }); + const manager = new OAuthManager(params); + manager.setOAuthConfig({ enterpriseManaged: true }); + + const outcome = await manager.handleAuthChallenge({ + reason: "token_expired", + }); + + expect(outcome.kind).toBe("failed"); + if (outcome.kind === "failed") { + expect(outcome.error.message).toBe("idp unreachable"); + } + refreshSpy.mockRestore(); + startSpy.mockRestore(); + }); + + it("wraps non-Error throw values when starting EMA IdP authorization fails", async () => { + const refreshSpy = vi + .spyOn(emaFlow, "refreshEmaResourceTokens") + .mockResolvedValue(undefined); + const startSpy = vi + .spyOn(emaFlow, "startEmaIdpAuthorization") + .mockRejectedValue("idp offline"); + const params = createMockParams({ + enterpriseManagedAuth: { + idp: { + issuer: "https://idp.example.com", + clientId: "app-client", + clientSecret: "secret", + }, + }, + }); + const manager = new OAuthManager(params); + manager.setOAuthConfig({ enterpriseManaged: true }); + + const outcome = await manager.handleAuthChallenge({ + reason: "token_expired", + }); + + expect(outcome.kind).toBe("failed"); + if (outcome.kind === "failed") { + expect(outcome.error.message).toBe("idp offline"); + } + refreshSpy.mockRestore(); + startSpy.mockRestore(); + }); + + it("returns interactive for EMA insufficient_scope with no prior scope and no configured fallback", async () => { + const silentSpy = vi + .spyOn(emaFlow, "trySilentEmaAuth") + .mockResolvedValue({ status: "no_idp_session" }); + const authUrl = new URL( + "https://idp.example.com/authorize?state=ema-empty", + ); + const startSpy = vi + .spyOn(emaFlow, "startEmaIdpAuthorization") + .mockResolvedValue(authUrl); + const params = createMockParams({ + enterpriseManagedAuth: { + idp: { + issuer: "https://idp.example.com", + clientId: "app-client", + clientSecret: "secret", + }, + }, + }); + const manager = new OAuthManager(params); + manager.setOAuthConfig({ enterpriseManaged: true }); + + const outcome = await manager.handleAuthChallenge( + { reason: "insufficient_scope" }, + { confirmedStepUp: true }, + ); + + expect(outcome).toEqual( + expect.objectContaining({ + kind: "interactive", + authorizationUrl: authUrl, + }), + ); + silentSpy.mockRestore(); + startSpy.mockRestore(); + }); + }); + + describe("beginInteractiveAuthorization", () => { + it("records flow state, navigates, and dispatches when state carries an authId", async () => { + const authorizationUrl = new URL( + "https://auth.example.com/authorize?state=begin-1", + ); + const onBeforeOAuthRedirect = vi.fn().mockResolvedValue(undefined); + const parseSpy = vi + .spyOn(await import("@inspector/core/auth/utils.js"), "parseOAuthState") + .mockReturnValue({ + execution: "quick", + authId: "begin-auth-id", + } as ReturnType< + typeof import("@inspector/core/auth/utils.js").parseOAuthState + >); + const params = createMockParams({ onBeforeOAuthRedirect }); + storageOf(params).getClientInformation.mockResolvedValue({ + client_id: "cid", + }); + const manager = new OAuthManager(params); + + await manager.beginInteractiveAuthorization(authorizationUrl); + + expect(onBeforeOAuthRedirect).toHaveBeenCalledWith("begin-auth-id"); + expect( + params.initialConfig.navigation!.navigateToAuthorization, + ).toHaveBeenCalledWith(authorizationUrl); + expect(manager.getOAuthFlowStep()).toBe("authorization_code"); + expect(manager.getOAuthFlowState()?.oauthClientInfo).toEqual({ + client_id: "cid", + }); + expect(params.dispatchOAuthAuthorizationRequired).toHaveBeenCalledWith({ + url: authorizationUrl, + }); + + parseSpy.mockRestore(); + }); + + it("skips onBeforeOAuthRedirect when there is no state param", async () => { + const authorizationUrl = new URL("https://auth.example.com/authorize"); + const onBeforeOAuthRedirect = vi.fn(); + const params = createMockParams({ onBeforeOAuthRedirect }); + const manager = new OAuthManager(params); + + await manager.beginInteractiveAuthorization(authorizationUrl); + + expect(onBeforeOAuthRedirect).not.toHaveBeenCalled(); + expect( + params.initialConfig.navigation!.navigateToAuthorization, + ).toHaveBeenCalledWith(authorizationUrl); + expect(params.dispatchOAuthAuthorizationRequired).toHaveBeenCalledWith({ + url: authorizationUrl, + }); + }); + + it("skips onBeforeOAuthRedirect when state param has no authId", async () => { + const authorizationUrl = new URL( + "https://auth.example.com/authorize?state=zzz", + ); + const parseSpy = vi + .spyOn(await import("@inspector/core/auth/utils.js"), "parseOAuthState") + .mockReturnValue(null); + const onBeforeOAuthRedirect = vi.fn(); + const params = createMockParams({ onBeforeOAuthRedirect }); + const manager = new OAuthManager(params); + + await manager.beginInteractiveAuthorization(authorizationUrl); + + expect(onBeforeOAuthRedirect).not.toHaveBeenCalled(); + + parseSpy.mockRestore(); + }); + + it("throws when navigation is not configured", async () => { + const params = createMockParams({ + initialConfig: { + storage: createMockParams().initialConfig.storage, + redirectUrlProvider: { + getRedirectUrl: vi + .fn() + .mockReturnValue("http://localhost/callback"), + }, + } as OAuthManagerConfig, + }); + const manager = new OAuthManager(params); + + await expect( + manager.beginInteractiveAuthorization( + new URL("https://auth.example.com/authorize"), + ), + ).rejects.toThrow("OAuth navigation is required."); + }); + }); + + describe("createOAuthProvider (clientId not configured)", () => { + it("skips savePreregisteredClientInformation when clientId is not configured", async () => { + mockedMcpAuth.mockResolvedValue("REDIRECT"); + const capturedUrl = new URL( + "https://auth.example.com/authorize?state=no-client-id", + ); + const params = createMockParams(); + params.initialConfig.clientId = undefined; + const manager = new OAuthManager(params); + const captureSpy = vi + .spyOn( + (await import("@inspector/core/auth/providers.js")) + .BaseOAuthClientProvider.prototype, + "getCapturedAuthUrl", + ) + .mockReturnValue(capturedUrl); + + await manager.authenticate(); + + expect( + storageOf(params).savePreregisteredClientInformation, + ).not.toHaveBeenCalled(); + captureSpy.mockRestore(); + }); + }); + + describe("completeOAuthFlow (oauthClientInfo null fallback)", () => { + it("stores null clientInfo when none is available and no flow state pre-exists", async () => { + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + const params = createMockParams(); + storageOf(params).getTokens.mockResolvedValue({ + access_token: "tok", + token_type: "Bearer", + }); + const manager = new OAuthManager(params); + + await manager.completeOAuthFlow("code"); + + expect(manager.getOAuthFlowState()?.oauthClientInfo).toBeNull(); + }); + + it("stores null clientInfo when none is available and flow state already exists", async () => { + const capturedUrl = new URL( + "https://auth.example.com/authorize?state=existing-flow", + ); + mockedMcpAuth + .mockResolvedValueOnce("REDIRECT") + .mockResolvedValueOnce("AUTHORIZED"); + const params = createMockParams(); + storageOf(params).getTokens.mockResolvedValue({ + access_token: "access", + token_type: "Bearer", + scope: "mcp", + }); + const manager = new OAuthManager(params); + const captureSpy = vi + .spyOn( + (await import("@inspector/core/auth/providers.js")) + .BaseOAuthClientProvider.prototype, + "getCapturedAuthUrl", + ) + .mockReturnValue(capturedUrl); + + await manager.handleAuthChallenge({ + reason: "insufficient_scope", + requiredScopes: ["weather:read"], + }); + expect(manager.getOAuthFlowState()).toBeDefined(); + + await manager.completeOAuthFlow("code"); + + expect(manager.getOAuthFlowState()?.oauthClientInfo).toBeNull(); + captureSpy.mockRestore(); + }); + }); + + describe("handleEnterpriseManagedAuthChallenge (scopeToPersist false arm)", () => { + it("returns satisfied without persisting scope when the EMA mint has no scope to record", async () => { + const silentSpy = vi + .spyOn(emaFlow, "trySilentEmaAuth") + .mockResolvedValue({ status: "success" }); + const params = createMockParams({ + enterpriseManagedAuth: { + idp: { + issuer: "https://idp.example.com", + clientId: "app-client", + clientSecret: "secret", + }, + }, + }); + storageOf(params) + .getTokens.mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce({ + access_token: "g", + token_type: "Bearer", + scope: "granted", + }) + .mockResolvedValueOnce({ access_token: "g2", token_type: "Bearer" }); + const manager = new OAuthManager(params); + manager.setOAuthConfig({ enterpriseManaged: true }); + + const outcome = await manager.handleAuthChallenge( + { reason: "insufficient_scope" }, + { confirmedStepUp: true }, + ); + + expect(outcome).toEqual({ kind: "satisfied" }); + expect(storageOf(params).saveScope).not.toHaveBeenCalled(); + silentSpy.mockRestore(); + }); + }); }); diff --git a/clients/web/src/test/integration/mcp/remote/remote-auth-branches.test.ts b/clients/web/src/test/integration/mcp/remote/remote-auth-branches.test.ts new file mode 100644 index 00000000..3a2100e5 --- /dev/null +++ b/clients/web/src/test/integration/mcp/remote/remote-auth-branches.test.ts @@ -0,0 +1,269 @@ +/** + * Supplemental coverage for createRemoteApp's /api/mcp/* routes + * (core/mcp/remote/node/server.ts), targeting branches the broader e2e + * suites (transport.test.ts, connect-crash.test.ts) don't reach: + * + * - POST /api/mcp/connect: connect-time AuthChallengeError (401 upstream) + * - POST /api/mcp/send: transport-dead short-circuit + * - POST /api/mcp/disconnect: unknown sessionId (no-op, still 200) + * - POST /api/mcp/auth-state: every guard branch (missing fields, unknown + * session, dead transport, no OAuth provider on the session, success) + */ + +import { describe, it, expect, afterEach, beforeEach } from "vitest"; +import { createServer } from "node:http"; +import type { Server } from "node:http"; +import { serve } from "@hono/node-server"; +import type { ServerType } from "@hono/node-server"; +import { createRemoteApp } from "@inspector/core/mcp/remote/node/server.js"; +import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server"; +import type { MCPServerConfig } from "@inspector/core/mcp/types.js"; + +interface Harness { + baseUrl: string; + server: ServerType; +} + +async function start(): Promise { + const { app } = createRemoteApp({ + dangerouslyOmitAuth: true, + initialConfig: { defaultEnvironment: {} }, + }); + return new Promise((resolve, reject) => { + const server = serve( + { fetch: app.fetch, port: 0, hostname: "127.0.0.1" }, + (info) => { + const port = + info && typeof info === "object" && "port" in info + ? (info as { port: number }).port + : 0; + resolve({ baseUrl: `http://127.0.0.1:${port}`, server }); + }, + ); + server.on("error", reject); + }); +} + +async function stop(h: Harness): Promise { + await new Promise((resolve) => h.server.close(() => resolve())); +} + +async function connect( + h: Harness, + config: MCPServerConfig, + authState?: { oauthTokens: { access_token: string; token_type: string } }, +): Promise { + return fetch(`${h.baseUrl}/api/mcp/connect`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ config, ...(authState && { authState }) }), + }); +} + +/** A raw HTTP server that returns 401 for every request (no MCP semantics). */ +async function startUnauthorizedUpstream(): Promise<{ + url: string; + server: Server; +}> { + const server = createServer((_req, res) => { + res.writeHead(401, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ error: "unauthorized" })); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const addr = server.address(); + const port = typeof addr === "object" && addr !== null ? addr.port : 0; + return { url: `http://127.0.0.1:${port}/mcp`, server }; +} + +/** Connect a stdio session whose process crashes almost immediately, then + * give the onclose handler time to mark the session's transport dead. */ +async function connectDeadSession(h: Harness): Promise { + const config: MCPServerConfig = { + type: "stdio", + command: process.execPath, + args: ["-e", "process.stderr.write('dying\\n'); process.exit(1);"], + }; + const res = await connect(h, config); + expect(res.status).toBe(200); + const { sessionId } = (await res.json()) as { sessionId: string }; + // Give the subprocess time to exit and the transport's onclose handler + // to mark the session dead (mirrors connect-crash.test.ts's technique). + await new Promise((resolve) => setTimeout(resolve, 300)); + return sessionId; +} + +describe("server.ts /api/mcp/* branch coverage", () => { + let h: Harness; + beforeEach(async () => { + h = await start(); + }); + afterEach(async () => { + await stop(h); + }); + + describe("POST /api/mcp/connect", () => { + it("returns 500 (not 401) when the upstream SSE connection is rejected for a non-auth reason", async () => { + // The SDK's SSEClientTransport wraps every connection failure — including + // a 401 — in its own SseError, so `err instanceof AuthChallengeError` at + // the connect catch site is unreachable via the real SSE/streamable-http + // SDK transports today (streamable-http's start() makes no network call + // at all; SSE's start() discards the thrown error's subclass). This test + // instead pins the *reachable* generic-failure branch: a non-401 refusal + // still surfaces as a 500 with the wrapped message, not a misleading 401. + const upstream = await startUnauthorizedUpstream(); + try { + const res = await connect(h, { + type: "sse", + url: upstream.url, + }); + expect(res.status).toBe(500); + const body = (await res.json()) as { error: string }; + expect(body.error).toMatch(/Failed to start transport/); + } finally { + await new Promise((r) => upstream.server.close(() => r())); + } + }); + }); + + describe("POST /api/mcp/send", () => { + it("short-circuits with a transport_error when the session's transport is already dead", async () => { + const sessionId = await connectDeadSession(h); + const res = await fetch(`${h.baseUrl}/api/mcp/send`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + sessionId, + message: { jsonrpc: "2.0", id: 1, method: "tools/list" }, + }), + }); + expect(res.status).toBe(200); + const body = (await res.json()) as { + ok: boolean; + kind?: string; + error?: string; + }; + expect(body.ok).toBe(false); + expect(body.kind).toBe("transport_error"); + expect(typeof body.error).toBe("string"); + }); + }); + + describe("POST /api/mcp/disconnect", () => { + it("returns ok:true as a no-op when the sessionId is unknown", async () => { + const res = await fetch(`${h.baseUrl}/api/mcp/disconnect`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ sessionId: "does-not-exist" }), + }); + expect(res.status).toBe(200); + expect((await res.json()).ok).toBe(true); + }); + }); + + describe("POST /api/mcp/auth-state", () => { + async function postAuthState(body: unknown): Promise { + return fetch(`${h.baseUrl}/api/mcp/auth-state`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); + } + + it("returns 400 when sessionId is missing", async () => { + const res = await postAuthState({ + authState: { oauthTokens: { access_token: "a", token_type: "Bearer" } }, + }); + expect(res.status).toBe(400); + expect((await res.json()).error).toMatch( + /Missing sessionId or authState/, + ); + }); + + it("returns 400 when authState is missing", async () => { + const res = await postAuthState({ sessionId: "some-session" }); + expect(res.status).toBe(400); + expect((await res.json()).error).toMatch( + /Missing sessionId or authState/, + ); + }); + + it("returns 400 on invalid JSON body", async () => { + const res = await fetch(`${h.baseUrl}/api/mcp/auth-state`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: "{not json", + }); + expect(res.status).toBe(400); + expect((await res.json()).error).toMatch(/Invalid JSON body/); + }); + + it("returns 404 when the session is not found", async () => { + const res = await postAuthState({ + sessionId: "unknown-session-id", + authState: { oauthTokens: { access_token: "a", token_type: "Bearer" } }, + }); + expect(res.status).toBe(404); + expect((await res.json()).error).toMatch(/Session not found/); + }); + + it("returns a transport_error when the session's transport is already dead", async () => { + const sessionId = await connectDeadSession(h); + const res = await postAuthState({ + sessionId, + authState: { oauthTokens: { access_token: "a", token_type: "Bearer" } }, + }); + expect(res.status).toBe(200); + const body = (await res.json()) as { ok: boolean; kind?: string }; + expect(body.ok).toBe(false); + expect(body.kind).toBe("transport_error"); + }); + + it("returns 400 when the session has no OAuth auth provider (connected without authState)", async () => { + const { command, args } = getTestMcpServerCommand(); + const res = await connect(h, { type: "stdio", command, args }); + expect(res.status).toBe(200); + const { sessionId } = (await res.json()) as { sessionId: string }; + + const authRes = await postAuthState({ + sessionId, + authState: { oauthTokens: { access_token: "a", token_type: "Bearer" } }, + }); + expect(authRes.status).toBe(400); + expect((await authRes.json()).error).toMatch( + /Session has no OAuth auth provider/, + ); + + await fetch(`${h.baseUrl}/api/mcp/disconnect`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ sessionId }), + }); + }); + + it("returns ok:true and hot-swaps the token when the session has an OAuth auth provider", async () => { + const { command, args } = getTestMcpServerCommand(); + const res = await connect( + h, + { type: "stdio", command, args }, + { oauthTokens: { access_token: "initial", token_type: "Bearer" } }, + ); + expect(res.status).toBe(200); + const { sessionId } = (await res.json()) as { sessionId: string }; + + const authRes = await postAuthState({ + sessionId, + authState: { + oauthTokens: { access_token: "rotated", token_type: "Bearer" }, + }, + }); + expect(authRes.status).toBe(200); + expect((await authRes.json()).ok).toBe(true); + + await fetch(`${h.baseUrl}/api/mcp/disconnect`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ sessionId }), + }); + }); + }); +}); diff --git a/clients/web/src/test/integration/mcp/remote/remote-session.test.ts b/clients/web/src/test/integration/mcp/remote/remote-session.test.ts index 960273a2..bb52a33b 100644 --- a/clients/web/src/test/integration/mcp/remote/remote-session.test.ts +++ b/clients/web/src/test/integration/mcp/remote/remote-session.test.ts @@ -182,6 +182,29 @@ describe("RemoteSession", () => { await expect(wait).resolves.toBeUndefined(); }); + it("endSend is a no-op when no send is active", () => { + const session = new RemoteSession("s-endsend-noop"); + expect(session.hasActiveSend()).toBe(false); + // No matching beginSend() — activeSendCount is already 0. + session.endSend(); + expect(session.hasActiveSend()).toBe(false); + }); + + it("waitForRequestResponse with timeoutMs=0 never schedules a timer and still resolves", async () => { + const session = new RemoteSession("s-no-timeout"); + const wait = session.waitForRequestResponse(7, 0); + session.onMessage({ jsonrpc: "2.0", id: 7, result: {} }); + await expect(wait).resolves.toBeUndefined(); + }); + + it("cancelRequestWait rejects a timeoutMs=0 wait without a timer to clear", async () => { + const session = new RemoteSession("s-no-timeout-cancel"); + const wait = session.waitForRequestResponse(8, 0); + const rejection = expect(wait).rejects.toThrow(/cancelled/); + session.cancelRequestWait(8); + await rejection; + }); + it("handleTransportAuthError rejects active request waits during send", async () => { const session = new RemoteSession("s-auth"); session.beginSend(); @@ -279,6 +302,28 @@ describe("RemoteSession", () => { vi.useRealTimers(); }); + it("setAuthState throws when no auth provider handle is set", () => { + const session = new RemoteSession("no-auth-provider"); + expect(() => + session.setAuthState({ + oauthTokens: { access_token: "x", token_type: "Bearer" }, + }), + ).toThrow(/Session has no OAuth auth provider/); + }); + + it("cancelRequestWait is a no-op when the requestId has no pending wait", () => { + const session = new RemoteSession("s-cancel-noop"); + // No wait was ever registered for this id — should not throw. + expect(() => session.cancelRequestWait(123)).not.toThrow(); + }); + + it("handleTransportAuthError returns false for a non-AuthChallengeError", () => { + const session = new RemoteSession("s-not-auth-error"); + expect(session.handleTransportAuthError(new Error("plain error"))).toBe( + false, + ); + }); + it("setAuthState updates the session auth provider", async () => { const { createRemoteAuthProvider } = await import("@inspector/core/mcp/remote/node/tokenAuthProvider.js"); diff --git a/clients/web/src/test/integration/mcp/remote/remoteClientTransport-unit.test.ts b/clients/web/src/test/integration/mcp/remote/remoteClientTransport-unit.test.ts index 559b29d8..4e07b3aa 100644 --- a/clients/web/src/test/integration/mcp/remote/remoteClientTransport-unit.test.ts +++ b/clients/web/src/test/integration/mcp/remote/remoteClientTransport-unit.test.ts @@ -16,6 +16,11 @@ import type { RemoteTransportOptions } from "@inspector/core/mcp/remote/remoteCl import type { MCPServerConfig } from "@inspector/core/mcp/types.js"; import type { RemoteEvent } from "@inspector/core/mcp/remote/types.js"; import type { JSONRPCMessage } from "@modelcontextprotocol/sdk/types.js"; +import { + AuthChallengeError, + AuthRecoveryRequiredError, + EMA_STEP_UP_PENDING_URL, +} from "@inspector/core/auth/challenge.js"; const CONFIG: MCPServerConfig = { type: "sse", @@ -69,6 +74,10 @@ interface MockFetchPlan { input: RequestInfo | URL, init?: RequestInit, ) => Response | Promise; + authState?: ( + input: RequestInfo | URL, + init?: RequestInit, + ) => Response | Promise; } /** @@ -87,6 +96,11 @@ function mockFetch(plan: MockFetchPlan): typeof fetch { if (url.includes("/api/mcp/events")) { return plan.events ? plan.events(input, init) : sseResponse([]); } + if (url.includes("/api/mcp/auth-state")) { + return plan.authState + ? plan.authState(input, init) + : jsonResponse({ ok: true }); + } if (url.includes("/api/mcp/send")) { return plan.send ? plan.send(input, init) : jsonResponse({ ok: true }); } @@ -101,6 +115,28 @@ function mockFetch(plan: MockFetchPlan): typeof fetch { return fn as unknown as typeof fetch; } +/** Push arbitrary RemoteEvent frames onto a controllable SSE stream. */ +function createPushableEventStream() { + const encoder = new TextEncoder(); + let controller: ReadableStreamDefaultController | null = null; + const stream = new ReadableStream({ + start(c) { + controller = c; + c.enqueue(encoder.encode(": keepalive\n\n")); + }, + }); + const push = (event: RemoteEvent) => { + controller?.enqueue(encoder.encode(sseFrame(event))); + }; + return { + response: new Response(stream, { + status: 200, + headers: { "content-type": "text/event-stream" }, + }), + push, + }; +} + function makeTransport( plan: MockFetchPlan, extra: Partial = {}, @@ -119,6 +155,19 @@ function makeTransport( /** Wait a tick so the detached consumeEventStream loop can advance. */ const tick = () => new Promise((r) => setTimeout(r, 10)); +/** + * An SSE response whose stream stays open (never closes on its own) until the + * transport cancels it. Use for tests that need the transport to remain + * started/open past the initial start() call — the plan's default `events()` + * handler returns an already-closed empty stream, which auto-closes the + * transport shortly after start() resolves. + */ +function openEventsResponse(): Response { + return new Response(new ReadableStream({ start() {} }), { + status: 200, + }); +} + describe("RemoteClientTransport (focused branch coverage)", () => { describe("start()", () => { it("throws Remote connect failed with status on non-OK connect", async () => { @@ -664,4 +713,638 @@ describe("RemoteClientTransport (focused branch coverage)", () => { expect(seen[0]!["x-mcp-remote-auth"]).toBeUndefined(); }); }); + + describe("start() connect-time auth challenge and error branches", () => { + it("accepts the ok:true connect response shape (ok-wrapped sessionId)", async () => { + const t = makeTransport({ + connect: () => jsonResponse({ ok: true, sessionId: "sess-ok" }), + }); + await t.start(); + expect(t.getRemoteBackendSessionId()).toBe("sess-ok"); + await t.close(); + }); + + it("throws a formatted error when connect returns a transport_error", async () => { + const t = makeTransport({ + connect: () => + jsonResponse({ + ok: false, + kind: "transport_error", + error: "upstream unreachable", + }), + }); + await expect(t.start()).rejects.toThrow( + /Remote connect failed: upstream unreachable/, + ); + }); + + it("throws AuthChallengeError immediately when connect returns an auth_challenge and no authRecovery is configured", async () => { + const t = makeTransport({ + connect: () => + jsonResponse({ + ok: false, + kind: "auth_challenge", + authChallenge: { reason: "token_expired" }, + }), + }); + try { + await t.start(); + throw new Error("expected start() to throw"); + } catch (e) { + expect(e).toBeInstanceOf(AuthChallengeError); + expect((e as AuthChallengeError).status).toBe(401); + } + }); + + it("preserves the raw httpStatus on the AuthChallengeError when no authRecovery is configured", async () => { + const t = makeTransport({ + connect: () => + jsonResponse({ + ok: false, + kind: "auth_challenge", + authChallenge: { + reason: "invalid_token", + raw: { httpStatus: 403 }, + }, + }), + }); + try { + await t.start(); + throw new Error("expected start() to throw"); + } catch (e) { + expect((e as AuthChallengeError).status).toBe(403); + } + }); + + it("throws AuthRecoveryRequiredError with the EMA pending URL for a step_up_confirm outcome", async () => { + const t = makeTransport( + { + connect: () => + jsonResponse({ + ok: false, + kind: "auth_challenge", + authChallenge: { reason: "insufficient_scope" }, + }), + }, + { + authRecovery: { + handleAuthChallenge: vi.fn().mockResolvedValue({ + kind: "step_up_confirm", + challenge: { reason: "insufficient_scope" }, + }), + }, + }, + ); + try { + await t.start(); + throw new Error("expected start() to throw"); + } catch (e) { + expect(e).toBeInstanceOf(AuthRecoveryRequiredError); + const err = e as AuthRecoveryRequiredError; + expect(err.authorizationUrl).toBe(EMA_STEP_UP_PENDING_URL); + expect(err.emaStepUpConfirm).toBe(true); + } + }); + + it("throws AuthRecoveryRequiredError with the authorization URL for an interactive outcome", async () => { + const authorizationUrl = new URL("https://idp.example/authorize"); + const t = makeTransport( + { + connect: () => + jsonResponse({ + ok: false, + kind: "auth_challenge", + authChallenge: { reason: "unauthorized" }, + }), + }, + { + authRecovery: { + handleAuthChallenge: vi.fn().mockResolvedValue({ + kind: "interactive", + authorizationUrl, + challenge: { reason: "unauthorized" }, + }), + }, + }, + ); + try { + await t.start(); + throw new Error("expected start() to throw"); + } catch (e) { + const err = e as AuthRecoveryRequiredError; + expect(err.authorizationUrl).toBe(authorizationUrl); + expect(err.emaStepUpConfirm).toBeUndefined(); + } + }); + + it("rethrows the recovery error for a failed outcome", async () => { + const recoveryError = new Error("recovery failed"); + const t = makeTransport( + { + connect: () => + jsonResponse({ + ok: false, + kind: "auth_challenge", + authChallenge: { reason: "unauthorized" }, + }), + }, + { + authRecovery: { + handleAuthChallenge: vi + .fn() + .mockResolvedValue({ kind: "failed", error: recoveryError }), + }, + }, + ); + await expect(t.start()).rejects.toBe(recoveryError); + }); + + it("succeeds after a satisfied outcome retries connect and receives a session", async () => { + let connectAttempt = 0; + const t = makeTransport( + { + connect: () => { + connectAttempt += 1; + if (connectAttempt === 1) { + return jsonResponse({ + ok: false, + kind: "auth_challenge", + authChallenge: { reason: "token_expired" }, + }); + } + return jsonResponse({ sessionId: "recovered-session" }); + }, + events: () => openEventsResponse(), + }, + { + authRecovery: { + handleAuthChallenge: vi + .fn() + .mockResolvedValue({ kind: "satisfied" }), + }, + }, + ); + await t.start(); + expect(connectAttempt).toBe(2); + expect(t.getRemoteBackendSessionId()).toBe("recovered-session"); + await t.close(); + }); + + it("throws AuthChallengeError when a retried connect still returns an auth_challenge", async () => { + const t = makeTransport( + { + connect: () => + jsonResponse({ + ok: false, + kind: "auth_challenge", + authChallenge: { reason: "token_expired" }, + }), + }, + { + authRecovery: { + handleAuthChallenge: vi + .fn() + .mockResolvedValue({ kind: "satisfied" }), + }, + }, + ); + await expect(t.start()).rejects.toBeInstanceOf(AuthChallengeError); + }); + }); + + describe("attachToSession while closed", () => { + it("un-closes a previously closed transport before reattaching", async () => { + const events = vi.fn(() => openEventsResponse()); + const t = makeTransport({ events }); + await t.close(); // never started; closed=true, no sessionId so disconnect is skipped + expect((t as unknown as { closed: boolean }).closed).toBe(true); + await t.attachToSession("resumed-session"); + expect((t as unknown as { closed: boolean }).closed).toBe(false); + expect(t.getRemoteBackendSessionId()).toBe("resumed-session"); + await t.close(); + }); + }); + + describe("fetchFn default", () => { + it("falls back to globalThis.fetch when no fetchFn option is configured", () => { + const t = new RemoteClientTransport( + { baseUrl: "http://remote.test" }, + CONFIG, + ); + expect((t as unknown as { fetchFn: typeof fetch }).fetchFn).toBe( + globalThis.fetch, + ); + }); + }); + + describe("parseSSE trailing buffer", () => { + it("ignores a trailing buffer line that is neither an event: nor a data: field", async () => { + const onmessage = vi.fn(); + const t = makeTransport({ + events: () => sseResponse([": trailing comment with no newline"]), + }); + t.onmessage = onmessage; + await t.start(); + await tick(); + expect(onmessage).not.toHaveBeenCalled(); + }); + }); + + describe("consumeEventStream stops after transport_error closes it", () => { + it("does not process a message frame that follows a transport_error frame in the same batch", async () => { + const m: JSONRPCMessage = { jsonrpc: "2.0", id: 99, result: {} }; + const onmessage = vi.fn(); + const onerror = vi.fn(); + const onclose = vi.fn(); + const t = makeTransport({ + events: () => + sseResponse([ + sseFrame({ type: "transport_error", data: { error: "died" } }), + sseFrame({ type: "message", data: m }), + ]), + }); + t.onmessage = onmessage; + t.onerror = onerror; + t.onclose = onclose; + await t.start(); + await tick(); + expect(onerror).toHaveBeenCalledTimes(1); + expect(onclose).toHaveBeenCalledTimes(1); + expect(onmessage).not.toHaveBeenCalled(); + }); + }); + + describe("consumeEventStream restart / re-entrancy suppression", () => { + it("suppresses transport_error handling entirely while a reconnect is in flight", async () => { + const sse = createPushableEventStream(); + const onerror = vi.fn(); + const onclose = vi.fn(); + const t = makeTransport({ events: () => sse.response }); + t.onerror = onerror; + t.onclose = onclose; + await t.start(); + await tick(); + // Simulate a reconnect (attachToSession/close) already in flight on this + // transport when a stale transport_error frame from the old stream lands. + ( + t as unknown as { restartingEventStream: boolean } + ).restartingEventStream = true; + sse.push({ type: "transport_error", data: { error: "stale" } }); + await tick(); + expect(onerror).not.toHaveBeenCalled(); + expect(onclose).not.toHaveBeenCalled(); + ( + t as unknown as { restartingEventStream: boolean } + ).restartingEventStream = false; + await t.close(); + }); + + it("does not double-fire onclose when the onerror handler synchronously closes the transport", async () => { + const t = makeTransport({ + events: () => + sseResponse([ + sseFrame({ type: "transport_error", data: { error: "boom" } }), + ]), + }); + const onclose = vi.fn(); + let closePromise: Promise | undefined; + t.onerror = () => { + closePromise = t.close(); + }; + t.onclose = onclose; + await t.start(); + await tick(); + await closePromise; + expect(onclose).toHaveBeenCalledTimes(1); + }); + }); + + describe("settleSseResponseWait guard", () => { + it("ignores a message-type SSE event with no id (notification)", async () => { + const onmessage = vi.fn(); + const t = makeTransport({ + events: () => + sseResponse([ + sseFrame({ + type: "message", + data: { + jsonrpc: "2.0", + method: "notifications/progress", + params: {}, + }, + }), + ]), + }); + t.onmessage = onmessage; + await t.start(); + await tick(); + expect(onmessage).toHaveBeenCalledTimes(1); + }); + + it("ignores a message-type SSE event that has an id but neither result nor error", async () => { + const onmessage = vi.fn(); + const t = makeTransport({ + events: () => + sseResponse([ + sseFrame({ + type: "message", + data: { jsonrpc: "2.0", id: 3, method: "sampling/createMessage" }, + }), + ]), + }); + t.onmessage = onmessage; + await t.start(); + await tick(); + expect(onmessage).toHaveBeenCalledTimes(1); + }); + }); + + describe("cancelSseResponseWait no-op when already settled", () => { + it("no-ops when the SSE response for the id already settled before the auth_challenge HTTP reply arrives", async () => { + const sse = createPushableEventStream(); + const t = makeTransport({ + events: () => sse.response, + send: async (_input, init) => { + const body = JSON.parse(String(init?.body)) as { + message: { id?: string | number }; + }; + // Simulate the SSE response for this id landing first... + sse.push({ + type: "message", + data: { jsonrpc: "2.0", id: body.message.id, result: {} }, + }); + // ...and let the independent SSE consumer settle it before the + // HTTP reply (an auth_challenge for the same id) comes back. + await tick(); + return jsonResponse({ + ok: false, + kind: "auth_challenge", + authChallenge: { reason: "token_expired" }, + }); + }, + }); + await t.start(); + await expect( + t.send({ jsonrpc: "2.0", id: 11, method: "tools/list" }), + ).rejects.toBeInstanceOf(AuthChallengeError); + await t.close(); + }); + }); + + describe("postSend catch skips cancel for requestId-less messages", () => { + it("does not attempt to cancel an SSE wait for a notification when requestSend throws", async () => { + const notif: JSONRPCMessage = { + jsonrpc: "2.0", + method: "notifications/cancelled", + }; + const t = makeTransport({ + events: () => openEventsResponse(), + send: () => { + throw new Error("network down"); + }, + }); + await t.start(); + await expect(t.send(notif)).rejects.toThrow(/network down/); + await t.close(); + }); + }); + + describe("send() generic transport_error and notification failure paths", () => { + it("throws the raw error message when send returns a non-auth_challenge failure kind", async () => { + const t = makeTransport({ + events: () => openEventsResponse(), + send: () => + jsonResponse({ + ok: false, + kind: "transport_error", + error: "upstream 503", + }), + }); + await t.start(); + await expect( + t.send({ jsonrpc: "2.0", id: 1, method: "ping" }), + ).rejects.toThrow(/upstream 503/); + await t.close(); + }); + + it("skips cancelSseResponseWait for a notification whose send fails", async () => { + const notif: JSONRPCMessage = { + jsonrpc: "2.0", + method: "notifications/cancelled", + }; + const t = makeTransport({ + events: () => openEventsResponse(), + send: () => + jsonResponse({ ok: false, kind: "transport_error", error: "boom" }), + }); + await t.start(); + await expect(t.send(notif)).rejects.toThrow(/boom/); + await t.close(); + }); + }); + + describe("send() auth_challenge without recovery configured", () => { + it("throws immediately when no authRecovery is configured", async () => { + const t = makeTransport({ + events: () => openEventsResponse(), + send: () => + jsonResponse({ + ok: false, + kind: "auth_challenge", + authChallenge: { reason: "token_expired" }, + }), + }); + await t.start(); + await expect( + t.send({ jsonrpc: "2.0", id: 1, method: "ping" }), + ).rejects.toBeInstanceOf(AuthChallengeError); + await t.close(); + }); + + it("throws immediately with the raw httpStatus when authRecovery has no pushAuthState handler", async () => { + const t = makeTransport( + { + events: () => openEventsResponse(), + send: () => + jsonResponse({ + ok: false, + kind: "auth_challenge", + authChallenge: { + reason: "token_expired", + raw: { httpStatus: 403 }, + }, + }), + }, + { authRecovery: { handleAuthChallenge: vi.fn() } }, + ); + await t.start(); + try { + await t.send({ jsonrpc: "2.0", id: 1, method: "ping" }); + throw new Error("expected send() to throw"); + } catch (e) { + expect((e as AuthChallengeError).status).toBe(403); + } + await t.close(); + }); + }); + + describe("pushAuthState guards", () => { + it("throws Transport not started when called before start()", async () => { + const t = makeTransport({}); + await expect( + t.pushAuthState({ + oauthTokens: { access_token: "a", token_type: "Bearer" }, + }), + ).rejects.toThrow(/Transport not started/); + }); + + it("throws Transport is closed after close()", async () => { + const t = makeTransport({}); + await t.start(); + await t.close(); + await expect( + t.pushAuthState({ + oauthTokens: { access_token: "a", token_type: "Bearer" }, + }), + ).rejects.toThrow(/Transport is closed/); + }); + + it("throws when the resolved auth state has neither oauthTokens nor oauthClient", async () => { + const t = makeTransport({ events: () => openEventsResponse() }); + await t.start(); + await expect(t.pushAuthState({})).rejects.toThrow( + /No auth state to push/, + ); + await t.close(); + }); + + it("throws No auth provider configured when called with no explicit state and no authProvider", async () => { + const t = makeTransport({ events: () => openEventsResponse() }); + await t.start(); + await expect(t.pushAuthState()).rejects.toThrow( + /No auth provider configured/, + ); + await t.close(); + }); + + it("throws No OAuth tokens available when the configured authProvider has no tokens", async () => { + const authProvider = { + tokens: async () => undefined, + } as unknown as NonNullable; + const t = makeTransport( + { events: () => openEventsResponse() }, + { authProvider }, + ); + await t.start(); + await expect(t.pushAuthState()).rejects.toThrow( + /No OAuth tokens available/, + ); + await t.close(); + }); + + it("throws a formatted error when the auth-state POST responds non-OK", async () => { + const t = makeTransport({ + events: () => openEventsResponse(), + authState: () => new Response("nope", { status: 500 }), + }); + await t.start(); + await expect( + t.pushAuthState({ + oauthTokens: { access_token: "a", token_type: "Bearer" }, + }), + ).rejects.toThrow(/Remote auth-state update failed \(500\)/); + await t.close(); + }); + + it("throws the server-provided error message when the auth-state POST reports ok:false", async () => { + const t = makeTransport({ + events: () => openEventsResponse(), + authState: () => jsonResponse({ ok: false, error: "session gone" }), + }); + await t.start(); + await expect( + t.pushAuthState({ + oauthTokens: { access_token: "a", token_type: "Bearer" }, + }), + ).rejects.toThrow(/session gone/); + await t.close(); + }); + + it("throws a default error message when the auth-state POST reports ok:false with no error field", async () => { + const t = makeTransport({ + events: () => openEventsResponse(), + authState: () => jsonResponse({ ok: false }), + }); + await t.start(); + await expect( + t.pushAuthState({ + oauthTokens: { access_token: "a", token_type: "Bearer" }, + }), + ).rejects.toThrow(/Remote auth-state update failed/); + await t.close(); + }); + + it("accepts an explicit authState argument without consulting authProvider", async () => { + const authProvider = { + tokens: vi.fn(), + } as unknown as NonNullable; + const authStateCalls: unknown[] = []; + const t = makeTransport( + { + events: () => openEventsResponse(), + authState: (_input, init) => { + authStateCalls.push(JSON.parse(String(init?.body))); + return jsonResponse({ ok: true }); + }, + }, + { authProvider }, + ); + await t.start(); + await t.pushAuthState({ + oauthTokens: { access_token: "explicit", token_type: "Bearer" }, + }); + expect(authStateCalls).toHaveLength(1); + await t.close(); + }); + }); + + describe("misc coverage", () => { + it("setOnAuthChallenge updates the ambient auth challenge handler", async () => { + const t = makeTransport({ + events: () => + sseResponse([ + sseFrame({ + type: "auth_challenge", + data: { reason: "token_expired" }, + }), + ]), + }); + const onAuthChallenge = vi.fn(); + t.setOnAuthChallenge(onAuthChallenge); + await t.start(); + await tick(); + expect(onAuthChallenge).toHaveBeenCalledWith({ reason: "token_expired" }); + }); + + it("is a no-op when start() is called again while already connected", async () => { + const connect = vi.fn(() => jsonResponse({ sessionId: "s" })); + const t = makeTransport({ connect, events: () => openEventsResponse() }); + await t.start(); + await t.start(); + expect(connect).toHaveBeenCalledTimes(1); + await t.close(); + }); + + it("rejects a pending SSE response wait when close() runs mid-send", async () => { + const t = makeTransport({ + events: () => openEventsResponse(), + send: () => jsonResponse({ ok: true }), + }); + await t.start(); + const sendPromise = t.send({ jsonrpc: "2.0", id: 1, method: "ping" }); + const rejection = expect(sendPromise).rejects.toThrow(/Transport closed/); + await t.close(); + await rejection; + }); + }); }); diff --git a/clients/web/src/test/integration/mcp/remote/server-extra-coverage.test.ts b/clients/web/src/test/integration/mcp/remote/server-extra-coverage.test.ts index b523a913..ba862f20 100644 --- a/clients/web/src/test/integration/mcp/remote/server-extra-coverage.test.ts +++ b/clients/web/src/test/integration/mcp/remote/server-extra-coverage.test.ts @@ -788,4 +788,113 @@ describe("server.ts supplemental coverage", () => { } }); }); + + describe("keychain-unavailable 503 on PUT/DELETE", () => { + class UnavailableOnWriteStore implements SecretStore { + async get(): Promise { + return null; + } + async set(): Promise { + throw new KeychainUnavailableError(new Error("libsecret missing")); + } + async delete(): Promise { + /* no-op */ + } + async deleteAllForServer(): Promise { + throw new KeychainUnavailableError(new Error("libsecret missing")); + } + } + + it("PUT returns 503 when the keychain write is unavailable", async () => { + const h = await start({ + secretStore: new UnavailableOnWriteStore(), + seedConfig: JSON.stringify({ + mcpServers: { + srv: { + type: "streamable-http", + url: "https://x.test/mcp", + oauth: { clientId: "cid", clientSecret: "shh" }, + }, + }, + }), + }); + try { + const res = await fetch(`${h.baseUrl}/api/servers/srv`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + config: { type: "streamable-http", url: "https://x.test/mcp" }, + settings: { + headers: [], + metadata: [], + connectionTimeout: 0, + requestTimeout: 0, + oauthClientSecret: "new-secret", + }, + }), + }); + expect(res.status).toBe(503); + expect((await res.json()).error).toMatch(/libsecret missing/); + } finally { + await stop(h); + } + }); + + it("DELETE returns 503 when the keychain sweep is unavailable", async () => { + const h = await start({ + secretStore: new UnavailableOnWriteStore(), + seedConfig: JSON.stringify({ + mcpServers: { srv: { type: "stdio", command: "node" } }, + }), + }); + try { + const res = await fetch(`${h.baseUrl}/api/servers/srv`, { + method: "DELETE", + }); + expect(res.status).toBe(503); + expect((await res.json()).error).toMatch(/libsecret missing/); + } finally { + await stop(h); + } + }); + }); + + describe("migratePlaintextSecrets rethrows a non-keychain error", () => { + class ThrowingOnGetStore implements SecretStore { + async get(): Promise { + throw new Error("disk full"); + } + async set(): Promise { + /* unreachable in this test */ + } + async delete(): Promise { + /* no-op */ + } + async deleteAllForServer(): Promise { + /* no-op */ + } + } + + it("GET /api/servers returns 500 when migration hits a non-KeychainUnavailableError", async () => { + const h = await start({ + secretStore: new ThrowingOnGetStore(), + seedConfig: JSON.stringify({ + mcpServers: { + srv: { + type: "streamable-http", + url: "https://x.test/mcp", + oauth: { clientId: "cid", clientSecret: "shh" }, + }, + }, + }), + }); + try { + const res = await fetch(`${h.baseUrl}/api/servers`); + expect(res.status).toBe(500); + expect((await res.json()).error).toMatch(/Failed to read server list/); + } finally { + await stop(h); + } + }); + }); }); diff --git a/clients/web/src/utils/browserTabVisibility.test.ts b/clients/web/src/utils/browserTabVisibility.test.ts index c4c9e98f..d852327a 100644 --- a/clients/web/src/utils/browserTabVisibility.test.ts +++ b/clients/web/src/utils/browserTabVisibility.test.ts @@ -41,4 +41,83 @@ describe("browserTabVisibility", () => { handler!(); expect(callback).toHaveBeenCalledOnce(); }); + + it("returned unsubscribe removes the visibilitychange listener", () => { + let handler: (() => void) | undefined; + vi.mocked(document.addEventListener).mockImplementation((_event, fn) => { + handler = fn as () => void; + }); + + const callback = vi.fn(); + const unsubscribe = onBrowserTabVisible(callback); + expect(handler).toBeDefined(); + + unsubscribe(); + expect(document.removeEventListener).toHaveBeenCalledWith( + "visibilitychange", + handler, + ); + + // After unsubscribing the stored handler must no longer fire the callback + // through the (now removed) listener path. + vi.mocked(document.addEventListener).mockClear(); + expect(document.addEventListener).not.toHaveBeenCalled(); + }); + + it("integrates with a real document: dispatch, hidden/visible, and unsubscribe", () => { + vi.unstubAllGlobals(); + const original = Object.getOwnPropertyDescriptor( + Document.prototype, + "visibilityState", + ); + const setVisibility = (value: string): void => { + Object.defineProperty(document, "visibilityState", { + configurable: true, + get: () => value, + }); + }; + + try { + setVisibility("visible"); + expect(isBrowserTabVisible()).toBe(true); + + const callback = vi.fn(); + const unsubscribe = onBrowserTabVisible(callback); + + // Hidden branch: listener fires but callback is not invoked. + setVisibility("hidden"); + document.dispatchEvent(new Event("visibilitychange")); + expect(callback).not.toHaveBeenCalled(); + expect(isBrowserTabVisible()).toBe(false); + + // Visible branch: callback invoked. + setVisibility("visible"); + document.dispatchEvent(new Event("visibilitychange")); + expect(callback).toHaveBeenCalledOnce(); + + // Unsubscribe: further events must not re-invoke the callback. + unsubscribe(); + document.dispatchEvent(new Event("visibilitychange")); + expect(callback).toHaveBeenCalledOnce(); + } finally { + if (original) { + Object.defineProperty(document, "visibilityState", original); + } else { + delete (document as { visibilityState?: string }).visibilityState; + } + } + }); + + it("no-ops safely when document is undefined (SSR guard)", () => { + vi.stubGlobal("document", undefined); + + expect(isBrowserTabVisible()).toBe(false); + + const callback = vi.fn(); + const unsubscribe = onBrowserTabVisible(callback); + expect(typeof unsubscribe).toBe("function"); + // The returned no-op must be callable without throwing. + expect(() => unsubscribe()).not.toThrow(); + expect(callback).not.toHaveBeenCalled(); + }); }); diff --git a/clients/web/src/utils/oauthResume.test.ts b/clients/web/src/utils/oauthResume.test.ts index 2ed83dc9..43969179 100644 --- a/clients/web/src/utils/oauthResume.test.ts +++ b/clients/web/src/utils/oauthResume.test.ts @@ -7,6 +7,7 @@ import { oauthResumeInsufficientScopeMessage, oauthResumeToastMessage, OAUTH_PENDING_SERVER_KEY, + OAUTH_RESUME_KEY, readOAuthResumeSnapshot, restoreTabUiFromSnapshot, writeOAuthResumeSnapshot, @@ -222,4 +223,273 @@ describe("oauthResume", () => { expect(clearGetPromptState).toHaveBeenCalledOnce(); expect(clearReadResourceState).toHaveBeenCalledOnce(); }); + + it("readOAuthResumeSnapshot returns undefined for a non-JSON string", () => { + storage.set(OAUTH_RESUME_KEY, "not-json{"); + expect(readOAuthResumeSnapshot()).toBeUndefined(); + }); + + it("readOAuthResumeSnapshot returns undefined when parsed JSON is null", () => { + storage.set(OAUTH_RESUME_KEY, "null"); + expect(readOAuthResumeSnapshot()).toBeUndefined(); + }); + + it("readOAuthResumeSnapshot rejects a wrong version", () => { + storage.set( + OAUTH_RESUME_KEY, + JSON.stringify({ + version: 2, + serverId: "srv-1", + activeTab: "Tools", + authKind: "reauth", + tabUi: {}, + }), + ); + expect(readOAuthResumeSnapshot()).toBeUndefined(); + }); + + it("readOAuthResumeSnapshot rejects a non-string serverId", () => { + storage.set( + OAUTH_RESUME_KEY, + JSON.stringify({ + version: 1, + serverId: 42, + activeTab: "Tools", + authKind: "reauth", + tabUi: {}, + }), + ); + expect(readOAuthResumeSnapshot()).toBeUndefined(); + }); + + it("readOAuthResumeSnapshot rejects an unknown authKind", () => { + storage.set( + OAUTH_RESUME_KEY, + JSON.stringify({ + version: 1, + serverId: "srv-1", + activeTab: "Tools", + authKind: "bogus", + tabUi: {}, + }), + ); + expect(readOAuthResumeSnapshot()).toBeUndefined(); + }); + + it("readOAuthResumeSnapshot rejects a non-string activeTab", () => { + storage.set( + OAUTH_RESUME_KEY, + JSON.stringify({ + version: 1, + serverId: "srv-1", + activeTab: 7, + authKind: "reauth", + tabUi: {}, + }), + ); + expect(readOAuthResumeSnapshot()).toBeUndefined(); + }); + + it("readOAuthResumeSnapshot accepts a snapshot with tabUi absent", () => { + storage.set( + OAUTH_RESUME_KEY, + JSON.stringify({ + version: 1, + serverId: "srv-1", + activeTab: "Tools", + authKind: "reauth", + }), + ); + const snapshot = readOAuthResumeSnapshot(); + expect(snapshot?.serverId).toBe("srv-1"); + expect(snapshot?.tabUi).toBeUndefined(); + }); + + it("readOAuthResumeSnapshot rejects non-object tabUi (string, array, null)", () => { + for (const tabUi of ['"nope"', "[]", "null"]) { + storage.set( + OAUTH_RESUME_KEY, + `{"version":1,"serverId":"srv-1","activeTab":"Tools","authKind":"reauth","tabUi":${tabUi}}`, + ); + expect(readOAuthResumeSnapshot()).toBeUndefined(); + } + }); + + it("writeOAuthResumeSnapshot swallows setItem failures", () => { + vi.stubGlobal("sessionStorage", { + getItem: () => null, + setItem: () => { + throw new Error("quota exceeded"); + }, + removeItem: () => {}, + }); + expect(() => + writeOAuthResumeSnapshot({ + version: 1, + serverId: "srv-1", + activeTab: "Tools", + authKind: "reauth", + tabUi: {}, + }), + ).not.toThrow(); + }); + + it("clearOAuthResumeSnapshot swallows removeItem failures", () => { + vi.stubGlobal("sessionStorage", { + getItem: () => null, + setItem: () => {}, + removeItem: () => { + throw new Error("blocked"); + }, + }); + expect(() => clearOAuthResumeSnapshot()).not.toThrow(); + }); + + it("legacy fallback returns undefined when the pending-key read throws", () => { + vi.stubGlobal("sessionStorage", { + getItem: (key: string) => { + if (key === OAUTH_PENDING_SERVER_KEY) { + throw new Error("blocked"); + } + return null; + }, + setItem: () => {}, + removeItem: () => {}, + }); + expect(readOAuthResumeSnapshot()).toBeUndefined(); + }); + + it("legacy fallback returns undefined when no pending server is stored", () => { + expect(readOAuthResumeSnapshot()).toBeUndefined(); + }); + + describe("without a window global", () => { + beforeEach(() => { + vi.stubGlobal("window", undefined); + }); + + it("writeOAuthResumeSnapshot is a no-op", () => { + expect(() => + writeOAuthResumeSnapshot({ + version: 1, + serverId: "srv-1", + activeTab: "Tools", + authKind: "reauth", + tabUi: {}, + }), + ).not.toThrow(); + expect(storage.size).toBe(0); + }); + + it("readOAuthResumeSnapshot returns undefined", () => { + expect(readOAuthResumeSnapshot()).toBeUndefined(); + }); + + it("clearOAuthResumeSnapshot is a no-op", () => { + expect(() => clearOAuthResumeSnapshot()).not.toThrow(); + }); + }); + + it("restoreTabUiFromSnapshot returns early when tabUi is undefined", () => { + const setToolsUi = vi.fn(); + restoreTabUiFromSnapshot(undefined, { + setToolsUi, + setPromptsUi: vi.fn(), + setResourcesUi: vi.fn(), + setAppsUi: vi.fn(), + setTasksUi: vi.fn(), + setLogsUi: vi.fn(), + setHistoryUi: vi.fn(), + setNetworkUi: vi.fn(), + }); + expect(setToolsUi).not.toHaveBeenCalled(); + }); + + it("restoreTabUiFromSnapshot skips keys that are not inspector tabs", () => { + const setters = { + setToolsUi: vi.fn(), + setPromptsUi: vi.fn(), + setResourcesUi: vi.fn(), + setAppsUi: vi.fn(), + setTasksUi: vi.fn(), + setLogsUi: vi.fn(), + setHistoryUi: vi.fn(), + setNetworkUi: vi.fn(), + }; + restoreTabUiFromSnapshot( + { NotATab: {} } as Record, + setters, + ); + for (const setter of Object.values(setters)) { + expect(setter).not.toHaveBeenCalled(); + } + }); + + it("restoreTabUiFromSnapshot restores every tab with a present value", () => { + const setters = { + setToolsUi: vi.fn(), + setPromptsUi: vi.fn(), + setResourcesUi: vi.fn(), + setAppsUi: vi.fn(), + setTasksUi: vi.fn(), + setLogsUi: vi.fn(), + setHistoryUi: vi.fn(), + setNetworkUi: vi.fn(), + }; + restoreTabUiFromSnapshot( + { + Tools: EMPTY_TOOLS_UI, + Prompts: EMPTY_PROMPTS_UI, + Resources: EMPTY_RESOURCES_UI, + Apps: EMPTY_APPS_UI, + Tasks: EMPTY_TASKS_UI, + Logs: EMPTY_LOGS_UI, + History: EMPTY_HISTORY_UI, + Network: EMPTY_NETWORK_UI, + }, + setters, + ); + expect(setters.setToolsUi).toHaveBeenCalledWith(EMPTY_TOOLS_UI); + expect(setters.setPromptsUi).toHaveBeenCalledWith(EMPTY_PROMPTS_UI); + expect(setters.setResourcesUi).toHaveBeenCalledWith(EMPTY_RESOURCES_UI); + expect(setters.setAppsUi).toHaveBeenCalledWith(EMPTY_APPS_UI); + expect(setters.setTasksUi).toHaveBeenCalledWith(EMPTY_TASKS_UI); + expect(setters.setLogsUi).toHaveBeenCalledWith(EMPTY_LOGS_UI); + expect(setters.setHistoryUi).toHaveBeenCalledWith(EMPTY_HISTORY_UI); + expect(setters.setNetworkUi).toHaveBeenCalledWith(EMPTY_NETWORK_UI); + }); + + it("restoreTabUiFromSnapshot falls back to EMPTY state for undefined tab values", () => { + const setters = { + setToolsUi: vi.fn(), + setPromptsUi: vi.fn(), + setResourcesUi: vi.fn(), + setAppsUi: vi.fn(), + setTasksUi: vi.fn(), + setLogsUi: vi.fn(), + setHistoryUi: vi.fn(), + setNetworkUi: vi.fn(), + }; + restoreTabUiFromSnapshot( + { + Tools: undefined, + Prompts: undefined, + Resources: undefined, + Apps: undefined, + Tasks: undefined, + Logs: undefined, + History: undefined, + Network: undefined, + }, + setters, + ); + expect(setters.setToolsUi).toHaveBeenCalledWith(EMPTY_TOOLS_UI); + expect(setters.setPromptsUi).toHaveBeenCalledWith(EMPTY_PROMPTS_UI); + expect(setters.setResourcesUi).toHaveBeenCalledWith(EMPTY_RESOURCES_UI); + expect(setters.setAppsUi).toHaveBeenCalledWith(EMPTY_APPS_UI); + expect(setters.setTasksUi).toHaveBeenCalledWith(EMPTY_TASKS_UI); + expect(setters.setLogsUi).toHaveBeenCalledWith(EMPTY_LOGS_UI); + expect(setters.setHistoryUi).toHaveBeenCalledWith(EMPTY_HISTORY_UI); + expect(setters.setNetworkUi).toHaveBeenCalledWith(EMPTY_NETWORK_UI); + }); }); diff --git a/core/auth/node/runner-interactive-oauth.ts b/core/auth/node/runner-interactive-oauth.ts index 77de2b17..f68a3280 100644 --- a/core/auth/node/runner-interactive-oauth.ts +++ b/core/auth/node/runner-interactive-oauth.ts @@ -76,6 +76,7 @@ export async function runRunnerInteractiveOAuth( onError: (params) => { flowReject( new Error( + /* v8 ignore next -- params.error is a required non-null string, so the "OAuth error" fallback is unreachable */ params.error_description ?? params.error ?? "OAuth error", ), ); @@ -89,6 +90,7 @@ export async function runRunnerInteractiveOAuth( options.callbackTimeoutMs ?? DEFAULT_RUNNER_INTERACTIVE_OAUTH_TIMEOUT_MS; const waitForCallback = Promise.race([ flowDone.finally(() => { + /* v8 ignore next 3 -- timeoutId is assigned synchronously while the race is constructed, before flowDone can settle, so the undefined arm is unreachable */ if (timeoutId !== undefined) { clearTimeout(timeoutId); } diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts index c029ccf0..601fc35b 100644 --- a/core/mcp/inspectorClient.ts +++ b/core/mcp/inspectorClient.ts @@ -2529,8 +2529,7 @@ export class InspectorClient extends InspectorClientEventTarget { ), { method: "completion/complete", - toolName: - ref.type === "ref/prompt" ? ref.name : ref.uri, + toolName: ref.type === "ref/prompt" ? ref.name : ref.uri, }, ); @@ -2941,6 +2940,7 @@ export class InspectorClient extends InspectorClientEventTarget { throw err; } const challenge = parseAuthChallengeFromError(err, context); + /* v8 ignore next 3 -- defensive: parseAuthChallengeFromError shares isAuthChallengeError's checks, so it always returns a truthy challenge once that guard passes */ if (!challenge) { throw err; } @@ -2994,7 +2994,10 @@ export class InspectorClient extends InspectorClientEventTarget { * Direct transports reconnect after token exchange so the live MCP session * picks up the new Bearer token (mirrors silent recovery reconnect). */ - async completeOAuthFlow(authorizationCode: string, iss?: string): Promise { + async completeOAuthFlow( + authorizationCode: string, + iss?: string, + ): Promise { await this.ensureOAuthManager().completeOAuthFlow(authorizationCode, iss); if (this.usesDirectAuthRecovery()) { await this.reconnectAfterAuthRecovery(); diff --git a/core/mcp/node/authChallengeFetch.ts b/core/mcp/node/authChallengeFetch.ts index e404b767..00361a39 100644 --- a/core/mcp/node/authChallengeFetch.ts +++ b/core/mcp/node/authChallengeFetch.ts @@ -17,6 +17,7 @@ export function createAuthChallengeInterceptFetch( } const challenge = parseAuthChallengeFromResponse(response); + /* v8 ignore next 3 -- parseAuthChallengeFromResponse only returns undefined for non-401/403, which the status guard above already excludes */ if (!challenge) { return response; } diff --git a/core/mcp/oauthManager.ts b/core/mcp/oauthManager.ts index 880f8c44..63c8ca14 100644 --- a/core/mcp/oauthManager.ts +++ b/core/mcp/oauthManager.ts @@ -469,9 +469,10 @@ export class OAuthManager { challenge, tokens.scope, ); + const authorizationScopesJoined = enriched.authorizationScopes?.join(" "); + /* v8 ignore next -- authorizationScopes from enrichChallengeWithAuthorizationScopes is always a defined array here, so this fallback is unreachable */ const scopeForAuth = - enriched.authorizationScopes?.join(" ") ?? - enriched.requiredScopes?.join(" "); + authorizationScopesJoined ?? enriched.requiredScopes?.join(" "); if (!scopeForAuth?.trim()) { return false; } @@ -556,7 +557,8 @@ export class OAuthManager { storage?.getScope(serverUrl), grantedTokenScope, ); - const requiredFromChallenge = challenge.requiredScopes?.filter(Boolean) ?? []; + const requiredFromChallenge = + challenge.requiredScopes?.filter(Boolean) ?? []; const grantedSet = new Set(parseScopeString(previousScope)); const missingRequired = requiredFromChallenge.filter( (scope) => !grantedSet.has(scope), @@ -576,9 +578,12 @@ export class OAuthManager { }; } - private resolveEmaScopeForChallenge(challenge: AuthChallenge): string | undefined { + private resolveEmaScopeForChallenge( + challenge: AuthChallenge, + ): string | undefined { if (challenge.reason === "insufficient_scope") { const fromChallenge = challenge.requiredScopes?.join(" ").trim(); + /* v8 ignore next 3 -- authorizationScopes is empty only when requiredScopes is also empty, so this branch is unreachable */ if (fromChallenge) { return fromChallenge; } @@ -600,7 +605,8 @@ export class OAuthManager { challenge: AuthChallenge, options?: HandleAuthChallengeOptions, ): Promise { - const enriched = await this.enrichChallengeWithAuthorizationScopes(challenge); + const enriched = + await this.enrichChallengeWithAuthorizationScopes(challenge); if (enriched.reason === "insufficient_scope" && !options?.confirmedStepUp) { return { kind: "step_up_confirm", challenge: enriched }; @@ -644,8 +650,7 @@ export class OAuthManager { enriched.reason === "insufficient_scope" && enriched.authorizationScopes?.length ) { - this.pendingAuthorizationScope = - enriched.authorizationScopes.join(" "); + this.pendingAuthorizationScope = enriched.authorizationScopes.join(" "); } return { kind: "interactive", authorizationUrl, challenge: enriched }; } catch (error) { diff --git a/core/mcp/remote/node/server.ts b/core/mcp/remote/node/server.ts index 3df7f60c..77804a09 100644 --- a/core/mcp/remote/node/server.ts +++ b/core/mcp/remote/node/server.ts @@ -2042,10 +2042,8 @@ export function createRemoteApp( // deliberate side-effect of using `readMcpConfig` + full rewrite // here. const existing = current.mcpServers[originalId]; + /* v8 ignore next 5 -- the `in` check above guarantees this branch is unreachable; narrowing without the non-null assertion keeps TS happy and makes the contract explicit for future refactors. */ if (!existing) { - // The `in` check above guarantees this branch is unreachable; - // narrowing without the non-null assertion keeps TS happy and - // makes the contract explicit for future refactors. return c.json({ error: `Server '${originalId}' not found` }, 404); } // Split the existing entry into its SDK-only config (no Inspector- diff --git a/core/mcp/remote/remoteClientTransport.ts b/core/mcp/remote/remoteClientTransport.ts index b9fde2f8..82c86b4f 100644 --- a/core/mcp/remote/remoteClientTransport.ts +++ b/core/mcp/remote/remoteClientTransport.ts @@ -103,7 +103,10 @@ function requestIdForMessage( function isConnectAuthChallenge( json: RemoteConnectResponse, -): json is Extract { +): json is Extract< + RemoteConnectResponse, + { ok: false; kind: "auth_challenge" } +> { return ( typeof json === "object" && json !== null && @@ -116,7 +119,10 @@ function isConnectAuthChallenge( function isConnectTransportError( json: RemoteConnectResponse, -): json is Extract { +): json is Extract< + RemoteConnectResponse, + { ok: false; kind: "transport_error" } +> { return ( typeof json === "object" && json !== null && @@ -212,7 +218,10 @@ export class RemoteClientTransport implements Transport { private eventStreamConsumeTask: Promise | null = null; private restartingEventStream = false; private closed = false; - private readonly sseResponseWaits = new Map(); + private readonly sseResponseWaits = new Map< + string | number, + SseResponseWait + >(); private readonly options: RemoteTransportOptions; private readonly config: import("../types.js").MCPServerConfig; @@ -287,10 +296,7 @@ export class RemoteClientTransport implements Transport { ): Promise { const recovery = this.options.authRecovery; if (!recovery) { - throw new AuthChallengeError( - challenge, - challenge.raw?.httpStatus ?? 401, - ); + throw new AuthChallengeError(challenge, challenge.raw?.httpStatus ?? 401); } const outcome = await recovery.handleAuthChallenge(challenge); @@ -536,7 +542,9 @@ export class RemoteClientTransport implements Transport { if (!res.ok) { const text = await res.text(); - throw new Error(`Remote auth-state update failed (${res.status}): ${text}`); + throw new Error( + `Remote auth-state update failed (${res.status}): ${text}`, + ); } const json = (await res.json()) as { ok?: boolean; error?: string };