mosaicstack/stack
1
0
Fork 0
Code Issues 19 Pull Requests Actions Packages Projects Releases 13 Wiki Activity

feat(federation): enrollment controller + single-use token flow (FED-M2-07) #497

Merged
jason.woltje merged 2 commits from feat/federation-m2-enrollment into main 2026-04-22 04:23:21 +00:00
Conversation 2 Commits 2 Files Changed 10 +4202 -3
jason.woltje commented 2026-04-22 04:08:40 +00:00
Owner
Copy Link

FED-M2-07: Enrollment Controller\n\nAdds single-use enrollment token flow for federation grant activation.\n\n### Changes\n- New DB table federation_enrollment_tokens (migration 0010)\n- EnrollmentService: token generation, redemption with replay protection, cert issuance via CaService, grant activation\n- EnrollmentController: POST /api/federation/enrollment/tokens (admin) + POST /api/federation/enrollment/:token (unauthenticated)\n- Unit tests for all success and error paths (12 tests)\n\nCloses #461 (partial — M2-07 of M2 milestone)

## FED-M2-07: Enrollment Controller\n\nAdds single-use enrollment token flow for federation grant activation.\n\n### Changes\n- New DB table `federation_enrollment_tokens` (migration 0010)\n- `EnrollmentService`: token generation, redemption with replay protection, cert issuance via CaService, grant activation\n- `EnrollmentController`: `POST /api/federation/enrollment/tokens` (admin) + `POST /api/federation/enrollment/:token` (unauthenticated)\n- Unit tests for all success and error paths (12 tests)\n\nCloses #461 (partial — M2-07 of M2 milestone)
jason.woltje added 1 commit 2026-04-22 04:08:41 +00:00
feat(federation): enrollment controller + single-use token flow (FED-M2-07)
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
Details
ci/woodpecker/pr/ci Pipeline was successful
Details
fe4dffde15 
Adds single-use enrollment token table, service, and controller enabling remote
peer gateways to enroll into a pending federation grant via CSR submission.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jason.woltje commented 2026-04-22 04:14:59 +00:00
Author
Owner
Copy Link

Independent Code Review — REQUEST_CHANGES

Reviewed by Opus 4.7 (independent agent, no shared context with author).

HIGH (must fix before merge)

H1 — TOCTOU: cert issued before token claimed (enrollment.service.ts:137-170, confidence 88)

Current order: check usedAt → issueCert → mark token used. Two concurrent requests both pass pre-checks, both mint a real cert at step-ca with distinct serials, then race on the UPDATE. The loser's cert is a valid mTLS credential with the correct mosaic OIDs, untracked in federation_peers.cert_serial, outside any CRL workflow, valid for 300s.

Fix: Move the UPDATE ... WHERE used_at IS NULL to BEFORE caService.issueCert(). If issueCert fails after the token is claimed, the token is consumed (pending grant stranded — admin must create new grant). This tradeoff prevents double cert issuance.

H2 — No transaction wrapping (enrollment.service.ts:154-196, confidence 84)

Four serial non-atomic writes: mark-used → activateGrant → update peer → audit. Any mid-flight failure leaves inconsistent state (grant active but peer missing cert serial → all inbound mTLS fails; audit row silently lost).

Fix: Wrap activateGrant + peer update + audit in await this.db.transaction(async (tx) => { ... }). Keep caService.issueCert() outside the transaction (network call).

MEDIUM (defer ok)

  • M1: rowCount PGlite fallback — prefer .returning({ token: ... }) + check array length; works on both drivers without driver inspection.
  • M2: crypto.randomUUID() doc says UUIDv7 but token column is text — cosmetic, but add TODO.

Checked OK

  • AdminGuard on POST /tokens ✓
  • Unauthenticated redeem (token IS the auth) ✓
  • Token entropy: crypto.randomBytes(32).toString('hex') (256 bits) ✓
  • Audit log uses correct columns (verb, resource, statusCode, outcome) ✓
  • DTO validation complete ✓
  • No any types, no hardcoded secrets, stack file untouched ✓
  • 12 unit tests including updateRowCount: 0 → GoneException ✓

Verdict: REQUEST_CHANGES — H1 and H2 must be fixed before merge.

