modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(oauth): pass the raw challenge scopes to the step-up scope filter

Browse source 
Copilot review round 10 on #2114 — a defect in round 8's fix, and the
precise loop that fix's extra argument was added to prevent.

`enrichChallengeWithScopes` narrows `requiredScopes` to the *missing*
subset, so a challenge requiring `offline_access tools:write` against a
stored scope that already carries `offline_access` arrives as just
`["tools:write"]`. Passing that narrowed list made the filter read the
server's explicit requirement as inherited and strip it, so
re-authorization would earn the same challenge forever.

Passes `challenge.requiredScopes` — the raw list — instead. The mixed
case is now covered, and fails against the narrowed list.

Also stops the checkbox promising more than the setting delivers: it
does not register anything for a preconfigured or CIMD client, and the
authorization server decides issuance either way. It now says it
declares the grant so the server *may* issue a refresh token.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-24 23:35:00 -04:00
commit fef8f5ca5a
3 changed files with 28 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

2
clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx
View file

@ -913,7 +913,7 @@ export function ServerSettingsForm({
/>
<Checkbox
label="Request refresh token"
description={`Registers the refresh_token grant, so access tokens renew without a new sign-in. Uncheck it for authorization servers that reject the offline_access scope and forced consent prompt the grant brings with it. Applies on the next connect.${refreshTokenEmaNote}`}
description={`Declares the refresh_token grant, so the authorization server may issue a refresh token and renew access tokens without a new sign-in. Uncheck it for authorization servers that reject the offline_access scope and forced consent prompt the grant brings with it. Applies on the next connect.${refreshTokenEmaNote}`}
checked={settings.oauthRequestRefreshToken ?? true}
onChange={handleRequestRefreshTokenChange}
/>

18
clients/web/src/test/core/mcp/oauthManager.test.ts
View file

@ -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 () => {

10
core/mcp/oauthManager.ts
View file

@ -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;