synAdminandClaude Opus 5
6742b15489
Groundwork only. Nothing reads any of this yet, and no behaviour changes. The Relying Party ID is derived from PUBLIC_URL rather than written down, because it is the one value in this feature that cannot be corrected afterwards: a credential is bound to it permanently, and a wrong one surfaces only as a customer unable to sign in with a passkey that no longer matches anything. PUBLIC_URL is what every customer-facing link is already built from, so the ID is correct wherever those links are, and wrong only where they were already wrong. hostname rather than host, so a port cannot reach an ID that must not contain one. Local development is the exception the issue's table did not cover. envValidation requires PUBLIC_URL only when SMTP is configured, so a local setup that cannot send mail legitimately has none and falls back to localhost, which browsers treat as a secure context. Two origins there rather than one: the app is served by Vite on 5173 during development and by Express on 3000 once built, and those differ only by port, which is not part of the RP ID. The challenge table is separate from customer_tokens, and the reason is structural rather than preference. customer_tokens.customer_id is NOT NULL, and an authentication challenge is issued before anyone is identified — a discoverable-credential sign-in has no customer to attach to at the moment the challenge exists. Storing it there would mean making that column nullable for every other kind of token. Two of the issue's open decisions are deliberately not made here, because they belong to the ceremony that enforces them rather than to the schema. What to do when the signature counter fails to increase is #39's: many synced passkeys report zero forever, so treating a non-increase as cloning is wrong for them and right for a hardware key, and this only has to hold the value. Whether a disabled account can authenticate is also #39's, and the schema takes the position that it should not cost the customer their devices: credentials survive disabling and are refused at the ceremony, so re-enabling does not mean re-registering everything. Deletion is different and is settled here — credentials cascade with the customer, since one outliving its owner could authenticate as an account that no longer exists. signature_counter is BIGINT because the spec allows a 32-bit unsigned value, which overflows a signed INTEGER at half its range. That is the first bigint column in this schema, so the generated mirror gains the Int8 alias with it. Both tables are added to resetDb's TRUNCATE list and to REQUIRED_TABLES, and the schema mirror is updated by hand to match what kysely-codegen emits — placement and all, so a real regenerate produces no diff. Skipping either is how #56 turned a green local run into a red main; the mirror drift guard exists precisely to catch it, and schemaLoss's count moves from 18 to 20 with them. Verified: tsc clean for src and tests, lint 0 errors with no new warnings, 494 unit tests across 34 suites including nine new ones for the RP derivation, frontend build green, and the migration parses. Not verified: the migration has not been run against a database, and the integration suite needs one this machine cannot provide. Worth knowing before this goes further: #313 changes the domain, and every passkey registered before that cutover stops working at it. This code needs no change — it follows PUBLIC_URL — but the credentials do not survive. That is free while production is not live and nobody holds one, and it stops being free the day the shop opens. Closes #37 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
77 lines
2.9 KiB
TypeScript
77 lines
2.9 KiB
TypeScript
import { pool } from '../../src/db';
|
|
import { assertSchemaPresent, migrate, resetDb, testPool, closeDb } from './setup/testDb';
|
|
|
|
/**
|
|
* The diagnostic for #154, exercised against a database that has actually lost
|
|
* its schema.
|
|
*
|
|
* The failure this guards is not hypothetical and not cheap: the integration run
|
|
* lost its schema partway through, and it presented as 36 assertion errors about
|
|
* categories, price filters and favourite notifications. The real message —
|
|
* `relation "items" does not exist` — was further down the same log.
|
|
*
|
|
* This suite destroys and rebuilds the schema, so it restores in afterAll and
|
|
* `migrate()` is what puts it back: dropping the schema takes `pgmigrations`
|
|
* with it, so node-pg-migrate re-runs everything rather than believing it has
|
|
* already done so.
|
|
*/
|
|
async function dropEverything(): Promise<void> {
|
|
await testPool.query('DROP SCHEMA public CASCADE; CREATE SCHEMA public;');
|
|
}
|
|
|
|
afterAll(async () => {
|
|
// Put it back before anything else in the run touches it. Ordering matters:
|
|
// jest runs these in band, so a suite after this one would otherwise find
|
|
// nothing at all.
|
|
await migrate();
|
|
await pool.end();
|
|
await closeDb();
|
|
});
|
|
|
|
describe('when the database loses its schema', () => {
|
|
it('assertSchemaPresent says so, rather than letting the suite guess', async () => {
|
|
await dropEverything();
|
|
|
|
await expect(assertSchemaPresent('in a test')).rejects.toThrow(/has no schema \(in a test\)/);
|
|
|
|
await migrate();
|
|
});
|
|
|
|
it('names the tables that are missing', async () => {
|
|
await dropEverything();
|
|
|
|
// The count and a few names, not the whole list: the point is that a reader
|
|
// can tell at a glance this is a missing schema rather than a logic bug.
|
|
await expect(assertSchemaPresent('in a test')).rejects.toThrow(/Missing 20 of 20 tables/);
|
|
await expect(assertSchemaPresent('in a test')).rejects.toThrow(/items/);
|
|
|
|
await migrate();
|
|
});
|
|
|
|
it('points at the cause rather than leaving it to be rediscovered', async () => {
|
|
await dropEverything();
|
|
|
|
await expect(assertSchemaPresent('in a test')).rejects.toThrow(/TRUNCATE does not/);
|
|
await expect(assertSchemaPresent('in a test')).rejects.toThrow(/#154/);
|
|
|
|
await migrate();
|
|
});
|
|
|
|
// The path a real run takes: resetDb's TRUNCATE is what fails first, and it
|
|
// has to turn that into the schema message rather than passing on a bare
|
|
// "relation does not exist".
|
|
it('resetDb turns a failed truncate into the schema message', async () => {
|
|
await dropEverything();
|
|
|
|
await expect(resetDb()).rejects.toThrow(/has no schema \(while resetting between tests\)/);
|
|
|
|
await migrate();
|
|
});
|
|
|
|
// And it must not swallow unrelated failures behind a schema message.
|
|
it('says nothing about the schema when the schema is fine', async () => {
|
|
await expect(assertSchemaPresent('in a test')).resolves.toBeUndefined();
|
|
await expect(resetDb()).resolves.toBeUndefined();
|
|
});
|
|
});
|