## Independent Code Review — REQUEST_CHANGES Reviewed by Opus 4.7 (independent agent, no shared context with author). ### HIGH (must fix before merge) **H1 — TOCTOU: cert issued before token claimed** (`enrollment.service.ts:137-170`, confidence 88) Current order: check usedAt → issueCert → mark token used. Two concurrent requests both pass pre-checks, both mint a real cert at step-ca with distinct serials, then race on the UPDATE. The loser's cert is a valid mTLS credential with the correct mosaic OIDs, untracked in `federation_peers.cert_serial`, outside any CRL workflow, valid for 300s. **Fix**: Move the `UPDATE ... WHERE used_at IS NULL` to BEFORE `caService.issueCert()`. If issueCert fails after the token is claimed, the token is consumed (pending grant stranded — admin must create new grant). This tradeoff prevents double cert issuance. **H2 — No transaction wrapping** (`enrollment.service.ts:154-196`, confidence 84) Four serial non-atomic writes: mark-used → activateGrant → update peer → audit. Any mid-flight failure leaves inconsistent state (grant active but peer missing cert serial → all inbound mTLS fails; audit row silently lost). **Fix**: Wrap `activateGrant` + peer update + audit in `await this.db.transaction(async (tx) => { ... })`. Keep `caService.issueCert()` outside the transaction (network call). ### MEDIUM (defer ok) - **M1**: `rowCount` PGlite fallback — prefer `.returning({ token: ... })` + check array length; works on both drivers without driver inspection. - **M2**: `crypto.randomUUID()` doc says UUIDv7 but token column is text — cosmetic, but add TODO. ### Checked OK - AdminGuard on `POST /tokens` ✓ - Unauthenticated redeem (token IS the auth) ✓ - Token entropy: `crypto.randomBytes(32).toString('hex')` (256 bits) ✓ - Audit log uses correct columns (`verb`, `resource`, `statusCode`, `outcome`) ✓ - DTO validation complete ✓ - No `any` types, no hardcoded secrets, stack file untouched ✓ - 12 unit tests including `updateRowCount: 0 → GoneException` ✓ ### Verdict: **REQUEST_CHANGES** — H1 and H2 must be fixed before merge.
jason.woltje added 1 commit 2026-04-22 04:20:56 +00:00
fix(federation): claim token before cert issuance; wrap activate+peer+audit in transaction (FED-M2-07)
All checks were successful
ci/woodpecker/push/ci Pipeline was successful
Details
ci/woodpecker/pr/ci Pipeline was successful
Details
e53121fa17
jason.woltje commented 2026-04-22 04:23:10 +00:00
Author
Owner
Copy Link

Independent Re-Review — APPROVE

Reviewed by Opus 4.7 (independent agent, no shared context with author).

H1 (TOCTOU: cert issued before token claimed): RESOLVED
Atomic claim (UPDATE WHERE used_at IS NULL RETURNING token) at line 143 fires before issueCert at line 160. claimed.length === 0 → GoneException without cert mint. Two new tests verify ordering and no-cert-on-race-loss.

H2 (no transaction): RESOLVED
activateGrant + peer update + audit wrapped in this.db.transaction(async (tx) => { ... }) at line 179. All three writes use tx. issueCert correctly stays outside. Test confirms: transaction called once, tx.update ×2, tx.insert ×1.

M1 (rowCount portability): RESOLVED
.returning({ token }) + claimed.length — works on both node-postgres and PGlite.

Gates on remediated branch: typecheck ✓ lint ✓ format ✓ test 429/429 ✓

No new HIGH issues.

Verdict: APPROVE

## Independent Re-Review — APPROVE Reviewed by Opus 4.7 (independent agent, no shared context with author). **H1 (TOCTOU: cert issued before token claimed): RESOLVED** Atomic claim (`UPDATE WHERE used_at IS NULL RETURNING token`) at line 143 fires before `issueCert` at line 160. `claimed.length === 0` → GoneException without cert mint. Two new tests verify ordering and no-cert-on-race-loss. **H2 (no transaction): RESOLVED** `activateGrant + peer update + audit` wrapped in `this.db.transaction(async (tx) => { ... })` at line 179. All three writes use `tx`. `issueCert` correctly stays outside. Test confirms: transaction called once, `tx.update` ×2, `tx.insert` ×1. **M1 (rowCount portability): RESOLVED** `.returning({ token })` + `claimed.length` — works on both node-postgres and PGlite. Gates on remediated branch: typecheck ✓ lint ✓ format ✓ test 429/429 ✓ No new HIGH issues. ### Verdict: **APPROVE**
jason.woltje merged commit 0bfaa56e9e into main 2026-04-22 04:23:21 +00:00
jason.woltje deleted branch feat/federation-m2-enrollment 2026-04-22 04:23:21 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
jason.woltje
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#497
Delete Branch "feat/federation-m2-enrollment"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?