-
Notifications
You must be signed in to change notification settings - Fork 62
fix: normalize email case to prevent duplicate accounts #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { Doc, MutationCtx } from "../types.js"; | ||
| import { normalizeEmail } from "../utils.js"; | ||
|
|
||
| export async function findAccountByProviderAndId( | ||
| ctx: MutationCtx, | ||
| provider: string, | ||
| providerAccountId: string, | ||
| ): Promise<Doc<"authAccounts"> | null> { | ||
| const exact = await ctx.db | ||
| .query("authAccounts") | ||
| .withIndex("providerAndAccountId", (q) => | ||
| q.eq("provider", provider).eq("providerAccountId", providerAccountId), | ||
| ) | ||
| .unique(); | ||
| if (exact !== null) return exact; | ||
|
|
||
| const normalized = normalizeEmail(providerAccountId); | ||
| if (normalized === providerAccountId) return null; | ||
|
|
||
| return await ctx.db | ||
| .query("authAccounts") | ||
| .withIndex("providerAndAccountId", (q) => | ||
| q.eq("provider", provider).eq("providerAccountId", normalized), | ||
| ) | ||
| .unique(); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { GenericId } from "convex/values"; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { Doc, MutationCtx, QueryCtx } from "./types.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { AuthProviderMaterializedConfig, ConvexAuthConfig } from "../types.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { LOG_LEVELS, logWithLevel } from "./utils.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { LOG_LEVELS, logWithLevel, normalizeEmail } from "./utils.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| type CreateOrUpdateUserArgs = { | ||||||||||||||||||||||||||||||||||||||||||||||||
| type: "oauth" | "credentials" | "email" | "phone" | "verification"; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -127,6 +127,9 @@ async function defaultCreateOrUpdateUser( | |||||||||||||||||||||||||||||||||||||||||||||||
| ...(emailVerified ? { emailVerificationTime: Date.now() } : null), | ||||||||||||||||||||||||||||||||||||||||||||||||
| ...(phoneVerified ? { phoneVerificationTime: Date.now() } : null), | ||||||||||||||||||||||||||||||||||||||||||||||||
| ...profile, | ||||||||||||||||||||||||||||||||||||||||||||||||
| ...(typeof profile.email === "string" | ||||||||||||||||||||||||||||||||||||||||||||||||
| ? { email: normalizeEmail(profile.email) } | ||||||||||||||||||||||||||||||||||||||||||||||||
| : null), | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| const existingOrLinkedUserId = userId; | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (userId !== null) { | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -169,7 +172,17 @@ async function uniqueUserWithVerifiedEmail(ctx: QueryCtx, email: string) { | |||||||||||||||||||||||||||||||||||||||||||||||
| .withIndex("email", (q) => q.eq("email", email)) | ||||||||||||||||||||||||||||||||||||||||||||||||
| .filter((q) => q.neq(q.field("emailVerificationTime"), undefined)) | ||||||||||||||||||||||||||||||||||||||||||||||||
| .take(2); | ||||||||||||||||||||||||||||||||||||||||||||||||
| return users.length === 1 ? users[0] : null; | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (users.length === 1) return users[0]; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const normalized = normalizeEmail(email); | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (normalized === email) return null; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const normalizedUsers = await ctx.db | ||||||||||||||||||||||||||||||||||||||||||||||||
| .query("users") | ||||||||||||||||||||||||||||||||||||||||||||||||
| .withIndex("email", (q) => q.eq("email", normalized)) | ||||||||||||||||||||||||||||||||||||||||||||||||
| .filter((q) => q.neq(q.field("emailVerificationTime"), undefined)) | ||||||||||||||||||||||||||||||||||||||||||||||||
| .take(2); | ||||||||||||||||||||||||||||||||||||||||||||||||
| return normalizedUsers.length === 1 ? normalizedUsers[0] : null; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+180
to
+190
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: ambiguous exact match ( The original implementation returned 🛡️ Proposed fix if (users.length === 1) return users[0];
+ if (users.length > 1) return null;
const normalized = normalizeEmail(email);
if (normalized === email) return null;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| async function uniqueUserWithVerifiedPhone(ctx: QueryCtx, phone: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -198,7 +211,7 @@ async function createOrUpdateAccount( | |||||||||||||||||||||||||||||||||||||||||||||||
| : await ctx.db.insert("authAccounts", { | ||||||||||||||||||||||||||||||||||||||||||||||||
| userId, | ||||||||||||||||||||||||||||||||||||||||||||||||
| provider: args.provider.id, | ||||||||||||||||||||||||||||||||||||||||||||||||
| providerAccountId: account.providerAccountId, | ||||||||||||||||||||||||||||||||||||||||||||||||
| providerAccountId: normalizeEmail(account.providerAccountId), | ||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: rg -n "providerAndAccountId" --type ts -C 4 src/Repository: get-convex/convex-auth Length of output: 3191 🏁 Script executed: cat -n src/server/implementation/mutations/userOAuth.ts | head -50Repository: get-convex/convex-auth Length of output: 2046
The OAuth sign-in flow in The normalized fallback already exists in Major OAuth providers (Google, GitHub) issue numeric 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
| secret: account.secret, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| // This is never used with the default `createOrUpdateUser` implementation, | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -210,7 +223,12 @@ async function createOrUpdateAccount( | |||||||||||||||||||||||||||||||||||||||||||||||
| await ctx.db.patch(accountId, { userId }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (args.profile.emailVerified) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| await ctx.db.patch(accountId, { emailVerified: args.profile.email }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| await ctx.db.patch(accountId, { | ||||||||||||||||||||||||||||||||||||||||||||||||
| emailVerified: | ||||||||||||||||||||||||||||||||||||||||||||||||
| typeof args.profile.email === "string" | ||||||||||||||||||||||||||||||||||||||||||||||||
| ? normalizeEmail(args.profile.email) | ||||||||||||||||||||||||||||||||||||||||||||||||
| : args.profile.email, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (args.profile.phoneVerified) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| await ctx.db.patch(accountId, { phoneVerified: args.profile.phone }); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n "providerAccountId" --type ts -C 3Repository: get-convex/convex-auth
Length of output: 11395
🏁 Script executed:
Repository: get-convex/convex-auth
Length of output: 945
🏁 Script executed:
cat -n src/providers/Email.ts | head -50Repository: get-convex/convex-auth
Length of output: 2005
🏁 Script executed:
rg -n "type EmailConfig|interface EmailConfig" src/server/types.ts -A 20Repository: get-convex/convex-auth
Length of output: 706
🏁 Script executed:
rg -n "account.providerAccountId" src/providers/ -B 2 -A 2Repository: get-convex/convex-auth
Length of output: 625
Remove the redundant
as stringcast onaccount.providerAccountId.The field is already typed as
stringby the schema definition (seeproviderAccountId: v.string()in types.ts). Theaccountparameter isGenericDoc<DataModel, "authAccounts">, which properly infers the field type from the database schema. The Phone provider demonstrates this works without a cast (line 36 of Phone.ts uses it directly without casting). The cast adds no safety benefit and should be removed.🤖 Prompt for AI Agents