diff --git a/clients/web/src/test/core/mcp/oauthManager.test.ts b/clients/web/src/test/core/mcp/oauthManager.test.ts index 716638dd..de7770a5 100644 --- a/clients/web/src/test/core/mcp/oauthManager.test.ts +++ b/clients/web/src/test/core/mcp/oauthManager.test.ts @@ -736,6 +736,70 @@ describe("OAuthManager", () => { }); }); + describe("persisting the requested scope after an ordinary grant (#2117)", () => { + const CAPTURED_URL = new URL( + "https://auth.example.com/authorize?state=abc", + ); + + async function authorizeThenComplete( + storedScope: string | undefined, + tokenScope: string | undefined, + ) { + mockedMcpAuth.mockResolvedValue("REDIRECT"); + const params = createMockParams(); + storageOf(params).getScope.mockResolvedValue(storedScope); + storageOf(params).getClientInformation.mockResolvedValue({ + client_id: "cid", + }); + const manager = new OAuthManager(params); + const captureSpy = vi + .spyOn( + (await import("@inspector/core/auth/providers.js")) + .BaseOAuthClientProvider.prototype, + "getCapturedAuthUrl", + ) + .mockReturnValue(CAPTURED_URL); + + await manager.authenticate(); + + mockedMcpAuth.mockResolvedValue("AUTHORIZED"); + storageOf(params).getTokens.mockResolvedValue({ + access_token: "access", + token_type: "Bearer", + ...(tokenScope === undefined ? {} : { scope: tokenScope }), + }); + storageOf(params).saveScope.mockClear(); + + await manager.completeOAuthFlow("code"); + captureSpy.mockRestore(); + return params; + } + + it("persists the requested scope when the token response omits scope", async () => { + const params = await authorizeThenComplete("mcp weather:read", undefined); + + expect(storageOf(params).saveScope).toHaveBeenCalledWith( + SERVER_URL, + "mcp weather:read", + ); + }); + + it("still prefers the granted scope when the AS echoes one", async () => { + const params = await authorizeThenComplete("mcp weather:read", "mcp"); + + expect(storageOf(params).saveScope).toHaveBeenCalledWith( + SERVER_URL, + "mcp", + ); + }); + + it("persists nothing when neither the request nor the response names a scope", async () => { + const params = await authorizeThenComplete(undefined, undefined); + + expect(storageOf(params).saveScope).not.toHaveBeenCalled(); + }); + }); + describe("completeOAuthFlow (EMA)", () => { it("mints resource tokens via the EMA path and dispatches complete", async () => { const tokens = { access_token: "EMA", token_type: "Bearer" }; @@ -762,6 +826,35 @@ describe("OAuthManager", () => { expect(manager.getOAuthFlowStep()).toBe("complete"); mintSpy.mockRestore(); }); + + it("persists the minted scope request when the mint response omits scope (#2117)", async () => { + const params = createMockParams({ + enterpriseManagedAuth: { + idp: { + issuer: "https://idp.example.com", + clientId: "app-client", + clientSecret: "secret", + }, + }, + }); + params.initialConfig.scope = "mcp weather:read"; + storageOf(params).getScope.mockResolvedValue(undefined); + const manager = new OAuthManager(params); + manager.setOAuthConfig({ enterpriseManaged: true }); + + const mintSpy = vi + .spyOn(emaFlow, "completeEmaIdpAuthorizationAndMint") + .mockResolvedValue({ access_token: "EMA", token_type: "Bearer" }); + storageOf(params).saveScope.mockClear(); + + await manager.completeOAuthFlow("ema-code"); + + expect(storageOf(params).saveScope).toHaveBeenCalledWith( + SERVER_URL, + "mcp weather:read", + ); + mintSpy.mockRestore(); + }); }); describe("trySilentEnterpriseManagedAuth", () => { diff --git a/core/mcp/oauthManager.ts b/core/mcp/oauthManager.ts index 530f3fed..5a8955ff 100644 --- a/core/mcp/oauthManager.ts +++ b/core/mcp/oauthManager.ts @@ -305,9 +305,10 @@ export class OAuthManager { resourceMetadataUrl, }); + const requestedScope = provider.scope; const result = await mcpAuth(provider, { serverUrl, - scope: provider.scope, + scope: requestedScope, fetchFn: this.params.effectiveAuthFetch, resourceMetadataUrl, }); @@ -323,6 +324,14 @@ export class OAuthManager { throw new Error("Failed to capture authorization URL"); } + // RFC 6749 5.1: an omitted `scope` in the token response means the grant + // is identical to what was requested. Carry the request across the + // redirect so completeOAuthFlow has that fallback to persist (#2117) -- + // the step-up paths already do this, and without it an ordinary grant + // whose AS echoes no scope leaves the previous stored scope standing as + // if it were still what the current token carries. + this.pendingAuthorizationScope = requestedScope; + const stateParam = capturedUrl.searchParams.get("state"); if (stateParam && this.params.onBeforeOAuthRedirect) { const parsedState = parseOAuthState(stateParam); @@ -357,10 +366,13 @@ export class OAuthManager { authorizationCode, iss, ); - const requestedScope = this.pendingAuthorizationScope; + // `scopeForMint` -- not `pendingAuthorizationScope` -- is what the + // mint actually asked for, so it is the RFC 6749 5.1 fallback here + // (the two differ on the ordinary leg, where only the config carries + // a scope). const scopeToPersist = resolvePersistedScopeAfterGrant( tokens.scope, - requestedScope, + scopeForMint, ); if (scopeToPersist) { await this.requireStorage().saveScope(