Skip to content

fix connected accounts tokens#1358

Merged
BilalG1 merged 3 commits intodevfrom
fix-connected-accounts-token-access
Apr 21, 2026
Merged

fix connected accounts tokens#1358
BilalG1 merged 3 commits intodevfrom
fix-connected-accounts-token-access

Conversation

@BilalG1
Copy link
Copy Markdown
Collaborator

@BilalG1 BilalG1 commented Apr 21, 2026

Summary by CodeRabbit

  • Bug Fixes
    • OAuth flows now consistently block extra scopes and access tokens for shared OAuth keys, enforcing restrictions earlier in the request processing and across all environments.
  • Tests
    • Added end-to-end regression tests to verify requests with extra scopes against shared OAuth providers return a 400 response indicating extra scopes/access tokens are not allowed.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
stack-auth-hosted-components Ready Ready Preview, Comment Apr 21, 2026 2:01am
stack-backend Ready Ready Preview, Comment Apr 21, 2026 2:01am
stack-dashboard Ready Ready Preview, Comment Apr 21, 2026 2:01am
stack-demo Ready Ready Preview, Comment Apr 21, 2026 2:01am
stack-docs Ready Ready Preview, Comment Apr 21, 2026 2:01am
stack-preview-backend Ready Ready Preview, Comment Apr 21, 2026 2:01am
stack-preview-dashboard Ready Ready Preview, Comment Apr 21, 2026 2:01am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c21c9023-9e8d-426a-932f-54f5948fe9b8

📥 Commits

Reviewing files that changed from the base of the PR and between 9218f05 and 943ab80.

📒 Files selected for processing (4)
  • apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/[provider_account_id]/access-token/crud.tsx
  • apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx
  • apps/backend/src/app/api/latest/connected-accounts/access-token-helpers.tsx
  • apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/[provider_account_id]/access-token/crud.tsx

📝 Walkthrough

Walkthrough

Validation for provider scope and access-token gating for shared OAuth providers was centralized and enforced earlier in the authorize flow and via a new helper, removing environment-based conditionals.

Changes

Cohort / File(s) Summary
Authorize route scope validation
apps/backend/src/app/api/latest/auth/oauth/authorize/[provider_id]/route.tsx
Moved provider scope validation for shared providers to run immediately after Turnstile assessment, so extra scopes are rejected regardless of whether an access token is present.
Access-token gating helper + tests
apps/backend/src/app/api/latest/connected-accounts/access-token-helpers.tsx
Added isSharedAccessTokenBlocked(providerIsShared: boolean) and unit tests; centralizes logic that allows shared access tokens only when STACK_ALLOW_SHARED_OAUTH_ACCESS_TOKENS === "true".
Connected-accounts access-token handlers
apps/backend/src/app/api/latest/connected-accounts/.../access-token/crud.tsx, apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx
Replaced inline environment/env-var checks with calls to isSharedAccessTokenBlocked(...); error behavior unchanged but decision is centralized.
E2E regression tests
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/authorize.test.ts
Added two tests asserting requests with provider_scope against a shared Spotify provider return 400 with OAUTH_EXTRA_SCOPE_NOT_AVAILABLE_WITH_SHARED_OAUTH_KEYS.

Sequence Diagram(s)

(Skipped — changes are not presented as a multi-component sequential flow that requires a diagram.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • oauth sign in scope and test #887: Related to provider_scope handling in the authorize/sign-in flow; likely overlaps server-side validation for provider_scope.
  • More connected accounts #1165: Related to shared-OAuth token gating and connected-accounts access-token logic; touches similar areas and centralization efforts.

Suggested reviewers

  • N2D4

Poem

"I’m a rabbit in code, hopping through scope and key,
I moved checks forward so extra scopes can’t be,
A helper now guards tokens with one tiny rule,
No env trickery—just a clear little tool,
🐇🎉"

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is empty except for a repository contribution guideline comment, providing no explanation of the changes, rationale, or testing approach. Add a substantive description explaining the refactoring: what was changed, why (e.g., code consolidation), and what testing was added to verify the fix.
Title check ❓ Inconclusive The title 'fix connected accounts tokens' is vague and generic, using a broad term 'fix' without specifying what the actual problem is or what the solution entails. Clarify the title to describe the specific change, e.g., 'Refactor OAuth shared token access checks into centralized helper' or 'Centralize shared OAuth token validation logic'.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-connected-accounts-token-access

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 21, 2026

Greptile Summary

This PR fixes two related bugs in the shared-OAuth-key enforcement logic. First, the provider_scope guard in the authorize route was only evaluated when a token query param was present, allowing unauthenticated flows to bypass the restriction — now the check runs unconditionally. Second, the access-token endpoints were skipping the shared-key block entirely in production environments due to a !getNodeEnvironment().includes('prod') clause, meaning production users with shared OAuth providers could retrieve access tokens that should have been disallowed — that exemption is now removed from both CRUD files.

Confidence Score: 5/5

Safe to merge — all three changes are targeted correctness fixes with no side effects on non-shared providers.

All findings are P2 or lower. The changes are small, well-scoped, and correctly fix two distinct bypasses in shared-OAuth enforcement. No new behaviour is introduced for projects using their own OAuth keys.

No files require special attention.

Important Files Changed

Filename Overview
apps/backend/src/app/api/latest/auth/oauth/authorize/[provider_id]/route.tsx Moved provider_scope + shared-key guard earlier in the handler so it blocks ALL flows, not just token-authenticated link flows.
apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/[provider_account_id]/access-token/crud.tsx Removed the !getNodeEnvironment().includes('prod') exemption so the shared-key access-token guard now applies in every environment.
apps/backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx Same fix as the sibling file — drops the production environment exemption from the shared-key guard; also removes the now-unused getNodeEnvironment import.

Sequence Diagram

sequenceDiagram
    participant Client
    participant AuthorizeRoute as /oauth/authorize
    participant AccessTokenRoute as /connected-accounts/.../access-token

    Note over AuthorizeRoute: BEFORE: provider_scope check only inside if(token) block
    Note over AuthorizeRoute: AFTER: provider_scope check runs for ALL flows

    Client->>AuthorizeRoute: GET ?provider_scope=X (no token)
    AuthorizeRoute->>AuthorizeRoute: Check provider.isShared && provider_scope
    AuthorizeRoute-->>Client: OAuthExtraScopeNotAvailableWithSharedOAuthKeys (now fixed)

    Note over AccessTokenRoute: BEFORE: shared-key guard skipped in production
    Note over AccessTokenRoute: AFTER: guard applies in all environments

    Client->>AccessTokenRoute: POST (shared provider, production env)
    AccessTokenRoute->>AccessTokenRoute: provider.isShared && env var != true
    AccessTokenRoute-->>Client: OAuthAccessTokenNotAvailableWithSharedOAuthKeys (now fixed)
Loading

Reviews (1): Last reviewed commit: "fix connected accounts tokens" | Re-trigger Greptile

@BilalG1 BilalG1 merged commit f89b97b into dev Apr 21, 2026
31 of 32 checks passed
@BilalG1 BilalG1 deleted the fix-connected-accounts-token-access branch April 21, 2026 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants