test(auth): polish register handler — comment, test isolation, +1 coverage

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".
This commit is contained in:
Jannis Braun
2026-04-28 20:58:16 +02:00
parent 0559ea369b
commit 87301bd4d2
3 changed files with 41 additions and 1 deletions
+2
View File
@@ -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"`. 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. 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 ### First-User Admin Promotion
+34 -1
View File
@@ -353,7 +353,7 @@ describe('POST /api/auth/register — federation gate split', () => {
it('federated registration: blocked even with valid token when federatedRegistrationOpen=false', async () => { it('federated registration: blocked even with valid token when federatedRegistrationOpen=false', async () => {
testDb.update(schema.instanceSettings) testDb.update(schema.instanceSettings)
.set({ registrationOpen: 0, federatedRegistrationOpen: 0 }) .set({ federatedRegistrationOpen: 0 })
.where(eq(schema.instanceSettings.id, 1)) .where(eq(schema.instanceSettings.id, 1))
.run(); .run();
const token = 'abcdefghijklmnopqrstuv'; const token = 'abcdefghijklmnopqrstuv';
@@ -454,4 +454,37 @@ describe('POST /api/auth/register — federation gate split', () => {
}); });
expect(res.statusCode).toBe(403); 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);
});
}); });
+5
View File
@@ -85,6 +85,11 @@ export async function authRoutes(app: FastifyInstance): Promise<void> {
const registrationOpen = instanceRow?.registrationOpen !== null && instanceRow?.registrationOpen !== undefined const registrationOpen = instanceRow?.registrationOpen !== null && instanceRow?.registrationOpen !== undefined
? instanceRow.registrationOpen === 1 ? instanceRow.registrationOpen === 1
: config.registrationOpen; : 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; const federatedRegistrationOpen = instanceRow?.federatedRegistrationOpen === 1;
// Optional invite token. Only meaningful for the local-closed path; ignored // Optional invite token. Only meaningful for the local-closed path; ignored