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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions modules/billing/config/billing.development.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 10 additions & 0 deletions modules/billing/tests/billing.referral.service.unit.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
34 changes: 25 additions & 9 deletions modules/invitations/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions modules/invitations/controllers/invitations.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>} 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);
Expand Down
8 changes: 6 additions & 2 deletions modules/invitations/repositories/invitations.repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<Array>}
*/
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
Expand Down
38 changes: 35 additions & 3 deletions modules/invitations/services/invitations.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<Object>} 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) {
Expand Down Expand Up @@ -291,10 +307,26 @@ 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<Array>}
*/
const list = () => InvitationRepository.list();
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([]);
return InvitationRepository.list({ invitedBy: userId });
};

/**
* @desc Get one invitation by id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' }]);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
48 changes: 45 additions & 3 deletions modules/invitations/tests/invitations.service.unit.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -335,10 +362,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({ id: 'a1', roles: ['user', 'admin'] });
expect(InvitationRepository.list).toHaveBeenCalledWith();
});
test('list(non-admin) → repository list scoped to { invitedBy: <caller id> } (#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();
expect(InvitationRepository.list).toHaveBeenCalledTimes(1);
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);
Expand Down
Loading