Skip to content

Fix CSP-blocked social login and add connected-accounts management - #683

Merged
kentcdodds merged 2 commits into
mainfrom
cursor/social-login-connections-247b
Jul 8, 2026
Merged

kentcdodds merged 2 commits into
mainfrom
cursor/social-login-connections-247b

Conversation

@kentcdodds

@kentcdodds kentcdodds commented Jul 8, 2026 •

Copy link
Copy Markdown
Owner

connections_card_connect_google_disconnect_demo.mp4

Fixes the two issues found while testing social login live on heykody.dev, in one PR:

1. Every provider button errored (CSP)

The login buttons were native form POSTs. The client router intercepts document form submits and replays them as fetch, whose followed redirect to the provider origin violates connect-src 'self' — and even unintercepted forms would hit form-action 'self' on the post-submit redirect. The mock providers masked this in tests because their authorize URL is same-origin.

Fix: POST /auth/:provider now returns { authorizeUrl } JSON when the request sends Accept: application/json, and the UI (login page + account card) fetches it and performs a top-level navigation to the provider, which CSP does not restrict. The 302 behavior remains for non-JSON clients, and the rate-limiter's 303-to-login fallback now only applies to non-JSON requests. No CSP loosening needed.

csp_fix_github_button_navigates_to_real_github.mp4
Demo with a real (non-mock) GitHub client id in dev: clicking "Continue with GitHub" now cleanly navigates to github.com with zero console errors.

2. Connect providers while signed in

/account gains a Connected accounts card (backed by GET/POST /account/connections.json):

  • Lists linked providers with the provider-side handle and offers "Connect GitHub / Google / X" buttons for enabled, unlinked providers — no logout needed.
  • Disconnect per provider, refused when the connection is the account's only sign-in method (no usable password, no passkey, no other connection). The guard lives inside the conditional DELETE itself so concurrent disconnects cannot race past a separate pre-check; the UI disables the button with an explanatory tooltip.
  • The callback is now session-aware: signed-in users always link (a provider identity already linked to a different user is a connection-conflict error, never an account switch), success redirects to /account?oauthLinked=<provider> with a confirmation message, and signed-in errors land on /account?oauthError=<code> instead of bouncing through /login.

Connected accounts card with disabled disconnect tooltip

Tests

  • auth-provider.node.test.ts: 3 new tests — JSON start mode (authorize URL + state cookie, JSON errors), signed-in link / re-link no-op / cross-user conflict / list / disconnect, and the disconnect guard for passwordless social-only accounts.
  • e2e/social-login.spec.ts: extended to cover the connections card end-to-end (disabled disconnect on the only sign-in method, connect Google, disconnect Google) via the mock providers.
System recap — extends existing primitives (medium risk)

Mode: recap · Base: main @ 1011ace3 · Head: 418ebf2f

Classification: extends — reshapes the social-login start contract (JSON mode) inside app-sessions and adds connection management; no new primitive (app-sessions code list updated in primitives.yaml).

Primitives touched

Primitive Group Impact
app-sessions auth extends — JSON start mode for CSP-safe navigation, session-aware callback (link-only when signed in), /account/connections.json list/disconnect with an atomic last-sign-in-method guard
app-ui surfaces extends — login buttons switch to fetch-then-navigate; new "Connected accounts" card on /account

System map

flowchart LR
	appUi["app-ui"]:::extended
	appSessions["app-sessions"]:::extended
	d1AppDb["d1-app-db"]:::untouched
	rbac["rbac"]:::untouched
	appUi --> appSessions --> d1AppDb
	appSessions --> rbac
	classDef touched fill:#1a7f37,color:#fff
	classDef extended fill:#9a6700,color:#fff
	classDef added fill:#cf222e,color:#fff
	classDef untouched fill:#57606a,color:#fff
Loading

Change flow

sequenceDiagram
	participant B as Browser (login or account card)
	participant K as Worker
	participant P as Provider
	B->>K: fetch POST /auth/:provider (Accept: json)
	K-->>B: { authorizeUrl } + kody_oauth_login cookie
	B->>P: top-level navigation (CSP-safe)
	P-->>B: 302 /auth/:provider/callback?code&state
	B->>K: GET callback
	alt signed in
		K-->>B: link -> /account?oauthLinked=… (conflict/em errors -> /account?oauthError=…)
	else signed out
		K-->>B: sign in / auto-link / signup as before
	end
