fix(oauth): honor a newly configured offline_access against stale storage
Copilot review round 6 on #2114. The early return handed back the persisted scope unchanged, so an `offline_access` the user had just typed into Scopes never reached the request when storage predated the change — and storage routinely does, since `createOAuthProvider` preserves a stored scope rather than reseeding from current settings. The docs promised configured scopes are honored and the form warned that `offline_access` "is still requested"; both were false in exactly that state. Honoring it means adding it, not merely declining to strip it. Only the token itself is added, not the rest of the configured scope: unioning unrelated configured tokens into the request is outside what this setting governs and could push an otherwise-satisfied connection into a step-up authorization. Three tests, all verified to fail against the previous behavior. 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:
parent
1bbdef03eb
commit
f153591042
1 changed files with 47 additions and 4 deletions
|
|
@ -89,6 +89,30 @@ describe("scopeForDeclinedRefreshGrant (#2068)", () => {
|
|||
).toBe("mcp offline_access");
|
||||
});
|
||||
|
||||
// Storage can predate the settings change — `createOAuthProvider` preserves a
|
||||
// stored scope rather than reseeding from current settings. Merely declining
|
||||
// to strip would silently omit a scope the user just typed, and the form's
|
||||
// "still requested" warning would be lying.
|
||||
it("adds a newly configured offline_access that stale storage lacks", () => {
|
||||
expect(scopeForDeclinedRefreshGrant("mcp", "mcp offline_access")).toBe(
|
||||
"mcp offline_access",
|
||||
);
|
||||
});
|
||||
|
||||
// Only the token itself is added; unrelated configured scopes are not unioned
|
||||
// in, since widening the request could force a step-up authorization.
|
||||
it("does not union unrelated configured scopes into the request", () => {
|
||||
expect(
|
||||
scopeForDeclinedRefreshGrant("mcp", "tools:read offline_access"),
|
||||
).toBe("mcp offline_access");
|
||||
});
|
||||
|
||||
it("does not duplicate an offline_access already present", () => {
|
||||
expect(
|
||||
scopeForDeclinedRefreshGrant(" mcp offline_access ", "offline_access"),
|
||||
).toBe("mcp offline_access");
|
||||
});
|
||||
|
||||
it("leaves a scope without offline_access untouched", () => {
|
||||
expect(scopeForDeclinedRefreshGrant("mcp tools:read", "mcp")).toBe(
|
||||
"mcp tools:read",
|
||||
|
|
|
|||
Loading…
Reference in a new issue