From 1c7595aa5de6fde9865cca3d1e8e9d14c10dde84 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 11 Jun 2026 12:29:37 +0200 Subject: [PATCH 1/4] fix(users): lowercase legacy mixed-case emails in the CI-index migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Edited in place (no timestamp bump): the migration has only ever run on ephemeral CI/per-pid test DBs β€” devkit has no prod, and Trawl prod first runs it at P9 (#3815). Post-chain binary lookups (signin/forgot/OAuth-link/E9) lowercase their input; un-normalized legacy rows would be unreachable accounts. --- MIGRATIONS.md | 2 +- ...60610120000-users-email-ci-unique-index.js | 17 +++++++ .../users.email.ci.index.integration.tests.js | 45 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/MIGRATIONS.md b/MIGRATIONS.md index 577c5b4f2..f3106a42c 100644 --- a/MIGRATIONS.md +++ b/MIGRATIONS.md @@ -58,7 +58,7 @@ Phase 3 of the invitations↔org decouple epic (#3811). Two downstream-relevant ### Action required for downstream projects (`/update-project`) 1. The model/repository/migration changes are devkit-owned β†’ arrive via `/update-stack` (`--theirs`). -2. **πŸ”΄ Before the first boot that carries the new schema, pre-check prod for case-variant duplicate emails** (`db.users.aggregate([{$group:{_id:{$toLower:'$email'},n:{$sum:1}}},{$match:{n:{$gt:1}}}])`). If any exist, resolve them BEFORE deploy β€” otherwise the migration aborts boot. (Trawl: handled in epic Phase 9 / #3815.) +2. **πŸ”΄ Before the first boot that carries the new schema, pre-check prod for case-variant duplicate emails** (`db.users.aggregate([{$group:{_id:{$toLower:'$email'},n:{$sum:1}}},{$match:{n:{$gt:1}}}])`). If any exist, resolve them BEFORE deploy β€” otherwise the migration aborts boot. Mixed-case **singles** need no action: the migration now lowercases them in place (post-dup-check, pre-index) so binary lookups keep finding those accounts. (Trawl: handled in epic Phase 9 / #3815.) 3. Migrations run at boot before `listen()`; the index swap + the `consumingAt` field land automatically once the dupe pre-check passes. 4. No client/contract change; existing 200/422 signup assertions pass unchanged. diff --git a/modules/users/migrations/20260610120000-users-email-ci-unique-index.js b/modules/users/migrations/20260610120000-users-email-ci-unique-index.js index f182ab59e..cbef21158 100644 --- a/modules/users/migrations/20260610120000-users-email-ci-unique-index.js +++ b/modules/users/migrations/20260610120000-users-email-ci-unique-index.js @@ -24,6 +24,7 @@ const CI_INDEX = 'email_ci_unique'; * ABORT (throw) WITHOUT touching indexes β€” auto-merging accounts is a * destructive product decision, not a migration's call. The thrown error * lists the colliding groups so an operator can remediate, then re-run. + * (a2) Normalize legacy mixed-case emails (post-dup-check, so it can never collide). * (b) Create the collation unique index FIRST, THEN drop the plain `email_1`. * There is never a window without a unique constraint on email. * (c) Idempotent: re-running after success is a no-op (the CI index already @@ -66,6 +67,22 @@ export async function up() { ); } + // ── (a2) Normalize legacy mixed-case emails to lowercase ── + // Post-chain every lookup lowercases its input (schema `lowercase:true` + + // repository normalizeEmail) and a plain findOne without per-query collation + // is a BINARY match β€” a legacy mixed-case row would be unreachable (signin, + // password-forgot, OAuth linkProviderByEmail, the invitations E9 guard all + // miss it). The dup pre-check above guarantees lowercasing cannot collide. + // Pipeline-update form so $toLower runs server-side; idempotent (the filter + // only matches rows still containing an uppercase letter). + const normalized = await collection.updateMany( + { email: { $type: 'string', $regex: /[A-Z]/ } }, + [{ $set: { email: { $toLower: '$email' } } }], + ); + if (normalized.modifiedCount > 0) { + console.info(`[migration] users-email-ci-unique-index: lowercased ${normalized.modifiedCount} legacy mixed-case email(s)`); + } + // Snapshot existing indexes once (listIndexes throws if the collection does not // exist yet β€” tolerate that: a fresh DB has no users collection and autoIndex / // syncIndexes will create the CI index from the schema declaration). diff --git a/modules/users/tests/users.email.ci.index.integration.tests.js b/modules/users/tests/users.email.ci.index.integration.tests.js index b26800e16..5be0e6605 100644 --- a/modules/users/tests/users.email.ci.index.integration.tests.js +++ b/modules/users/tests/users.email.ci.index.integration.tests.js @@ -7,6 +7,8 @@ import mongoose from 'mongoose'; import { beforeAll, afterAll, afterEach, describe, test, expect } from '@jest/globals'; import { bootstrap } from '../../../lib/app.js'; +import { up } from '../migrations/20260610120000-users-email-ci-unique-index.js'; + /** * E3 β€” case-insensitive unique email index integration tests. * @@ -89,4 +91,47 @@ describe('E3 case-insensitive unique email index:', () => { expect(found).not.toBeNull(); expect(found.email).toBe('ci-variant@example.com'); }); + + test('lowercases legacy mixed-case emails so post-chain binary lookups can find them', async () => { + const col = mongoose.connection.db.collection('users'); + try { + // Bypass the schema (lowercase:true) β€” simulate a PRE-chain legacy row. + await col.insertOne({ + email: 'Legacy.MixedCase@Example.COM', + firstName: 'Legacy', + lastName: 'Row', + provider: 'local', + roles: ['user'], + }); + await up(); + // Raw read (binary): the stored value itself must now be lowercase. + const raw = await col.findOne({ email: 'legacy.mixedcase@example.com' }); + expect(raw).not.toBeNull(); + const mixed = await col.findOne({ email: 'Legacy.MixedCase@Example.COM' }); + expect(mixed).toBeNull(); + } finally { + // Binary deletes β€” target BOTH forms so a failing (pre-fix) run cannot leak the row. + await col.deleteMany({ email: { $in: ['legacy.mixedcase@example.com', 'Legacy.MixedCase@Example.COM'] } }); + } + }); + + test('still ABORTS on case-variant duplicates BEFORE normalizing anything', async () => { + const col = mongoose.connection.db.collection('users'); + try { + // The bootstrapped DB already carries the CI unique index, which would reject the + // 2nd case-variant insert below β€” drop it to simulate the PRE-migration state the + // dup pre-check exists for (the finally re-runs up() to restore the end state). + try { await col.dropIndex('email_ci_unique'); } catch (_) { /* already absent */ } + await col.insertOne({ email: 'Dupe@x.com', firstName: 'A', lastName: 'A', provider: 'local', roles: ['user'] }); + await col.insertOne({ email: 'dupe@x.com', firstName: 'B', lastName: 'B', provider: 'local', roles: ['user'] }); + await expect(up()).rejects.toThrow(/case-variant duplicate/); + // Neither row was modified (abort happens before the normalization pass). + expect(await col.findOne({ email: 'Dupe@x.com' })).not.toBeNull(); + expect(await col.findOne({ email: 'dupe@x.com' })).not.toBeNull(); + } finally { + await col.deleteMany({ email: { $in: ['Dupe@x.com', 'dupe@x.com'] } }); + // Restore the migrated end state (idempotent β€” the dup rows are gone). + await up(); + } + }); }); From 3155df9546e6ccd57c0c8e48c5c8bf6e58b00455 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 11 Jun 2026 12:32:12 +0200 Subject: [PATCH 2/4] =?UTF-8?q?fix(auth):=20invite=20finalize/release=20ar?= =?UTF-8?q?e=20best-effort=20=E2=80=94=20never=20422=20a=20created=20accou?= =?UTF-8?q?nt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- modules/auth/controllers/auth.controller.js | 38 +++++++++++++++++-- .../tests/invitations.integration.tests.js | 22 ++++++++++- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/modules/auth/controllers/auth.controller.js b/modules/auth/controllers/auth.controller.js index 3d61f999f..3936b6da9 100644 --- a/modules/auth/controllers/auth.controller.js +++ b/modules/auth/controllers/auth.controller.js @@ -101,7 +101,9 @@ const signup = async (req, res) => { // leave it stuck mid-claim (the lazy sweep would also recover it; this is // immediate). Only the closed-signup path claims, so gate the release on it to // avoid a no-op release (+ misleading log) on an open-signup presented token. - if (invite && !config.sign.up) await eligibility?.release?.(); + if (invite && !config.sign.up) { + try { await eligibility?.release?.(); } catch (_releaseErr) { /* best-effort β€” the sweep recovers */ } + } return responses.error(res, 404, 'Signup error', 'Registration is currently deactivated')(); } // Force default role on public signup β€” clients must not self-assign admin @@ -198,8 +200,22 @@ const signup = async (req, res) => { // checker (invitations module owns it; auth never imports invitation code). This // is the last pre-response step, and every earlier failure path (create-throw, // verify-failure, org-failure) already released the claim, so reaching finalize - // means the claim is still ours to burn. - if (invite && !config.sign.up) await eligibility?.finalize?.(user._id || user.id); + // means the claim is still ours to burn. finalize itself is best-effort (see + // catch below). + if (invite && !config.sign.up) { + try { + await eligibility?.finalize?.(user._id || user.id); + } catch (finalizeErr) { + // Best-effort: the account exists and the response is about to succeed β€” + // a finalize DB hiccup must not convert a created account into a 422. + // The claim stays stamped and the 15-min lazy sweep releases it; the + // invite is reconcilable from its pending state + the account's email. + logger.warn('[signup] invite finalize failed post-create (left to the sweep)', { + userId: String(user._id || user.id), + err: finalizeErr?.message, + }); + } + } const token = jwt.sign({ userId: user.id }, config.jwt.secret, { expiresIn: config.jwt.expiresIn, @@ -540,7 +556,21 @@ const checkOAuthUserProfile = async (profil, key, provider) => { // release on failure here β€” finalize marks it accepted+used (single-use). The // finalize filter (acceptedAt:null) + the unique-email index are the single-use // backstop against two concurrent OAuth signups for the same invited email. - if (oauthInvite) await oauthEligibility?.finalize?.(createdUser._id || createdUser.id); + // finalize itself is best-effort (see catch below). + if (oauthInvite) { + try { + await oauthEligibility?.finalize?.(createdUser._id || createdUser.id); + } catch (finalizeErr) { + // Best-effort: the account exists and the response is about to succeed β€” + // a finalize DB hiccup must not convert a created account into a 422. + // The claim stays stamped and the 15-min lazy sweep releases it; the + // invite is reconcilable from its pending state + the account's email. + logger.warn('[oauth] invite finalize failed post-create (left to the sweep)', { + userId: String(createdUser._id || createdUser.id), + err: finalizeErr?.message, + }); + } + } return createdUser; } catch (err) { if (err instanceof AppError) throw err; diff --git a/modules/invitations/tests/invitations.integration.tests.js b/modules/invitations/tests/invitations.integration.tests.js index f60e29203..44a3008cd 100644 --- a/modules/invitations/tests/invitations.integration.tests.js +++ b/modules/invitations/tests/invitations.integration.tests.js @@ -410,7 +410,7 @@ describe('Signup invitations:', () => { afterEach(async () => { config.sign.up = originalUp; config.sign.cap = originalCap; jest.restoreAllMocks(); - for (const email of ['e2-replay@example.com', 'e2-claimed@example.com', 'e2-stale@example.com', 'e2-createthrow@example.com', 'e2-concurrent@example.com']) { + for (const email of ['e2-replay@example.com', 'e2-claimed@example.com', 'e2-stale@example.com', 'e2-createthrow@example.com', 'e2-concurrent@example.com', 'e2-finalize-throw@example.com']) { try { const existing = await UserService.getBrut({ email }); if (existing) await UserService.remove(existing); @@ -570,6 +570,26 @@ describe('Signup invitations:', () => { expect(swept.consumingAt == null).toBe(true); // claim released expect(await InvitationService.findValid(token, email)).not.toBeNull(); // reusable again }); + + test('a finalize throw does NOT convert a successful signup into an error (best-effort)', async () => { + const adminAgent = await createAdminAndSignin(); + const email = 'e2-finalize-throw@example.com'; + const created = await adminAgent.post('/api/invitations').send({ email }); + const { token } = created.body.data; + config.sign.up = false; config.sign.cap = null; + + // finalize is the last pre-response step of an ALREADY-successful signup β€” make + // it blow up once (the eligibility closure routes finalize through accept, P8a). + const acceptSpy = jest.spyOn(InvitationService, 'accept').mockRejectedValueOnce(new Error('simulated finalize DB hiccup')); + + const res = await request(app) + .post(`/api/auth/signup?inviteToken=${token}`) + .send({ email, password: 'Sup3rStr0ng!' }); + // The account was created before finalize β€” the response must still succeed. + expect(acceptSpy).toHaveBeenCalledTimes(1); + expect(res.status).toBe(200); + expect(res.body.user.email).toBe(email); + }); }); describe('E6 no auto-verify on invite (mailer off)', () => { From 240c4fe8488302718b6bbcbcab56bb846cbeca39 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 11 Jun 2026 12:33:53 +0200 Subject: [PATCH 3/4] docs(auth): fix stale case-sensitive index comments + re-gate alias-removal TODO on P9 --- modules/auth/controllers/auth.controller.js | 6 +++--- modules/auth/routes/auth.routes.js | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/modules/auth/controllers/auth.controller.js b/modules/auth/controllers/auth.controller.js index 3936b6da9..72c32154e 100644 --- a/modules/auth/controllers/auth.controller.js +++ b/modules/auth/controllers/auth.controller.js @@ -109,14 +109,14 @@ const signup = async (req, res) => { // Force default role on public signup β€” clients must not self-assign admin const safeBody = { ...req.body, roles: ['user'] }; // Invite-gated signup: canonicalize the account email to the invite's pinned - // (lowercased) email. Enforces the pin exactly AND makes the case-sensitive - // unique-email index a reliable single-use backstop β€” concurrent case-variant + // (lowercased) email. Enforces the pin exactly AND makes the case-insensitive + // unique-email index (email_ci_unique, collation strength-2) a reliable single-use backstop β€” concurrent case-variant // signups on the same invite collide on the index instead of creating two accounts. if (invite) safeBody.email = invite.email; // E2: the invite was atomically CLAIMED (consumingAt stamped) before we got here, // so a throw FROM create itself must release the claim too β€” otherwise the invite // stays locked until the 15-min sweep. The most realistic throw is an E11000 from - // the case-sensitive unique-email index when two case-variant signups race the same + // the case-insensitive unique-email index (email_ci_unique) when two case-variant signups race the same // invited email (validation/transient errors land here as well). Mirror the three // release sites below + the same `!config.sign.up` gating (only the closed-signup // path claimed). Best-effort: a release failure must not mask the create error. diff --git a/modules/auth/routes/auth.routes.js b/modules/auth/routes/auth.routes.js index 017e593a4..80d69ad62 100644 --- a/modules/auth/routes/auth.routes.js +++ b/modules/auth/routes/auth.routes.js @@ -9,8 +9,10 @@ import policy from '../../../lib/middlewares/policy.js'; import UsersSchema from '../../users/models/users.schema.js'; import auth from '../controllers/auth.controller.js'; import authPassword from '../controllers/auth.password.controller.js'; -// TODO(P6, pierreb-devkit/Node#4279): Remove these two imports and the three routes -// below once Vue migrates to the canonical /api/invitations mount. +// TODO(P9, pierreb-devkit/Node#3815): Remove these two imports and the three routes +// below ONLY after (a) downstream (Trawl) has absorbed Vue P6 via /update-stack AND +// (b) Vue auth.store `verifyInvite` is repointed to the canonical +// /api/invitations/verify/:token (Vue pre-P9 fix PR) β€” it still calls the alias. // Deprecation alias only β€” the invitations feature lives in modules/invitations. // This block MUST stay here (before the greedy `/api/auth/:strategy` wildcard) so // the legacy `/api/auth/invitations*` paths the Vue admin store still calls keep From b4cacd04cd8e0bf3575681d0ffeac8fef29008b8 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 11 Jun 2026 14:43:37 +0200 Subject: [PATCH 4/4] fix(auth): log release failure + capture finalize stack; cover catch arms Addresses review threads: the capacity-gate release swallow is now logged, both finalize warns include the stack, and the alias-coupling comment is gated on P9. Adds OAuth-finalize-throw + cap-release-throw integration tests covering the 3 codecov-missed catch lines. --- modules/auth/controllers/auth.controller.js | 13 ++++- modules/auth/routes/auth.routes.js | 2 +- .../tests/invitations.integration.tests.js | 50 ++++++++++++++++++- 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/modules/auth/controllers/auth.controller.js b/modules/auth/controllers/auth.controller.js index 72c32154e..893429d68 100644 --- a/modules/auth/controllers/auth.controller.js +++ b/modules/auth/controllers/auth.controller.js @@ -102,7 +102,16 @@ const signup = async (req, res) => { // immediate). Only the closed-signup path claims, so gate the release on it to // avoid a no-op release (+ misleading log) on an open-signup presented token. if (invite && !config.sign.up) { - try { await eligibility?.release?.(); } catch (_releaseErr) { /* best-effort β€” the sweep recovers */ } + try { + await eligibility?.release?.(); + } catch (releaseErr) { + // Best-effort β€” the sweep recovers; log it so a burst of "signup blocked" + // reports is diagnosable (a silently stuck claim looks like a dead invite). + logger.warn('[signup] invite release failed on capacity gate (left to the sweep)', { + err: releaseErr?.message, + stack: releaseErr?.stack, + }); + } } return responses.error(res, 404, 'Signup error', 'Registration is currently deactivated')(); } @@ -213,6 +222,7 @@ const signup = async (req, res) => { logger.warn('[signup] invite finalize failed post-create (left to the sweep)', { userId: String(user._id || user.id), err: finalizeErr?.message, + stack: finalizeErr?.stack, }); } } @@ -568,6 +578,7 @@ const checkOAuthUserProfile = async (profil, key, provider) => { logger.warn('[oauth] invite finalize failed post-create (left to the sweep)', { userId: String(createdUser._id || createdUser.id), err: finalizeErr?.message, + stack: finalizeErr?.stack, }); } } diff --git a/modules/auth/routes/auth.routes.js b/modules/auth/routes/auth.routes.js index 80d69ad62..a5ba69ffc 100644 --- a/modules/auth/routes/auth.routes.js +++ b/modules/auth/routes/auth.routes.js @@ -18,7 +18,7 @@ import authPassword from '../controllers/auth.password.controller.js'; // the legacy `/api/auth/invitations*` paths the Vue admin store still calls keep // working; it points at the MOVED controller. Canonical mount: /api/invitations // (modules/invitations/routes/invitations.routes.js). -// INTENTIONAL coreβ†’optional import: temporary coupling until P6 removes these routes. +// INTENTIONAL coreβ†’optional import: temporary coupling until P9 removes these routes. import invitations from '../../invitations/controllers/invitations.controller.js'; import InvitationSchema from '../../invitations/models/invitations.schema.js'; diff --git a/modules/invitations/tests/invitations.integration.tests.js b/modules/invitations/tests/invitations.integration.tests.js index 44a3008cd..74660366f 100644 --- a/modules/invitations/tests/invitations.integration.tests.js +++ b/modules/invitations/tests/invitations.integration.tests.js @@ -410,7 +410,7 @@ describe('Signup invitations:', () => { afterEach(async () => { config.sign.up = originalUp; config.sign.cap = originalCap; jest.restoreAllMocks(); - for (const email of ['e2-replay@example.com', 'e2-claimed@example.com', 'e2-stale@example.com', 'e2-createthrow@example.com', 'e2-concurrent@example.com', 'e2-finalize-throw@example.com']) { + for (const email of ['e2-replay@example.com', 'e2-claimed@example.com', 'e2-stale@example.com', 'e2-createthrow@example.com', 'e2-concurrent@example.com', 'e2-finalize-throw@example.com', 'e2-oauth-finalize-throw@example.com', 'e2-release-throw@example.com']) { try { const existing = await UserService.getBrut({ email }); if (existing) await UserService.remove(existing); @@ -590,6 +590,54 @@ describe('Signup invitations:', () => { expect(res.status).toBe(200); expect(res.body.user.email).toBe(email); }); + + test('OAuth: a finalize throw does NOT convert a successful OAuth signup into an error (best-effort)', async () => { + const adminAgent = await createAdminAndSignin(); + const email = 'e2-oauth-finalize-throw@example.com'; + const created = await adminAgent.post('/api/invitations').send({ email }); + expect(created.status).toBe(200); + config.sign.up = false; config.sign.cap = null; + + // Mirror of the local-signup test above on the OAuth path: the user is created, + // then finalize (the same accept closure β€” no claim to release on OAuth) blows up. + const AuthController = (await import(path.resolve('./modules/auth/controllers/auth.controller.js'))).default; + const acceptSpy = jest.spyOn(InvitationService, 'accept').mockRejectedValueOnce(new Error('simulated finalize DB hiccup')); + + const createdUser = await AuthController.checkOAuthUserProfile({ + firstName: 'Invited', + lastName: 'OAuth', + email, + avatar: '', + providerData: { sub: 'google-e2-oauth-finalize-throw-sub' }, + emailVerifiedByProvider: true, + }, 'sub', 'google'); + + // The account was created before finalize β€” the call must still resolve with it. + expect(acceptSpy).toHaveBeenCalledTimes(1); + expect(createdUser).toBeDefined(); + expect(createdUser.email).toBe(email); + }); + + test('cap-reached release: a release throw is swallowed β€” the gate still answers 404, not a 5xx', async () => { + const adminAgent = await createAdminAndSignin(); + const email = 'e2-release-throw@example.com'; + const created = await adminAgent.post('/api/invitations').send({ email }); + const { token } = created.body.data; + // Closed signup + a positive cap already filled: the checker CLAIMS the invite, + // then the capacity gate rejects and must release the claim β€” make that release + // blow up. Best-effort: the 404 must come back unchanged (no 5xx), the stuck + // claim is the sweep's job. + config.sign.up = false; + config.sign.cap = await UserService.count(); // > 0 (the admin exists) and reached + + const releaseSpy = jest.spyOn(InvitationService, 'release').mockRejectedValueOnce(new Error('simulated release DB hiccup')); + + const res = await request(app) + .post(`/api/auth/signup?inviteToken=${token}`) + .send({ email, password: 'Sup3rStr0ng!' }); + expect(releaseSpy).toHaveBeenCalledTimes(1); + expect(res.status).toBe(404); // swallowed β€” the capacity answer, not a masked 5xx + }); }); describe('E6 no auto-verify on invite (mailer off)', () => {