Loading

Invariants

  • per-user-isolation: connections list/disconnect operate strictly on the authenticated user's rows (WHERE user_id = ?); linking never reassigns an identity that belongs to another user.
Open in Web Open in Cursor 

Summary by CodeRabbit

  • New Features
    • Added a “Connected accounts” section to account settings for viewing, linking, and disconnecting social sign-in providers.
    • Social sign-in now supports a JSON-based provider start flow so the app UI can launch OAuth and navigate reliably.
  • Bug Fixes
    • Improved OAuth callback handling for signed-in users, including clearer linking behavior, identity conflict handling, and more precise error redirects.
    • Prevented disconnecting the last remaining sign-in method for an account.
    • Updated social-login rate-limit behavior to match JSON vs browser navigation expectations.
  • Documentation / Tests
    • Updated social-login documentation and expanded e2e coverage for multi-provider linking/disconnecting.

@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds JSON-based social login start handling, signed-in account linking/disconnect flows, a new account connections API and UI, rate-limit branching for JSON clients, and matching tests/docs updates.

Changes

Social Login JSON Flow and Account Connections

Layer / File(s) Summary
OAuth start/callback handling
packages/worker/src/app/handlers/auth-provider.ts
OAuth start returns JSON authorizeUrl responses for JSON-preferring clients; callback handling routes signed-in linking, conflict cases, and account-scoped errors.
Account connections API
packages/worker/src/app/handlers/account-connections.ts, packages/worker/src/app/router.ts, packages/worker/src/app/routes.ts
Adds the authenticated /account/connections.json API for listing connections and processing disconnect requests, plus router and route wiring.
Shared social-sign-in client helpers
packages/worker/client/social-sign-in.ts, packages/worker/client/routes/login.tsx
Adds reusable provider discovery and sign-in helpers, and updates the login route to use them from a click-driven provider list.
Connected accounts UI
packages/worker/client/routes/account.tsx
Adds connected-account state, callback message handling, connection loading, connect/disconnect actions, and the connected-accounts card UI.
Rate limits, tests, and docs
packages/worker/src/index.ts, packages/worker/src/app/handlers/auth-provider.node.test.ts, e2e/social-login.spec.ts, docs/contributing/...
Updates JSON-aware rate-limit handling, adds unit and e2e coverage for the new flows, and refreshes related architecture and social-login documentation.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant UI
  participant AuthProviderHandler
  participant OAuthProvider
  participant AccountConnectionsAPI
  UI->>AuthProviderHandler: POST /auth/:provider (Accept: application/json)
  AuthProviderHandler-->>UI: { ok: true, authorizeUrl } + cookie
  UI->>OAuthProvider: navigate to authorizeUrl
  OAuthProvider-->>AuthProviderHandler: callback with code
  AuthProviderHandler->>AuthProviderHandler: check session and existing connection
  AuthProviderHandler-->>UI: redirect /account?oauthLinked or oauthError
  UI->>AccountConnectionsAPI: GET /account/connections.json
  AccountConnectionsAPI-->>UI: connections + availableProviders
  UI->>AccountConnectionsAPI: POST disconnect
  AccountConnectionsAPI-->>UI: updated connections or 400 error
Loading

Possibly related PRs

  • kentcdodds/kody#582: Both PRs modify auth-rate-limiting behavior in packages/worker/src/index.ts, affecting the same social-login response path.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: fixing CSP-blocked social login and adding connected-accounts management.
✨ 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 cursor/social-login-connections-247b

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.

@kentcdodds
kentcdodds marked this pull request as ready for review July 8, 2026 18:24
@github-actions

github-actions Bot commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

🔎 Preview deployed: https://kody-pr-683.kody-a99.workers.dev

Worker: kody-pr-683
D1: kody-pr-683-db
KV: kody-pr-683-oauth-kv

Mocks:

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e9892ae. Configure here.

Comment thread packages/worker/src/app/handlers/account-connections.ts Outdated
@kentcdodds
kentcdodds merged commit ecfeed7 into main Jul 8, 2026
5 checks passed
@kentcdodds
kentcdodds deleted the cursor/social-login-connections-247b branch July 8, 2026 18:43
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