diff --git a/modules/billing/tests/billing.quota.unit.tests.js b/modules/billing/tests/billing.quota.unit.tests.js index e62fb0b0d..0970bbab0 100644 --- a/modules/billing/tests/billing.quota.unit.tests.js +++ b/modules/billing/tests/billing.quota.unit.tests.js @@ -1,7 +1,7 @@ /** * Module dependencies. */ -import { jest, describe, test, beforeEach, afterEach, expect } from '@jest/globals'; +import { jest, describe, test, beforeAll, afterAll, beforeEach, afterEach, expect } from '@jest/globals'; import AppError from '../../../lib/helpers/AppError.js'; /** @@ -17,6 +17,23 @@ describe('requireQuota middleware:', () => { let req; let res; let next; + let originalNodeEnv; + + beforeAll(() => { + // rule 3 (Node#4020): lib/helpers/responses.js only serializes the raw + // `payload.error` blob in a dev-grade NODE_ENV (lib/helpers/config.js + // isProd()/DEV_ENVS). The assertions below read that field, so force an + // explicit dev-grade value here rather than depend on whichever NODE_ENV + // a consumer happens to run its own test suite under. Same value for + // every test in this file, so set once for the suite rather than per test. + originalNodeEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'test'; + }); + + afterAll(() => { + if (originalNodeEnv === undefined) delete process.env.NODE_ENV; + else process.env.NODE_ENV = originalNodeEnv; + }); beforeEach(async () => { jest.resetModules(); diff --git a/modules/billing/tests/billing.unit.tests.js b/modules/billing/tests/billing.unit.tests.js index d2075c8df..d7aa16bfd 100644 --- a/modules/billing/tests/billing.unit.tests.js +++ b/modules/billing/tests/billing.unit.tests.js @@ -1,6 +1,7 @@ /** * Module dependencies. */ +import { jest } from '@jest/globals'; import config from '../../../config/index.js'; import schema from '../models/billing.subscription.schema.js'; @@ -8,13 +9,32 @@ import schema from '../models/billing.subscription.schema.js'; * Unit tests */ describe('Billing unit tests:', () => { + // rule 1 (Node#4020): pick a plan from the LOADED config instead of hardcoding + // a devkit-default plan id ('pro'/'starter') — a consumer with a different plan + // catalogue must still pass these SubscriptionUpdate assertions. The `?? plans[0]` + // fallback only fires when EVERY plan in the catalogue equals defaultPlan (a + // single-distinct-plan catalogue) — there is no other plan to pick in that case, + // by construction. The round-trip these tests assert (partial update preserves + // the exact plan provided, no default silently injected) still holds either + // way; only the variable's "non-default" framing stops applying. + const nonDefaultPlan = config.billing.plans.find((plan) => plan !== config.billing.defaultPlan) ?? config.billing.plans[0]; + + // rule 1 (Node#4020): fixture baseline plan for tests that need "some valid + // plan" (not testing default-resolution itself) — derived from the loaded + // config so a consumer whose catalogue lacks the devkit-shipped 'free' id + // still gets a value schema.Subscription actually accepts. Prefers + // defaultPlan (readable, guaranteed-valid by convention) but falls back to + // plans[0] in case a consumer's defaultPlan is ever missing from its own + // plans array. + const fixturePlan = config.billing.plans.includes(config.billing.defaultPlan) ? config.billing.defaultPlan : config.billing.plans[0]; + describe('Subscription schema', () => { let subscription; beforeEach(() => { subscription = { organization: '507f1f77bcf86cd799439011', - plan: 'free', + plan: fixturePlan, status: 'active', }; }); @@ -105,7 +125,7 @@ describe('Billing unit tests:', () => { }); test('should strip unknown fields with SubscriptionUpdate', (done) => { - const update = { plan: 'pro', unknown: 'field' }; + const update = { plan: nonDefaultPlan, unknown: 'field' }; const result = schema.SubscriptionUpdate.safeParse(update); expect(result.error).toBeFalsy(); expect(result.data?.unknown).toBeUndefined(); @@ -113,10 +133,10 @@ describe('Billing unit tests:', () => { }); test('should allow partial updates with SubscriptionUpdate', (done) => { - const update = { plan: 'starter' }; + const update = { plan: nonDefaultPlan }; const result = schema.SubscriptionUpdate.safeParse(update); expect(result.error).toBeFalsy(); - expect(result.data.plan).toBe('starter'); + expect(result.data.plan).toBe(nonDefaultPlan); done(); }); @@ -173,10 +193,10 @@ describe('Billing unit tests:', () => { }); test('should not inject defaults in SubscriptionUpdate partial', (done) => { - const update = { plan: 'pro' }; + const update = { plan: nonDefaultPlan }; const result = schema.SubscriptionUpdate.safeParse(update); expect(result.error).toBeFalsy(); - expect(result.data.plan).toBe('pro'); + expect(result.data.plan).toBe(nonDefaultPlan); expect(result.data.status).toBeUndefined(); done(); }); @@ -188,7 +208,7 @@ describe('Billing unit tests:', () => { beforeEach(() => { subscription = { organization: '507f1f77bcf86cd799439011', - plan: 'free', + plan: fixturePlan, status: 'active', }; }); @@ -330,4 +350,75 @@ describe('Billing unit tests:', () => { expect(result.error).toBeDefined(); }); }); + + // rule 1 (Node#4020): the two SubscriptionUpdate fixes above prove the tests no + // longer HARDCODE a devkit-default plan id, but they still run against the real + // devkit-default catalogue (free/starter/pro/enterprise). This block proves the + // schema itself is config-driven — not merely re-labeling the same default — + // by rebuilding it against a synthetic consumer profile with a totally different + // plan catalogue and confirming the enum follows the config, not a stack default. + describe('Subscription schema — synthetic consumer plan catalogue:', () => { + const syntheticPlans = ['launch', 'grow', 'scale']; + const syntheticDefaultPlan = 'launch'; + let SyntheticSchema; + + beforeAll(async () => { + jest.resetModules(); + jest.unstable_mockModule('../../../config/index.js', () => ({ + default: { + billing: { + plans: syntheticPlans, + defaultPlan: syntheticDefaultPlan, + statuses: ['active', 'canceled'], + }, + }, + })); + SyntheticSchema = (await import('../models/billing.subscription.schema.js')).default; + }); + + test('accepts every plan from the synthetic catalogue (none of which exist in the devkit default)', () => { + for (const plan of syntheticPlans) { + const result = SyntheticSchema.Subscription.safeParse({ + organization: '507f1f77bcf86cd799439011', + plan, + status: 'active', + }); + expect(result.error).toBeFalsy(); + } + }); + + test('rejects a devkit-default plan id that is absent from the synthetic catalogue', () => { + const result = SyntheticSchema.Subscription.safeParse({ + organization: '507f1f77bcf86cd799439011', + plan: 'free', + status: 'active', + }); + expect(result.error).toBeDefined(); + }); + + // Pre-push panel finding (P1): the shared `fixturePlan` derivation above + // (`config.billing.plans.includes(config.billing.defaultPlan) ? ... : + // plans[0]`) is what the 'Subscription schema' + 'meter fields' blocks' + // fixtures now use in place of a hardcoded 'free'. Proves that SAME + // derivation, applied to THIS synthetic catalogue (no 'free', no + // devkit-shipped id at all), still resolves to a value the schema accepts + // — the mechanism those ~15 fixture-driven tests now rely on genuinely + // survives a catalogue that lacks every devkit-default plan id. + test('the fixturePlan derivation mechanism resolves to a value the synthetic schema accepts', () => { + const syntheticFixturePlan = syntheticPlans.includes(syntheticDefaultPlan) ? syntheticDefaultPlan : syntheticPlans[0]; + const result = SyntheticSchema.Subscription.safeParse({ + organization: '507f1f77bcf86cd799439011', + plan: syntheticFixturePlan, + status: 'active', + }); + expect(result.error).toBeFalsy(); + expect(result.data.plan).toBe(syntheticFixturePlan); + }); + + test('SubscriptionUpdate round-trips a synthetic-catalogue plan id', () => { + const result = SyntheticSchema.SubscriptionUpdate.safeParse({ plan: syntheticPlans[1] }); + expect(result.error).toBeFalsy(); + expect(result.data.plan).toBe(syntheticPlans[1]); + }); + }); }); diff --git a/modules/invitations/tests/invitations.integration.tests.js b/modules/invitations/tests/invitations.integration.tests.js index c974542e6..f6cc04f1d 100644 --- a/modules/invitations/tests/invitations.integration.tests.js +++ b/modules/invitations/tests/invitations.integration.tests.js @@ -405,18 +405,25 @@ describe('Signup invitations:', () => { describe('E2 two-phase claim/finalize hardening', () => { let InvitationService; let InvitationRepository; - let originalUp; let originalCap; + let originalUp; let originalCap; let originalUserFacing; beforeAll(async () => { InvitationService = (await import(path.resolve('./modules/invitations/services/invitations.service.js'))).default; InvitationRepository = (await import(path.resolve('./modules/invitations/repositories/invitations.repository.js'))).default; }); - beforeEach(() => { originalUp = config.sign.up; originalCap = config.sign.cap; }); + beforeEach(() => { + originalUp = config.sign.up; + originalCap = config.sign.cap; + // rules 1+3 (Node#4020): saved/restored here even though only the + // OPEN-signup test below touches it — same block-level pattern as + // sign.up/sign.cap, so that test needs no bespoke try/finally of its own. + originalUserFacing = config.invitations.userFacing; + }); afterEach(async () => { - config.sign.up = originalUp; config.sign.cap = originalCap; + config.sign.up = originalUp; config.sign.cap = originalCap; config.invitations.userFacing = originalUserFacing; 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', 'e2-oauth-finalize-throw@example.com', 'e2-release-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', 'e2-opensignup@example.com']) { try { const existing = await UserService.getBrut({ email }); if (existing) await UserService.remove(existing); @@ -536,6 +543,15 @@ describe('Signup invitations:', () => { const created = await adminAgent.post('/api/invitations').send({ email }); const { token } = created.body.data; + // rules 1+3 (Node#4020): this P2 invariant holds specifically for + // userFacing:false (the byte-for-byte-unchanged baseline — the userFacing:true + // variant is covered separately by the dedicated "Open-signup userFacing" + // block below). Install that value explicitly rather than ride the config's + // ambient default, so this assertion never silently flips under a consumer + // whose default deployment config sets userFacing:true. Restored by this + // describe block's own afterEach, same as sign.up/sign.cap below. + config.invitations.userFacing = false; + // Public signup OPEN: a token may be presented but is not required. config.sign.up = true; config.sign.cap = null; const res = await request(app) @@ -546,11 +562,6 @@ describe('Signup invitations:', () => { // The invite must remain VALID (presented, not consumed) — not stuck mid-claim. const verify = await request(app).get(`/api/invitations/verify/${token}`); expect(verify.body.data.valid).toBe(true); - - try { - const u = await UserService.getBrut({ email }); - if (u) await UserService.remove(u); - } catch (_) { /* cleanup */ } }); test('crash recovery: a stale claim (older than the window) is swept and the invite becomes reusable', async () => { diff --git a/modules/organizations/tests/organizations.abilities.unit.tests.js b/modules/organizations/tests/organizations.abilities.unit.tests.js new file mode 100644 index 000000000..e93f38ed9 --- /dev/null +++ b/modules/organizations/tests/organizations.abilities.unit.tests.js @@ -0,0 +1,140 @@ +/** + * Unit tests for organizationAbilities — the CASL ability-definition function + * for the organizations module. Covers the full admin/user × owner/admin/member/ + * no-membership matrix (Node#4020, generic policy coverage the stack lacked). + * + * `can`/`cannot` are plain spies (no real CASL AbilityBuilder) — same convention + * as modules/invitations/tests/invitations.policy.unit.tests.js. + */ +import { describe, test, expect, jest } from '@jest/globals'; + +import { organizationAbilities } from '../policies/organizations.policy.js'; +import { MEMBERSHIP_ROLES } from '../lib/constants.js'; + +/** + * Build a membership fixture with a populated organizationId (the shape + * MembershipRepository.findOne's defaultPopulate returns in production). + * @param {string} role - one of MEMBERSHIP_ROLES + * @param {string} organizationId - hex-ish id string + * @returns {Object} membership fixture + */ +function membershipFixture(role, organizationId = '507f1f77bcf86cd799439011') { + return { role, organizationId: { _id: organizationId } }; +} + +/** + * Call organizationAbilities with fresh can/cannot spies and return them. + * @param {Object} user - user fixture + * @param {Object|null} membership - membership fixture + * @returns {{can: jest.Mock, cannot: jest.Mock}} the spies, populated by the call + */ +function callAbilities(user, membership) { + const can = jest.fn(); + const cannot = jest.fn(); + organizationAbilities(user, membership, { can, cannot }); + return { can, cannot }; +} + +describe('organizationAbilities:', () => { + describe('admin (roles includes "admin"):', () => { + test('grants manage all, regardless of membership (no membership)', () => { + const { can, cannot } = callAbilities({ roles: ['admin'] }, null); + + expect(can).toHaveBeenCalledWith('manage', 'all'); + expect(can).toHaveBeenCalledTimes(1); + expect(cannot).not.toHaveBeenCalled(); + }); + + test('grants manage all and short-circuits even when a membership is present (owner)', () => { + const { can, cannot } = callAbilities({ roles: ['admin'] }, membershipFixture(MEMBERSHIP_ROLES.OWNER)); + + expect(can).toHaveBeenCalledWith('manage', 'all'); + // The admin branch returns immediately — never also grants the non-admin/ + // membership-scoped abilities below. + expect(can).toHaveBeenCalledTimes(1); + expect(cannot).not.toHaveBeenCalled(); + }); + }); + + describe('non-admin user — no membership:', () => { + test('grants only create Organization', () => { + const { can, cannot } = callAbilities({ roles: ['user'] }, null); + + expect(can).toHaveBeenCalledWith('create', 'Organization'); + expect(can).toHaveBeenCalledTimes(1); + expect(cannot).not.toHaveBeenCalled(); + }); + + test('grants create Organization even when roles is absent (non-array falls through, not admin)', () => { + const { can } = callAbilities({}, null); + + expect(can).toHaveBeenCalledWith('create', 'Organization'); + expect(can).toHaveBeenCalledTimes(1); + }); + }); + + describe('non-admin user — membership.role = owner:', () => { + test('grants create Organization + manage Organization/Membership scoped to the org', () => { + const membership = membershipFixture(MEMBERSHIP_ROLES.OWNER, 'org-owner-1'); + const { can, cannot } = callAbilities({ roles: ['user'] }, membership); + + expect(can).toHaveBeenCalledWith('create', 'Organization'); + expect(can).toHaveBeenCalledWith('manage', 'Organization', { _id: 'org-owner-1' }); + expect(can).toHaveBeenCalledWith('manage', 'Membership', { organizationId: 'org-owner-1' }); + expect(can).toHaveBeenCalledTimes(3); + expect(cannot).not.toHaveBeenCalled(); + }); + }); + + describe('non-admin user — membership.role = admin:', () => { + test('grants create Organization + read/update Organization (not delete) + read/create/delete Membership', () => { + const membership = membershipFixture(MEMBERSHIP_ROLES.ADMIN, 'org-admin-1'); + const { can, cannot } = callAbilities({ roles: ['user'] }, membership); + + expect(can).toHaveBeenCalledWith('create', 'Organization'); + expect(can).toHaveBeenCalledWith('read', 'Organization', { _id: 'org-admin-1' }); + expect(can).toHaveBeenCalledWith('update', 'Organization', { _id: 'org-admin-1' }); + expect(can).toHaveBeenCalledWith('read', 'Membership', { organizationId: 'org-admin-1' }); + expect(can).toHaveBeenCalledWith('create', 'Membership', { organizationId: 'org-admin-1' }); + expect(can).toHaveBeenCalledWith('delete', 'Membership', { organizationId: 'org-admin-1' }); + expect(can).toHaveBeenCalledTimes(6); + + expect(cannot).toHaveBeenCalledWith('delete', 'Organization'); + expect(cannot).toHaveBeenCalledTimes(1); + }); + }); + + describe('non-admin user — membership.role = member:', () => { + test('grants create Organization + read-only Organization/Membership, no write abilities', () => { + const membership = membershipFixture(MEMBERSHIP_ROLES.MEMBER, 'org-member-1'); + const { can, cannot } = callAbilities({ roles: ['user'] }, membership); + + expect(can).toHaveBeenCalledWith('create', 'Organization'); + expect(can).toHaveBeenCalledWith('read', 'Organization', { _id: 'org-member-1' }); + expect(can).toHaveBeenCalledWith('read', 'Membership', { organizationId: 'org-member-1' }); + expect(can).toHaveBeenCalledTimes(3); + expect(cannot).not.toHaveBeenCalled(); + + expect(can).not.toHaveBeenCalledWith('manage', 'Organization', expect.anything()); + expect(can).not.toHaveBeenCalledWith('update', 'Organization', expect.anything()); + expect(can).not.toHaveBeenCalledWith('delete', expect.anything(), expect.anything()); + }); + }); + + describe('membership.organizationId shape — populated vs. raw id:', () => { + test('scopes abilities to the raw id when organizationId is NOT populated (no ._id)', () => { + const membership = { role: MEMBERSHIP_ROLES.OWNER, organizationId: 'org-raw-1' }; + const { can } = callAbilities({ roles: ['user'] }, membership); + + expect(can).toHaveBeenCalledWith('manage', 'Organization', { _id: 'org-raw-1' }); + expect(can).toHaveBeenCalledWith('manage', 'Membership', { organizationId: 'org-raw-1' }); + }); + + test('scopes abilities to the populated ._id when organizationId IS populated', () => { + const membership = membershipFixture(MEMBERSHIP_ROLES.OWNER, 'org-populated-1'); + const { can } = callAbilities({ roles: ['user'] }, membership); + + expect(can).toHaveBeenCalledWith('manage', 'Organization', { _id: 'org-populated-1' }); + }); + }); +}); diff --git a/modules/organizations/tests/organizations.emailVerification.unit.tests.js b/modules/organizations/tests/organizations.emailVerification.unit.tests.js index 9f3c20e27..1eeb74665 100644 --- a/modules/organizations/tests/organizations.emailVerification.unit.tests.js +++ b/modules/organizations/tests/organizations.emailVerification.unit.tests.js @@ -6,6 +6,19 @@ import { jest, describe, test, expect, beforeEach } from '@jest/globals'; // --- Mocks --- +// rule 2 (Node#4020): isolate the org-creation seam GENERICALLY — mock the hook +// mechanism itself (lib/events.js, a plain EventEmitter registry any consumer +// module can subscribe to), never a specific consumer's own listener module. +// A bare double (not a real EventEmitter) — same convention already used by +// organizations.service.silent.catch.unit.tests.js and +// organizations.service.signup.unit.tests.js, which already covers the +// organization.created/organization.provisioned payload shape and ordering +// generically via this exact mock, so this file only needs isolation, not a +// second copy of that coverage. +jest.unstable_mockModule('../lib/events.js', () => ({ + default: { emit: jest.fn(), on: jest.fn() }, +})); + const mockIsConfigured = jest.fn(); jest.unstable_mockModule('../../../lib/helpers/mailer/index.js', () => ({ default: { isConfigured: mockIsConfigured, sendMail: jest.fn() }, diff --git a/modules/organizations/tests/organizations.service.silent.catch.unit.tests.js b/modules/organizations/tests/organizations.service.silent.catch.unit.tests.js index 0c8a3c904..e1ac7a928 100644 --- a/modules/organizations/tests/organizations.service.silent.catch.unit.tests.js +++ b/modules/organizations/tests/organizations.service.silent.catch.unit.tests.js @@ -68,6 +68,15 @@ jest.unstable_mockModule('../helpers/organizations.slug.js', () => ({ generateOrganizationSlug: jest.fn().mockResolvedValue('test-org'), })); +// rule 2 (Node#4020): isolate the org-creation seam GENERICALLY — mock the hook +// mechanism itself (lib/events.js, a plain EventEmitter registry any consumer +// module can subscribe to), never a specific consumer's own listener module. +// This keeps the rollback-logging assertion below independent of whatever a +// consumer has registered on 'organization.created' / 'organization.provisioned'. +jest.unstable_mockModule('../lib/events.js', () => ({ + default: { emit: jest.fn(), on: jest.fn() }, +})); + const { default: OrgService } = await import('../services/organizations.service.js'); describe('organizations.service silent-catch error logging:', () => {