diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx index 69e3b87e..fec3f25a 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx @@ -913,7 +913,7 @@ export function ServerSettingsForm({ /> diff --git a/clients/web/src/test/core/mcp/oauthManager.test.ts b/clients/web/src/test/core/mcp/oauthManager.test.ts index eadca821..76a01e88 100644 --- a/clients/web/src/test/core/mcp/oauthManager.test.ts +++ b/clients/web/src/test/core/mcp/oauthManager.test.ts @@ -1213,6 +1213,24 @@ describe("OAuthManager", () => { ); expect(sent?.split(/\s+/)).toContain("offline_access"); }); + + // The mixed case, and the one that made the guard above insufficient: + // `enrichChallengeWithScopes` narrows `requiredScopes` to the *missing* + // subset, so a challenge requiring both — against a stored scope that + // already carries `offline_access` — arrives as just `["tools:write"]`. + // Reading the narrowed list treats the server's explicit requirement as + // inherited and strips it, and re-authorization then earns the same + // challenge forever. + it("preserves a required offline_access when the challenge also names a missing scope", async () => { + const sent = await scopeSentForChallenge( + "mcp offline_access", + "mcp", + ["offline_access", "tools:write"], + false, + ); + expect(sent?.split(/\s+/)).toContain("offline_access"); + expect(sent?.split(/\s+/)).toContain("tools:write"); + }); }); it("short-circuits handleAuthChallenge when scope already satisfied", async () => { diff --git a/core/mcp/oauthManager.ts b/core/mcp/oauthManager.ts index 0f0b4727..c80033a5 100644 --- a/core/mcp/oauthManager.ts +++ b/core/mcp/oauthManager.ts @@ -804,7 +804,15 @@ export class OAuthManager { ? scopeForDeclinedRefreshGrant( requestedScopeForAuth, this.oauthConfig.scope, - enriched.requiredScopes, + // The RAW challenge scopes, not `enriched.requiredScopes`: + // `enrichChallengeWithScopes` narrows that to the *missing* subset, + // so a challenge requiring `offline_access tools:write` against a + // stored scope that already has `offline_access` arrives here as + // just `["tools:write"]`. Passing the narrowed list would read the + // server's explicit requirement as inherited and strip it — then + // re-authorization earns the same challenge forever, which is the + // loop this argument exists to prevent. + challenge.requiredScopes, ) : requestedScopeForAuth;