From 87301bd4d2bafaa844f94446877a747f12aa9971 Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Tue, 28 Apr 2026 20:58:16 +0200 Subject: [PATCH] =?UTF-8?q?test(auth):=20polish=20register=20handler=20?= =?UTF-8?q?=E2=80=94=20comment,=20test=20isolation,=20+1=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quality-review polish on Task 11: 1. One-line comment near the federatedRegistrationOpen default behavior noting that the missing-row case is unreachable post-migration but falls federation-closed defensively (asymmetric with registrationOpen which falls back to env config — by design). 2. The "federated gate blocks token registration" test now sets ONLY federatedRegistrationOpen=0, isolating the federated-gate-alone effect rather than a both-gates-closed compound. 3. New test: open registration + revoked token → 201 (silently ignored). Locks the spec §5.7 invariant "no validation when registration is open" against future "let's just validate it for safety" regressions. 4. auth.md prose explicitly notes that federated stub upgrade and new- account paths do NOT enter redeemInvite — surfacing the structural enforcement of spec §1.3 "tokens never unlock federated creation". --- docs/systems/auth.md | 2 ++ packages/server/src/routes/auth.test.ts | 35 ++++++++++++++++++++++++- packages/server/src/routes/auth.ts | 5 ++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/docs/systems/auth.md b/docs/systems/auth.md index f3642804..be71f6e3 100644 --- a/docs/systems/auth.md +++ b/docs/systems/auth.md @@ -165,6 +165,8 @@ db.transaction(() => { If any step throws (concurrent revoke, last-slot race, username collision against the unique index), the entire transaction rolls back -- `usedCount` is never incremented on a failed registration. The route catches `InviteUnavailableError` from `redeemInvite()` and surfaces it as 403 `"Invalid or expired invite"`. +- **Federated stub upgrade and federated new-account paths do NOT enter `redeemInvite`.** They are gated only by `federatedRegistrationOpen` and never consume tokens, even if a token is provided in the request body. This is the structural enforcement of the spec §1.3 invariant "tokens never unlock federated creation". + The `/api/auth/check-invite` debounced UX endpoint pre-validates a token from the register page; the in-txn re-derive inside `redeemInvite()` is the authoritative enforcement point. ### First-User Admin Promotion diff --git a/packages/server/src/routes/auth.test.ts b/packages/server/src/routes/auth.test.ts index 57f41432..c51f17a0 100644 --- a/packages/server/src/routes/auth.test.ts +++ b/packages/server/src/routes/auth.test.ts @@ -353,7 +353,7 @@ describe('POST /api/auth/register — federation gate split', () => { it('federated registration: blocked even with valid token when federatedRegistrationOpen=false', async () => { testDb.update(schema.instanceSettings) - .set({ registrationOpen: 0, federatedRegistrationOpen: 0 }) + .set({ federatedRegistrationOpen: 0 }) .where(eq(schema.instanceSettings.id, 1)) .run(); const token = 'abcdefghijklmnopqrstuv'; @@ -454,4 +454,37 @@ describe('POST /api/auth/register — federation gate split', () => { }); expect(res.statusCode).toBe(403); }); + + it('open registration: revoked/expired token is silently ignored, registration still succeeds', async () => { + // Spec §5.7: when registration is open, the token field is not even + // validated. A revoked token in the request body must NOT block signup + // and must NOT be consumed. + const adminId = ADMIN_ID; + const token = 'r'.repeat(22); + testDb.insert(schema.inviteLinks).values({ + id: 'inv-revoked', + token, + name: 'revoked', + createdBy: adminId, + createdAt: Date.now(), + maxUses: 10, + usedCount: 0, + expiresAt: null, + revokedAt: Date.now(), // revoked! + }).run(); + + const res = await app.inject({ + method: 'POST', + url: '/api/auth/register', + payload: { username: 'eve', password: 'password123', inviteToken: token }, + }); + expect(res.statusCode).toBe(201); + + // Revoked invite still revoked — usedCount unchanged, no redemption row. + const inv = testDb.select().from(schema.inviteLinks).where(eq(schema.inviteLinks.id, 'inv-revoked')).get(); + expect(inv?.usedCount).toBe(0); + expect(inv?.revokedAt).not.toBeNull(); + const reds = testDb.select().from(schema.inviteRedemptions).where(eq(schema.inviteRedemptions.inviteId, 'inv-revoked')).all(); + expect(reds).toHaveLength(0); + }); }); diff --git a/packages/server/src/routes/auth.ts b/packages/server/src/routes/auth.ts index 157bbb4e..da3f6473 100644 --- a/packages/server/src/routes/auth.ts +++ b/packages/server/src/routes/auth.ts @@ -85,6 +85,11 @@ export async function authRoutes(app: FastifyInstance): Promise { const registrationOpen = instanceRow?.registrationOpen !== null && instanceRow?.registrationOpen !== undefined ? instanceRow.registrationOpen === 1 : config.registrationOpen; + // instanceRow is guaranteed by ensureDefaults() (migrate.ts) to have id=1 + // post-boot, with federatedRegistrationOpen NOT NULL DEFAULT 1. The optional + // chain is defensive against the impossible-in-production case of a missing + // row (e.g., a hand-cleared DB) — falls open-closed rather than open-open + // for federation, which is the safer default. const federatedRegistrationOpen = instanceRow?.federatedRegistrationOpen === 1; // Optional invite token. Only meaningful for the local-closed path; ignored