diff --git a/clients/web/src/test/core/mcp/oauthManager.test.ts b/clients/web/src/test/core/mcp/oauthManager.test.ts index de7770a5..7e8e4de6 100644 --- a/clients/web/src/test/core/mcp/oauthManager.test.ts +++ b/clients/web/src/test/core/mcp/oauthManager.test.ts @@ -737,13 +737,18 @@ describe("OAuthManager", () => { }); describe("persisting the requested scope after an ordinary grant (#2117)", () => { - const CAPTURED_URL = new URL( - "https://auth.example.com/authorize?state=abc", - ); + function capturedUrlWithScope(scope?: string): URL { + const url = new URL("https://auth.example.com/authorize?state=abc"); + if (scope !== undefined) { + url.searchParams.set("scope", scope); + } + return url; + } async function authorizeThenComplete( storedScope: string | undefined, tokenScope: string | undefined, + authorizeUrlScope?: string, ) { mockedMcpAuth.mockResolvedValue("REDIRECT"); const params = createMockParams(); @@ -758,7 +763,7 @@ describe("OAuthManager", () => { .BaseOAuthClientProvider.prototype, "getCapturedAuthUrl", ) - .mockReturnValue(CAPTURED_URL); + .mockReturnValue(capturedUrlWithScope(authorizeUrlScope)); await manager.authenticate(); @@ -798,6 +803,23 @@ describe("OAuthManager", () => { expect(storageOf(params).saveScope).not.toHaveBeenCalled(); }); + + it("persists the scope the authorize URL actually carried, not the provider's", async () => { + // The SDK augments the request with `offline_access` when the AS + // advertises it and the client wants a refresh token, so the authorize + // URL is a superset of `provider.scope`. Persisting the provider's copy + // would understate the grant the AS implied by staying silent. + const params = await authorizeThenComplete( + "mcp", + undefined, + "mcp offline_access", + ); + + expect(storageOf(params).saveScope).toHaveBeenCalledWith( + SERVER_URL, + "mcp offline_access", + ); + }); }); describe("completeOAuthFlow (EMA)", () => { diff --git a/core/mcp/oauthManager.ts b/core/mcp/oauthManager.ts index 5a8955ff..4dbfbb9d 100644 --- a/core/mcp/oauthManager.ts +++ b/core/mcp/oauthManager.ts @@ -324,13 +324,20 @@ 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 + // 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) -- + // 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; + // + // Read it off the authorize URL rather than from `provider.scope`: the + // SDK augments the request with `offline_access` when the AS advertises + // it and the client asks for a refresh token, so the provider's scope can + // be a strict subset of what was actually asked for — and persisting the + // subset would understate the grant in exactly the way this fixes. + this.pendingAuthorizationScope = + capturedUrl.searchParams.get("scope")?.trim() || requestedScope; const stateParam = capturedUrl.searchParams.get("state"); if (stateParam && this.params.onBeforeOAuthRedirect) { @@ -366,8 +373,8 @@ export class OAuthManager { authorizationCode, iss, ); - // `scopeForMint` -- not `pendingAuthorizationScope` -- is what the - // mint actually asked for, so it is the RFC 6749 5.1 fallback here + // `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(