Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/providers/Email.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { GenericDataModel } from "convex/server";
import { EmailConfig, EmailUserConfig } from "../server/types.js";
import { normalizeEmail } from "../server/implementation/utils.js";

/**
* Email providers send a token to the user's email address
Expand Down Expand Up @@ -47,7 +48,10 @@ export function Email<DataModel extends GenericDataModel>(
"Token verification requires an `email` in params of `signIn`.",
);
}
if (account.providerAccountId !== params.email) {
if (
normalizeEmail(account.providerAccountId as string) !==
normalizeEmail(params.email)
) {
Comment on lines +51 to +54

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n "providerAccountId" --type ts -C 3

Repository: get-convex/convex-auth

Length of output: 11395


🏁 Script executed:

cat -n src/providers/Email.ts | sed -n '40,65p'

Repository: get-convex/convex-auth

Length of output: 945


🏁 Script executed:

cat -n src/providers/Email.ts | head -50

Repository: get-convex/convex-auth

Length of output: 2005


🏁 Script executed:

rg -n "type EmailConfig|interface EmailConfig" src/server/types.ts -A 20

Repository: get-convex/convex-auth

Length of output: 706


🏁 Script executed:

rg -n "account.providerAccountId" src/providers/ -B 2 -A 2

Repository: get-convex/convex-auth

Length of output: 625


Remove the redundant as string cast on account.providerAccountId.

The field is already typed as string by the schema definition (see providerAccountId: v.string() in types.ts). The account parameter is GenericDoc<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
Verify each finding against the current code and only fix it if needed.

In `@src/providers/Email.ts` around lines 51 - 54, Remove the redundant type
assertion on account.providerAccountId in Email.ts: update the conditional using
normalizeEmail(account.providerAccountId as string) to call
normalizeEmail(account.providerAccountId) directly (the account parameter is a
GenericDoc<DataModel, "authAccounts"> and providerAccountId is already typed as
string); keep the rest of the comparison with params.email unchanged and ensure
normalizeEmail is used the same way as in Phone.ts.

throw new Error(
"Short verification code requires a matching `email` " +
"in params of `signIn`.",
Expand Down
3 changes: 2 additions & 1 deletion src/providers/Password.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
} from "convex/server";
import { Value } from "convex/values";
import { Scrypt } from "lucia";
import { normalizeEmail } from "../server/implementation/utils.js";

/**
* The available options to a {@link Password} provider for Convex Auth.
Expand Down Expand Up @@ -256,6 +257,6 @@ function validateDefaultPasswordRequirements(password: string) {

function defaultProfile(params: Record<string, unknown>) {
return {
email: params.email as string,
email: normalizeEmail(params.email as string),
};
}
34 changes: 34 additions & 0 deletions src/server/implementation/mutations/accountLookup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { Doc, MutationCtx } from "../types.js";
import { normalizeEmail } from "../utils.js";

/**
* Look up an auth account by provider and account ID.
*
* Tries an exact match first, then falls back to a normalized (lowercased)
* lookup. This makes reads backward-compatible: accounts created before
* email normalization was introduced are still reachable by their original
* casing, while new (normalized) accounts are found on the fallback query.
*/
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();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { ConvexCredentialsConfig } from "../../types.js";
import { upsertUserAndAccount } from "../users.js";
import { getAuthSessionId } from "../sessions.js";
import { LOG_LEVELS, logWithLevel, maybeRedact } from "../utils.js";
import { findAccountByProviderAndId } from "./accountLookup.js";

