fix(auth): handle P2002 race condition in OAuth sign-in to prevent account lockout#28844
fix(auth): handle P2002 race condition in OAuth sign-in to prevent account lockout#28844pmartin-dev wants to merge 1 commit intocalcom:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds tests exercising recovery from Prisma 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
915b1fc to
009906a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/features/auth/lib/next-auth-custom-adapter.test.ts (1)
46-90: Exercise the real sign-in callback instead of a copy.
simulateSignInNewUserCreation()mirrors the production catch branch fromnext-auth-options.ts, so these tests can stay green even when the real callback drifts. For example, failures from the shipped recovery path afteradapter.linkAccount()would not be caught here unless the duplicate helper is updated the same way. Prefer extracting the recovery logic into a shared helper or invokinggetOptions(...).callbacks.signInwith mocks so this suite validates the code that actually ships.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/features/auth/lib/next-auth-custom-adapter.test.ts` around lines 46 - 90, The test currently implements a standalone simulateSignInNewUserCreation() that duplicates the sign-in recovery logic; replace it by invoking the actual sign-in callback from your production config (e.g., call getOptions(...).callbacks.signIn or extract the shared recovery helper used by next-auth-options.ts) with the same mocked prisma and adapter to exercise real behavior. Specifically, remove simulateSignInNewUserCreation, import the function that returns NextAuth options (or the shared recovery helper), construct the same mocks for PrismaClient and CalComAdapter, call the signIn callback with a mock account/profile/context, and assert that on Prisma P2002 the adapter.linkAccount and the returned value match production; update test names and fixtures accordingly so tests validate the shipped logic rather than a duplicated implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/features/auth/lib/next-auth-options.ts`:
- Around line 1455-1458: The recovery lookup uses prisma.user.findFirst to set
existingUser but currently matches email with exact equality; update the where
clause to use the equals predicate with mode: "insensitive" (i.e., where: {
email: { equals: user.email, mode: "insensitive" } }) so it matches the
preflight/existence check semantics used elsewhere; keep the same select fields
(id, email, twoFactorEnabled) and the same variable name existingUser.
- Around line 1452-1469: On Prisma P2002 for email/username, do not directly
construct AdapterAccountPresenter.fromCalAccount and call
calcomAdapter.linkAccount; instead, fetch the existing record with
prisma.user.findFirst and route that existingUser back into the same
email-conflict merge/decision helper used earlier in this file (the routine that
runs the pre-create checks/merge logic between lines ~1199-1355), so the same
unverified-CAL / provider/invite/conversion branches are applied; only after
that helper returns success should you call calcomAdapter.linkAccount (and keep
its errors propagated so they hit the outer catch and surface as
/auth/error?error=user-creation-error); preserve the two-factor fast-path by
still calling loginWithTotp(existingUser.email) if
existingUser.twoFactorEnabled.
---
Nitpick comments:
In `@packages/features/auth/lib/next-auth-custom-adapter.test.ts`:
- Around line 46-90: The test currently implements a standalone
simulateSignInNewUserCreation() that duplicates the sign-in recovery logic;
replace it by invoking the actual sign-in callback from your production config
(e.g., call getOptions(...).callbacks.signIn or extract the shared recovery
helper used by next-auth-options.ts) with the same mocked prisma and adapter to
exercise real behavior. Specifically, remove simulateSignInNewUserCreation,
import the function that returns NextAuth options (or the shared recovery
helper), construct the same mocks for PrismaClient and CalComAdapter, call the
signIn callback with a mock account/profile/context, and assert that on Prisma
P2002 the adapter.linkAccount and the returned value match production; update
test names and fixtures accordingly so tests validate the shipped logic rather
than a duplicated implementation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d6448a3-de1f-44ef-be60-74001e7bb5d3
📒 Files selected for processing (3)
packages/features/auth/lib/next-auth-custom-adapter.test.tspackages/features/auth/lib/next-auth-custom-adapter.tspackages/features/auth/lib/next-auth-options.ts
009906a to
d67f5a3
Compare
What does this PR do?
When multiple concurrent OAuth sign-in requests hit the system (e.g., after rapid API automation), Prisma throws
P2002 unique constraint errors on
user.create()oraccount.create(). The current catch-all handler masks these asa generic "Error creating a new user", leaving the account in a broken state where subsequent logins fail
indefinitely.
This PR adds targeted P2002 recovery at two levels:
next-auth-custom-adapter.ts—linkAccount(): On P2002, looks up the existing account instead of crashing.This prevents the adapter from propagating unhandled errors upstream.
next-auth-options.ts—signIncallback: On P2002 during user creation (email/username conflict), recoversby finding the existing user and linking the OAuth account normally, instead of returning a fatal error redirect.
Both fixes follow the same P2002 handling pattern already used in
calcomSignupHandler.tsandselfHostedHandler.ts.Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Run the unit tests:
TZ=UTC yarn vitest run packages/features/auth/lib/next-auth-custom-adapter.test.tsAll 9 tests should pass across 3 describe blocks:
signIn callback – full flow (4 tests):
linkAccount before fix (1 test):
linkAccount after fix (4 tests):
To reproduce the original bug: trigger ~40-50 rapid API calls, then attempt OAuth dashboard login. Without the
fix, login fails with "Error creating a new user". With the fix, the race condition is recovered gracefully.
Checklist