Skip to content

Commit 3b8035b

Browse files
feat(invitations): edge-case hardening — two-phase claim, CI email index, cap unify (P3) (#3821)
* feat(invitations): edge-case hardening — two-phase claim, CI email index, cap unify (P3) Security-sensitive hardening of the platform-invitation gate (#3811), building on the P2 module + auth.eligibility seam. E2 two-phase claim/finalize (replaces naive consume-after-create): - add invitation.consumingAt; evolve the eligibility seam from { invite, consume } to { invite, finalize, release }. - closed-signup path atomically CLAIMS the invite (findOneAndUpdate token+pending+ consumingAt:null) BEFORE user create — a replay/concurrent-accept loses the race and throws 422. Open signup resolves WITHOUT claiming (preserves the !sign.up gating: a presented token is never burned/locked). - finalize(userId) on full success (status:accepted+acceptedAt+acceptedUserId+ usedAt, checks the FOAU return); release() ($unset consumingAt) on any pre- response failure (verification setup / org provisioning throw) or cap-exhausted gate. finalize is acceptedAt-guarded (idempotent) so it also burns the unclaimed OAuth-by-email accept. - bypass guard: findValid AND findValidByEmail exclude consumingAt-set rows — a claimed-but-unfinalized invite is invisible to both, so the OAuth email-resolved path cannot re-accept it. - LAZY stale-claim sweep (no cron — stack has no scheduler): release consumingAt older than 15min w/ no acceptedAt, at boot + lazily before each validity read. Crash between claim and finalize recovers. E3 case-insensitive unique email index (users migration): - users.email: drop inline unique, add lowercase, declare an explicit { email:1 } unique collation index named email_ci_unique (distinct from the legacy email_1 so autoIndex/migration don't collide on name). Normalize email in every users-repo exact-match query (findByEmail/get/linkProviderByEmail/remove). - migration: abort+remediate on case-variant dup emails (no auto-merge); create the collation index FIRST then drop email_1 (never a window without a unique index); idempotent up(), down() no-op-warn. Schema declares the SAME index so syncIndexes stays idempotent vs the autoIndex race. - errors.getUniqueMessage now derives the field from err.keyPattern (index-name agnostic) so the "Email already exists." message survives the index rename. E4 cap unify: route signup + checkOAuthUserProfile through computeSignupCapacity (blank cap '' => uncapped, not Number('')→0→everyone-blocked); drop the hand-rolled inline parse. TOCTOU overshoot documented. E5 pin-in-checker: assertInvited keeps the token-present+no-email and mismatch rejects; dedicated tests. E6 no auto-verify on invite: mailer-off branch no longer sets emailVerified for invite-created accounts (token proves inviter, not signer). E8 soft-delete revoke: add revokedAt + status (pending|accepted|revoked); revoke() sets status:revoked instead of deleteOne; findValid excludes non-pending — invitedBy/acceptedUserId preserved for the referral phase. E9 already-registered guard: invitations.create rejects 422 when UserService .findByEmail(email) exists. Token-in-logs redaction: morgan :url token override via lib/helpers/redactUrl.js scrubs ?inviteToken from logged URLs (dev+prod patterns). Tests: full suite green (172 suites / 2289 tests), lint clean. OAuth E7 behavior unchanged. No org/Vue/referral changes (other phases). * fix(invitations): close leaked-claim window on create-throw + concurrency test (P3 review) Code-review (Approved-with-minors) follow-up to be74956: - Important: release the claimed invite when UserService.create itself throws (most realistically an E11000 from the case-sensitive unique-email index when two case-variant signups race the same invited email). Previously a create throw left the invite consumingAt-stamped and locked until the 15-min stale sweep — the cap / verify-failure / org-failure paths released, create did not. Wraps create in try/catch mirroring the three existing release sites + the same `invite && !config.sign.up` gating. Updates the finalize comment so it is accurate (every earlier failure path now releases). - Minor: add a real concurrency test — two parallel InvitationRepository.claim() on the same pending token via Promise.all assert EXACTLY ONE wins (the atomic findOneAndUpdate CAS), the genuine double-claim atomicity proof (the existing replay test was sequential / replay-after-completion only). - Minor: add the create-throw release regression test (verified to fail on the pre-fix code at the consumingAt assertion). - Minor: comment the cap:0 arm — a cap:0 deployment's rejection rides the `!sign.up && !invite` arm (invites bypass a zero cap per computeSignupCapacity), not capReached. Lint clean. Unit 1848/1848, integration 431/431. * docs(migrations): document email-CI-index + consumingAt downstream actions (P3) * fix(invitations): harden claim CAS + neutral E9 message + test cleanup - claim(): add usedAt:null + expiresAt:{$gt:now} to the findOneAndUpdate filter so the CAS step is self-contained (expired or already-used invite cannot be claimed even in the narrow window between findValid and claim) - invitations.service: remove org-coupling from E9 error message ("add them to the organization directly") — module is decoupled from orgs - afterAll test cleanup: replace leftover 'user@ci.example.com' with the actual test address 'ci-variant@example.com' - invitations.repository unit test: update claim assertion to verify the new usedAt + expiresAt guards in the filter All 1848 unit tests pass. * fix(users): harden normalizeEmail null-guard, migration PII + hasCi tighten - users.repository: normalizeEmail returns null (not the raw input) for non-string values; every caller (get/remove/findByEmail/linkProviderByEmail) now guards the null and returns a safe default — prevents a non-string value (e.g. Mongo operator object) from slipping into an exact-match filter - migration: replace raw d.emails in the abort error with IDs/counts only (no PII leaked in operator-facing logs/alerts) - migration: tighten hasCi to require the exact expected shape (name AND key AND unique AND collation locale+strength) instead of the prior OR-logic that would accept any unique/collated email index regardless of name All 1848 unit tests pass.
1 parent d52c3a1 commit 3b8035b

21 files changed

Lines changed: 1296 additions & 139 deletions

MIGRATIONS.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,24 @@ Breaking changes and upgrade notes for downstream projects.
44

55
---
66

7+
## Invitations hardening: case-insensitive unique email index + two-phase invite claim (2026-06-10)
8+
9+
Phase 3 of the invitations↔org decouple epic (#3811). Two downstream-relevant changes.
10+
11+
### What changed (this repo)
12+
13+
- **`users.email` is now lowercased + case-insensitively unique.** Inline `unique:true` removed; `lowercase:true` added; an explicit collation index `{ email:1 }, { unique:true, name:'email_ci_unique', collation:{ locale:'en', strength:2 } }` declared on the schema. Email inputs normalized to lowercase in `findByEmail` / get-by-email / `linkProviderByEmail` / `remove`. New migration **`modules/users/migrations/20260610120000-users-email-ci-unique-index.js`**: pre-checks for case-variant duplicate emails and **ABORTS boot if any exist** (no auto-merge), then creates the collation index FIRST and drops the legacy `email_1` (never a window without a unique index). `lib/helpers/errors.js` `getUniqueMessage` now derives the field from `err.keyPattern` (index-name-agnostic) so the collation index still yields a friendly "Email already exists." message.
14+
- **Invitation `consumingAt` two-phase claim** (new optional field on the `invitations` collection): the signup gate now atomically claims an invite before user creation and finalizes/releases after, with a lazy stale-claim sweep (15 min, no scheduler). Pure additive schema change — no data migration needed.
15+
16+
### Action required for downstream projects (`/update-project`)
17+
18+
1. The model/repository/migration changes are devkit-owned → arrive via `/update-stack` (`--theirs`).
19+
2. **🔴 Before the first boot that carries the new schema, pre-check prod for case-variant duplicate emails** (`db.users.aggregate([{$group:{_id:{$toLower:'$email'},n:{$sum:1}}},{$match:{n:{$gt:1}}}])`). If any exist, resolve them BEFORE deploy — otherwise the migration aborts boot. (Trawl: handled in epic Phase 9 / #3815.)
20+
3. Migrations run at boot before `listen()`; the index swap + the `consumingAt` field land automatically once the dupe pre-check passes.
21+
4. No client/contract change; existing 200/422 signup assertions pass unchanged.
22+
23+
---
24+
725
## @casl/ability v6 → v7 (2026-05-22)
826

927
`@casl/ability` upgraded from `^6.8.1` to `^7.0.0`.

lib/helpers/errors.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@
66
const getUniqueMessage = (err) => {
77
let output;
88

9+
// Primary: derive the field from the driver-supplied keyPattern (e.g. { email: 1 }).
10+
// This is index-NAME-agnostic, so it works for both the legacy `email_1` index and
11+
// the E3 case-insensitive `email_ci_unique` collation index — the old errmsg parse
12+
// below assumed a `{field}_1` index name and broke on any other name (and the
13+
// collation index's keyValue is binary-garbled, so keyPattern is the right source).
14+
const keyField = err.keyPattern && typeof err.keyPattern === 'object' ? Object.keys(err.keyPattern)[0] : null;
15+
if (keyField) {
16+
return `${keyField.charAt(0).toUpperCase() + keyField.slice(1)} already exists`;
17+
}
18+
919
try {
1020
let begin = 0;
1121
if (err.errmsg.lastIndexOf('.$') !== -1) {

lib/helpers/redactUrl.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* Sensitive query-string parameters that must never reach the request log.
3+
*
4+
* `inviteToken` rides the query string of `POST /api/auth/signup?inviteToken=…`
5+
* (the Vue client puts it there, not in the body). The morgan log pattern logs
6+
* `:url`, so without redaction a live single-use invite token lands in prod logs
7+
* (and any log shipper / aggregator downstream). Redact it to `REDACTED`.
8+
*/
9+
const SENSITIVE_QUERY_KEYS = ['inviteToken'];
10+
11+
/**
12+
* @desc Redact sensitive query-string parameters from a request URL for logging.
13+
* Preserves the path and every other query parameter; only the value of a
14+
* sensitive key is replaced with `REDACTED`. Tolerant of a missing/empty URL and
15+
* URLs with no query string (returned unchanged). Pure + synchronous so it is
16+
* safe to call on every logged request.
17+
* @param {String} url - the raw request URL (path + optional query string)
18+
* @returns {String} the URL with sensitive query values redacted
19+
*/
20+
const redactUrl = (url) => {
21+
if (!url || typeof url !== 'string') return url;
22+
const queryStart = url.indexOf('?');
23+
if (queryStart === -1) return url;
24+
25+
const pathPart = url.slice(0, queryStart);
26+
const queryPart = url.slice(queryStart + 1);
27+
28+
const redactedQuery = queryPart
29+
.split('&')
30+
.map((pair) => {
31+
const eq = pair.indexOf('=');
32+
const key = eq === -1 ? pair : pair.slice(0, eq);
33+
if (SENSITIVE_QUERY_KEYS.includes(key)) return `${key}=REDACTED`;
34+
return pair;
35+
})
36+
.join('&');
37+
38+
return `${pathPart}?${redactedQuery}`;
39+
};
40+
41+
export default redactUrl;
42+
export { SENSITIVE_QUERY_KEYS };
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { describe, test, expect } from '@jest/globals';
2+
import errors from '../errors.js';
3+
4+
/**
5+
* getUniqueMessage (via errors.getMessage on an E11000) must derive the friendly
6+
* "X already exists." message from the driver keyPattern — index-NAME-agnostic — so
7+
* it survives the E3 rename of the email unique index from `email_1` to the
8+
* collation-named `email_ci_unique`.
9+
*/
10+
describe('errors.getMessage — E11000 unique violation (keyPattern, index-name agnostic)', () => {
11+
test('derives the field from keyPattern for the collation email index (no _1 suffix)', () => {
12+
const err = {
13+
code: 11000,
14+
keyPattern: { email: 1 },
15+
// collation index name + a binary-garbled key value (real-world shape)
16+
errmsg: 'E11000 duplicate key error collection: db.users index: email_ci_unique collation: { locale: "en" } dup key: { email: "??" }',
17+
};
18+
expect(errors.getMessage(err)).toBe('Email already exists.');
19+
});
20+
21+
test('still works via keyPattern for a legacy email_1 index', () => {
22+
const err = {
23+
code: 11000,
24+
keyPattern: { email: 1 },
25+
errmsg: 'E11000 duplicate key error collection: db.users index: email_1 dup key: { : "a@b.co" }',
26+
};
27+
expect(errors.getMessage(err)).toBe('Email already exists.');
28+
});
29+
30+
test('falls back to the legacy errmsg parse when keyPattern is absent (older drivers)', () => {
31+
const err = {
32+
code: 11000,
33+
errmsg: 'E11000 duplicate key error collection: db.users index: email_1 dup key: { : "a@b.co" }',
34+
};
35+
expect(errors.getMessage(err)).toBe('Email already exists.');
36+
});
37+
});
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { describe, test, expect } from '@jest/globals';
2+
import redactUrl, { SENSITIVE_QUERY_KEYS } from '../redactUrl.js';
3+
4+
describe('redactUrl', () => {
5+
test('redacts an inviteToken value, preserving the path', () => {
6+
expect(redactUrl('/api/auth/signup?inviteToken=abc123deadbeef')).toBe('/api/auth/signup?inviteToken=REDACTED');
7+
});
8+
9+
test('redacts inviteToken among other query params, leaving the rest intact', () => {
10+
expect(redactUrl('/api/auth/signup?foo=1&inviteToken=secret&bar=2')).toBe('/api/auth/signup?foo=1&inviteToken=REDACTED&bar=2');
11+
});
12+
13+
test('redacts inviteToken when it is the first of several params', () => {
14+
expect(redactUrl('/x?inviteToken=secret&bar=2')).toBe('/x?inviteToken=REDACTED&bar=2');
15+
});
16+
17+
test('leaves a URL without a query string unchanged', () => {
18+
expect(redactUrl('/api/auth/signup')).toBe('/api/auth/signup');
19+
});
20+
21+
test('leaves a URL whose query has no sensitive key unchanged', () => {
22+
expect(redactUrl('/api/users?page=2&perPage=10')).toBe('/api/users?page=2&perPage=10');
23+
});
24+
25+
test('handles a bare sensitive key with no value (no = sign)', () => {
26+
// `?inviteToken` with no value: still scrubbed (becomes inviteToken=REDACTED) so a
27+
// malformed/edge query can never leak a partial token form downstream.
28+
expect(redactUrl('/x?inviteToken')).toBe('/x?inviteToken=REDACTED');
29+
});
30+
31+
test('does not redact a key that merely CONTAINS the sensitive name as a substring', () => {
32+
expect(redactUrl('/x?notinviteTokenish=keepme')).toBe('/x?notinviteTokenish=keepme');
33+
});
34+
35+
test('is tolerant of falsy / non-string input', () => {
36+
expect(redactUrl('')).toBe('');
37+
expect(redactUrl(undefined)).toBeUndefined();
38+
expect(redactUrl(null)).toBeNull();
39+
});
40+
41+
test('exports inviteToken as a sensitive key', () => {
42+
expect(SENSITIVE_QUERY_KEYS).toContain('inviteToken');
43+
});
44+
});

lib/services/express.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import redoc from 'redoc-express';
1919

2020
import config from '../../config/index.js';
2121
import guidesHelper from '../helpers/guides.js';
22+
import redactUrl from '../helpers/redactUrl.js';
2223
import logger from './logger.js';
2324
import requestId from '../middlewares/requestId.js';
2425
import { posthogContextMiddleware } from '../middlewares/posthog-context.middleware.js';
@@ -238,6 +239,10 @@ const initMiddleware = (app) => {
238239
morgan.token('email', (req) => _.get(req, 'user.email') || 'Unknown email');
239240
morgan.token('requestId', (req) => req.id || '-');
240241
morgan.token('orgId', (req) => _.get(req, 'organization.id') || _.get(req, 'organization._id', '-'));
242+
// Override the built-in :url token to scrub single-use secrets (e.g. inviteToken)
243+
// from the query string before they hit the log stream. Both the dev and prod
244+
// log patterns interpolate :url, so the override covers every logged request.
245+
morgan.token('url', (req) => redactUrl(req.originalUrl || req.url));
241246
app.use(morgan(logger.getLogFormat(), logger.getMorganOptions()));
242247
}
243248
// Request body parsing middleware should be above methodOverride

0 commit comments

Comments
 (0)