fix(oauth): rescue legacy grants via grant.scope fallback (closes #29)#33
Open
stackbilt-admin wants to merge 1 commit intomainfrom
Open
fix(oauth): rescue legacy grants via grant.scope fallback (closes #29)#33stackbilt-admin wants to merge 1 commit intomainfrom
stackbilt-admin wants to merge 1 commit intomainfrom
Conversation
…uth (#29) The C-1a remediation (commit 256ba06, 2026-04-10) removed the hardcoded ['generate','read'] path in resolveAuth and started enforcing the actual scopes threaded through props.scopes. PR #32 fixed the grant-creation side so empty-scope OAuth initiates get routed through the consent page and mint grants with populated scopes. This closes the last gap: the READ side. Any grant minted before C-1a carries empty props.scopes (the pre-C-1a path wrote the grant top-level scope but never populated props), so every authenticated request from that legacy cohort still hits a (none) scopes error at tool dispatch - exactly the UX symptom #29 and #30 describe. resolveAuth now calls a new resolveLegacyGrantScopes helper when oauthProps.scopes is empty. The helper extracts the bearer, calls the OAuth provider's unwrapToken (which denormalizes the top-level grant.scope onto the token record), and returns it as the effective scope set. Successful rescues are logged so we can measure the legacy cohort shrinking over time and know when to retire both this fallback and the future backfill script. What this does NOT rescue: - Grants minted between C-1a (2026-04-10) and #32 (2026-04-11 20:43Z) where grant.scope and props.scopes are both empty. These users must disconnect and reauth through the #32 consent page - nothing exists on the server to fall back to. Architecture: The @cloudflare/workers-oauth-provider library transitively imports cloudflare:* protocol modules that vitest's node ESM loader cannot resolve, so getOAuthApi is dynamically imported inside the fallback helper. Every other unit test that transitively imports gateway.ts continues to work without touching the library. OAuthProvider configuration was extracted to src/oauth-config.ts so gateway.ts can pass the same options to getOAuthApi without a circular import back through index.ts. In-flight safety: deploy this READ fallback first. The backfill script that rewrites stale KV grants comes in a separate PR and must run AFTER this deploy is live - a partial backfill against workers running old code will 500 in-flight sessions on mixed blob versions. Tests (6 new, 131/131 total): - props.scopes present -> uses them directly, never touches unwrapToken - props.scopes empty + grant.scope populated -> fallback rescues, session created with rescued scopes, tool call succeeds - both empty -> scopes stay empty, tool call rejects with (none) error, which is the UX signal pointing users to reauth - unwrapToken returns null -> graceful empty fallback - unwrapToken throws (KV outage) -> caught, logged, empty fallback - no bearer header -> fallback is inert Related: #28 (closed by #32), #29 (this PR), #30 (should resolve after this deploys + users reconnect). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #29. This is the read-side half of the scope cluster — #32 already fixed grant creation so empty-scope OAuth initiates get routed through the consent page. This PR fixes grant reads so the legacy cohort (grants minted before the C-1a remediation on 2026-04-10) no longer hits
(none) scopesat tool dispatch.resolveAuthcalls a newresolveLegacyGrantScopeshelper wheneveroauthProps.scopesis empty. The helper runsunwrapTokenon the bearer and reads the denormalized top-levelgrant.scopeoff the token record.legacy-grant scope fallback rescued grant=...) so we can measure the legacy cohort shrinking over time and know when to retire the fallback + backfill.tsc --noEmitclean, no eslint config in this repo.What this does NOT rescue
Grants minted between C-1a (2026-04-10) and #32 deploy (2026-04-11 20:43Z) have empty
grant.scopeAND emptyprops.scopes— nothing to fall back to. Those users must disconnect and reauth through the #32 consent page to mint a fresh grant. That's unavoidable given the data loss at grant-creation time.In-flight deploy safety (READ FIRST)
Deploy this PR BEFORE running any backfill script. A partial backfill that rewrites stale
grant:*/mcp_session:*KV records while workers are still running pre-fallback code will 500 in-flight sessions on mixed blob versions. Sequence:wrangler deploythis PR[gateway] legacy-grant scope fallback rescuedlogs for 24h to confirm the rescue path is actually firing for real usersencryptedProps.scopesfromgrant.scopeso the fallback becomes dead codeArchitecture note
@cloudflare/workers-oauth-providertransitively importscloudflare:*protocol modules that vitest's node ESM loader cannot resolve. Eagerly importinggetOAuthApiat the top ofgateway.tsbreaks every unit test that transitively imports it, even tests that never hit the fallback. Fix: dynamicimport()inside the helper, confined to the runtime path. Tests that need to stub it (the newgateway-legacy-scope.test.ts) do so viavi.mock.OAuthProviderconfiguration was extracted tosrc/oauth-config.tsso bothindex.ts(real provider instantiation) andgateway.ts(getOAuthApifor helpers) can reference the same options without a circular import.Rejected alternatives
grantIdintopropsat creation time + lookup grant by ID on every request. Doesn't help the legacy cohort — their existing props are frozen without a grantId. Would only benefit future grants, which don't need the rescue.authenticated: falsewith areauth_requirederror code when both scope sources are empty. Considered but rejected — keepingauthenticated: truewith empty scopes preserves audit trail shape and lets the existing scope enforcement at tool dispatch emit the familiar(none)message that users can already map to "need to reconnect."Test plan
npm test— 131/131 passing (125 before + 6 new)npx tsc --noEmit— cleanlegacy-grant scope fallback rescuedand confirm rate drops over timeimage_list_modelsreturns the catalog (validates end-to-end that fix(oauth): route empty-scope initiates through consent screen (closes #28) #32 + oauth: fall back to grant.scope when ctx.props.scopes is empty (fixes legacy grants) #29 are live)Related: #28 (closed by #32), #30 (should resolve after this deploys + users reconnect), future backfill PR.