Honor the offline_access scope opt-in for refresh tokens (SEP-2207)#192
Conversation
) AS metadata now advertises scopes_supported: [offline_access], which per the MCP spec invites clients that want refresh tokens to request it explicitly. PRM deliberately continues to omit scopes_supported (refresh tokens are not a resource requirement). A new opt-in config, mcp.refreshTokenRequiresOfflineAccess (default false), makes the authorization_code exchange withhold refresh tokens unless the granted scope carries offline_access; the default policy is unchanged so existing MCP clients that never send offline_access keep receiving refresh tokens. Existing refresh-token families are unaffected either way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185AoLyjuZUuKAwg9NxUDiq
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the mcp.refreshTokenRequiresOfflineAccess configuration option to support the SEP-2207 specification, allowing operators to require the offline_access scope before issuing refresh tokens. The changes include configuration normalization, token issuance logic updates, and metadata advertising, accompanied by corresponding documentation and unit tests. There are no review comments, and I have no feedback to provide.
|
Reviewed; no blockers found. |
This comment has been minimized.
This comment has been minimized.
|
Re the gemini-review "blocker" (rotation bypasses the opt-in): adjudicated as not a blocker — the premise is incorrect. Rotation keeps the family's original 🤖 Posted by Claude on Nathan's behalf |
…ing)
An unresolved ${FLAG} placeholder (or any other non-coercible value) on a
documented boolean survived normalization as a truthy string and flipped
whichever direction the consuming gate tested — e.g. activating the
default-off refreshTokenRequiresOfflineAccess gate (caught in review).
normalizeMcpSecurityConfig now guarantees every documented boolean
(enabled, refreshTokenRequiresOfflineAccess, clientCredentials.enabled,
dynamicClientRegistration.enabled, clientIdMetadataDocuments.enabled) is
a real boolean or absent: coercible values coerce, anything else is
dropped with a startup warning so the documented default applies.
Consumers keep gating on plain truthiness.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0185AoLyjuZUuKAwg9NxUDiq
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Re the updated gemini-review comment (drain on rotation, now non-blocking): the adjudication above stands — rotation keeps the family's original 🤖 Posted by Claude on Nathan's behalf |
heskew
left a comment
heskew
left a comment
There was a problem hiding this comment.
Reviewed the current head (160992d). No blockers found.
The OAuth/MCP-specific behavior looks sound: offline_access is advertised by the authorization server but omitted from protected-resource metadata, invalid boolean configuration is normalized at the boundary, and the opt-in gate applies to authorization-code issuance without retroactively revoking existing refresh families. Those families retain their original bounded expires_at, so refresh rotation does not create an indefinite bypass.
The branch passes the unit suite, lint, unsuppressed TypeScript checking, and git diff --check.
🤖 Posted by Codex on Nathan's behalf.
# Conflicts: # docs/configuration.md # test/lib/mcp/wellKnown.test.js
2 participants
Part of #156 (item 2 — the SEP-2207
offline_accessMAY; SEP-2350 remains blocked on per-tool scopes and the July 28 final-spec re-verify stays tracked there).Summary
scopes_supported: ["offline_access"]— per the MCP spec, clients MAY addoffline_accessto their scope when AS metadata lists it. RFC 8414 permits a partial list; upstream-provider scopes stay opaque pass-through.scopes_supported(refresh tokens are not a resource requirement) — now documented in-code and pinned by a test.mcp.refreshTokenRequiresOfflineAccess(default false): when set, the authorization_code exchange withholds refresh tokens unless the granted scope carriesoffline_access. Normalization of the flag is total (normalizeMcpSecurityConfig): coercible strings coerce, and any other present value — including an unresolved${FLAG}placeholder, which review caught surviving as a truthy string — is dropped with a startup warning so the documented default applies. The same total normalization now covers every documented boolean in themcpblock.Design decision to weigh
Default behavior is unchanged: any client whose registered
grant_typesincluderefresh_tokenstill receives one without requestingoffline_access. Making the scope mandatory by default would force hourly re-auth for every MCP client that never sends it (most of them), so strictness is opt-in. If you'd rather flip the default, it's a one-line config-default change.Review notes
Cross-model review ran (Codex + Claude domain pass; Gemini leg unavailable). No blockers; the config-coercion finding was addressed as above. Existing refresh-token families are unaffected when the flag is flipped on (deliberate — no retro-revocation).
1084 unit tests pass (10 new: gate on/off/grant_types interplay, metadata advertisement, PRM omission, config coercion).
Generated by an LLM (Claude Fable 5) pairing with @heskew.
🤖 Generated with Claude Code
https://claude.ai/code/session_0185AoLyjuZUuKAwg9NxUDiq