diff --git a/modules/billing/billing.init.js b/modules/billing/billing.init.js index 7efab419b..e0bf4ee4e 100644 --- a/modules/billing/billing.init.js +++ b/modules/billing/billing.init.js @@ -6,6 +6,7 @@ import config from '../../config/index.js'; import AnalyticsService from '../../lib/services/analytics.js'; import logger from '../../lib/services/logger.js'; import billingEvents from './lib/events.js'; +import invitationEvents from '../invitations/lib/events.js'; import BillingUsageRepository from './repositories/billing.usage.repository.js'; import { getAlertThresholdPercents } from './lib/billing.constants.js'; import { setupBillingEmails } from './billing.email.js'; @@ -44,6 +45,26 @@ export default async (app) => { // Wire billing email listeners (quota warnings + payment-failed notifications). setupBillingEmails(); + // Referral substrate (#5) — billing is an OPTIONAL consumer of the invitations + // fire-and-forget `invitation.accepted` event (dependency direction billing → + // invitations is fine: billing imports the events singleton, invitations never + // imports billing). This listener is a deliberate NO-OP that only PROVES the event + // seam works end-to-end (the payload arrives here on every invite acceptance); the + // actual credit-grant logic lands in #5. Wrapped so a future grant impl can't crash + // boot/signup. Mirrors the other cross-module listeners wired on this init. + /** + * @desc No-op referral seam listener for invitation acceptance events (P8a). + * Proves the cross-module event contract end-to-end; credit-grant logic lands in #5. + * @param {{invitationId: string, email: string, invitedBy: (string|null), acceptedUserId: string}} payload - Accepted invitation event payload. + * @returns {void} + */ + // eslint-disable-next-line no-unused-vars + invitationEvents.on('invitation.accepted', (payload) => { + // TODO(#5): grant referral credits to payload.invitedBy (skip when invitedBy is null). + // TODO(#5): async grant listener must self-guard rejections — the emit-site try/catch only catches sync throws. + // No-op for P8a — the seam is the deliverable, not the grant. + }); + // Update analytics group properties when a subscription plan changes billingEvents.on('plan.changed', ({ organizationId, newPlan }) => { try { diff --git a/modules/billing/tests/billing.init.unit.tests.js b/modules/billing/tests/billing.init.unit.tests.js index 82cb79faf..05261d609 100644 --- a/modules/billing/tests/billing.init.unit.tests.js +++ b/modules/billing/tests/billing.init.unit.tests.js @@ -13,6 +13,7 @@ describe('billing.init unit tests:', () => { let mockDistinct; let mockMongoose; let mockLogger; + let mockInvitationEvents; const mockApp = {}; @@ -58,6 +59,13 @@ describe('billing.init unit tests:', () => { default: { on: jest.fn(), emit: jest.fn() }, })); + // P8a: billing is an optional consumer of the invitations `invitation.accepted` + // event — stub the singleton so the unit test asserts the listener wiring in isolation. + mockInvitationEvents = { on: jest.fn(), emit: jest.fn() }; + jest.unstable_mockModule('../../invitations/lib/events.js', () => ({ + default: mockInvitationEvents, + })); + // Stub billing.email so boot validator tests don't wire real email listeners jest.unstable_mockModule('../billing.email.js', () => ({ setupBillingEmails: jest.fn(), @@ -79,6 +87,17 @@ describe('billing.init unit tests:', () => { await expect(billingInit(mockApp)).resolves.toBeUndefined(); }); + test('P8a: wires a (no-op) invitation.accepted listener that does not throw on emit', async () => { + await billingInit(mockApp); + // The seam is proven by the listener being registered on the invitations emitter. + const acceptedCall = mockInvitationEvents.on.mock.calls.find(([evt]) => evt === 'invitation.accepted'); + expect(acceptedCall).toBeDefined(); + const handler = acceptedCall[1]; + expect(typeof handler).toBe('function'); + // No-op: invoking it with a payload must not throw (and returns nothing). + expect(() => handler({ invitationId: 'i1', email: 'a@b.co', invitedBy: 'x', acceptedUserId: 'u1' })).not.toThrow(); + }); + test('resolves without error when meterMode=true and no legacy docs', async () => { mockConfig.billing.meterMode = true; mockConfig.billing.plans = ['free', 'growth', 'pro']; diff --git a/modules/invitations/invitations.init.js b/modules/invitations/invitations.init.js index 2d7a7da64..67b5fedef 100644 --- a/modules/invitations/invitations.init.js +++ b/modules/invitations/invitations.init.js @@ -80,10 +80,25 @@ export default async () => { } if (!invite) return undefined; // Return the resolved (+claimed, local) invite plus finalize/release closures - // bound to its id. finalize/release logic stays in this module; auth just relays. + // bound to it. The accept/release logic stays in this module; auth just relays. + // P8a: `finalize` now routes through InvitationsService.accept, which finalizes + // the invite AND wires the referral substrate (#5) — stamps referredBy on the new + // user (server-side) + emits `invitation.accepted`. The closure name stays + // `finalize` so auth.controller relays it unchanged (auth never imports us); accept + // is a superset of finalize. Fires on BOTH the token AND the OAuth path (both go + // through this same closure), so OAuth-invited users are credited too. + + /** + * @desc Finalize accepted invite and run referral side-effects (P8a). + * Delegates to InvitationsService.accept so auth stays import-free. + * @param {String} userId - the just-created user id + * @returns {Promise} finalized invitation document, or null if not finalized + */ + const finalizeInvite = (userId) => InvitationsService.accept(invite, userId); + return { invite, - finalize: (userId) => InvitationsService.finalize(invite.id, userId), + finalize: finalizeInvite, release: () => InvitationsService.release(invite.id), }; }); diff --git a/modules/invitations/lib/events.js b/modules/invitations/lib/events.js index b2b1bf155..ecde1bb78 100644 --- a/modules/invitations/lib/events.js +++ b/modules/invitations/lib/events.js @@ -7,10 +7,27 @@ import { EventEmitter } from 'events'; * Singleton emitter for invitation events. Config-free / import-safe. * * Events: - * - `invitation.accepted` — emitted when an invite is consumed by a signup - * Payload: { invitationId, email, invitedBy, acceptedUserId } + * - `invitation.accepted` — emitted (P8a) by InvitationsService.accept when an invite + * is consumed by a successful signup (BOTH the local two-phase token path AND the + * OAuth-by-email path go through the same accept seam). Fire-and-forget. Always + * emitted on accept. + * ⚠️ The try/catch around the `emit` call (accept seam) only guards against a + * SYNCHRONOUS listener throw — `EventEmitter.emit` is synchronous, so it returns + * before any async listener settles. An ASYNC listener (e.g. `async (p) => { await + * grantCredits() }`) that REJECTS escapes the emit-site try/catch as an + * unhandledRejection AFTER emit returns. Therefore a future async listener (e.g. the + * #5 credit-grant) MUST own its own internal try/catch and never let a rejection + * escape, OR the emit seam must switch to an awaited `Promise.allSettled` fan-out — + * the current synchronous guard will NOT catch an async rejection. + * Payload: { + * invitationId: String — the accepted invite's id + * email: String — the invite's pinned (lowercased) email + * invitedBy: ObjectId|null — the inviter user id, or null for an admin-created + * invite with no inviter + * acceptedUserId: String — the just-created user's id (= the new user's referredBy + * source when invitedBy is set) + * } * - * NOTE: no event is emitted yet — P8 wires the actual `invitation.accepted` emit. * This file ships the singleton + the error-listener hook (registered in * invitations.init.js after config is ready) so it stays config-free and * importable without ordering hazards (mirrors billing/lib/events.js). diff --git a/modules/invitations/services/invitations.service.js b/modules/invitations/services/invitations.service.js index 45e57d196..400de6bec 100644 --- a/modules/invitations/services/invitations.service.js +++ b/modules/invitations/services/invitations.service.js @@ -6,6 +6,7 @@ import crypto from 'crypto'; import InvitationRepository from '../repositories/invitations.repository.js'; import { DEFAULT_INVITE_EXPIRES_IN_DAYS, STALE_CLAIM_MINUTES } from '../lib/constants.js'; import UserService from '../../users/services/users.service.js'; +import invitationEvents from '../lib/events.js'; import config from '../../../config/index.js'; import mails from '../../../lib/helpers/mailer/index.js'; import getBaseUrl from '../../../lib/helpers/getBaseUrl.js'; @@ -201,6 +202,79 @@ const finalize = async (id, userId) => { return result; }; +/** + * @desc P8a — the shared ACCEPT seam, invoked by the eligibility closure on FULL + * signup success for BOTH paths (local two-phase token AND OAuth-by-email). It: + * 1. finalizes the invite (status:'accepted', acceptedAt, acceptedUserId:userId, + * usedAt — burns single-use; already idempotent on acceptedAt:null in the repo), + * 2. stamps `referredBy = invite.invitedBy` on the just-created user, SERVER-SIDE, + * via UserService.updateById (raw update — bypasses the client whitelist + Zod, + * so this is the ONLY way the field is ever written; never from a client body). + * invitations already depends on users (the E9 guard), so this keeps auth import-free. + * 3. emits `invitation.accepted` so optional consumers (billing #5 credit-grant) can + * react fire-and-forget. + * + * Referral substrate (#5) — NO credit-grant logic here; this only wires the field + event. + * + * `invitedBy` may be null (admin-created invite with no inviter). We DECIDE to ALWAYS + * emit on accept (the canonical "an invite was consumed" signal) with `invitedBy:null` + * in that case, but to SKIP the referredBy write when there is no inviter (leaving the + * user's `referredBy` at its `default:null` — writing null is a redundant no-op). + * + * If finalize() returns null (duplicate-accept / revoked / missing invite), the method + * returns null immediately — no referral side-effects fire for non-finalized invites. + * + * Both side-effects are best-effort: a referredBy-write failure or a listener throw must + * NOT roll back an already-burned invite or break the signup response (the invite is + * finalized first; the referral wiring is downstream of account creation). Errors are + * logged. The emit is wrapped because a synchronous listener throw would otherwise + * propagate out of `emit` into the signup flow. + * + * @param {Object} invite - the resolved invite doc (has id, invitedBy, email) + * @param {String} userId - the just-created user's id + * @returns {Promise} the finalized invite doc, or null if nothing to finalize + */ +const accept = async (invite, userId) => { + const result = await finalize(invite.id, userId); + // Guard: only wire referral side-effects when the invite was actually finalized. + // finalize() returns null on duplicate-accept / revoked / missing — we must not + // emit a consumed-invite signal or stamp referredBy for an invite that did not land. + if (!result) { + return null; + } + const invitedBy = invite.invitedBy || null; + // Stamp the referral link server-side (skip the redundant null write). + if (invitedBy) { + try { + await UserService.updateById(userId, { referredBy: invitedBy }); + } catch (err) { + // Best-effort: the invite is already accepted; a referredBy write failure must + // not break signup. Surface it as a warning for follow-up (referral attribution + // would be lost for this user, but the account is valid). + logger.warn('invitations: failed to set referredBy on accepted signup', { + invitationId: String(invite.id), + userId: String(userId), + invitedBy: String(invitedBy), + message: err?.message, + }); + } + } + // Always emit on accept (invitedBy may be null) so the event is the single canonical + // "invite consumed" signal. Guard the emit so a synchronous listener throw cannot + // escape into the signup flow (an emitted 'error' would crash without the init listener). + try { + invitationEvents.emit('invitation.accepted', { + invitationId: invite.id, + email: invite.email, + invitedBy, + acceptedUserId: userId, + }); + } catch (err) { + logger.warn('invitations: invitation.accepted listener threw', { message: err?.message }); + } + return result; +}; + /** * @desc E2 — release a claimed-but-unfinalized invite (on any pre-response signup * failure) so it can be retried. Checks the repository return. @@ -244,6 +318,7 @@ export default { assertInvitedByEmail, claim, finalize, + accept, release, sweepStaleClaims, list, diff --git a/modules/invitations/tests/invitations.init.unit.tests.js b/modules/invitations/tests/invitations.init.unit.tests.js index 7e24da9fd..b0588bdae 100644 --- a/modules/invitations/tests/invitations.init.unit.tests.js +++ b/modules/invitations/tests/invitations.init.unit.tests.js @@ -20,6 +20,7 @@ const mockService = { assertInvitedByEmail: jest.fn(), claim: jest.fn(), finalize: jest.fn(), + accept: jest.fn(), release: jest.fn(), sweepStaleClaims: jest.fn(), }; @@ -74,9 +75,13 @@ describe('invitations.init', () => { // E2: the resolved invite was atomically claimed by token before returning. expect(mockService.claim).toHaveBeenCalledWith('tok'); expect(result.invite).toBe(invite); - // returned finalize/release closures bind exactly this invite id + // returned finalize/release closures bind exactly this invite. P8a: the `finalize` + // closure routes through accept(invite, userId) (finalize + referral wiring); the + // closure name is unchanged so auth relays it verbatim. accept receives the WHOLE + // invite (needs invitedBy/email for referredBy + the event payload), not just the id. await result.finalize('u9'); - expect(mockService.finalize).toHaveBeenCalledWith('i7', 'u9'); + expect(mockService.accept).toHaveBeenCalledWith(invite, 'u9'); + expect(mockService.finalize).not.toHaveBeenCalled(); await result.release(); expect(mockService.release).toHaveBeenCalledWith('i7'); }); @@ -117,8 +122,10 @@ describe('invitations.init', () => { // OAuth has no token to claim — claim must NOT be invoked on this path. expect(mockService.claim).not.toHaveBeenCalled(); expect(result.invite).toBe(invite); + // P8a: the OAuth path's finalize closure also routes through accept — so an + // OAuth-invited user gets referredBy set + invitation.accepted emitted too. await result.finalize('u2'); - expect(mockService.finalize).toHaveBeenCalledWith('o1', 'u2'); + expect(mockService.accept).toHaveBeenCalledWith(invite, 'u2'); }); test('(c) OAuth: does NOT resolve an invite when the provider email is unverified (E7)', async () => { diff --git a/modules/invitations/tests/invitations.integration.tests.js b/modules/invitations/tests/invitations.integration.tests.js index 7747a087c..f60e29203 100644 --- a/modules/invitations/tests/invitations.integration.tests.js +++ b/modules/invitations/tests/invitations.integration.tests.js @@ -623,6 +623,105 @@ describe('Signup invitations:', () => { }); }); + describe('P8a referral substrate (referredBy + invitation.accepted)', () => { + let invitationEvents; + let originalUp; let originalCap; + + beforeAll(async () => { + invitationEvents = (await import(path.resolve('./modules/invitations/lib/events.js'))).default; + }); + + beforeEach(() => { originalUp = config.sign.up; originalCap = config.sign.cap; }); + afterEach(async () => { + config.sign.up = originalUp; config.sign.cap = originalCap; + jest.restoreAllMocks(); + for (const email of ['p8a-token@example.com', 'p8a-oauth@example.com', 'p8a-event@example.com']) { + try { + const existing = await UserService.getBrut({ email }); + if (existing) await UserService.remove(existing); + } catch (_) { /* cleanup */ } + } + }); + + test('token path: invited signup sets referredBy = inviter id; a client-supplied referredBy in the body is IGNORED', async () => { + const adminAgent = await createAdminAndSignin(); + const email = 'p8a-token@example.com'; + const created = await adminAgent.post('/api/invitations').send({ email }); + const { token } = created.body.data; + // The inviter is the admin who created the invite. + const admin = await UserService.getBrut({ email: 'inv-admin@test.com' }); + const inviterId = String(admin._id); + + config.sign.up = false; config.sign.cap = null; + + // Attacker tries to self-assign a DIFFERENT referrer via the signup body. + const res = await request(app) + .post(`/api/auth/signup?inviteToken=${token}`) + .send({ email, password: 'Sup3rStr0ng!', referredBy: '64b2f0000000000000000999' }); + expect(res.status).toBe(200); + + const brut = await UserService.getBrut({ email }); + // referredBy is the SERVER-resolved inviter, NOT the client-supplied id. + expect(String(brut.referredBy)).toBe(inviterId); + expect(String(brut.referredBy)).not.toBe('64b2f0000000000000000999'); + }); + + test('OAuth path: an email-matched invited OAuth signup ALSO sets referredBy = inviter id', async () => { + const AuthController = (await import(path.resolve('./modules/auth/controllers/auth.controller.js'))).default; + const adminAgent = await createAdminAndSignin(); + const email = 'p8a-oauth@example.com'; + await adminAgent.post('/api/invitations').send({ email }); + const admin = await UserService.getBrut({ email: 'inv-admin@test.com' }); + const inviterId = String(admin._id); + + config.sign.up = false; config.sign.cap = null; + + const profil = { + firstName: 'Referred', // NB: the name Zod regex rejects digits — no 'P8a' + lastName: 'OAuth', + email, + avatar: '', + providerData: { sub: 'google-p8a-oauth-sub' }, + emailVerifiedByProvider: true, + }; + const createdUser = await AuthController.checkOAuthUserProfile(profil, 'sub', 'google'); + expect(createdUser.email).toBe(email); + + const brut = await UserService.getBrut({ email }); + // OAuth-invited users are credited too (the same accept seam runs). + expect(String(brut.referredBy)).toBe(inviterId); + }); + + test('invitation.accepted fires with the documented payload on accept (proves the billing listener seam end-to-end)', async () => { + const adminAgent = await createAdminAndSignin(); + const email = 'p8a-event@example.com'; + const created = await adminAgent.post('/api/invitations').send({ email }); + const { token } = created.body.data; + const invitationId = created.body.data.id; + const admin = await UserService.getBrut({ email: 'inv-admin@test.com' }); + const inviterId = String(admin._id); + + // Spy on the REAL events singleton (billing's no-op listener is also attached + // to it at boot — this proves the event reaches consumers end-to-end). + const emitSpy = jest.spyOn(invitationEvents, 'emit'); + + config.sign.up = false; config.sign.cap = null; + const res = await request(app) + .post(`/api/auth/signup?inviteToken=${token}`) + .send({ email, password: 'Sup3rStr0ng!' }); + expect(res.status).toBe(200); + const newUser = await UserService.getBrut({ email }); + + const acceptedCall = emitSpy.mock.calls.find(([evt]) => evt === 'invitation.accepted'); + expect(acceptedCall).toBeDefined(); + const payload = acceptedCall[1]; + expect(payload.invitationId).toBe(invitationId); + expect(payload.email).toBe(email); + expect(String(payload.invitedBy)).toBe(inviterId); + expect(String(payload.acceptedUserId)).toBe(String(newUser._id)); + }); + }); + describe('E4 cap unify — blank cap means UNCAPPED at the signup gate', () => { let originalUp; let originalCap; beforeEach(() => { originalUp = config.sign.up; originalCap = config.sign.cap; }); diff --git a/modules/invitations/tests/invitations.service.unit.tests.js b/modules/invitations/tests/invitations.service.unit.tests.js index d30d6a5f9..72f884b6d 100644 --- a/modules/invitations/tests/invitations.service.unit.tests.js +++ b/modules/invitations/tests/invitations.service.unit.tests.js @@ -16,7 +16,7 @@ jest.unstable_mockModule('../repositories/invitations.repository.js', () => ({ }, })); -const mockUserService = { findByEmail: jest.fn() }; +const mockUserService = { findByEmail: jest.fn(), updateById: jest.fn() }; jest.unstable_mockModule('../../users/services/users.service.js', () => ({ default: mockUserService })); const mockMailer = { isConfigured: jest.fn(() => false), sendMail: jest.fn() }; @@ -41,11 +41,14 @@ jest.unstable_mockModule('../../../lib/services/logger.js', () => ({ const InvitationRepository = (await import('../repositories/invitations.repository.js')).default; const InvitationService = (await import('../services/invitations.service.js')).default; +// Real events singleton — the service emits on it; we spy to assert the payload. +const invitationEvents = (await import('../lib/events.js')).default; beforeEach(() => { // Default: no stale claims to sweep, no pre-existing user (E9). InvitationRepository.releaseStaleClaims.mockResolvedValue({ modifiedCount: 0 }); mockUserService.findByEmail.mockResolvedValue(null); + mockUserService.updateById.mockResolvedValue({}); }); describe('InvitationService.findValid', () => { @@ -186,6 +189,96 @@ describe('InvitationService.finalize / release (E2)', () => { }); }); +describe('InvitationService.accept (P8a — referral substrate seam)', () => { + let emitSpy; + beforeEach(() => { + InvitationRepository.finalize.mockResolvedValue({ id: 'i1', status: 'accepted' }); + emitSpy = jest.spyOn(invitationEvents, 'emit'); + }); + afterEach(() => { + emitSpy.mockRestore(); + }); + + test('finalizes the invite (burns single-use) AND returns the finalized doc', async () => { + const doc = { id: 'i1', status: 'accepted' }; + InvitationRepository.finalize.mockResolvedValue(doc); + const invite = { id: 'i1', email: 'a@b.co', invitedBy: 'inviter1' }; + expect(await InvitationService.accept(invite, 'u1')).toBe(doc); + expect(InvitationRepository.finalize).toHaveBeenCalledWith('i1', 'u1'); + }); + + test('stamps referredBy = invite.invitedBy on the new user, SERVER-SIDE via updateById', async () => { + const invite = { id: 'i1', email: 'a@b.co', invitedBy: 'inviter1' }; + await InvitationService.accept(invite, 'u1'); + // updateById is the raw path that bypasses the client whitelist + Zod — the ONLY + // way referredBy is written. Asserts the server sets it from the invite, not a body. + expect(mockUserService.updateById).toHaveBeenCalledWith('u1', { referredBy: 'inviter1' }); + }); + + test('emits invitation.accepted with the documented payload', async () => { + const invite = { id: 'i1', email: 'a@b.co', invitedBy: 'inviter1' }; + await InvitationService.accept(invite, 'u1'); + expect(emitSpy).toHaveBeenCalledWith('invitation.accepted', { + invitationId: 'i1', + email: 'a@b.co', + invitedBy: 'inviter1', + acceptedUserId: 'u1', + }); + }); + + test('invitedBy null (admin-created invite): SKIPS the referredBy write but STILL emits (invitedBy:null)', async () => { + const invite = { id: 'i2', email: 'c@d.co', invitedBy: null }; + await InvitationService.accept(invite, 'u2'); + // No inviter → writing null is a redundant no-op, so updateById is skipped. + expect(mockUserService.updateById).not.toHaveBeenCalled(); + // The event still fires (canonical "invite consumed" signal) with invitedBy:null. + expect(emitSpy).toHaveBeenCalledWith('invitation.accepted', { + invitationId: 'i2', + email: 'c@d.co', + invitedBy: null, + acceptedUserId: 'u2', + }); + }); + + test('invitedBy undefined is normalized to null in the payload', async () => { + const invite = { id: 'i3', email: 'e@f.co' }; // no invitedBy key at all + await InvitationService.accept(invite, 'u3'); + expect(mockUserService.updateById).not.toHaveBeenCalled(); + expect(emitSpy).toHaveBeenCalledWith('invitation.accepted', expect.objectContaining({ invitedBy: null })); + }); + + test('best-effort: a referredBy write failure is swallowed (still emits, still returns the doc)', async () => { + mockUserService.updateById.mockRejectedValueOnce(new Error('db down')); + const invite = { id: 'i1', email: 'a@b.co', invitedBy: 'inviter1' }; + // Must not throw — the invite is already accepted; signup must not break. + const result = await InvitationService.accept(invite, 'u1'); + expect(result).toMatchObject({ status: 'accepted' }); + expect(emitSpy).toHaveBeenCalledWith('invitation.accepted', expect.objectContaining({ acceptedUserId: 'u1' })); + }); + + test('best-effort: a synchronous listener throw is caught at the emit site (does not break signup)', async () => { + const thrower = () => { throw new Error('listener boom'); }; + invitationEvents.on('invitation.accepted', thrower); + try { + const invite = { id: 'i1', email: 'a@b.co', invitedBy: 'inviter1' }; + await expect(InvitationService.accept(invite, 'u1')).resolves.toMatchObject({ status: 'accepted' }); + } finally { + invitationEvents.removeListener('invitation.accepted', thrower); + } + }); + + test('returns null immediately when finalize() returns null — no side-effects fire', async () => { + // Covers: duplicate-accept / revoked / missing invite → finalize returns null. + // referredBy must NOT be written and invitation.accepted must NOT be emitted. + InvitationRepository.finalize.mockResolvedValue(null); + const invite = { id: 'i1', email: 'a@b.co', invitedBy: 'inviter1' }; + const result = await InvitationService.accept(invite, 'u1'); + expect(result).toBeNull(); + expect(mockUserService.updateById).not.toHaveBeenCalled(); + expect(emitSpy).not.toHaveBeenCalledWith('invitation.accepted', expect.anything()); + }); +}); + describe('InvitationService.sweepStaleClaims (E2)', () => { test('computes a cutoff and delegates to releaseStaleClaims', async () => { InvitationRepository.releaseStaleClaims.mockResolvedValue({ modifiedCount: 2 }); diff --git a/modules/users/models/users.model.mongoose.js b/modules/users/models/users.model.mongoose.js index bb0a5bd0c..89184c812 100644 --- a/modules/users/models/users.model.mongoose.js +++ b/modules/users/models/users.model.mongoose.js @@ -52,6 +52,18 @@ const UserMongoose = new Schema( type: Schema.ObjectId, ref: 'Organization', }, + // Referral substrate (#5): the user whose invite this account accepted. Set + // SERVER-SIDE ONLY on invite acceptance (the invitations finalize seam, via + // UserService.updateById which bypasses the client whitelist) — NEVER from a + // client signup/update body. It is intentionally absent from the Zod schemas + // and from every users update whitelist, so a client cannot self-assign a + // referrer. null for self-serve signups and admin-created invites with no + // inviter. No index yet (default:null, no referral-list query in P8a). + referredBy: { + type: mongoose.Schema.Types.ObjectId, + ref: 'User', + default: null, + }, complementary: {}, // put your specific project private data here }, { diff --git a/modules/users/models/users.schema.js b/modules/users/models/users.schema.js index ba0f58437..32f441cc6 100644 --- a/modules/users/models/users.schema.js +++ b/modules/users/models/users.schema.js @@ -50,6 +50,19 @@ const User = z.object({ lockUntil: z.coerce.date().nullable().optional().default(null), // organization context currentOrganization: z.string().trim().optional(), + // Referral substrate (#5) — `referredBy` is DELIBERATELY NOT declared here. + // This is a server-only field set on invite acceptance (the invitations finalize + // seam, via UserService.updateById, which bypasses Zod + the update whitelist). + // The signup route validates the body with `model.isValid(User)`, and for POST it + // replaces req.body with the FULL Zod output — so any field present in this schema + // becomes client-writable on signup. Omitting `referredBy` makes Zod strip it from + // every client-facing parse (signup POST + PUT /api/users), guaranteeing a client + // can never self-assign a referrer. Do NOT add it here. (The Mongoose model + the + // updateById raw path are all the server needs to persist it.) + // When a future feature (e.g. the P8b referrals view) needs to READ/EXPOSE + // `referredBy`, do it via a response projection / the read whitelist — NEVER by + // adding it to this Zod schema (which is also the signup-POST write surface, so + // adding it here reintroduces the client-writable hole this omission closes). // others complementary: z.record(z.string(), z.unknown()).nullable().optional(), }); diff --git a/modules/users/tests/user.account.integration.tests.js b/modules/users/tests/user.account.integration.tests.js index 5d1eab93a..63d2514d2 100644 --- a/modules/users/tests/user.account.integration.tests.js +++ b/modules/users/tests/user.account.integration.tests.js @@ -291,6 +291,33 @@ describe('User integration tests:', () => { } }); + // P8a / E20: referredBy is a server-only referral field — a client must never be + // able to self-assign a referrer via the self-update endpoint. It is absent from + // config.whitelists.users.update, so removeSensitive strips it. This proves the + // whitelist blocks it (the deliverable is the test, not new stripping code). + test('should NOT be able to set referredBy via PUT /api/users (server-only referral field)', async () => { + try { + const userUpdate = { + firstName: 'refUpdateFirst', + referredBy: '64b2f0000000000000000abc', // attacker-supplied referrer id + }; + await agent.put('/api/users').send(userUpdate).expect(200); + } catch (err) { + console.log(err); + expect(err).toBeFalsy(); + } + + try { + // Read the raw doc — the whitelisted firstName landed, referredBy did NOT. + const result = await UserService.getBrut({ id: user.id }); + expect(result.firstName).toBe('refUpdateFirst'); + expect(result.referredBy == null).toBe(true); + } catch (err) { + console.log(err); + expect(err).toBeFalsy(); + } + }); + test('should be able to remove current user', async () => { // Init user edited _userEdited.email = 'register_new_user_@test.com'; diff --git a/modules/users/tests/user.admin.integration.tests.js b/modules/users/tests/user.admin.integration.tests.js index 5ca8e62d2..0d7e63043 100644 --- a/modules/users/tests/user.admin.integration.tests.js +++ b/modules/users/tests/user.admin.integration.tests.js @@ -303,6 +303,48 @@ describe('User admin integration tests:', () => { } }); + // P8a / E20: referredBy is a server-only referral field — even an admin update + // (config.whitelists.users.updateAdmin) must NOT persist it from the request body. + // It is absent from updateAdmin, so removeSensitive strips it. Proves the admin + // whitelist blocks it too (mirrors the self-update negative test). + test('should NOT persist referredBy via the admin user-update path (server-only)', async () => { + try { + userEdited = await signupAndPromoteAdmin(agent, { ..._userEdited, roles: ['user', 'admin'] }); + } catch (err) { + console.log(err); + expect(err).toBeFalsy(); + } + + try { + const userUpdate = { + firstName: 'admin_ref_first', + referredBy: '64b2f0000000000000000def', // attacker-supplied referrer id + }; + const result = await agent.put(`/api/admin/users/${userEdited._id}`).send(userUpdate).expect(200); + expect(result.body.data.firstName).toBe('admin_ref_first'); + } catch (err) { + console.log(err); + expect(err).toBeFalsy(); + } + + try { + // Raw read — the whitelisted firstName landed, referredBy did NOT. + const result = await UserService.getBrut({ id: userEdited._id }); + expect(result.firstName).toBe('admin_ref_first'); + expect(result.referredBy == null).toBe(true); + } catch (err) { + console.log(err); + expect(err).toBeFalsy(); + } + + try { + await UserService.remove(userEdited); + } catch (err) { + console.log(err); + expect(err).toBeFalsy(); + } + }); + test('should be able to remove a single user if admin', async () => { try { userEdited = await signupAndPromoteAdmin(agent, { ..._userEdited, roles: ['user', 'admin'] });