From d10778e64cded71e2ac5b60593307c0c5a5dd5d7 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 13 Jun 2026 14:41:29 +0200 Subject: [PATCH 1/6] fix(invitations): guard revoke to pending-only invitations Revoking an accepted invitation flipped it to revoked and silently dropped it out of the accepted set used for referral attribution. The repository CAS now filters status:'pending'; the controller maps a null update on an existing invitation to 409 Conflict. The revoke-mid-claim race is benign and documented at the CAS. refs #3834 --- .../controllers/invitations.controller.js | 13 ++++++-- .../repositories/invitations.repository.js | 16 ++++++++-- .../services/invitations.service.js | 2 +- .../invitations.controller.unit.tests.js | 6 ++++ .../tests/invitations.integration.tests.js | 31 +++++++++++++++++++ .../invitations.repository.unit.tests.js | 6 ++-- 6 files changed, 66 insertions(+), 8 deletions(-) diff --git a/modules/invitations/controllers/invitations.controller.js b/modules/invitations/controllers/invitations.controller.js index f951d0740..2bd6fea1e 100644 --- a/modules/invitations/controllers/invitations.controller.js +++ b/modules/invitations/controllers/invitations.controller.js @@ -40,16 +40,23 @@ const list = async (req, res) => { }; /** - * @desc Admin: revoke an invitation + * @desc Admin: revoke a PENDING invitation (pending-only guard in the repository) * @param {Object} req - Express request object * @param {Object} req.invitation - Loaded invitation document (set by invitationByID middleware) * @param {string} req.invitation.id - Invitation id * @param {Object} res - Express response object - * @returns {Promise} Sends HTTP 200 with deleted id or 422 on error + * @returns {Promise} Sends HTTP 200 with deleted id, 409 when not pending, or 422 on error */ const remove = async (req, res) => { try { - await InvitationService.revoke(req.invitation.id); + const revoked = await InvitationService.revoke(req.invitation.id); + if (!revoked) { + // The invitation EXISTS (invitationByID loaded it) but the guarded CAS + // matched nothing → it is no longer pending (accepted or already + // revoked). Refuse: revoking an accepted invite would silently drop it + // from the accepted set used for referral attribution. + return responses.error(res, 409, 'Conflict', 'Only pending invitations can be revoked')(); + } responses.success(res, 'invitation deleted')({ id: req.invitation.id }); } catch (err) { responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err); diff --git a/modules/invitations/repositories/invitations.repository.js b/modules/invitations/repositories/invitations.repository.js index 89f68796c..807040e5e 100644 --- a/modules/invitations/repositories/invitations.repository.js +++ b/modules/invitations/repositories/invitations.repository.js @@ -129,12 +129,24 @@ const get = (id) => (mongoose.Types.ObjectId.isValid(id) ? Invitation.findById(i * revokedAt instead of deleting, so invitedBy/acceptedUserId survive for the * referral phase. A revoked invite never re-opens the gate (findValid filters * on status:'pending'). + * + * Guarded on status:'pending': an ACCEPTED invite must never flip to 'revoked' + * — it would silently drop out of the accepted set findAccepted() scans for + * referral attribution. Returns null when the row exists but is no longer + * pending; the controller maps that to 409. + * + * Benign race (revoke-mid-claim): a signup can claim/finalize between the + * controller's param-load and this CAS — the filter then matches nothing and + * we return null (→ 409) instead of corrupting the just-accepted invite. The + * inverse interleaving (revoke lands first, mid-claim) is also safe: finalize + * is guarded on status:{$ne:'revoked'}, so the in-flight accept no-ops with a + * logged warning and the invite stays revoked. * @param {String} id - * @returns {Promise} the revoked doc, or null when no such id + * @returns {Promise} the revoked doc, or null when missing / not pending */ const revoke = (id) => Invitation.findOneAndUpdate( - { _id: id }, + { _id: id, status: 'pending' }, { $set: { status: 'revoked', revokedAt: new Date() } }, { returnDocument: 'after' }, ).exec(); diff --git a/modules/invitations/services/invitations.service.js b/modules/invitations/services/invitations.service.js index 1860c2a15..deac55cd1 100644 --- a/modules/invitations/services/invitations.service.js +++ b/modules/invitations/services/invitations.service.js @@ -339,7 +339,7 @@ const get = (id) => InvitationRepository.get(id); * @desc E8 — soft-delete (revoke) an invitation by id (status:'revoked' + revokedAt, * NOT a hard delete), preserving invitedBy/acceptedUserId for the referral phase. * @param {String} id - * @returns {Promise} the revoked invitation + * @returns {Promise} the revoked invitation, or null when not pending */ const revoke = (id) => InvitationRepository.revoke(id); diff --git a/modules/invitations/tests/invitations.controller.unit.tests.js b/modules/invitations/tests/invitations.controller.unit.tests.js index 73515632b..44f691655 100644 --- a/modules/invitations/tests/invitations.controller.unit.tests.js +++ b/modules/invitations/tests/invitations.controller.unit.tests.js @@ -62,6 +62,12 @@ describe('invitations.controller.remove', () => { expect(mockService.revoke).toHaveBeenCalledWith('i9'); expect(successInner).toHaveBeenCalledWith({ id: 'i9' }); }); + test('responds 409 Conflict when revoke matches no pending row (accepted / already revoked)', async () => { + mockService.revoke.mockResolvedValue(null); + await controller.remove({ invitation: { id: 'i9' } }, makeRes()); + expect(error).toHaveBeenCalledWith(expect.anything(), 409, 'Conflict', 'Only pending invitations can be revoked'); + expect(success).not.toHaveBeenCalled(); + }); }); describe('invitations.controller.verify', () => { diff --git a/modules/invitations/tests/invitations.integration.tests.js b/modules/invitations/tests/invitations.integration.tests.js index 74660366f..2102b1fcb 100644 --- a/modules/invitations/tests/invitations.integration.tests.js +++ b/modules/invitations/tests/invitations.integration.tests.js @@ -814,4 +814,35 @@ describe('Signup invitations:', () => { expect(res.status).toBe(200); }); }); + + describe('Invitation ops — revoke guard, duplicate-pending, resend', () => { + let mails; + + beforeAll(async () => { + mails = (await import(path.resolve('./lib/helpers/mailer/index.js'))).default; + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('revoking an ACCEPTED invitation is refused with 409 (referral attribution preserved)', async () => { + const adminAgent = await createAdminAndSignin(); + const created = await adminAgent.post('/api/invitations').send({ email: 'ops-accepted@example.com' }); + const id = created.body.data.id; + + // Flip to accepted directly — the full signup-accept path is covered elsewhere. + const Invitation = (await import('mongoose')).default.model('Invitation'); + await Invitation.updateOne({ _id: id }, { $set: { status: 'accepted', acceptedAt: new Date() } }).exec(); + + const res = await adminAgent.delete(`/api/invitations/${id}`); + expect(res.status).toBe(409); + expect(res.body.description).toBe('Only pending invitations can be revoked'); + + // The accepted row is untouched — still in the accepted set. + const after = await Invitation.findById(id).exec(); + expect(after.status).toBe('accepted'); + expect(after.revokedAt).toBeNull(); + }); + }); }); diff --git a/modules/invitations/tests/invitations.repository.unit.tests.js b/modules/invitations/tests/invitations.repository.unit.tests.js index 0e8a81ea9..0956a93d1 100644 --- a/modules/invitations/tests/invitations.repository.unit.tests.js +++ b/modules/invitations/tests/invitations.repository.unit.tests.js @@ -160,11 +160,13 @@ describe('InvitationRepository', () => { expect(result).toEqual({ id: 'i1' }); }); - test('revoke soft-deletes by id (status:revoked + revokedAt), not deleteOne (E8)', async () => { + test('revoke soft-deletes by id GUARDED on status:pending (E8 + revoke-guard), not deleteOne', async () => { exec.mockResolvedValue({ id: 'i1', status: 'revoked' }); const result = await InvitationRepository.revoke('i1'); const [filter, update] = InvitationModel.findOneAndUpdate.mock.calls.at(-1); - expect(filter).toEqual({ _id: 'i1' }); + // An accepted invite must never flip to revoked — that would silently drop + // it out of the accepted set findAccepted() scans for referral attribution. + expect(filter).toEqual({ _id: 'i1', status: 'pending' }); expect(update.$set.status).toBe('revoked'); expect(update.$set).toHaveProperty('revokedAt'); expect(InvitationModel.deleteOne).not.toHaveBeenCalled(); From 623a5c4c8ebdc37f4748b98122a2ffa57038d62e Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 13 Jun 2026 14:43:55 +0200 Subject: [PATCH 2/6] fix(invitations): reject duplicate pending invitation with 409 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create() allowed N live tokens for the same email — any one of them opens the signup gate. Guard on the existing pending-only findByEmail (expired/revoked/accepted invites never block a re-invite) and thread the 409 through the controller instead of the hardcoded 422. refs #3834 --- .../controllers/invitations.controller.js | 9 +++++++-- .../invitations/services/invitations.service.js | 14 ++++++++++++++ .../tests/invitations.controller.unit.tests.js | 5 +++++ .../tests/invitations.integration.tests.js | 17 +++++++++++++++++ .../tests/invitations.service.unit.tests.js | 16 +++++++++++++++- 5 files changed, 58 insertions(+), 3 deletions(-) diff --git a/modules/invitations/controllers/invitations.controller.js b/modules/invitations/controllers/invitations.controller.js index 2bd6fea1e..4acda5e48 100644 --- a/modules/invitations/controllers/invitations.controller.js +++ b/modules/invitations/controllers/invitations.controller.js @@ -11,14 +11,19 @@ import InvitationService from '../services/invitations.service.js'; * @param {string} req.body.email - Email address to invite * @param {Object} req.user - Authenticated admin user * @param {Object} res - Express response object - * @returns {Promise} Sends HTTP 200 with created invitation or 422 on error + * @returns {Promise} Sends HTTP 200 with created invitation, 409 on duplicate pending, or 422 on error */ const create = async (req, res) => { try { const invitation = await InvitationService.create(req.body.email, req.user); responses.success(res, 'invitation created')(invitation); } catch (err) { - responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err); + // Thread the service-thrown status — a 409 (duplicate pending) must not + // flatten to the legacy hardcoded 422. Anything else keeps the + // historical 422 mapping (same style as the billing admin controller). + const status = err.status === 409 ? 409 : 422; + const title = status === 409 ? 'Conflict' : 'Unprocessable Entity'; + responses.error(res, status, title, errors.getMessage(err))(err); } }; diff --git a/modules/invitations/services/invitations.service.js b/modules/invitations/services/invitations.service.js index deac55cd1..87e40a14d 100644 --- a/modules/invitations/services/invitations.service.js +++ b/modules/invitations/services/invitations.service.js @@ -51,6 +51,20 @@ const create = async (email, invitedBy) => { details: { message: 'A user with this email already exists.' }, }); } + // One LIVE invite per email. findByEmail is already pending-only + // (status:'pending', unused, unclaimed, unexpired), so an expired, revoked + // or accepted prior invite never blocks a re-invite — only a genuinely + // outstanding one does. Best-effort guard (no partial unique index on + // email+status): two racing creates can still both land; acceptable for an + // admin/owner surface, and any one resolved token still single-uses. + const outstanding = await InvitationRepository.findByEmail(normalizedEmail); + if (outstanding) { + throw new AppError('A pending invitation already exists for this email', { + status: 409, + code: 'CONFLICT', + details: { message: 'A pending invitation already exists for this email.' }, + }); + } const token = crypto.randomBytes(20).toString('hex'); const days = config.sign?.inviteExpiresInDays || DEFAULT_INVITE_EXPIRES_IN_DAYS; const expiresAt = new Date(Date.now() + days * 24 * 3600000); diff --git a/modules/invitations/tests/invitations.controller.unit.tests.js b/modules/invitations/tests/invitations.controller.unit.tests.js index 44f691655..772946568 100644 --- a/modules/invitations/tests/invitations.controller.unit.tests.js +++ b/modules/invitations/tests/invitations.controller.unit.tests.js @@ -43,6 +43,11 @@ describe('invitations.controller.create', () => { await controller.create({ body: { email: 'a@b.co' }, user: {} }, makeRes()); expect(error).toHaveBeenCalledWith(expect.anything(), 422, 'Unprocessable Entity', 'boom'); }); + test('threads a service 409 (duplicate pending) through as Conflict', async () => { + mockService.create.mockRejectedValue(Object.assign(new Error('A pending invitation already exists for this email'), { status: 409 })); + await controller.create({ body: { email: 'a@b.co' }, user: {} }, makeRes()); + expect(error).toHaveBeenCalledWith(expect.anything(), 409, 'Conflict', 'A pending invitation already exists for this email'); + }); }); describe('invitations.controller.list', () => { diff --git a/modules/invitations/tests/invitations.integration.tests.js b/modules/invitations/tests/invitations.integration.tests.js index 2102b1fcb..95e933149 100644 --- a/modules/invitations/tests/invitations.integration.tests.js +++ b/modules/invitations/tests/invitations.integration.tests.js @@ -844,5 +844,22 @@ describe('Signup invitations:', () => { expect(after.status).toBe('accepted'); expect(after.revokedAt).toBeNull(); }); + + test('a second invite for the SAME email is refused with 409 while the first is pending', async () => { + const adminAgent = await createAdminAndSignin(); + const first = await adminAgent.post('/api/invitations').send({ email: 'ops-dup@example.com' }); + expect(first.status).toBe(200); + + // Case-insensitive: the service lowercases before the guard. + const second = await adminAgent.post('/api/invitations').send({ email: 'OPS-DUP@example.com' }); + expect(second.status).toBe(409); + expect(second.body.description).toBe('A pending invitation already exists for this email.'); + + // Revoking the pending invite re-opens creation (findByEmail is pending-only). + const revoked = await adminAgent.delete(`/api/invitations/${first.body.data.id}`); + expect(revoked.status).toBe(200); + const third = await adminAgent.post('/api/invitations').send({ email: 'ops-dup@example.com' }); + expect(third.status).toBe(200); + }); }); }); diff --git a/modules/invitations/tests/invitations.service.unit.tests.js b/modules/invitations/tests/invitations.service.unit.tests.js index 196ecae63..efbd91240 100644 --- a/modules/invitations/tests/invitations.service.unit.tests.js +++ b/modules/invitations/tests/invitations.service.unit.tests.js @@ -45,8 +45,11 @@ const InvitationService = (await import('../services/invitations.service.js')).d const invitationEvents = (await import('../lib/events.js')).default; beforeEach(() => { - // Default: no stale claims to sweep, no pre-existing user (E9). + // Default: no stale claims to sweep, no pre-existing user (E9), no outstanding + // pending invite (the duplicate-pending guard). clearMocks resets calls but NOT + // mockResolvedValue, so a prior test's findByEmail value would otherwise leak in. InvitationRepository.releaseStaleClaims.mockResolvedValue({ modifiedCount: 0 }); + InvitationRepository.findByEmail.mockResolvedValue(undefined); mockUserService.findByEmail.mockResolvedValue(null); mockUserService.updateById.mockResolvedValue({}); }); @@ -317,6 +320,17 @@ describe('InvitationService.create', () => { expect(mockUserService.findByEmail).toHaveBeenCalledWith('taken@example.com'); expect(InvitationRepository.create).not.toHaveBeenCalled(); }); + + test('rejects (409 CONFLICT) when a PENDING invitation already exists for the email', async () => { + InvitationRepository.findByEmail.mockResolvedValue({ id: 'i0', email: 'dup@example.com', status: 'pending' }); + await expect(InvitationService.create('Dup@Example.com', { id: 'admin1' })).rejects.toMatchObject({ + status: 409, + code: 'CONFLICT', + }); + // checks the lowercased email + never persists a second live invite + expect(InvitationRepository.findByEmail).toHaveBeenCalledWith('dup@example.com'); + expect(InvitationRepository.create).not.toHaveBeenCalled(); + }); }); describe('InvitationService.create — #3833 self-invite guard', () => { From 73e6a3a8d4f7895de5aafa67739ca071d30a9192 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 13 Jun 2026 14:46:25 +0200 Subject: [PATCH 3/6] feat(invitations): resend endpoint for pending invitations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/invitations/:invitationId/resend (admin CASL chain, also on the legacy /api/auth/invitations alias for mount consistency). Re-sends the signup-invite email with the EXISTING token — the list endpoint strips tokens, so a lost email previously meant a dead invite. 409 when not pending, 422 when the mailer is unconfigured; the send is awaited (the email IS the operation). refs #3834 --- modules/auth/routes/auth.routes.js | 6 +- .../controllers/invitations.controller.js | 21 +++++- .../invitations/routes/invitations.routes.js | 6 ++ .../services/invitations.service.js | 66 ++++++++++++++++--- .../invitations.controller.unit.tests.js | 21 ++++++ .../tests/invitations.integration.tests.js | 42 ++++++++++++ .../tests/invitations.service.unit.tests.js | 42 ++++++++++++ 7 files changed, 192 insertions(+), 12 deletions(-) diff --git a/modules/auth/routes/auth.routes.js b/modules/auth/routes/auth.routes.js index a5ba69ffc..cefc6c996 100644 --- a/modules/auth/routes/auth.routes.js +++ b/modules/auth/routes/auth.routes.js @@ -9,7 +9,7 @@ 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'; -// TODO(P9, pierreb-devkit/Node#3815): Remove these two imports and the three routes +// TODO(P9, pierreb-devkit/Node#3815): Remove these two imports and the four routes // below ONLY after (a) downstream (Trawl) has absorbed Vue P6 via /update-stack AND // (b) Vue auth.store `verifyInvite` is repointed to the canonical // /api/invitations/verify/:token (Vue pre-P9 fix PR) — it still calls the alias. @@ -47,6 +47,10 @@ export default (app) => { .route('/api/auth/invitations/:invitationId') .all(passport.authenticate('jwt', { session: false }), policy.isAllowed) .delete(invitations.remove); + app + .route('/api/auth/invitations/:invitationId/resend') + .all(passport.authenticate('jwt', { session: false }), policy.isAllowed) + .post(invitations.resend); // Auth config — optional JWT: public fields for everyone, org details for authenticated users /** diff --git a/modules/invitations/controllers/invitations.controller.js b/modules/invitations/controllers/invitations.controller.js index 4acda5e48..a2ec55ea8 100644 --- a/modules/invitations/controllers/invitations.controller.js +++ b/modules/invitations/controllers/invitations.controller.js @@ -68,6 +68,25 @@ const remove = async (req, res) => { } }; +/** + * @desc Admin: re-send the invitation email for a pending invitation (existing token) + * @param {Object} req - Express request object + * @param {Object} req.invitation - Loaded invitation document (set by invitationByID middleware) + * @param {string} req.invitation.id - Invitation id + * @param {Object} res - Express response object + * @returns {Promise} Sends HTTP 200 with the invitation, 409 when not pending, or 422 on error + */ +const resend = async (req, res) => { + try { + const invitation = await InvitationService.resend(req.invitation.id); + responses.success(res, 'invitation resent')(invitation); + } catch (err) { + const status = err.status === 409 ? 409 : 422; + const title = status === 409 ? 'Conflict' : 'Unprocessable Entity'; + responses.error(res, status, title, errors.getMessage(err))(err); + } +}; + /** * @desc Public: report whether a token is a valid invite (+ prefill email) * @param {Object} req - Express request object @@ -103,4 +122,4 @@ const invitationByID = async (req, res, next, id) => { } }; -export default { create, list, remove, verify, invitationByID }; +export default { create, list, remove, resend, verify, invitationByID }; diff --git a/modules/invitations/routes/invitations.routes.js b/modules/invitations/routes/invitations.routes.js index ce8980f57..39aa25b74 100644 --- a/modules/invitations/routes/invitations.routes.js +++ b/modules/invitations/routes/invitations.routes.js @@ -36,5 +36,11 @@ export default (app) => { .all(passport.authenticate('jwt', { session: false }), policy.isAllowed) .delete(invitations.remove); + // Admin — re-send the invitation email (pending invites only, existing token). + app + .route('/api/invitations/:invitationId/resend') + .all(passport.authenticate('jwt', { session: false }), policy.isAllowed) + .post(invitations.resend); + app.param('invitationId', invitations.invitationByID); }; diff --git a/modules/invitations/services/invitations.service.js b/modules/invitations/services/invitations.service.js index 87e40a14d..5de58871a 100644 --- a/modules/invitations/services/invitations.service.js +++ b/modules/invitations/services/invitations.service.js @@ -13,6 +13,24 @@ import getBaseUrl from '../../../lib/helpers/getBaseUrl.js'; import logger from '../../../lib/services/logger.js'; import AppError from '../../../lib/helpers/AppError.js'; +/** + * @desc Build the signup-invite email payload for an invitation. Shared by + * create() (fire-and-forget) and resend() (awaited) so the template, subject + * and link format can never drift between the two send sites. + * @param {Object} invitation - persisted invitation (email + token) + * @returns {Object} sendMail arguments + */ +const inviteMailPayload = (invitation) => ({ + template: 'signup-invite', + to: invitation.email, + subject: `You're invited to ${config.app.title}`, + params: { + url: `${getBaseUrl()}/signup?inviteToken=${invitation.token}`, + appName: config.app.title, + appContact: config.app.contact, + }, +}); + /** * @desc Create an invitation for an email, generate a single-use token, persist, * and (best-effort) email the signup link. Token/expiry are server-controlled. @@ -76,16 +94,7 @@ const create = async (email, invitedBy) => { }); if (mails.isConfigured()) { mails - .sendMail({ - template: 'signup-invite', - to: invitation.email, - subject: `You're invited to ${config.app.title}`, - params: { - url: `${getBaseUrl()}/signup?inviteToken=${token}`, - appName: config.app.title, - appContact: config.app.contact, - }, - }) + .sendMail(inviteMailPayload(invitation)) .catch((err) => logger.warn('invitations: email failed', { message: err?.message })); } return invitation; @@ -357,6 +366,42 @@ const get = (id) => InvitationRepository.get(id); */ const revoke = (id) => InvitationRepository.revoke(id); +/** + * @desc Re-send the invitation email for a still-PENDING invitation, with the + * EXISTING token — never regenerated, so a previously emailed or copied link + * stays valid. Unlike create()'s best-effort send, the email here IS the + * operation: sendMail is awaited and a transport failure surfaces as an error. + * @param {String} id - invitation id + * @returns {Promise} the (unchanged) invitation + * @throws {AppError} 404 unknown id, 409 not pending, 422 mailer unconfigured + */ +const resend = async (id) => { + const invitation = await InvitationRepository.get(id); + if (!invitation) { + throw new AppError('no invitation with that identifier has been found', { + status: 404, + code: 'NOT_FOUND', + details: { message: 'No invitation with that identifier has been found.' }, + }); + } + if (invitation.status !== 'pending') { + throw new AppError('Only pending invitations can be resent', { + status: 409, + code: 'CONFLICT', + details: { message: 'Only pending invitations can be resent.' }, + }); + } + if (!mails.isConfigured()) { + throw new AppError('mailer is not configured', { + status: 422, + code: 'VALIDATION_ERROR', + details: { message: 'Email sending is not configured on this server.' }, + }); + } + await mails.sendMail(inviteMailPayload(invitation)); + return invitation; +}; + export default { create, findValid, @@ -371,4 +416,5 @@ export default { list, get, revoke, + resend, }; diff --git a/modules/invitations/tests/invitations.controller.unit.tests.js b/modules/invitations/tests/invitations.controller.unit.tests.js index 772946568..4891c8ecf 100644 --- a/modules/invitations/tests/invitations.controller.unit.tests.js +++ b/modules/invitations/tests/invitations.controller.unit.tests.js @@ -4,6 +4,7 @@ const mockService = { create: jest.fn(), list: jest.fn(), revoke: jest.fn(), + resend: jest.fn(), findValid: jest.fn(), get: jest.fn(), }; @@ -75,6 +76,26 @@ describe('invitations.controller.remove', () => { }); }); +describe('invitations.controller.resend', () => { + test('resends via the service and responds success', async () => { + mockService.resend.mockResolvedValue({ id: 'i1', status: 'pending' }); + await controller.resend({ invitation: { id: 'i1' } }, makeRes()); + expect(mockService.resend).toHaveBeenCalledWith('i1'); + expect(success).toHaveBeenCalledWith(expect.anything(), 'invitation resent'); + expect(successInner).toHaveBeenCalledWith({ id: 'i1', status: 'pending' }); + }); + test('threads a 409 (non-pending) as Conflict', async () => { + mockService.resend.mockRejectedValue(Object.assign(new Error('Only pending invitations can be resent'), { status: 409 })); + await controller.resend({ invitation: { id: 'i1' } }, makeRes()); + expect(error).toHaveBeenCalledWith(expect.anything(), 409, 'Conflict', expect.any(String)); + }); + test('maps a 422 (mailer unconfigured) as Unprocessable Entity', async () => { + mockService.resend.mockRejectedValue(Object.assign(new Error('mailer is not configured'), { status: 422 })); + await controller.resend({ invitation: { id: 'i1' } }, makeRes()); + expect(error).toHaveBeenCalledWith(expect.anything(), 422, 'Unprocessable Entity', expect.any(String)); + }); +}); + describe('invitations.controller.verify', () => { test('reports valid + email for a fresh token', async () => { mockService.findValid.mockResolvedValue({ email: 'a@b.co' }); diff --git a/modules/invitations/tests/invitations.integration.tests.js b/modules/invitations/tests/invitations.integration.tests.js index 95e933149..846682b73 100644 --- a/modules/invitations/tests/invitations.integration.tests.js +++ b/modules/invitations/tests/invitations.integration.tests.js @@ -861,5 +861,47 @@ describe('Signup invitations:', () => { const third = await adminAgent.post('/api/invitations').send({ email: 'ops-dup@example.com' }); expect(third.status).toBe(200); }); + + test('resend re-sends the EXISTING token for a pending invite (no regeneration)', async () => { + const adminAgent = await createAdminAndSignin(); + const created = await adminAgent.post('/api/invitations').send({ email: 'ops-resend@example.com' }); + const { id, token } = created.body.data; + + jest.spyOn(mails, 'isConfigured').mockReturnValue(true); + const sendSpy = jest.spyOn(mails, 'sendMail').mockResolvedValue({}); + + const res = await adminAgent.post(`/api/invitations/${id}/resend`); + expect(res.status).toBe(200); + expect(sendSpy).toHaveBeenCalledTimes(1); + const mail = sendSpy.mock.calls[0][0]; + expect(mail.template).toBe('signup-invite'); + expect(mail.to).toBe('ops-resend@example.com'); + expect(mail.params.url).toContain(`inviteToken=${token}`); // SAME token + }); + + test('resend guards: 422 with the mailer unconfigured, 409 once non-pending', async () => { + const adminAgent = await createAdminAndSignin(); + const created = await adminAgent.post('/api/invitations').send({ email: 'ops-resend-guards@example.com' }); + const { id } = created.body.data; + + // Test env mailer is unconfigured (placeholder `from`) → 422 with no mocks. + const unconfigured = await adminAgent.post(`/api/invitations/${id}/resend`); + expect(unconfigured.status).toBe(422); + + // Revoke it, then resend → 409 (status guard fires before the mailer guard). + await adminAgent.delete(`/api/invitations/${id}`); + const conflicted = await adminAgent.post(`/api/invitations/${id}/resend`); + expect(conflicted.status).toBe(409); + expect(conflicted.body.description).toBe('Only pending invitations can be resent.'); + }); + + test('resend resolves through the legacy /api/auth/invitations alias too (kept consistent)', async () => { + const adminAgent = await createAdminAndSignin(); + const created = await adminAgent.post('/api/invitations').send({ email: 'ops-resend-alias@example.com' }); + jest.spyOn(mails, 'isConfigured').mockReturnValue(true); + jest.spyOn(mails, 'sendMail').mockResolvedValue({}); + const res = await adminAgent.post(`/api/auth/invitations/${created.body.data.id}/resend`); + expect(res.status).toBe(200); + }); }); }); diff --git a/modules/invitations/tests/invitations.service.unit.tests.js b/modules/invitations/tests/invitations.service.unit.tests.js index efbd91240..aa1180c5d 100644 --- a/modules/invitations/tests/invitations.service.unit.tests.js +++ b/modules/invitations/tests/invitations.service.unit.tests.js @@ -407,3 +407,45 @@ describe('InvitationService.list / get / revoke', () => { expect(InvitationRepository.revoke).toHaveBeenCalledWith('id-1'); }); }); + +describe('InvitationService.resend', () => { + const pending = { id: 'i1', email: 'a@b.co', token: 'tok-1', status: 'pending' }; + beforeEach(() => { + mockMailer.isConfigured.mockReturnValue(true); + mockMailer.sendMail.mockResolvedValue({}); + }); + afterEach(() => { + mockMailer.isConfigured.mockReturnValue(false); + }); + + test('re-sends the signup-invite email with the EXISTING token and returns the invitation', async () => { + InvitationRepository.get.mockResolvedValue(pending); + const result = await InvitationService.resend('i1'); + expect(result).toBe(pending); + expect(mockMailer.sendMail).toHaveBeenCalledTimes(1); + const mail = mockMailer.sendMail.mock.calls[0][0]; + expect(mail.template).toBe('signup-invite'); + expect(mail.to).toBe('a@b.co'); + // No token regeneration: a previously emailed/shared link must stay valid. + expect(mail.params.url).toBe('http://localhost:3000/signup?inviteToken=tok-1'); + }); + + test('404 when the invitation does not exist', async () => { + InvitationRepository.get.mockResolvedValue(null); + await expect(InvitationService.resend('missing')).rejects.toMatchObject({ status: 404 }); + expect(mockMailer.sendMail).not.toHaveBeenCalled(); + }); + + test('409 when the invitation is not pending (accepted/revoked never re-email)', async () => { + InvitationRepository.get.mockResolvedValue({ ...pending, status: 'accepted' }); + await expect(InvitationService.resend('i1')).rejects.toMatchObject({ status: 409, code: 'CONFLICT' }); + expect(mockMailer.sendMail).not.toHaveBeenCalled(); + }); + + test('422 when the mailer is not configured (resend IS the email — fail loudly)', async () => { + mockMailer.isConfigured.mockReturnValue(false); + InvitationRepository.get.mockResolvedValue(pending); + await expect(InvitationService.resend('i1')).rejects.toMatchObject({ status: 422 }); + expect(mockMailer.sendMail).not.toHaveBeenCalled(); + }); +}); From 2c8fe46fefd1a2a72c29b4cbc48631bd17318607 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 13 Jun 2026 15:03:06 +0200 Subject: [PATCH 4/6] docs(auth): neutralize downstream name in deprecation-alias comment Replaces a downstream consumer name in the alias-removal TODO with neutral wording (public-OSS no-downstream-refs rule). refs #3834 --- modules/auth/routes/auth.routes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/auth/routes/auth.routes.js b/modules/auth/routes/auth.routes.js index cefc6c996..ca5267748 100644 --- a/modules/auth/routes/auth.routes.js +++ b/modules/auth/routes/auth.routes.js @@ -10,7 +10,7 @@ import UsersSchema from '../../users/models/users.schema.js'; import auth from '../controllers/auth.controller.js'; import authPassword from '../controllers/auth.password.controller.js'; // TODO(P9, pierreb-devkit/Node#3815): Remove these two imports and the four routes -// below ONLY after (a) downstream (Trawl) has absorbed Vue P6 via /update-stack AND +// below ONLY after (a) the downstream consumer has absorbed Vue P6 via /update-stack AND // (b) Vue auth.store `verifyInvite` is repointed to the canonical // /api/invitations/verify/:token (Vue pre-P9 fix PR) — it still calls the alias. // Deprecation alias only — the invitations feature lives in modules/invitations. From fecdaa15f9018e3edba35cf9e814a53770b3d7b0 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 13 Jun 2026 15:09:35 +0200 Subject: [PATCH 5/6] fix(invitations): thread service status codes + anchor param loader resend()/create() now pass through any AppError status (e.g. a 404) instead of flattening all-but-409 to 422; app.param anchored before its routes. Addresses critical-review H1 + M1 on #3857. refs #3834 --- .../controllers/invitations.controller.js | 19 ++++++++++++------- .../invitations/routes/invitations.routes.js | 7 +++++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/modules/invitations/controllers/invitations.controller.js b/modules/invitations/controllers/invitations.controller.js index a2ec55ea8..59f51be7f 100644 --- a/modules/invitations/controllers/invitations.controller.js +++ b/modules/invitations/controllers/invitations.controller.js @@ -18,11 +18,12 @@ const create = async (req, res) => { const invitation = await InvitationService.create(req.body.email, req.user); responses.success(res, 'invitation created')(invitation); } catch (err) { - // Thread the service-thrown status — a 409 (duplicate pending) must not - // flatten to the legacy hardcoded 422. Anything else keeps the - // historical 422 mapping (same style as the billing admin controller). - const status = err.status === 409 ? 409 : 422; - const title = status === 409 ? 'Conflict' : 'Unprocessable Entity'; + // Thread ANY service-thrown AppError status (409 duplicate-pending, etc.) + // rather than flattening everything but 409 to 422 (same style as the + // billing admin controller). create() does not throw 404 today, but the + // shared form keeps it consistent with resend(). + const status = err.status ?? 422; + const title = status === 409 ? 'Conflict' : status === 404 ? 'Not Found' : 'Unprocessable Entity'; responses.error(res, status, title, errors.getMessage(err))(err); } }; @@ -81,8 +82,12 @@ const resend = async (req, res) => { const invitation = await InvitationService.resend(req.invitation.id); responses.success(res, 'invitation resent')(invitation); } catch (err) { - const status = err.status === 409 ? 409 : 422; - const title = status === 409 ? 'Conflict' : 'Unprocessable Entity'; + // Thread ANY service-thrown AppError status (409 duplicate-pending, 404 + // unknown id, etc.) rather than flattening everything but 409 to 422 — a + // 404 must not surface as Unprocessable Entity if a future caller reaches + // the service without the invitationByID param-loader's 404 guard. + const status = err.status ?? 422; + const title = status === 409 ? 'Conflict' : status === 404 ? 'Not Found' : 'Unprocessable Entity'; responses.error(res, status, title, errors.getMessage(err))(err); } }; diff --git a/modules/invitations/routes/invitations.routes.js b/modules/invitations/routes/invitations.routes.js index 39aa25b74..c0d757103 100644 --- a/modules/invitations/routes/invitations.routes.js +++ b/modules/invitations/routes/invitations.routes.js @@ -23,6 +23,11 @@ export default (app) => { // Public: report whether a token is a valid invite (+ prefill email). app.route('/api/invitations/verify/:token').get(authLimiter, invitations.verify); + // Param loader for :invitationId — anchored before the routes that use it + // (app.param is global regardless of position, but keeping it here is the + // house convention and avoids a future reader missing it for a new route). + app.param('invitationId', invitations.invitationByID); + // Admin CRUD — list + create. app .route('/api/invitations') @@ -41,6 +46,4 @@ export default (app) => { .route('/api/invitations/:invitationId/resend') .all(passport.authenticate('jwt', { session: false }), policy.isAllowed) .post(invitations.resend); - - app.param('invitationId', invitations.invitationByID); }; From 45c2c0f3403dbf4ad42b7525d282ad99b710e7bb Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Sat, 13 Jun 2026 15:13:06 +0200 Subject: [PATCH 6/6] docs(invitations): document 409 in create throws + capitalize 404 msg create() @throws now lists the 409 duplicate-pending case; the resend 404 message is capitalized to match invitationByID. Review feedback on #3857. refs #3834 --- modules/invitations/services/invitations.service.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/invitations/services/invitations.service.js b/modules/invitations/services/invitations.service.js index 5de58871a..5dc6e3510 100644 --- a/modules/invitations/services/invitations.service.js +++ b/modules/invitations/services/invitations.service.js @@ -40,7 +40,7 @@ const inviteMailPayload = (invitation) => ({ * @param {String} email - invitee email (any case) * @param {Object} invitedBy - the admin user creating the invite * @returns {Promise} created invitation - * @throws {AppError} 422 when the invitee email is the inviter's own, or a user already exists for this email + * @throws {AppError} 409 when a pending invitation already exists for the email; 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(); @@ -378,7 +378,7 @@ const revoke = (id) => InvitationRepository.revoke(id); const resend = async (id) => { const invitation = await InvitationRepository.get(id); if (!invitation) { - throw new AppError('no invitation with that identifier has been found', { + throw new AppError('No invitation with that identifier has been found', { status: 404, code: 'NOT_FOUND', details: { message: 'No invitation with that identifier has been found.' },