fix(auth): reconcile shadow user by email so api_keys FK holds#11
Conversation
The CP can re-issue a user's UUID (e.g. after the DB migration), leaving a stale local user row with the same email under the old id. Because email is unique, the shadow upsert's plain insert of the new id threw on the email constraint (swallowed), so no row for the current id existed — and creating an API key failed with api_keys_user_id_user_id_fk. Reconcile in a transaction: free the email from any other shadow id (rename it uniquely, keeping old dependents valid), then upsert the current id with the real email. Self-heals on the next request.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe shadow-user upsert logic in getServerSession was changed from a simple best-effort insert with onConflictDoNothing to a database transaction that first stales any other shadow row sharing the same email, then inserts/upserts the current user by id with onConflictDoUpdate. ChangesShadow User Reconciliation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant getServerSession
participant Database
Client->>getServerSession: request session
getServerSession->>Database: begin transaction
getServerSession->>Database: update stale email (same email, different id)
getServerSession->>Database: insert user, onConflictDoUpdate on id
Database-->>getServerSession: transaction result
getServerSession-->>Client: return { user }
Related issues: None specified. Related PRs: None specified. Suggested labels: backend, database, auth Suggested reviewers: hallelx2 🐰 A shadow user once caused a fright, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/lib/server-auth.tsOops! Something went wrong! :( ESLint: 9.39.1 TypeError: Converting circular structure to JSON ... [truncated 455 characters] ... c/dist/eslintrc.cjs:3261:25) 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/lib/server-auth.ts`:
- Around line 101-105: The session reconciliation in server-auth.ts is clearing
optional CP fields by defaulting missing email_verified and avatar_url values to
false/null, which can overwrite existing local data. Update the set block in the
auth refresh flow so the logic around user.email_verified and user.avatar_url
only assigns those fields when the CP response actually includes them, and
otherwise leaves the stored values unchanged. Use the existing mapping in the
user update path as the place to apply this conditional behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 778e0e3b-2180-4417-9239-f33b55e8e5cd
📒 Files selected for processing (1)
apps/web/lib/server-auth.ts
| set: { | ||
| email: user.email, | ||
| name: user.name || user.email, | ||
| emailVerified: !!user.email_verified, | ||
| image: user.avatar_url || null, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Avoid clearing optional CP fields during refresh.
Line 104 and Line 105 turn omitted email_verified / avatar_url values into false / null, so an existing local row can lose stored verification or avatar data on any session reconciliation. Only update those fields when the CP response includes them.
Proposed fix
set: {
email: user.email,
name: user.name || user.email,
- emailVerified: !!user.email_verified,
- image: user.avatar_url || null,
+ ...(typeof user.email_verified === "boolean"
+ ? { emailVerified: user.email_verified }
+ : {}),
+ ...(user.avatar_url !== undefined
+ ? { image: user.avatar_url || null }
+ : {}),
updatedAt: new Date(),
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| set: { | |
| email: user.email, | |
| name: user.name || user.email, | |
| emailVerified: !!user.email_verified, | |
| image: user.avatar_url || null, | |
| set: { | |
| email: user.email, | |
| name: user.name || user.email, | |
| ...(typeof user.email_verified === "boolean" | |
| ? { emailVerified: user.email_verified } | |
| : {}), | |
| ...(user.avatar_url !== undefined | |
| ? { image: user.avatar_url || null } | |
| : {}), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/server-auth.ts` around lines 101 - 105, The session
reconciliation in server-auth.ts is clearing optional CP fields by defaulting
missing email_verified and avatar_url values to false/null, which can overwrite
existing local data. Update the set block in the auth refresh flow so the logic
around user.email_verified and user.avatar_url only assigns those fields when
the CP response actually includes them, and otherwise leaves the stored values
unchanged. Use the existing mapping in the user update path as the place to
apply this conditional behavior.
Bug: creating an API key failed with
insert or update on table "api_keys" violates foreign key constraint "api_keys_user_id_user_id_fk".Cause: the CP re-issued the user's UUID (after the DB migration). The local
usertable still held the old UUID under the same email. The shadow upsert didonConflictDoNothing(target: id), so inserting the new UUID hit the email-unique constraint, threw, was swallowed — and no row for the current id existed, breaking the FK.Fix: reconcile in a transaction — free the email from any other shadow id (rename it uniquely so old dependent rows stay valid), then upsert the current id with the real email. Self-heals on the next authenticated request.
Summary by CodeRabbit