Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the NextAuth sign-in callback to preferentially identify OAuth users via their stable providerAccountId (instead of email), preventing duplicate/orphaned accounts when an OAuth provider returns a renamed email address (e.g., Gmail rename).
Changes:
- Add OAuth user lookup by
(provider, providerAccountId)with an email-rename update path insignInCallback. - Keep email-based lookup as a fallback when no linked OAuth account exists.
- Add Jest coverage for providerAccountId-first lookup, email update behavior, and fallback paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/server/callbacks/signin.ts | Adds providerAccountId-based lookup and updates stored email when it changes. |
| src/server/api/tests/callbacks/signinOAuthLookup.test.ts | Adds tests covering providerAccountId lookup, email update/no-update, fallback behavior, and user creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Update the user's email if it changed (e.g. Gmail rename) | ||
| if (userExist && user.email && userExist.email !== user.email) { | ||
| await prisma.user.update({ | ||
| where: { id: userExist.id }, | ||
| data: { email: user.email }, | ||
| }); | ||
| userExist.email = user.email; | ||
| } |
There was a problem hiding this comment.
prisma.user.update({ data: { email: user.email } }) can throw a unique-constraint error if the renamed email is already in use (including by a duplicate account created before this fix). Because it’s inside the main try/catch, that will cause the entire sign-in to return false and lock the user out even though the providerAccountId link exists. Consider catching Prisma P2002 (or doing a pre-check) and either (a) skip the email update while still allowing sign-in, or (b) return a deterministic error/redirect telling the user/admin how to resolve the conflict (e.g., merge accounts).
| browser: "Chrome", | ||
| os: "Linux", |
There was a problem hiding this comment.
The mocked parseUA return value is missing required fields (browserVersion, osVersion). This can break type-checking under ts-jest and also makes the test less representative of production behavior (those fields are required by DeviceInfo / the Prisma UserDevice model). Update the mock to include the full ParsedUA shape.
| browser: "Chrome", | |
| os: "Linux", | |
| browser: "Chrome", | |
| browserVersion: "120.0.0.0", | |
| os: "Linux", | |
| osVersion: "unknown", |
| // Enable OAuth registration | ||
| process.env.OAUTH_ALLOW_NEW_USERS = "true"; | ||
| process.env.OAUTH_EXCLUSIVE_LOGIN = "true"; | ||
|
|
There was a problem hiding this comment.
This test mutates process.env.OAUTH_ALLOW_NEW_USERS and process.env.OAUTH_EXCLUSIVE_LOGIN but never restores the previous values. Since Jest can run test files in any order, this can leak state into other suites and cause flaky failures. Save the old values and restore them in afterEach/afterAll (or use jest.resetModules() + per-test env setup).
When a user renames their email (e.g., Gmail's new rename feature), the OAuth provider sends the updated email on next login. Previously, the sign-in callback looked up users solely by email, which would fail to find the existing account and create a duplicate, orphaning the original user and all their ZeroTier network configurations.
Resovles #882