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/auth/routes/auth.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ 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
// below ONLY after (a) downstream (Trawl) has absorbed Vue P6 via /update-stack AND
// TODO(P9, pierreb-devkit/Node#3815): Remove these two imports and the four routes
// 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.
Expand Down Expand Up @@ -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
/**
Expand Down
48 changes: 42 additions & 6 deletions modules/invitations/controllers/invitations.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,20 @@ 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<void>} Sends HTTP 200 with created invitation or 422 on error
* @returns {Promise<void>} 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 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);
}
};

Expand All @@ -40,22 +46,52 @@ 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<void>} Sends HTTP 200 with deleted id or 422 on error
* @returns {Promise<void>} 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);
}
};

/**
* @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<void>} 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) {
// 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);
}
};

/**
* @desc Public: report whether a token is a valid invite (+ prefill email)
* @param {Object} req - Express request object
Expand Down Expand Up @@ -91,4 +127,4 @@ const invitationByID = async (req, res, next, id) => {
}
};

export default { create, list, remove, verify, invitationByID };
export default { create, list, remove, resend, verify, invitationByID };
16 changes: 14 additions & 2 deletions modules/invitations/repositories/invitations.repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<Object|null>} the revoked doc, or null when no such id
* @returns {Promise<Object|null>} 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();
Expand Down
11 changes: 10 additions & 1 deletion modules/invitations/routes/invitations.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -36,5 +41,9 @@ export default (app) => {
.all(passport.authenticate('jwt', { session: false }), policy.isAllowed)
.delete(invitations.remove);

app.param('invitationId', invitations.invitationByID);
// 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);
};
84 changes: 72 additions & 12 deletions modules/invitations/services/invitations.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -22,7 +40,7 @@ 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 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();
Expand Down Expand Up @@ -51,6 +69,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.' },
});
}
Comment thread
PierreBrisorgueil marked this conversation as resolved.
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);
Expand All @@ -62,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;
Expand Down Expand Up @@ -339,10 +362,46 @@ 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<Object|null>} the revoked invitation
* @returns {Promise<Object|null>} the revoked invitation, or null when not pending
*/
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<Object>} 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.' },
});
}
Comment thread
PierreBrisorgueil marked this conversation as resolved.
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,
Expand All @@ -357,4 +416,5 @@ export default {
list,
get,
revoke,
resend,
};
32 changes: 32 additions & 0 deletions modules/invitations/tests/invitations.controller.unit.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const mockService = {
create: jest.fn(),
list: jest.fn(),
revoke: jest.fn(),
resend: jest.fn(),
findValid: jest.fn(),
get: jest.fn(),
};
Expand Down Expand Up @@ -43,6 +44,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', () => {
Expand All @@ -62,6 +68,32 @@ 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.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', () => {
Expand Down
Loading
Loading