refactor(invitations): extract platform-invitation module + invert auth eligibility hook (P2) (#3820)

* refactor(invitations): extract platform-invite module + eligibility hook (#3810)

Phase 2 of the invitations↔org decouple epic. Move #3715's platform-invite
code out of modules/auth into a glob-discovered optional modules/invitations,
and invert the dependency so auth no longer imports invitation code.

- New modules/invitations: controller/service/repository/model/schema/policy/
  doc moved from auth (renamed auth.invitation.* → invitations.*), Mongoose
  model name kept "Invitation" → collection "invitations" (live Trawl prod
  docs; collection NOT renamed). Fields + indexes unchanged.
- New auth.eligibility.js: generic signup-eligibility hook registry
  (registerSignupEligibility / assertSignupEligible / _reset). invitations
  plugs in at boot via invitations.init.js (invitations → auth; auth never
  imports invitations). init also wires the events 'error' listener
  (mirrors billing.init.js crash guard).
- auth.controller signup + OAuth gates now call the generic hook; the
  checker resolves the invite and stashes it (+ a bound single-use consume
  closure) on the ctx carrier so the controller can pin the account email +
  consume without importing invitation code. Cap logic unchanged (P3).
- Routes: canonical /api/invitations mount in modules/invitations; the legacy
  /api/auth/invitations* alias stays in auth.routes.js (before the greedy
  /api/auth/:strategy wildcard) pointing at the moved controller. invitationId
  param registered once (app-global) by the canonical routes — no double DB hit.
- Policy registers BOTH /api/invitations and /api/auth/invitations → Invitation.
- config: inviteExpiresInDays moved to invitations config (deep-merged into
  config.sign); audit.routeTypeMap.invitations → Invitation.

Behavior-preserving refactor + deprecation alias only. No P3 hardening
(two-phase claim / email index / cap unify / soft-revoke). Signup behavior
identical. Tests: moved + split into per-layer unit tests, added init/
controller/repository unit + e2e + alias/OAuth-wildcard integration tests.
Full suite green (169 suites, 2246 tests); lint clean.

* refactor(invitations): return-value eligibility seam (drop req stash)

Code-review fix-up on the Phase-2 eligibility hook:

- Replace the req._signupInvite / req._consumeSignupInvite stash + synthetic
  oauthCtx={} carrier with a return-value seam. assertSignupEligible now runs
  every check and returns the first non-null result (opaque to auth); the
  invitations checker returns { invite, consume } instead of stashing. Deletes
  both req._-prefixed props. auth.controller stays import-free of invitation
  code (only comments + the generic Eligibility import).
- Fix the misleading body.inviteToken comment: HTTP signup strips unknown body
  keys via the model middleware, so query is the effective source; body
  fallback is for non-HTTP/direct callers. Rename the init test accordingly.
- Inline findValidByEmail into assertInvitedByEmail (drop the pure pass-through;
  no other caller); update its service unit test.
- Retitle the init 'registers exactly one checker' test to what it verifies
  (one init() => one checker) and document that init() is not idempotent.

Lint clean. 34 touched unit tests + 17 invitation integration/e2e tests green;
full suite 2246/2246 (one pre-existing socket-hang-up flake in auth lockout,
unrelated, passes on re-run).

* docs(auth): note throw-aborts-chain + loose-null seam semantics for P5/P8 checkers

* fix(invitations): address CodeRabbit + Copilot review findings

- auth.routes.js: add TODO(P6, #4279) with explicit rationale for
  the intentional core→optional import (deprecation alias until P6)
- auth.eligibility.js: guard registerSignupEligibility against
  non-function input with TypeError for earlier error surfacing
- auth.eligibility.unit.tests.js: add TypeError test for the new guard
- invitations.controller.unit.tests.js: add JSDoc to makeRes per
  coding guidelines for named test helpers
- invitations.init.unit.tests.js: fix misleading comment that said
  init() must not duplicate — clarify it is not idempotent by design,
  boots exactly once at startup
- invitations.repository.unit.tests.js: add jest.clearAllMocks() in
  beforeEach to isolate mock call state across test cases

Author: PierreBrisorgueil

lib/services/tests/analytics.identify.unit.tests.js

@@ -47,15 +47,12 @@ describe('Analytics identify on auth events:', () => {
         },
       }));
 
-      jest.unstable_mockModule('../../../modules/auth/services/auth.invitation.service.js', () => ({
+      // auth.controller imports the generic eligibility registry (not invitation code).
+      jest.unstable_mockModule('../../../modules/auth/services/auth.eligibility.js', () => ({
         default: {
-          findValid: jest.fn().mockResolvedValue(null),
-          findValidByEmail: jest.fn().mockResolvedValue(null),
-          consume: jest.fn().mockResolvedValue(null),
-          create: jest.fn(),
-          list: jest.fn(),
-          get: jest.fn(),
-          revoke: jest.fn(),
+          registerSignupEligibility: jest.fn(),
+          assertSignupEligible: jest.fn().mockResolvedValue(undefined),
+          _reset: jest.fn(),
         },
       }));
 
@@ -243,15 +240,12 @@ describe('Analytics identify on auth events:', () => {
         default: { getBrut: jest.fn(), update: jest.fn(), count: jest.fn().mockResolvedValue(0) },
       }));
 
-      jest.unstable_mockModule('../../../modules/auth/services/auth.invitation.service.js', () => ({
+      // auth.controller imports the generic eligibility registry (not invitation code).
+      jest.unstable_mockModule('../../../modules/auth/services/auth.eligibility.js', () => ({
         default: {
-          findValid: jest.fn().mockResolvedValue(null),
-          findValidByEmail: jest.fn().mockResolvedValue(null),
-          consume: jest.fn().mockResolvedValue(null),
-          create: jest.fn(),
-          list: jest.fn(),
-          get: jest.fn(),
-          revoke: jest.fn(),
+          registerSignupEligibility: jest.fn(),
+          assertSignupEligible: jest.fn().mockResolvedValue(undefined),
+          _reset: jest.fn(),
         },
       }));
 
@@ -359,15 +353,12 @@ describe('Analytics identify on auth events:', () => {
         default: { getBrut: jest.fn(), update: jest.fn(), count: jest.fn().mockResolvedValue(0) },
       }));
 
-      jest.unstable_mockModule('../../../modules/auth/services/auth.invitation.service.js', () => ({
+      // auth.controller imports the generic eligibility registry (not invitation code).
+      jest.unstable_mockModule('../../../modules/auth/services/auth.eligibility.js', () => ({
         default: {
-          findValid: jest.fn().mockResolvedValue(null),
-          findValidByEmail: jest.fn().mockResolvedValue(null),
-          consume: jest.fn().mockResolvedValue(null),
-          create: jest.fn(),
-          list: jest.fn(),
-          get: jest.fn(),
-          revoke: jest.fn(),
+          registerSignupEligibility: jest.fn(),
+          assertSignupEligible: jest.fn().mockResolvedValue(undefined),
+          _reset: jest.fn(),
         },
       }));
 

modules/auth/config/auth.development.config.js

@@ -14,7 +14,7 @@ const config = {
     in: true, // disable signin
     up: true, // disable signup
     cap: null, // null = unlimited; integer = hard ceiling on TOTAL accounts (invited included)
-    inviteExpiresInDays: 14, // signup invite link validity
+    // inviteExpiresInDays moved to modules/invitations/config (deep-merged into config.sign)
   },
   // jwt is for token authentication
   jwt: {

modules/auth/controllers/auth.controller.js

@@ -6,7 +6,7 @@ import passport from 'passport';
 import jwt from 'jsonwebtoken';
 
 import UserService from '../../users/services/users.service.js';
-import InvitationService from '../services/auth.invitation.service.js';
+import Eligibility from '../services/auth.eligibility.js';
 import { computeSignupCapacity } from '../services/auth.signupCapacity.js';
 import config from '../../../config/index.js';
 import model from '../../../lib/middlewares/model.js';
@@ -68,19 +68,18 @@ const signup = async (req, res) => {
   try {
     // Two AND-ed gates: (1) capacity — a hard ceiling on total accounts,
     // invited users included; (2) eligibility — public signup open OR a valid
-    // invite token (read from query: model.isValid strips unknown body keys).
+    // invite token. The eligibility check is supplied by optional modules via the
+    // generic registry (auth never imports invitation code). The invitations
+    // checker resolves the email-pinned invite and RETURNS `{ invite, consume }`,
+    // which auth relays back here verbatim (opaque result) so this controller can
+    // canonicalize the account email + consume it below.
     // Short-circuit count() when cap is not set (null = unlimited) to avoid a
     // collection-wide count on every signup request for uncapped deployments.
     const cap = config.sign.cap != null ? Number(config.sign.cap) : null;
     const capReached = cap != null && Number.isFinite(cap) && (await UserService.count()) >= cap;
-    let invite = null;
-    if (req.query?.inviteToken) {
-      invite = await InvitationService.findValid(req.query.inviteToken, req.body.email);
-      // An invite is bound to its email; never honor it for a signup that supplies
-      // no email to match. findValid stays lenient on a falsy email because the
-      // public verify endpoint reuses it, so enforce the pin here.
-      if (invite && invite.email && !req.body.email) invite = null;
-    }
+    const eligibility = await Eligibility.assertSignupEligible({ email: req.body.email, body: req.body, req });
+    // null when no optional module opened the gate (registry empty or no valid invite).
+    const invite = eligibility?.invite || null;
     if (capReached || (!config.sign.up && !invite)) {
       return responses.error(res, 404, 'Signup error', 'Registration is currently deactivated')();
     }
@@ -149,7 +148,9 @@ const signup = async (req, res) => {
     // Single-use: consume only when the invite actually opened the gate (signup
     // was closed, so the invite was required). When signup is open, a token can
     // be presented but is not required — consuming it would silently burn it.
-    if (invite && !config.sign.up) await InvitationService.consume(invite.id);
+    // Consume runs through the closure returned by the eligibility checker
+    // (invitations module owns consume; auth never imports invitation code).
+    if (invite && !config.sign.up) await eligibility?.consume?.();
 
     const token = jwt.sign({ userId: user.id }, config.jwt.secret, {
       expiresIn: config.jwt.expiresIn,
@@ -443,7 +444,20 @@ const checkOAuthUserProfile = async (profil, key, provider) => {
     // provider returning an arbitrary unverified email could open the gate on an
     // invite meant for someone else.
     const hasVerifiedProviderEmail = !!(profil.email && profil.emailVerifiedByProvider);
-    const oauthInvite = config.sign.up || !hasVerifiedProviderEmail ? null : await InvitationService.findValidByEmail(profil.email);
+    // Eligibility via the generic hook (auth never imports invitation code). OAuth
+    // carries no token through the redirect, so the invite is matched on the
+    // provider-verified email — the checker honors it only when emailVerifiedByProvider.
+    // Skip the hook entirely when signup is open: a token/invite is presented but not
+    // required, and resolving one would silently burn it (preserves prior short-circuit).
+    // The checker returns `{ invite, consume }` (opaque to auth); no synthetic req carrier.
+    let oauthEligibility = null;
+    if (!config.sign.up) {
+      oauthEligibility = await Eligibility.assertSignupEligible({
+        email: profil.email,
+        oauth: { emailVerifiedByProvider: hasVerifiedProviderEmail },
+      });
+    }
+    const oauthInvite = oauthEligibility?.invite || null;
     if (capReached || (!config.sign.up && !oauthInvite)) {
       // Mirror the local signup endpoint's error shape so clients see the same
       // `message`/`description` regardless of signup method (see `signup` above).
@@ -468,7 +482,8 @@ const checkOAuthUserProfile = async (profil, key, provider) => {
     // else return req.body with the data after Zod validation
     if (oauthInvite) result.value.email = oauthInvite.email;
     const createdUser = await UserService.create(result.value);
-    if (oauthInvite) await InvitationService.consume(oauthInvite.id);
+    // Consume through the returned closure (invitations owns consume; auth stays import-free).
+    if (oauthInvite) await oauthEligibility?.consume?.();
     return createdUser;
   } catch (err) {
     if (err instanceof AppError) throw err;

modules/auth/routes/auth.routes.js

@@ -9,8 +9,16 @@ 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';
-import invitations from '../controllers/auth.invitation.controller.js';
-import InvitationSchema from '../models/auth.invitation.schema.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.
+// 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
+// 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.
+import invitations from '../../invitations/controllers/invitations.controller.js';
+import InvitationSchema from '../../invitations/models/invitations.schema.js';
 
 /**
  * Register authentication routes on the Express application.
@@ -20,9 +28,13 @@ import InvitationSchema from '../models/auth.invitation.schema.js';
 export default (app) => {
   const authLimiter = limiters.auth;
 
-  // Signup invitations — public verify + admin CRUD. MUST be declared before the
-  // greedy `/api/auth/:strategy` wildcard below, which would otherwise capture
-  // `/api/auth/invitations`.
+  // Signup invitations — DEPRECATION ALIAS for the canonical /api/invitations mount
+  // (modules/invitations). MUST be declared before the greedy `/api/auth/:strategy`
+  // wildcard below, which would otherwise capture `/api/auth/invitations`. A separate
+  // glob-loaded module cannot guarantee it loads before that wildcard, so the alias
+  // stays here, pointing at the MOVED controller. The `invitationId` param callback is
+  // registered once (app-global) by the canonical invitations.routes.js, so it resolves
+  // for this alias route too — no need (and harmful: double DB lookup) to re-register it.
   app.route('/api/auth/invitations/verify/:token').get(authLimiter, invitations.verify);
   app
     .route('/api/auth/invitations')
@@ -33,7 +45,6 @@ export default (app) => {
     .route('/api/auth/invitations/:invitationId')
     .all(passport.authenticate('jwt', { session: false }), policy.isAllowed)
     .delete(invitations.remove);
-  app.param('invitationId', invitations.invitationByID);
 
   // Auth config — optional JWT: public fields for everyone, org details for authenticated users
   /**

modules/auth/services/auth.eligibility.js

@@ -0,0 +1,63 @@
+/**
+ * Generic signup-eligibility hook registry.
+ *
+ * Inverts the auth → invitations dependency: auth never imports invitation code.
+ * Optional modules (e.g. invitations) register a check at boot via
+ * registerSignupEligibility; auth runs them all in the signup / OAuth paths.
+ *
+ * Each check (a) may THROW an AppError to block signup, and (b) may RETURN a
+ * result object that auth hands back verbatim to whoever cares. The result is
+ * OPAQUE to auth — auth never reads its shape, it just relays the first non-null
+ * one. This is the return-value seam that replaces stashing carrier props on req.
+ */
+
+const checks = [];
+
+/**
+ * Register a signup eligibility check. Each fn(ctx) may throw an AppError to block
+ * signup, and/or return a result object (opaque to auth) — or undefined.
+ * @param {(ctx: { email?: string, body?: Object, req?: Object, oauth?: Object }) => (any|Promise<any>)} fn
+ * @returns {void}
+ */
+export const registerSignupEligibility = (fn) => {
+  if (typeof fn !== 'function') {
+    throw new TypeError('registerSignupEligibility: fn must be a function');
+  }
+  checks.push(fn);
+};
+
+/**
+ * Run EVERY registered eligibility check with the same ctx. A check may throw
+ * (to block signup — propagated to the caller) and/or return a result object.
+ * Returns the FIRST non-null result collected across all checks (in registration
+ * order), or null when no check returned one. The result is opaque to auth: it is
+ * handed straight back to the caller (e.g. the invitations checker returns
+ * `{ invite, consume }`, which auth relays without importing any invitation code).
+ *
+ * NOTE for future check authors (P5/P8 will register more): a THROW from ANY
+ * check aborts the whole chain and blocks signup — even a check that runs AFTER
+ * an earlier one already resolved a non-null result. Throwing is "block signup",
+ * not "I have no opinion"; return `undefined` for the latter. Order matters.
+ * @param {{ email?: string, body?: Object, req?: Object, oauth?: Object }} ctx
+ * @returns {Promise<any|null>} the first non-null check result, or null
+ */
+export const assertSignupEligible = async (ctx) => {
+  let result = null;
+  for (const check of checks) {
+    const r = await check(ctx);
+    // Loose `== null`: a check returning either undefined or null counts as "no
+    // result" (a check can `return;` to mean "no opinion"). First non-null wins.
+    if (result == null && r != null) result = r;
+  }
+  return result;
+};
+
+/**
+ * Test helper — clears the registry (mirrors discoverPolicies registry clearing).
+ * @returns {void}
+ */
+export const _reset = () => {
+  checks.length = 0;
+};
+
+export default { registerSignupEligibility, assertSignupEligible, _reset };

modules/auth/tests/auth.config.controller.unit.tests.js

@@ -37,8 +37,13 @@ describe('auth.controller getConfig:', () => {
     jest.unstable_mockModule('../../../modules/users/services/users.service.js', () => ({
       default: { create: jest.fn(), getBrut: jest.fn(), update: jest.fn(), remove: jest.fn(), search: jest.fn(), count: jest.fn().mockResolvedValue(0) },
     }));
-    jest.unstable_mockModule('../../../modules/auth/services/auth.invitation.service.js', () => ({
-      default: { findValid: jest.fn().mockResolvedValue(null), consume: jest.fn().mockResolvedValue(null) },
+    // auth.controller imports the generic eligibility registry (not invitation code).
+    jest.unstable_mockModule('../../../modules/auth/services/auth.eligibility.js', () => ({
+      default: {
+        registerSignupEligibility: jest.fn(),
+        assertSignupEligible: jest.fn().mockResolvedValue(undefined),
+        _reset: jest.fn(),
+      },
     }));
     jest.unstable_mockModule('../../../modules/users/repositories/users.repository.js', () => ({
       default: { update: jest.fn() },

modules/auth/tests/auth.eligibility.unit.tests.js

@@ -0,0 +1,77 @@
+import { jest } from '@jest/globals';
+
+const Eligibility = (await import('../services/auth.eligibility.js')).default;
+const { registerSignupEligibility, assertSignupEligible, _reset } = await import('../services/auth.eligibility.js');
+
+beforeEach(() => {
+  _reset();
+});
+
+describe('auth.eligibility registry', () => {
+  test('assertSignupEligible returns null when no checks are registered', async () => {
+    await expect(assertSignupEligible({ email: 'a@b.co' })).resolves.toBeNull();
+  });
+
+  test('returns null when every registered check returns nothing', async () => {
+    registerSignupEligibility(() => {});
+    registerSignupEligibility(async () => undefined);
+    await expect(assertSignupEligible({ email: 'a@b.co' })).resolves.toBeNull();
+  });
+
+  test('runs every registered check, in order, with the same ctx', async () => {
+    const order = [];
+    const ctx = { email: 'a@b.co', body: { x: 1 }, req: {} };
+    registerSignupEligibility((c) => { order.push(['a', c]); });
+    registerSignupEligibility(async (c) => { order.push(['b', c]); });
+    await assertSignupEligible(ctx);
+    expect(order.map((o) => o[0])).toEqual(['a', 'b']);
+    expect(order[0][1]).toBe(ctx);
+    expect(order[1][1]).toBe(ctx);
+  });
+
+  test('returns the first non-null check result and still runs every check', async () => {
+    const r1 = { invite: { id: 'a' } };
+    const r2 = { invite: { id: 'b' } };
+    const third = jest.fn();
+    registerSignupEligibility(() => undefined); // contributes no result
+    registerSignupEligibility(() => r1); // first non-null → this one wins
+    registerSignupEligibility(() => r2); // later non-null is ignored
+    registerSignupEligibility(third); // but EVERY check still runs (side-effects fire)
+    const result = await assertSignupEligible({});
+    expect(result).toBe(r1);
+    expect(third).toHaveBeenCalledTimes(1);
+  });
+
+  test('registerSignupEligibility throws TypeError when fn is not a function', () => {
+    expect(() => registerSignupEligibility('not-a-function')).toThrow(TypeError);
+    expect(() => registerSignupEligibility(null)).toThrow(TypeError);
+    expect(() => registerSignupEligibility(42)).toThrow(TypeError);
+  });
+
+  test('a throwing check aborts the chain (blocks signup) and propagates', async () => {
+    const second = jest.fn();
+    registerSignupEligibility(() => { throw new Error('blocked'); });
+    registerSignupEligibility(second);
+    await expect(assertSignupEligible({})).rejects.toThrow('blocked');
+    expect(second).not.toHaveBeenCalled();
+  });
+
+  test('a rejecting async check propagates', async () => {
+    registerSignupEligibility(async () => { throw new Error('async-blocked'); });
+    await expect(assertSignupEligible({})).rejects.toThrow('async-blocked');
+  });
+
+  test('_reset clears the registry', async () => {
+    const check = jest.fn();
+    registerSignupEligibility(check);
+    _reset();
+    await assertSignupEligible({});
+    expect(check).not.toHaveBeenCalled();
+  });
+
+  test('default export exposes the same functions', () => {
+    expect(typeof Eligibility.registerSignupEligibility).toBe('function');
+    expect(typeof Eligibility.assertSignupEligible).toBe('function');
+    expect(typeof Eligibility._reset).toBe('function');
+  });
+});

modules/auth/tests/auth.signout.controller.unit.tests.js

@@ -22,15 +22,12 @@ describe('auth.controller signout:', () => {
     jest.unstable_mockModule('../../../modules/users/services/users.service.js', () => ({
       default: { create: jest.fn(), getBrut: jest.fn(), update: jest.fn(), remove: jest.fn(), search: jest.fn(), count: jest.fn().mockResolvedValue(0) },
     }));
-    jest.unstable_mockModule('../../../modules/auth/services/auth.invitation.service.js', () => ({
+    // auth.controller imports the generic eligibility registry (not invitation code).
+    jest.unstable_mockModule('../../../modules/auth/services/auth.eligibility.js', () => ({
       default: {
-        findValid: jest.fn().mockResolvedValue(null),
-        findValidByEmail: jest.fn().mockResolvedValue(null),
-        consume: jest.fn().mockResolvedValue(null),
-        create: jest.fn(),
-        list: jest.fn(),
-        get: jest.fn(),
-        revoke: jest.fn(),
+        registerSignupEligibility: jest.fn(),
+        assertSignupEligible: jest.fn().mockResolvedValue(undefined),
+        _reset: jest.fn(),
       },
     }));
     jest.unstable_mockModule('../../../modules/users/repositories/users.repository.js', () => ({

modules/auth/tests/auth.silent.catch.unit.tests.js

@@ -39,15 +39,13 @@ describe('auth.controller silent-catch error logging:', () => {
         },
       }));
 
-      jest.unstable_mockModule('../../../modules/auth/services/auth.invitation.service.js', () => ({
+      // auth.controller no longer imports invitation code — it runs the generic
+      // eligibility registry. Mock it inert (no checks registered ⇒ no invite stashed).
+      jest.unstable_mockModule('../../../modules/auth/services/auth.eligibility.js', () => ({
         default: {
-          findValid: jest.fn().mockResolvedValue(null),
-          findValidByEmail: jest.fn().mockResolvedValue(null),
-          consume: jest.fn().mockResolvedValue(null),
-          create: jest.fn(),
-          list: jest.fn(),
-          get: jest.fn(),
-          revoke: jest.fn(),
+          registerSignupEligibility: jest.fn(),
+          assertSignupEligible: jest.fn().mockResolvedValue(undefined),
+          _reset: jest.fn(),
         },
       }));