export const createAccountFromCredentialsArgs = v.object({
provider: v.string(),
Expand Down Expand Up @@ -37,12 +38,11 @@ export async function createAccountFromCredentialsImpl(
shouldLinkViaPhone,
} = args;
const provider = getProviderOrThrow(providerId) as ConvexCredentialsConfig;
const existingAccount = await ctx.db
.query("authAccounts")
.withIndex("providerAndAccountId", (q) =>
q.eq("provider", provider.id).eq("providerAccountId", account.id),
)
.unique();
const existingAccount = await findAccountByProviderAndId(
ctx,
provider.id,
account.id,
);
if (existingAccount !== null) {
if (
account.secret !== undefined &&
Expand Down
19 changes: 8 additions & 11 deletions src/server/implementation/mutations/createVerificationCode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import * as Provider from "../provider.js";
import { EmailConfig, PhoneConfig } from "../../types.js";
import { getAccountOrThrow, upsertUserAndAccount } from "../users.js";
import { getAuthSessionId } from "../sessions.js";
import { LOG_LEVELS, logWithLevel, sha256 } from "../utils.js";
import { LOG_LEVELS, logWithLevel, normalizeEmail, sha256 } from "../utils.js";
import { findAccountByProviderAndId } from "./accountLookup.js";

export const createVerificationCodeArgs = v.object({
accountId: v.optional(v.id("authAccounts")),
Expand Down Expand Up @@ -37,14 +38,7 @@ export async function createVerificationCodeImpl(
const existingAccount =
existingAccountId !== undefined
? await getAccountOrThrow(ctx, existingAccountId)
: await ctx.db
.query("authAccounts")
.withIndex("providerAndAccountId", (q) =>
q
.eq("provider", providerId)
.eq("providerAccountId", email ?? phone!),
)
.unique();
: await findAccountByProviderAndId(ctx, providerId, email ?? phone!);

const provider = getProviderOrThrow(providerId, allowExtraProviders) as
| EmailConfig
Expand All @@ -54,7 +48,10 @@ export async function createVerificationCodeImpl(
await getAuthSessionId(ctx),
existingAccount !== null
? { existingAccount }
: { providerAccountId: email ?? phone! },
: {
providerAccountId:
email !== undefined ? normalizeEmail(email) : phone!,
},
provider.type === "email"
? { type: "email", provider, profile: { email: email! } }
: { type: "phone", provider, profile: { phone: phone! } },
Expand Down Expand Up @@ -103,7 +100,7 @@ async function generateUniqueVerificationCode(
provider,
code: await sha256(code),
expirationTime,
emailVerified: email,
emailVerified: email !== undefined ? normalizeEmail(email) : undefined,
phoneVerified: phone,
});
}
12 changes: 6 additions & 6 deletions src/server/implementation/mutations/modifyAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Infer, v } from "convex/values";
import { ActionCtx, MutationCtx } from "../types.js";
import { GetProviderOrThrowFunc, hash } from "../provider.js";
import { LOG_LEVELS, logWithLevel, maybeRedact } from "../utils.js";
import { findAccountByProviderAndId } from "./accountLookup.js";

export const modifyAccountArgs = v.object({
provider: v.string(),
Expand All @@ -21,12 +22,11 @@ export async function modifyAccountImpl(
secret: maybeRedact(account.secret ?? ""),
},
});
const existingAccount = await ctx.db
.query("authAccounts")
.withIndex("providerAndAccountId", (q) =>
q.eq("provider", provider).eq("providerAccountId", account.id),
)
.unique();
const existingAccount = await findAccountByProviderAndId(
ctx,
provider,
account.id,
);
if (existingAccount === null) {
throw new Error(
`Cannot modify account with ID ${account.id} because it does not exist`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
} from "../rateLimit.js";
import * as Provider from "../provider.js";
import { LOG_LEVELS, logWithLevel, maybeRedact } from "../utils.js";
import { findAccountByProviderAndId } from "./accountLookup.js";

export const retrieveAccountWithCredentialsArgs = v.object({
provider: v.string(),
Expand All @@ -33,12 +34,11 @@ export async function retrieveAccountWithCredentialsImpl(
secret: maybeRedact(account.secret ?? ""),
},
});
const existingAccount = await ctx.db
.query("authAccounts")
.withIndex("providerAndAccountId", (q) =>
q.eq("provider", providerId).eq("providerAccountId", account.id),
)
.unique();
const existingAccount = await findAccountByProviderAndId(
ctx,
providerId,
account.id,
);
if (existingAccount === null) {
return "InvalidAccountId";
}
Expand Down
7 changes: 5 additions & 2 deletions src/server/implementation/mutations/verifyCodeAndSignIn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
maybeGenerateTokensForSession,
} from "../sessions.js";
import { ConvexAuthConfig } from "../../types.js";
import { LOG_LEVELS, logWithLevel, sha256 } from "../utils.js";
import { LOG_LEVELS, logWithLevel, normalizeEmail, sha256 } from "../utils.js";
import { upsertUserAndAccount } from "../users.js";

export const verifyCodeAndSignInArgs = v.object({
Expand All @@ -39,7 +39,10 @@ export async function verifyCodeAndSignInImpl(
allowExtraProviders: args.allowExtraProviders,
});
const { generateTokens, provider, allowExtraProviders } = args;
const identifier = args.params.email ?? args.params.phone;
const identifier =
args.params.email !== undefined
? normalizeEmail(args.params.email)
: args.params.phone;
if (identifier !== undefined) {
if (await isSignInRateLimited(ctx, identifier, config)) {
logWithLevel(
Expand Down
29 changes: 26 additions & 3 deletions src/server/implementation/users.ts
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";
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -163,13 +166,28 @@ async function defaultCreateOrUpdateUser(
return userId;
}

/**
* Find a unique user with a verified email, using the same "try exact then
* normalized" strategy as {@link findAccountByProviderAndId} so that
* pre-normalization user records are still discoverable for account linking.
*/
async function uniqueUserWithVerifiedEmail(ctx: QueryCtx, email: string) {
const users = await ctx.db
.query("users")
.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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Minor: ambiguous exact match (users.length === 2) silently falls through to the normalized lookup.

The original implementation returned null for any count ≠ 1. The new logic does return users[0] when exactly one exact match exists, but for the users.length === 2 case (two users share the same verbatim email — a data-integrity anomaly), execution falls through to the normalized query. If that second query returns exactly one hit, the function returns that third, unrelated user instead of signalling ambiguity with null. Adding an early-exit guard preserves the intended "ambiguous ⇒ no linking" invariant:

🛡️ 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

‼️ 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.

Suggested change
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;
if (users.length === 1) return users[0];
if (users.length > 1) return null;
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;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/server/implementation/users.ts` around lines 180 - 190, The current logic
lets the ambiguous exact-match case (users.length === 2) fall through to the
normalized lookup and possibly return an unrelated user; add an early-exit guard
so any ambiguous exact-match returns null. Concretely, in the function that
queries into users (the block using `users` and `normalizeEmail(email)`), change
the first check to explicitly return the single match when `users.length === 1`,
but return null immediately when `users.length > 1` (ambiguous) before computing
`normalized` and running the `normalizedUsers` query; keep the rest of the
normalized lookup unchanged.

}

async function uniqueUserWithVerifiedPhone(ctx: QueryCtx, phone: string) {
Expand Down Expand Up @@ -210,7 +228,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 });
Expand Down
11 changes: 11 additions & 0 deletions src/server/implementation/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@ export function generateRandomString(length: number, alphabet: string) {
return osloGenerateRandomString(random, alphabet, length);
}

/**
* Normalize an email address for storage and comparison.
*
* Per RFC 5321 the local part *can* be case-sensitive, but in practice
* no major provider treats it that way. We lowercase and trim so that
* `User@Example.com` and `user@example.com` resolve to the same account.
*/
export function normalizeEmail(email: string): string {
return email.toLowerCase().trim();
}

export function logError(error: unknown) {
logWithLevel(
LOG_LEVELS.ERROR,
Expand Down
69 changes: 69 additions & 0 deletions test/convex/email.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,75 @@ test("redirectTo with email", async () => {
vi.unstubAllGlobals();
});

test("OTP sign-in with different email casing reuses same account", async () => {
setupEnv();
const t = convexTest(schema);

// First OTP sign in with mixed case
let code;
vi.stubGlobal(
"fetch",
vi.fn(async (input, init) => {
if (
typeof input === "string" &&
input === "https://api.resend.com/emails"
) {
code = init.body.match(/\?code=([^\s\\]+)/)?.[1];
return new Response(null, { status: 200 });
}
throw new Error("Unexpected fetch");
}),
);

await t.action(api.auth.signIn, {
provider: "resend",
params: { email: "Tom@Gmail.COM" },
});
vi.unstubAllGlobals();

const { tokens } = await t.action(api.auth.signIn, {
params: { code },
});
expect(tokens).not.toBeNull();

// Second OTP sign in with lowercase
let code2;
vi.stubGlobal(
"fetch",
vi.fn(async (input, init) => {
if (
typeof input === "string" &&
input === "https://api.resend.com/emails"
) {
code2 = init.body.match(/\?code=([^\s\\]+)/)?.[1];
return new Response(null, { status: 200 });
}
throw new Error("Unexpected fetch");
}),
);

await t.action(api.auth.signIn, {
provider: "resend",
params: { email: "tom@gmail.com" },
});
vi.unstubAllGlobals();

const { tokens: tokens2 } = await t.action(api.auth.signIn, {
params: { code: code2 },
});
expect(tokens2).not.toBeNull();

// Verify only one user and one account
await t.run(async (ctx) => {
const users = await ctx.db.query("users").collect();
expect(users).toHaveLength(1);
expect(users[0].email).toBe("tom@gmail.com");
const accounts = await ctx.db.query("authAccounts").collect();
expect(accounts).toHaveLength(1);
expect(accounts[0].providerAccountId).toBe("tom@gmail.com");
});
});

function setupEnv() {
process.env.SITE_URL = "http://localhost:5173";
process.env.CONVEX_SITE_URL = CONVEX_SITE_URL;
Expand Down
Loading
Loading