From b4612f9c9e2f41febdcbcad5eaca6b82df43b98a Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 13 Jun 2026 08:43:24 +0200 Subject: [PATCH 1/4] fix(invitations): scope list by invitedBy for non-admin callers GET /api/invitations stays admin-only by CASL, but the controller now threads req.user and the service keys the repository filter on the caller's role: admins read the platform-global list, anyone else only the invitations they sent (the invitedBy index covers it). Prep so the referral phase can widen the Invitation abilities without leaking invitee emails platform-wide. refs #3833 --- .../controllers/invitations.controller.js | 6 ++++-- .../repositories/invitations.repository.js | 8 +++++-- .../services/invitations.service.js | 16 ++++++++++++-- .../invitations.controller.unit.tests.js | 7 ++++--- .../invitations.repository.unit.tests.js | 9 ++++++++ .../tests/invitations.service.unit.tests.js | 21 ++++++++++++++++--- 6 files changed, 55 insertions(+), 12 deletions(-) diff --git a/modules/invitations/controllers/invitations.controller.js b/modules/invitations/controllers/invitations.controller.js index e3400abaa..f951d0740 100644 --- a/modules/invitations/controllers/invitations.controller.js +++ b/modules/invitations/controllers/invitations.controller.js @@ -23,14 +23,16 @@ const create = async (req, res) => { }; /** - * @desc Admin: list all signup invitations + * @desc List signup invitations — role-keyed by the service (#3833): + * admins get the platform-global list, any other caller only their own. * @param {Object} req - Express request object + * @param {Object} req.user - Authenticated caller (scoping key) * @param {Object} res - Express response object * @returns {Promise} Sends HTTP 200 with invitation array or 422 on error */ const list = async (req, res) => { try { - const invitations = await InvitationService.list(); + const invitations = await InvitationService.list(req.user); responses.success(res, 'invitation list')(invitations); } catch (err) { responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err); diff --git a/modules/invitations/repositories/invitations.repository.js b/modules/invitations/repositories/invitations.repository.js index d66446cbf..89f68796c 100644 --- a/modules/invitations/repositories/invitations.repository.js +++ b/modules/invitations/repositories/invitations.repository.js @@ -97,10 +97,14 @@ const releaseStaleClaims = (cutoff) => ).exec(); /** - * @desc List all invitations (admin), newest first + * @desc List invitations, newest first, token always stripped. Optional filter + * (#3833): the service passes { invitedBy } for non-admin callers so a user only + * ever reads the invitations THEY sent (invitee emails are PII); admins list + * unfiltered. The { invitedBy: 1 } index (#3842) covers the scoped query. + * @param {Object} [filter] - mongo filter (default {} = all) * @returns {Promise} */ -const list = () => Invitation.find({}).select('-token').populate('invitedBy', 'email firstName lastName').sort('-createdAt').exec(); +const list = (filter = {}) => Invitation.find(filter).select('-token').populate('invitedBy', 'email firstName lastName').sort('-createdAt').exec(); /** * @desc List ALL accepted invitations, lean + minimal projection — the referral diff --git a/modules/invitations/services/invitations.service.js b/modules/invitations/services/invitations.service.js index 0b1fda2ed..458532cc1 100644 --- a/modules/invitations/services/invitations.service.js +++ b/modules/invitations/services/invitations.service.js @@ -291,10 +291,22 @@ const release = async (id) => { }; /** - * @desc List all invitations (admin) + * @desc List invitations, role-keyed (#3833): platform admins read the global + * list; any other caller reads only the invitations THEY sent (scoped on + * invitedBy — invitee emails are PII, an unscoped list would leak them + * platform-wide). The admin check mirrors invitations.policy.js. CASL still only + * routes admins to this today; the scoping ships FIRST so the referral phase can + * widen the Invitation abilities without touching this seam again. A caller with + * no resolvable id gets [] — never the { invitedBy: null } admin-created rows. + * @param {Object} user - the authenticated caller (req.user) * @returns {Promise} */ -const list = () => InvitationRepository.list(); +const list = (user) => { + if (Array.isArray(user?.roles) && user.roles.includes('admin')) return InvitationRepository.list(); + const userId = user?.id || user?._id; + if (!userId) return Promise.resolve([]); + return InvitationRepository.list({ invitedBy: userId }); +}; /** * @desc Get one invitation by id diff --git a/modules/invitations/tests/invitations.controller.unit.tests.js b/modules/invitations/tests/invitations.controller.unit.tests.js index f7a674014..73515632b 100644 --- a/modules/invitations/tests/invitations.controller.unit.tests.js +++ b/modules/invitations/tests/invitations.controller.unit.tests.js @@ -46,10 +46,11 @@ describe('invitations.controller.create', () => { }); describe('invitations.controller.list', () => { - test('lists invitations', async () => { + test('passes the authenticated caller to the service (role-keyed scoping, #3833)', async () => { mockService.list.mockResolvedValue([{ id: 'i1' }]); - await controller.list({}, makeRes()); - expect(mockService.list).toHaveBeenCalledTimes(1); + const req = { user: { id: 'u1', roles: ['user'] } }; + await controller.list(req, makeRes()); + expect(mockService.list).toHaveBeenCalledWith({ id: 'u1', roles: ['user'] }); expect(successInner).toHaveBeenCalledWith([{ id: 'i1' }]); }); }); diff --git a/modules/invitations/tests/invitations.repository.unit.tests.js b/modules/invitations/tests/invitations.repository.unit.tests.js index 5fcac7fa9..0e8a81ea9 100644 --- a/modules/invitations/tests/invitations.repository.unit.tests.js +++ b/modules/invitations/tests/invitations.repository.unit.tests.js @@ -127,6 +127,15 @@ describe('InvitationRepository', () => { expect(chain.sort).toHaveBeenCalledWith('-createdAt'); }); + test('list({ invitedBy }) threads the filter (#3833 non-admin scoping), token still stripped', async () => { + exec.mockResolvedValue([]); + await InvitationRepository.list({ invitedBy: 'u1' }); + expect(InvitationModel.find).toHaveBeenCalledWith({ invitedBy: 'u1' }); + expect(chain.select).toHaveBeenCalledWith('-token'); + expect(chain.populate).toHaveBeenCalledWith('invitedBy', 'email firstName lastName'); + expect(chain.sort).toHaveBeenCalledWith('-createdAt'); + }); + test('findAccepted scans status:accepted lean with the minimal referral projection (#3842)', async () => { exec.mockResolvedValue([{ _id: 'i1', invitedBy: 'u1', acceptedUserId: 'u2' }]); const result = await InvitationRepository.findAccepted(); diff --git a/modules/invitations/tests/invitations.service.unit.tests.js b/modules/invitations/tests/invitations.service.unit.tests.js index 72f884b6d..c3ad97f25 100644 --- a/modules/invitations/tests/invitations.service.unit.tests.js +++ b/modules/invitations/tests/invitations.service.unit.tests.js @@ -335,10 +335,25 @@ describe('InvitationService.create — email sending branch', () => { }); describe('InvitationService.list / get / revoke', () => { - test('list delegates to repository', async () => { + test('list(admin) → unscoped repository list (#3833)', async () => { InvitationRepository.list.mockResolvedValue([]); - await InvitationService.list(); - expect(InvitationRepository.list).toHaveBeenCalledTimes(1); + await InvitationService.list({ id: 'a1', roles: ['user', 'admin'] }); + expect(InvitationRepository.list).toHaveBeenCalledWith(); + }); + test('list(non-admin) → repository list scoped to { invitedBy: } (#3833)', async () => { + InvitationRepository.list.mockResolvedValue([]); + await InvitationService.list({ id: 'u1', roles: ['user'] }); + expect(InvitationRepository.list).toHaveBeenCalledWith({ invitedBy: 'u1' }); + }); + test('list(non-admin with _id only) → scoped on _id (mirrors create() id resolution)', async () => { + InvitationRepository.list.mockResolvedValue([]); + await InvitationService.list({ _id: 'u2', roles: ['user'] }); + expect(InvitationRepository.list).toHaveBeenCalledWith({ invitedBy: 'u2' }); + }); + test('list with no resolvable caller id → empty list, repository untouched (never leaks the invitedBy:null admin rows)', async () => { + expect(await InvitationService.list(undefined)).toEqual([]); + expect(await InvitationService.list({ roles: ['user'] })).toEqual([]); + expect(InvitationRepository.list).not.toHaveBeenCalled(); }); test('get delegates to repository', async () => { InvitationRepository.get.mockResolvedValue(null); From ada3214aa25235a3528c9ee9d893887b02e9514a Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 13 Jun 2026 08:44:09 +0200 Subject: [PATCH 2/4] fix(invitations): explicit self-invite 422 before registered-email check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create() now fails fast with 'You cannot invite yourself' when the invitee email equals the inviter's own. E9 already caught the exact match as a registered email, but with a misleading message; this makes the intent explicit. Also pins the grant-side floor (#3833 belt): expectedGrantKeys implies no referrer key when invitedBy equals acceptedUserId (same-account pairs only — alias multi-account self-invites remain an accepted residual risk, out of scope here). refs #3833 --- .../billing.referral.service.unit.tests.js | 10 +++++++ .../services/invitations.service.js | 18 ++++++++++++- .../tests/invitations.service.unit.tests.js | 27 +++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/modules/billing/tests/billing.referral.service.unit.tests.js b/modules/billing/tests/billing.referral.service.unit.tests.js index 9a2ddec71..3d7b2ec89 100644 --- a/modules/billing/tests/billing.referral.service.unit.tests.js +++ b/modules/billing/tests/billing.referral.service.unit.tests.js @@ -115,6 +115,16 @@ describe('billing.referral.service unit tests:', () => { expect(BillingReferralService.expectedGrantKeys(invite, { ...cfg, enabled: false })).toEqual([]); expect(BillingReferralService.expectedGrantKeys({ invitedBy: inviterId, acceptedUserId: refereeId }, cfg)).toEqual([]); }); + + test('#3833 belt: a SAME-account self-referral pair implies NO referrer key (listener AND cron derive from this one rule; alias multi-account self-invites are NOT covered by this floor)', () => { + const cfg = mockConfig.billing.referral; + const keys = BillingReferralService.expectedGrantKeys( + { _id: invitationId, invitedBy: refereeId, acceptedUserId: refereeId }, + cfg, + ); + expect(keys).toEqual([{ side: 'referee', key: `referral:${invitationId}:referee` }]); + expect(keys.some((k) => k.side === 'referrer')).toBe(false); + }); }); test('disabled → no-op: returns skipped and never touches the ledger or users', async () => { diff --git a/modules/invitations/services/invitations.service.js b/modules/invitations/services/invitations.service.js index 458532cc1..f303b08ee 100644 --- a/modules/invitations/services/invitations.service.js +++ b/modules/invitations/services/invitations.service.js @@ -22,10 +22,26 @@ import AppError from '../../../lib/helpers/AppError.js'; * @param {String} email - invitee email (any case) * @param {Object} invitedBy - the admin user creating the invite * @returns {Promise} created invitation - * @throws {AppError} 422 when a user already exists for this email + * @throws {AppError} 422 when the invitee email is the inviter's own, or a user already exists for this email */ const create = async (email, invitedBy) => { const normalizedEmail = String(email).toLowerCase().trim(); + // #3833: explicit self-invite guard. The inviter's own email is by definition a + // registered email, so E9 below would also reject it — but with a misleading + // "already registered" message. Failing fast here makes the intent explicit. + // Alias/variant self-emails (a SEPARATE account on a second personal address) are + // NOT covered here or anywhere: the grant-side floor (billing expectedGrantKeys + // skips invitedBy === acceptedUserId) only suppresses same-account pairs. Accepted + // residual risk — sock-puppet prevention is out of #3833's scope (fraud review / + // email-normalization dedup territory, revisit before any paid-rewards launch). + const inviterEmail = typeof invitedBy?.email === 'string' ? invitedBy.email.toLowerCase().trim() : null; + if (inviterEmail && inviterEmail === normalizedEmail) { + throw new AppError('You cannot invite yourself', { + status: 422, + code: 'VALIDATION_ERROR', + details: { message: 'You cannot invite yourself.' }, + }); + } // E9: an already-registered email must not be invited — they are already a user. const existing = await UserService.findByEmail(normalizedEmail); if (existing) { diff --git a/modules/invitations/tests/invitations.service.unit.tests.js b/modules/invitations/tests/invitations.service.unit.tests.js index c3ad97f25..196ecae63 100644 --- a/modules/invitations/tests/invitations.service.unit.tests.js +++ b/modules/invitations/tests/invitations.service.unit.tests.js @@ -319,6 +319,33 @@ describe('InvitationService.create', () => { }); }); +describe('InvitationService.create — #3833 self-invite guard', () => { + test('rejects 422 "You cannot invite yourself" BEFORE the E9 lookup and any persistence', async () => { + await expect(InvitationService.create('Me@Example.com', { id: 'u1', email: 'me@example.com' })).rejects.toMatchObject({ + status: 422, + code: 'VALIDATION_ERROR', + }); + await expect(InvitationService.create('me@example.com', { id: 'u1', email: 'me@example.com' })).rejects.toThrow('You cannot invite yourself'); + // Fires before E9: the registered-email lookup is never reached, nothing persists. + expect(mockUserService.findByEmail).not.toHaveBeenCalled(); + expect(InvitationRepository.create).not.toHaveBeenCalled(); + }); + test('matches case-insensitively and trims whitespace (same normalization both sides)', async () => { + await expect(InvitationService.create(' ME@EXAMPLE.COM ', { id: 'u1', email: 'Me@Example.com' })).rejects.toMatchObject({ status: 422 }); + }); + test('a different invitee email passes the guard — E9 path unchanged', async () => { + InvitationRepository.create.mockImplementation((doc) => Promise.resolve({ ...doc, id: '1' })); + const inv = await InvitationService.create('friend@example.com', { id: 'u1', email: 'me@example.com' }); + expect(inv.email).toBe('friend@example.com'); + expect(mockUserService.findByEmail).toHaveBeenCalledWith('friend@example.com'); + }); + test('an inviter without an email (defensive) skips the guard, E9 still protects', async () => { + InvitationRepository.create.mockImplementation((doc) => Promise.resolve({ ...doc, id: '2' })); + const inv = await InvitationService.create('someone@example.com', { id: 'u1' }); + expect(inv.email).toBe('someone@example.com'); + }); +}); + describe('InvitationService.create — email sending branch', () => { test('sends email when mailer is configured', async () => { mockMailer.isConfigured.mockReturnValue(true); From 08119cc8c5e7e8ee47d482c6b4aab1b5031eefb9 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 13 Jun 2026 08:44:52 +0200 Subject: [PATCH 3/4] docs(invitations): referral requires closed signup; #3833 gate status Documents the open-signup invariant kept on purpose: with sign.up true a presented token is resolved but never claimed/finalized, so invitation.accepted never fires and referral grants are a silent no-op. Marks gates 1-2 shipped in the README and mirrors the caveat in the billing referral config block. refs #3833 --- .../config/billing.development.config.js | 8 +++-- modules/invitations/README.md | 34 ++++++++++++++----- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/modules/billing/config/billing.development.config.js b/modules/billing/config/billing.development.config.js index 7ca1f84e5..7ad20b2af 100644 --- a/modules/billing/config/billing.development.config.js +++ b/modules/billing/config/billing.development.config.js @@ -140,8 +140,12 @@ const config = { * Pair with the reconcile cron (crons/billing.referralReconcile.js): the listener * is in-process fire-and-forget (latency); the cron back-fills missed grants (truth). * - * ⚠️ Merging is safe everywhere (default OFF); do NOT enable until - * pierreb-devkit/Node#3833 lands — only the cheap self-referral floor ships here. + * ⚠️ Referral grants require CLOSED signup (sign.up: false). With public signup + * open, a presented invite token is resolved but never claimed/finalized (the + * "open signup never burns a token" invariant), so `invitation.accepted` never + * fires and enabling this block is a silent no-op. The #3833 gates shipped: + * role-keyed list scoping + the create() self-invite guard (the grant-side + * floor here skips invitedBy === acceptedUserId as the belt). */ referral: { enabled: false, diff --git a/modules/invitations/README.md b/modules/invitations/README.md index 9ded19cd7..4281fa403 100644 --- a/modules/invitations/README.md +++ b/modules/invitations/README.md @@ -135,15 +135,31 @@ and hard to cap/expire/audit ("when was this credited?"). Good for simple boosts > Recommended: **A + reconcile cron** for credit/cashback economies; **B** for static > entitlement boosts. The substrate supports both simultaneously. -## Gates before shipping rewards (#3833 — read it first) - -1. **Scope the list**: `GET /api/invitations` is platform-global today (admin-only by - CASL). Before widening `create Invitation` to regular users, add an `invitedBy`-scoped - `/mine` (PII: invitee emails). -2. **Self-referral guard** — alternate-email self-invites become valuable once credits exist. -3. **Open-signup hole** — claim/finalize are gated on `!config.sign.up`: with public signup - open the event never fires. Resolve (finalize-without-claim) or hide the Referrals tab - before any open deployment enables rewards. +## Gates before shipping rewards (#3833 — status) + +1. **Scope the list — SHIPPED (#3833)**: `GET /api/invitations` is role-keyed in the + service: admins read the platform-global list; any other caller reads only the + invitations they sent (`invitedBy`-scoped — the `{ invitedBy: 1 }` index covers + it; a caller with no resolvable id gets `[]`, never the `invitedBy:null` + admin-created rows). CASL still grants the route to admins only — widening the + `Invitation` abilities to regular users is the referral phase's flip; the scoping + ships first so that flip can never leak invitee emails (PII) platform-wide. +2. **Self-referral guard — SHIPPED (#3833), with a known residual**: `create()` + rejects 422 "You cannot invite yourself" when the invitee email equals the + inviter's own, before the E9 registered-email check. The grant-side floor + (`expectedGrantKeys` drops the referrer side when + `invitedBy === acceptedUserId` — pinned in the billing unit tests) covers + same-account pairs only. **Alias/variant self-invites (a second personal email + → a separate account) are NOT prevented** — accepted residual risk; revisit + (fraud review / email-normalization dedup) before any paid-rewards launch. +3. **Open-signup hole — DOCUMENTED, intentionally NOT changed**: claim/finalize stay + gated on `!config.sign.up` (the "open signup never burns a token" invariant). + **Referral rewards therefore require `sign.up: false`.** On an open-signup + deployment a presented token is *resolved* but never *claimed/finalized*, so + `invitation.accepted` never fires and no grant occurs — enabling + `billing.referral` there is a silent no-op. The Vue Referrals tab reads + `GET /api/auth/config` (`sign.up`) and replaces the invite form with an + informational state when signup is open (the referrals list stays read-only). 4. **Index `referredBy`** alongside the first real referral query. ## UI From af141a0aa626b52319a69fadd0070112f975743e Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 13 Jun 2026 09:39:35 +0200 Subject: [PATCH 4/4] docs(invitations): document list() defence-in-depth scoping seam The admin role-check mirrors invitations.policy.js; any CASL ability widening for this route must be reviewed against this seam. refs #3833 --- modules/invitations/services/invitations.service.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/invitations/services/invitations.service.js b/modules/invitations/services/invitations.service.js index f303b08ee..1860c2a15 100644 --- a/modules/invitations/services/invitations.service.js +++ b/modules/invitations/services/invitations.service.js @@ -318,6 +318,10 @@ const release = async (id) => { * @returns {Promise} */ const list = (user) => { + // Defence-in-depth seam: this role check is a deliberate MIRROR of invitations.policy.js, + // not the authorization itself (CASL is). It is what keeps a non-admin scoped to their own + // rows once the referral phase widens the Invitation abilities — any such widening MUST be + // reviewed against this seam so the unscoped branch is never reachable by a regular user. if (Array.isArray(user?.roles) && user.roles.includes('admin')) return InvitationRepository.list(); const userId = user?.id || user?._id; if (!userId) return Promise.resolve([]);