From d932f22598167165cf930dfe3968f8bae400e7fd Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Tue, 13 Oct 2026 22:53:52 -0500 Subject: [PATCH 1/3] fix(auth): match account emails regardless of case Account lookups by email were exact, but Apple returns the address with the casing the user gave their Apple ID, while an email typed into the apps is usually lowercase. The passkey sign-up's "already registered" check lowercases the address before looking it up, so it never found a mixed-case Apple account: an iOS user who then signed up with a passkey on Android got a second, empty account, and bought a second subscription on it. UserDB.getUser and PasskeyDB.getUserByEmail now compare lower(email). Active accounts that differ only in case already exist, so the exact spelling is preferred, then the older account: every lookup that matched before returns the same row. Linking Apple or Google to an existing account by email can now find an account spelled differently, so that login's token carries the account's stored email. For accounts that predate external_id storage the token's email is the S3 prefix, and the provider's spelling would point at an empty one. Logins with a credential the account already has keep the provider's email, as before. A migration adds a partial index on lower(email) for active accounts; it can't be unique while the case-duplicate pairs remain. --- .../controllers/UserController.test.ts | 64 +++++++++++++++++++ .../services/EmailVerificationService.test.ts | 19 ++++++ src/__tests__/services/PasskeyService.test.ts | 24 +++++++ src/__tests__/services/UserServices.test.ts | 41 ++++++++++++ src/controllers/UserController.ts | 5 +- ...20261013120000_users_lower_email_active.ts | 23 +++++++ src/services/db/PasskeyDB.ts | 5 +- src/services/db/UserDB.ts | 11 ++-- 8 files changed, 186 insertions(+), 6 deletions(-) create mode 100644 src/database/migrations/20261013120000_users_lower_email_active.ts diff --git a/src/__tests__/controllers/UserController.test.ts b/src/__tests__/controllers/UserController.test.ts index 6ef6b2a..bc742fd 100644 --- a/src/__tests__/controllers/UserController.test.ts +++ b/src/__tests__/controllers/UserController.test.ts @@ -99,3 +99,67 @@ describe('UserController.secondOnboarding', () => { expect(res.json).toHaveBeenCalledWith({}); }); }); + +describe('UserController.initLogin', () => { + let userService: any; + let controller: UserController; + + beforeEach(() => { + userService = { + verifyToken: jest.fn(), + getAuthMethodByExternalId: jest.fn(), + getUser: jest.fn(), + addNewUser: jest.fn(), + addAuthMethod: jest.fn(), + addNewDevice: jest.fn(), + tokenUser: jest.fn<() => Promise>().mockResolvedValue('jwt'), + }; + controller = new UserController(userService, {} as any); + }); + + const appleLogin = () => + controller.initLogin({ body: { token_id: 'apple-token' }, headers: {} } as any, makeRes()); + + it("links Apple to an account whose email differs in case, and the token carries the account's email", async () => { + // The account was created by a passkey sign-up typed lowercase; Apple keeps the casing + userService.verifyToken.mockResolvedValue({ email: 'Jane.Doe@Example.com', sub: 'apple-sub' }); + userService.getAuthMethodByExternalId.mockResolvedValue(null); + userService.getUser.mockResolvedValue({ + id_user: 7, + email: 'jane.doe@example.com', + external_id: 'uuid-7', + }); + + await appleLogin(); + + expect(userService.addNewUser).not.toHaveBeenCalled(); + expect(userService.addAuthMethod).toHaveBeenCalledWith( + expect.objectContaining({ user_id: 7, auth_type: 'apple', is_primary: false }), + ); + // An email-prefixed account's files live under its own spelling + expect(userService.tokenUser).toHaveBeenCalledWith( + expect.objectContaining({ id_user: 7, email: 'jane.doe@example.com' }), + ); + }); + + it("keeps the provider's email in the token for a credential the account already has", async () => { + userService.verifyToken.mockResolvedValue({ email: 'new@example.com', sub: 'apple-sub' }); + userService.getAuthMethodByExternalId.mockResolvedValue({ + user_id: 7, + id_auth_method: 1, + email: 'old@example.com', + }); + userService.getUser.mockResolvedValue({ + id_user: 7, + email: 'old@example.com', + external_id: 'uuid-7', + session: 'apple-sub', + }); + + await appleLogin(); + + expect(userService.tokenUser).toHaveBeenCalledWith( + expect.objectContaining({ id_user: 7, email: 'new@example.com' }), + ); + }); +}); diff --git a/src/__tests__/services/EmailVerificationService.test.ts b/src/__tests__/services/EmailVerificationService.test.ts index 7604ef4..9777d1d 100644 --- a/src/__tests__/services/EmailVerificationService.test.ts +++ b/src/__tests__/services/EmailVerificationService.test.ts @@ -1,10 +1,12 @@ import { describe, it, expect, beforeEach } from '@jest/globals'; import { EmailVerificationService } from '../../services/EmailVerificationService'; +import { UserServices } from '../../services/UserServices'; import { getTestTransaction, mockEmailService, mockLoggerService, mockUserService, + createTestUser, createTestVerificationCode, } from '../setup'; import moment from 'moment'; @@ -116,6 +118,23 @@ describe('EmailVerificationService', () => { expect(mockEmailService.sendEmail).not.toHaveBeenCalled(); }); + it('should refuse an email an account already has in another case', async () => { + const trx = getTestTransaction(); + // An Apple sign-in stored the address with its casing + await createTestUser(trx, { email: 'Jane.Doe@Example.com' }); + const userService = new UserServices(); + (userService as any)._userDB.db = trx; + (userService as any)._userDB._logger = mockLoggerService; + (service as any)._userService = userService; + + // A passkey sign-up on the other app, typed lowercase + const result = await service.sendVerificationCode('jane.doe@example.com'); + + expect(result.success).toBe(false); + expect(result.error_code).toBe('EMAIL_ALREADY_REGISTERED'); + expect(mockEmailService.sendEmail).not.toHaveBeenCalled(); + }); + it('should allow sending after rate limit window expires', async () => { const trx = getTestTransaction(); const email = 'test@example.com'; diff --git a/src/__tests__/services/PasskeyService.test.ts b/src/__tests__/services/PasskeyService.test.ts index 5686b05..297003c 100644 --- a/src/__tests__/services/PasskeyService.test.ts +++ b/src/__tests__/services/PasskeyService.test.ts @@ -162,6 +162,30 @@ describe('PasskeyService', () => { expect(result!.external_id).toBe(user.external_id); }); + it('should find the user whatever the email casing', async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx, { email: 'Jane.Doe@Example.com' }); + + const result = await service.getUserByEmail('jane.doe@example.com'); + + expect(result).not.toBeNull(); + expect(result!.id_user).toBe(user.id_user); + expect(result!.email).toBe('Jane.Doe@Example.com'); + }); + + it('should prefer the exact spelling when two users differ only in case', async () => { + const trx = getTestTransaction(); + const mixedCase = await createTestUser(trx, { email: 'Split@Example.com' }); + const lowerCase = await createTestUser(trx, { email: 'split@example.com' }); + + expect((await service.getUserByEmail('Split@Example.com'))!.id_user).toBe( + mixedCase.id_user, + ); + expect((await service.getUserByEmail('split@example.com'))!.id_user).toBe( + lowerCase.id_user, + ); + }); + it('should return null for non-existent email', async () => { const result = await service.getUserByEmail('nonexistent@example.com'); expect(result).toBeNull(); diff --git a/src/__tests__/services/UserServices.test.ts b/src/__tests__/services/UserServices.test.ts index a75c6cb..3f9932f 100644 --- a/src/__tests__/services/UserServices.test.ts +++ b/src/__tests__/services/UserServices.test.ts @@ -19,6 +19,47 @@ describe('UserServices - Duplicate Prevention', () => { mockLoggerService.log.mockClear(); }); + describe('GetUser', () => { + it('should find the account whatever the email casing', async () => { + const trx = getTestTransaction(); + + // Apple keeps the casing of the address; the other app's user types it lowercase + const user = await createTestUser(trx, { email: 'Jane.Doe@Example.com' }); + + const result = await service.getUser({ email: 'jane.doe@example.com' }); + + expect(result).toBeDefined(); + expect(result.id_user).toBe(user.id_user); + expect(result.email).toBe('Jane.Doe@Example.com'); + }); + + it('should prefer the exact spelling when two accounts differ only in case', async () => { + const trx = getTestTransaction(); + + const mixedCase = await createTestUser(trx, { email: 'Split@Example.com' }); + const lowerCase = await createTestUser(trx, { email: 'split@example.com' }); + + const byMixed = await service.getUser({ email: 'Split@Example.com' }); + const byLower = await service.getUser({ email: 'split@example.com' }); + const byOther = await service.getUser({ email: 'SPLIT@EXAMPLE.COM' }); + + expect(byMixed.id_user).toBe(mixedCase.id_user); + expect(byLower.id_user).toBe(lowerCase.id_user); + // No exact match: the older account + expect(byOther.id_user).toBe(mixedCase.id_user); + }); + + it('should not return an inactive account', async () => { + const trx = getTestTransaction(); + + await createTestUser(trx, { email: 'Gone@Example.com', active: false }); + + const result = await service.getUser({ email: 'gone@example.com' }); + + expect(result).toBeUndefined(); + }); + }); + describe('GetAuthMethodByExternalId', () => { it('should return auth method and user info when found', async () => { const trx = getTestTransaction(); diff --git a/src/controllers/UserController.ts b/src/controllers/UserController.ts index e79af9b..299c6ea 100644 --- a/src/controllers/UserController.ts +++ b/src/controllers/UserController.ts @@ -143,7 +143,10 @@ export class UserController { const token = await this._userService.tokenUser({ id_user: user.id_user, - email: authData.email, + // This claim is the S3 prefix of an account that predates external_id + // storage. A credential just linked by email may spell the address in a + // different case than the account does, so it carries the account's. + email: existingAuthMethod ? authData.email : user.email, external_id: user.external_id, session: authData.external_id, }); diff --git a/src/database/migrations/20261013120000_users_lower_email_active.ts b/src/database/migrations/20261013120000_users_lower_email_active.ts new file mode 100644 index 0000000..7f75627 --- /dev/null +++ b/src/database/migrations/20261013120000_users_lower_email_active.ts @@ -0,0 +1,23 @@ +import type { Knex } from 'knex'; + +// CREATE INDEX CONCURRENTLY can't run inside a transaction, and it keeps +// `users` writable (sign-ups) while the index builds. +export const config = { transaction: false }; + +// Account lookups by email ignore case (`lower(email) = lower(?)`), which +// `users_email_active_unique` can't serve. Not unique: active accounts whose +// emails differ only in case already exist and are merged by hand. +export async function up(knex: Knex): Promise { + // A concurrent build that fails leaves an INVALID index the planner never + // uses; IF NOT EXISTS would then skip it for good. Drop any leftover first. + await knex.raw('DROP INDEX CONCURRENTLY IF EXISTS users_lower_email_active'); + await knex.raw(` + CREATE INDEX CONCURRENTLY users_lower_email_active + ON users (lower(email)) + WHERE active = true + `); +} + +export async function down(knex: Knex): Promise { + await knex.raw('DROP INDEX CONCURRENTLY IF EXISTS users_lower_email_active'); +} diff --git a/src/services/db/PasskeyDB.ts b/src/services/db/PasskeyDB.ts index 99b2611..61f5ec2 100644 --- a/src/services/db/PasskeyDB.ts +++ b/src/services/db/PasskeyDB.ts @@ -305,9 +305,12 @@ export class PasskeyDB { ): Promise { try { const db = trx || this.db; + // Case-insensitive, exact spelling first: see UserDB.getUser. const user = await db('users') - .where({ email, active: true }) + .whereRaw('lower(email) = lower(?)', [email]) + .where('active', true) .select('id_user', 'email', 'external_id', 'active') + .orderByRaw('email = ? DESC, id_user', [email]) .first(); return user || null; } catch (err) { diff --git a/src/services/db/UserDB.ts b/src/services/db/UserDB.ts index c9a18a6..797413f 100644 --- a/src/services/db/UserDB.ts +++ b/src/services/db/UserDB.ts @@ -29,10 +29,13 @@ export class UserDB { db.raw('?', [session || '']), ); }) - .where({ - email, - 'usr.active': true, - }) + .whereRaw('lower(usr.email) = lower(?)', [email]) + .where('usr.active', true) + // Apple keeps the address's casing (`Jane@example.com`) while a typed + // one is often lowercase; an exact match split one person into two + // accounts. Some such pairs exist, so the exact spelling wins and the + // older account breaks a remaining tie. + .orderByRaw('usr.email = ? DESC, usr.id_user', [email]) .first(); return user; } catch (err) { From efabbbdfa8a9fc6e58c3ec9956491b1013c4458d Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Tue, 13 Oct 2026 23:08:28 -0500 Subject: [PATCH 2/3] fix(auth): allow one active account per email, whatever its case The six pairs of active accounts whose emails differ only in case are merged before this deploys, so the lower(email) index can be unique and the lookups no longer need an order to pick one of a pair. The migration fails, naming the email, while any pair is left; it can run again once that pair is merged. --- .../database/usersLowerEmailUnique.test.ts | 27 +++++++++++++++++++ src/__tests__/services/PasskeyService.test.ts | 13 --------- src/__tests__/services/UserServices.test.ts | 16 ----------- ...120000_users_lower_email_active_unique.ts} | 14 +++++----- src/services/db/PasskeyDB.ts | 3 +-- src/services/db/UserDB.ts | 8 +++--- 6 files changed, 39 insertions(+), 42 deletions(-) create mode 100644 src/__tests__/database/usersLowerEmailUnique.test.ts rename src/database/migrations/{20261013120000_users_lower_email_active.ts => 20261013120000_users_lower_email_active_unique.ts} (57%) diff --git a/src/__tests__/database/usersLowerEmailUnique.test.ts b/src/__tests__/database/usersLowerEmailUnique.test.ts new file mode 100644 index 0000000..c3e7a3a --- /dev/null +++ b/src/__tests__/database/usersLowerEmailUnique.test.ts @@ -0,0 +1,27 @@ +import { describe, it, expect } from '@jest/globals'; +import { getTestTransaction, createTestUser } from '../setup'; + +// Migration 20261013120000: account lookups ignore the email's case, so only +// one active account may hold an email, whatever its case. +describe('users_lower_email_active_unique', () => { + it('rejects a second active account whose email differs only in case', async () => { + const trx = getTestTransaction(); + await createTestUser(trx, { email: 'Jane.Doe@Example.com' }); + + // A savepoint, so the failed insert doesn't abort the test's transaction + await expect( + trx.transaction((savepoint) => + createTestUser(savepoint, { email: 'jane.doe@example.com' }), + ), + ).rejects.toThrow(/users_lower_email_active_unique/); + }); + + it('allows it once the other account is inactive', async () => { + const trx = getTestTransaction(); + await createTestUser(trx, { email: 'Gone.Doe@Example.com', active: false }); + + const user = await createTestUser(trx, { email: 'gone.doe@example.com' }); + + expect(user.email).toBe('gone.doe@example.com'); + }); +}); diff --git a/src/__tests__/services/PasskeyService.test.ts b/src/__tests__/services/PasskeyService.test.ts index 297003c..08d080c 100644 --- a/src/__tests__/services/PasskeyService.test.ts +++ b/src/__tests__/services/PasskeyService.test.ts @@ -173,19 +173,6 @@ describe('PasskeyService', () => { expect(result!.email).toBe('Jane.Doe@Example.com'); }); - it('should prefer the exact spelling when two users differ only in case', async () => { - const trx = getTestTransaction(); - const mixedCase = await createTestUser(trx, { email: 'Split@Example.com' }); - const lowerCase = await createTestUser(trx, { email: 'split@example.com' }); - - expect((await service.getUserByEmail('Split@Example.com'))!.id_user).toBe( - mixedCase.id_user, - ); - expect((await service.getUserByEmail('split@example.com'))!.id_user).toBe( - lowerCase.id_user, - ); - }); - it('should return null for non-existent email', async () => { const result = await service.getUserByEmail('nonexistent@example.com'); expect(result).toBeNull(); diff --git a/src/__tests__/services/UserServices.test.ts b/src/__tests__/services/UserServices.test.ts index 3f9932f..4c4dc03 100644 --- a/src/__tests__/services/UserServices.test.ts +++ b/src/__tests__/services/UserServices.test.ts @@ -33,22 +33,6 @@ describe('UserServices - Duplicate Prevention', () => { expect(result.email).toBe('Jane.Doe@Example.com'); }); - it('should prefer the exact spelling when two accounts differ only in case', async () => { - const trx = getTestTransaction(); - - const mixedCase = await createTestUser(trx, { email: 'Split@Example.com' }); - const lowerCase = await createTestUser(trx, { email: 'split@example.com' }); - - const byMixed = await service.getUser({ email: 'Split@Example.com' }); - const byLower = await service.getUser({ email: 'split@example.com' }); - const byOther = await service.getUser({ email: 'SPLIT@EXAMPLE.COM' }); - - expect(byMixed.id_user).toBe(mixedCase.id_user); - expect(byLower.id_user).toBe(lowerCase.id_user); - // No exact match: the older account - expect(byOther.id_user).toBe(mixedCase.id_user); - }); - it('should not return an inactive account', async () => { const trx = getTestTransaction(); diff --git a/src/database/migrations/20261013120000_users_lower_email_active.ts b/src/database/migrations/20261013120000_users_lower_email_active_unique.ts similarity index 57% rename from src/database/migrations/20261013120000_users_lower_email_active.ts rename to src/database/migrations/20261013120000_users_lower_email_active_unique.ts index 7f75627..ab26d21 100644 --- a/src/database/migrations/20261013120000_users_lower_email_active.ts +++ b/src/database/migrations/20261013120000_users_lower_email_active_unique.ts @@ -4,20 +4,22 @@ import type { Knex } from 'knex'; // `users` writable (sign-ups) while the index builds. export const config = { transaction: false }; -// Account lookups by email ignore case (`lower(email) = lower(?)`), which -// `users_email_active_unique` can't serve. Not unique: active accounts whose -// emails differ only in case already exist and are merged by hand. +// One active account per email, whatever its case. Account lookups match +// `lower(email)`, which `users_email_active_unique` can't serve, and an exact +// match had split people into two accounts (`Jane@` from Apple, `jane@` typed +// at a passkey sign-up). Those pairs were merged before this ran; if one is +// left, the build fails, and the migration can run again once it's merged. export async function up(knex: Knex): Promise { // A concurrent build that fails leaves an INVALID index the planner never // uses; IF NOT EXISTS would then skip it for good. Drop any leftover first. - await knex.raw('DROP INDEX CONCURRENTLY IF EXISTS users_lower_email_active'); + await knex.raw('DROP INDEX CONCURRENTLY IF EXISTS users_lower_email_active_unique'); await knex.raw(` - CREATE INDEX CONCURRENTLY users_lower_email_active + CREATE UNIQUE INDEX CONCURRENTLY users_lower_email_active_unique ON users (lower(email)) WHERE active = true `); } export async function down(knex: Knex): Promise { - await knex.raw('DROP INDEX CONCURRENTLY IF EXISTS users_lower_email_active'); + await knex.raw('DROP INDEX CONCURRENTLY IF EXISTS users_lower_email_active_unique'); } diff --git a/src/services/db/PasskeyDB.ts b/src/services/db/PasskeyDB.ts index 61f5ec2..c8bf28f 100644 --- a/src/services/db/PasskeyDB.ts +++ b/src/services/db/PasskeyDB.ts @@ -305,12 +305,11 @@ export class PasskeyDB { ): Promise { try { const db = trx || this.db; - // Case-insensitive, exact spelling first: see UserDB.getUser. + // Case-insensitive: see UserDB.getUser. const user = await db('users') .whereRaw('lower(email) = lower(?)', [email]) .where('active', true) .select('id_user', 'email', 'external_id', 'active') - .orderByRaw('email = ? DESC, id_user', [email]) .first(); return user || null; } catch (err) { diff --git a/src/services/db/UserDB.ts b/src/services/db/UserDB.ts index 797413f..df970f7 100644 --- a/src/services/db/UserDB.ts +++ b/src/services/db/UserDB.ts @@ -29,13 +29,11 @@ export class UserDB { db.raw('?', [session || '']), ); }) - .whereRaw('lower(usr.email) = lower(?)', [email]) - .where('usr.active', true) // Apple keeps the address's casing (`Jane@example.com`) while a typed // one is often lowercase; an exact match split one person into two - // accounts. Some such pairs exist, so the exact spelling wins and the - // older account breaks a remaining tie. - .orderByRaw('usr.email = ? DESC, usr.id_user', [email]) + // accounts. users_lower_email_active_unique keeps it to one. + .whereRaw('lower(usr.email) = lower(?)', [email]) + .where('usr.active', true) .first(); return user; } catch (err) { From 1a0c97d03c3ffc46f0cc70beb76b72dba067d484 Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Tue, 13 Oct 2026 23:11:55 -0500 Subject: [PATCH 3/3] fix: address review feedback (round 2) Later sign-ins with a credential linked across a case difference took the existing-credential branch and put the provider's spelling back in the token, pointing older accounts at an empty S3 prefix. The token now carries the account's email whenever the two match ignoring case, on either branch; a provider email that changed altogether is still kept. --- .../controllers/UserController.test.ts | 23 ++++++++++++++++++- src/controllers/UserController.ts | 10 +++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/__tests__/controllers/UserController.test.ts b/src/__tests__/controllers/UserController.test.ts index bc742fd..5caee0e 100644 --- a/src/__tests__/controllers/UserController.test.ts +++ b/src/__tests__/controllers/UserController.test.ts @@ -142,7 +142,28 @@ describe('UserController.initLogin', () => { ); }); - it("keeps the provider's email in the token for a credential the account already has", async () => { + it("keeps the account's email in the token on later sign-ins with the linked credential", async () => { + userService.verifyToken.mockResolvedValue({ email: 'Jane.Doe@Example.com', sub: 'apple-sub' }); + userService.getAuthMethodByExternalId.mockResolvedValue({ + user_id: 7, + id_auth_method: 1, + email: 'jane.doe@example.com', + }); + userService.getUser.mockResolvedValue({ + id_user: 7, + email: 'jane.doe@example.com', + external_id: 'uuid-7', + session: 'apple-sub', + }); + + await appleLogin(); + + expect(userService.tokenUser).toHaveBeenCalledWith( + expect.objectContaining({ id_user: 7, email: 'jane.doe@example.com' }), + ); + }); + + it("keeps the provider's email in the token when it changed altogether", async () => { userService.verifyToken.mockResolvedValue({ email: 'new@example.com', sub: 'apple-sub' }); userService.getAuthMethodByExternalId.mockResolvedValue({ user_id: 7, diff --git a/src/controllers/UserController.ts b/src/controllers/UserController.ts index 299c6ea..02647be 100644 --- a/src/controllers/UserController.ts +++ b/src/controllers/UserController.ts @@ -144,9 +144,13 @@ export class UserController { const token = await this._userService.tokenUser({ id_user: user.id_user, // This claim is the S3 prefix of an account that predates external_id - // storage. A credential just linked by email may spell the address in a - // different case than the account does, so it carries the account's. - email: existingAuthMethod ? authData.email : user.email, + // storage. Accounts are found by email whatever its case, so the + // provider may spell it differently than the account: carry the + // account's. A provider email that changed altogether is kept, as before. + email: + authData.email.toLowerCase() === user.email.toLowerCase() + ? user.email + : authData.email, external_id: user.external_id, session: authData.external_id, });