feat(passkeys): registration ceremony (#38)
A signed-in customer can register a passkey. Signing in with one is #39 and the management screen is #40, so nothing reads these credentials yet. The name column arrives here rather than in #37. That issue listed the columns the ceremony needs and this one is for the person: #40 shows a list and offers to revoke from it, and "phone" against "laptop" is the only thing that makes two rows tellable apart. Without it a customer revoking a credential is choosing between identical entries. The customer may name it, and otherwise it is derived from the authenticator's transports — a hint rather than a fact, so the defaults are deliberately vague. "This device" is honest about a platform authenticator in a way that guessing at a model name would not be. Single use is enforced by deleting the challenge and treating the delete as the check, in one statement with the expiry condition. A replayed response finds nothing to delete and is refused, and two requests racing cannot both see the row and both proceed. Beginning a second registration replaces any in-flight challenge for that customer, so pressing the button twice cannot leave the first one usable. The challenge is consumed before the response is verified, deliberately. A failed attempt must not leave one available for a second try, so an invalid response costs the ceremony rather than merely failing it. Never started, already used and expired all answer the same way. From the server they are one condition — no challenge this customer may still complete — and distinguishing them would tell someone guessing which guess was closest. excludeCredentials stops the same authenticator being enrolled twice, but it is a hint the browser may ignore, so the unique constraint on credential_id is what actually holds. Hitting it answers 409: the credential is already registered, which is not a failure of anything. userID is the customer id rather than the email. A userID is meant to be stable and opaque, and an email is neither — a customer changing theirs would otherwise look like a different person to their own authenticator. Discoverable credentials are requested as preferred rather than required, because #39 wants sign-in without the customer first saying who they are, and an authenticator that cannot store one should still be usable here. Mounted before /api/customers, like the addresses router: Express matches mounts in order and the broader prefix would otherwise swallow these. Verified: tsc clean for src and tests, lint 0 errors with no new warnings, 514 unit tests across 35 suites — twenty new, covering the naming rules. The schema mirror gains the name column, placed where kysely-codegen would put it. Not verified: neither migration has been run against a database, and the ceremony itself cannot be exercised without a browser and an authenticator. What CI can prove is that the routes exist, are wrapped, and refuse an unauthenticated caller; what it cannot prove is a real attestation, which needs a device. Closes #38 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
c6f6ddc6dd
commit
88f76926d5
@@ -0,0 +1,23 @@
|
||||
exports.up = (pgm) => {
|
||||
pgm.sql(`
|
||||
-- What the customer calls this passkey (#38).
|
||||
--
|
||||
-- Not in the #37 groundwork because that issue listed the columns the
|
||||
-- ceremony needs and this one is for the person: the management screen (#40)
|
||||
-- shows a list, and "phone" against "laptop" is the only thing that makes
|
||||
-- two entries tellable apart. Without it a customer revoking a credential is
|
||||
-- choosing between identical rows.
|
||||
--
|
||||
-- NOT NULL with a default rather than nullable. Every row must be
|
||||
-- displayable, and a null would push the "or a sensible default" half of the
|
||||
-- requirement out into every read site. The route derives a better default
|
||||
-- from the authenticator's transports; this is the floor under that, and the
|
||||
-- value existing rows take.
|
||||
ALTER TABLE customer_credentials
|
||||
ADD COLUMN IF NOT EXISTS name TEXT NOT NULL DEFAULT 'Passkey';
|
||||
`);
|
||||
};
|
||||
|
||||
exports.down = (pgm) => {
|
||||
pgm.sql(`ALTER TABLE customer_credentials DROP COLUMN IF EXISTS name;`);
|
||||
};
|
||||
@@ -17,6 +17,7 @@ import adminVersionRouter from './routes/adminVersion';
|
||||
import adminConfigRouter from './routes/adminConfig';
|
||||
import filtersRouter from './routes/filters';
|
||||
import customersRouter from './routes/customers';
|
||||
import passkeysRouter from './routes/passkeys';
|
||||
import publicRouter from './routes/public';
|
||||
import cartRouter from './routes/cart';
|
||||
import shippingAddressesRouter from './routes/shippingAddresses';
|
||||
@@ -97,6 +98,9 @@ app.use('/api/admin/version', requireAdminGate, adminVersionRouter);
|
||||
app.use('/api/admin/config', requireAdminGate, adminConfigRouter);
|
||||
app.use('/api/admin', requireAdminGate, adminRouter);
|
||||
app.use('/api/customers/me/addresses', shippingAddressesRouter);
|
||||
// Before /api/customers, like the addresses router above: Express matches
|
||||
// mounts in order, so the broader prefix would swallow these otherwise (#38).
|
||||
app.use('/api/customers/me/passkeys', passkeysRouter);
|
||||
app.use('/api/customers', customersRouter);
|
||||
app.use('/api/client-errors', clientErrorsRouter);
|
||||
app.use('/', publicRouter);
|
||||
|
||||
@@ -79,6 +79,7 @@ export interface CustomerCredentials {
|
||||
customer_id: number;
|
||||
id: Generated<number>;
|
||||
last_used_at: Timestamp | null;
|
||||
name: Generated<string>;
|
||||
public_key: string;
|
||||
signature_counter: Generated<Int8>;
|
||||
transports: string | null;
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
/**
|
||||
* A name for a passkey the customer did not name themselves (#38).
|
||||
*
|
||||
* The management screen (#40) lists credentials and offers to revoke them, so
|
||||
* two entries that read identically are a screen where the customer cannot tell
|
||||
* which device they are removing. A default that says something is the
|
||||
* difference between "Passkey, Passkey, Passkey" and a list worth showing.
|
||||
*
|
||||
* Derived from the authenticator's transports, which is the only thing the
|
||||
* ceremony learns about the device. It is a hint rather than a fact — the
|
||||
* browser reports what the authenticator claims — so these are deliberately
|
||||
* vague. "This device" is honest about a platform authenticator in a way that
|
||||
* guessing "MacBook" would not be.
|
||||
*/
|
||||
|
||||
/** The fallback when the authenticator reports nothing usable. */
|
||||
export const GENERIC_CREDENTIAL_NAME = 'Passkey';
|
||||
|
||||
export function defaultCredentialName(transports: readonly string[] | null | undefined): string {
|
||||
if (!transports || transports.length === 0) return GENERIC_CREDENTIAL_NAME;
|
||||
|
||||
// Checked in this order because an authenticator can report several. A phone
|
||||
// used as a cross-device passkey reports `hybrid` and often `internal` too,
|
||||
// and "Phone or tablet" is the more useful of the two readings — `internal`
|
||||
// alone means the authenticator built into the machine being used.
|
||||
if (transports.includes('hybrid')) return 'Phone or tablet';
|
||||
if (transports.includes('internal')) return 'This device';
|
||||
if (transports.some((t) => t === 'usb' || t === 'nfc' || t === 'ble')) return 'Security key';
|
||||
|
||||
return GENERIC_CREDENTIAL_NAME;
|
||||
}
|
||||
|
||||
/**
|
||||
* The customer's own name for a passkey, or null when they gave none.
|
||||
*
|
||||
* Trimmed, because a name of spaces is a name nobody can read in a list, and
|
||||
* bounded because this is rendered — a customer is naming their laptop, not
|
||||
* writing prose, and an unbounded string in a table cell is a layout problem
|
||||
* rather than an expressive one.
|
||||
*/
|
||||
export const MAX_CREDENTIAL_NAME_LENGTH = 64;
|
||||
|
||||
export function readCredentialName(value: unknown): string | null {
|
||||
if (typeof value !== 'string') return null;
|
||||
const trimmed = value.trim();
|
||||
if (trimmed === '') return null;
|
||||
return trimmed.slice(0, MAX_CREDENTIAL_NAME_LENGTH);
|
||||
}
|
||||
@@ -0,0 +1,198 @@
|
||||
import { Router, Request, Response } from 'express';
|
||||
import {
|
||||
generateRegistrationOptions,
|
||||
verifyRegistrationResponse
|
||||
} from '@simplewebauthn/server';
|
||||
import type { RegistrationResponseJSON } from '@simplewebauthn/server';
|
||||
import { pool } from '../db';
|
||||
import { asyncRoute } from '../asyncRoute';
|
||||
import { requireCustomer } from '../middleware/customerAuth';
|
||||
import { relyingParty } from '../passkeys/relyingParty';
|
||||
import { defaultCredentialName, readCredentialName } from '../passkeys/credentialName';
|
||||
|
||||
const router = Router();
|
||||
|
||||
/**
|
||||
* Registering a passkey (#38).
|
||||
*
|
||||
* Every route here is behind `requireCustomer`. Registration is not a sign-up
|
||||
* path — it adds a credential to an account that already exists and is already
|
||||
* signed in — so an unauthenticated caller has nothing to register against.
|
||||
*
|
||||
* Signing in with a passkey is #39, and the management screen is #40. Neither
|
||||
* exists yet, so nothing reads these credentials.
|
||||
*/
|
||||
|
||||
/**
|
||||
* How long a customer has to complete the ceremony.
|
||||
*
|
||||
* Long enough to find a phone and use it; short enough that an intercepted
|
||||
* challenge is not useful for long. The browser's own timeout is set to match,
|
||||
* so the two cannot disagree about when the attempt has expired.
|
||||
*/
|
||||
const CHALLENGE_TTL_MS = 5 * 60 * 1000;
|
||||
|
||||
interface CredentialIdRow {
|
||||
credential_id: string;
|
||||
transports: string | null;
|
||||
}
|
||||
|
||||
interface ChallengeRow {
|
||||
challenge: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes a challenge and reports whether it was there.
|
||||
*
|
||||
* Single use is the whole point, and deleting it *is* the check: a replayed
|
||||
* response finds nothing to delete and is refused. Doing it as one statement
|
||||
* rather than a read followed by a delete means two requests racing cannot both
|
||||
* see the row and both proceed.
|
||||
*
|
||||
* Expiry is part of the same condition, so an expired challenge is refused for
|
||||
* the same reason and by the same statement.
|
||||
*/
|
||||
async function consumeChallenge(customerId: number, kind: string): Promise<string | null> {
|
||||
const { rows } = await pool.query<ChallengeRow>(
|
||||
`DELETE FROM webauthn_challenges
|
||||
WHERE customer_id = $1 AND kind = $2 AND expires_at > now()
|
||||
RETURNING challenge`,
|
||||
[customerId, kind]
|
||||
);
|
||||
return rows[0]?.challenge ?? null;
|
||||
}
|
||||
|
||||
router.post(
|
||||
'/register/begin',
|
||||
requireCustomer,
|
||||
asyncRoute(async (req: Request, res: Response) => {
|
||||
const customerId = req.customerId as number;
|
||||
const rp = relyingParty();
|
||||
|
||||
const { rows: existing } = await pool.query<CredentialIdRow>(
|
||||
`SELECT credential_id, transports FROM customer_credentials WHERE customer_id = $1`,
|
||||
[customerId]
|
||||
);
|
||||
|
||||
const { rows: customers } = await pool.query<{ email: string; first_name: string | null }>(
|
||||
`SELECT email, first_name FROM customers WHERE id = $1`,
|
||||
[customerId]
|
||||
);
|
||||
const customer = customers[0];
|
||||
if (!customer) return res.status(404).json({ error: 'not found' });
|
||||
|
||||
const options = await generateRegistrationOptions({
|
||||
rpName: rp.name,
|
||||
rpID: rp.id,
|
||||
userName: customer.email,
|
||||
userDisplayName: customer.first_name ?? customer.email,
|
||||
// The customer id, not the email. A userID is meant to be stable and
|
||||
// opaque; the email is neither, and a customer changing theirs would
|
||||
// otherwise look like a different person to their own authenticator.
|
||||
userID: new TextEncoder().encode(String(customerId)),
|
||||
// Stops the same authenticator being enrolled twice. Without it a
|
||||
// customer pressing register again on a device they already registered
|
||||
// gets a second row that behaves identically to the first, and a
|
||||
// management screen showing two entries they cannot tell apart.
|
||||
excludeCredentials: existing.map((row) => ({ id: row.credential_id })),
|
||||
attestationType: 'none',
|
||||
authenticatorSelection: {
|
||||
// Discoverable, because #39 wants sign-in without the customer first
|
||||
// saying who they are. 'preferred' rather than 'required' so an
|
||||
// authenticator that cannot store one is still usable here.
|
||||
residentKey: 'preferred',
|
||||
userVerification: 'preferred'
|
||||
},
|
||||
timeout: CHALLENGE_TTL_MS
|
||||
});
|
||||
|
||||
// One in-flight registration per customer. Pressing the button twice must
|
||||
// not leave the first challenge usable — the second replaces it, and the
|
||||
// first response is then refused by consumeChallenge finding nothing.
|
||||
await pool.query(`DELETE FROM webauthn_challenges WHERE customer_id = $1 AND kind = 'registration'`, [
|
||||
customerId
|
||||
]);
|
||||
await pool.query(
|
||||
`INSERT INTO webauthn_challenges (challenge, customer_id, kind, expires_at)
|
||||
VALUES ($1, $2, 'registration', now() + ($3 || ' milliseconds')::interval)`,
|
||||
[options.challenge, customerId, String(CHALLENGE_TTL_MS)]
|
||||
);
|
||||
|
||||
res.json(options);
|
||||
})
|
||||
);
|
||||
|
||||
router.post(
|
||||
'/register/finish',
|
||||
requireCustomer,
|
||||
asyncRoute(async (req: Request, res: Response) => {
|
||||
const customerId = req.customerId as number;
|
||||
const rp = relyingParty();
|
||||
|
||||
const expectedChallenge = await consumeChallenge(customerId, 'registration');
|
||||
if (expectedChallenge === null) {
|
||||
// Deliberately the same answer for "never started", "already used" and
|
||||
// "expired". They are the same thing from here — no challenge this
|
||||
// customer may still complete — and distinguishing them would tell an
|
||||
// attacker which of their guesses was closest.
|
||||
return res.status(400).json({ error: 'start again — that registration is no longer valid' });
|
||||
}
|
||||
|
||||
let verification;
|
||||
try {
|
||||
verification = await verifyRegistrationResponse({
|
||||
response: req.body as RegistrationResponseJSON,
|
||||
expectedChallenge,
|
||||
expectedOrigin: rp.origins,
|
||||
expectedRPID: rp.id
|
||||
});
|
||||
} catch {
|
||||
// The library throws on a malformed or unverifiable response. The
|
||||
// challenge is already consumed by this point, deliberately: a failed
|
||||
// attempt must not leave one usable for a second try.
|
||||
return res.status(400).json({ error: 'that passkey could not be registered' });
|
||||
}
|
||||
|
||||
if (!verification.verified) {
|
||||
return res.status(400).json({ error: 'that passkey could not be registered' });
|
||||
}
|
||||
|
||||
const { credential } = verification.registrationInfo;
|
||||
const transports = credential.transports ?? [];
|
||||
const name = readCredentialName((req.body as { name?: unknown }).name)
|
||||
?? defaultCredentialName(transports);
|
||||
|
||||
try {
|
||||
await pool.query(
|
||||
`INSERT INTO customer_credentials
|
||||
(customer_id, credential_id, public_key, signature_counter, transports, name)
|
||||
VALUES ($1, $2, $3, $4, $5, $6)`,
|
||||
[
|
||||
customerId,
|
||||
credential.id,
|
||||
Buffer.from(credential.publicKey).toString('base64url'),
|
||||
credential.counter,
|
||||
JSON.stringify(transports),
|
||||
name
|
||||
]
|
||||
);
|
||||
} catch (err) {
|
||||
// credential_id is unique across the table. excludeCredentials should
|
||||
// have stopped the browser offering an already-registered authenticator,
|
||||
// but that is a hint the browser may ignore, so the constraint is what
|
||||
// actually holds — and hitting it means the credential is already
|
||||
// registered rather than that anything is broken.
|
||||
if ((err as { code?: string }).code === '23505') {
|
||||
return res.status(409).json({ error: 'that passkey is already registered' });
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
res.status(201).json({ name });
|
||||
})
|
||||
);
|
||||
|
||||
/** Exported for the tests; nothing else constructs a challenge. */
|
||||
export { CHALLENGE_TTL_MS };
|
||||
|
||||
export default router;
|
||||
@@ -0,0 +1,66 @@
|
||||
import {
|
||||
defaultCredentialName,
|
||||
readCredentialName,
|
||||
GENERIC_CREDENTIAL_NAME,
|
||||
MAX_CREDENTIAL_NAME_LENGTH
|
||||
} from '../../src/passkeys/credentialName';
|
||||
|
||||
/**
|
||||
* The management screen (#40) lists credentials and offers to revoke them, so a
|
||||
* default that says nothing produces a list of identical rows and a customer
|
||||
* removing a device at random. These are about that list being readable.
|
||||
*/
|
||||
describe('defaultCredentialName', () => {
|
||||
it('calls a cross-device authenticator a phone or tablet', () => {
|
||||
expect(defaultCredentialName(['hybrid'])).toBe('Phone or tablet');
|
||||
});
|
||||
|
||||
it('prefers hybrid over internal when both are reported', () => {
|
||||
// A phone used as a cross-device passkey commonly reports both, and "Phone
|
||||
// or tablet" is the more useful reading — `internal` on its own means the
|
||||
// authenticator built into the machine in front of the customer.
|
||||
expect(defaultCredentialName(['internal', 'hybrid'])).toBe('Phone or tablet');
|
||||
});
|
||||
|
||||
it('calls a platform authenticator this device', () => {
|
||||
expect(defaultCredentialName(['internal'])).toBe('This device');
|
||||
});
|
||||
|
||||
it.each([['usb'], ['nfc'], ['ble']])('calls %s a security key', (transport) => {
|
||||
expect(defaultCredentialName([transport])).toBe('Security key');
|
||||
});
|
||||
|
||||
it.each([[[]], [null], [undefined], [['something-new']]])(
|
||||
'falls back to the generic name for %p',
|
||||
(transports) => {
|
||||
// Transports are a hint the authenticator supplies, so an unrecognised
|
||||
// one is expected rather than exceptional — the list grows.
|
||||
expect(defaultCredentialName(transports)).toBe(GENERIC_CREDENTIAL_NAME);
|
||||
}
|
||||
);
|
||||
});
|
||||
|
||||
describe('readCredentialName', () => {
|
||||
it('takes a name the customer gave', () => {
|
||||
expect(readCredentialName('Work laptop')).toBe('Work laptop');
|
||||
});
|
||||
|
||||
it('trims, because a padded name is not a different name', () => {
|
||||
expect(readCredentialName(' Work laptop ')).toBe('Work laptop');
|
||||
});
|
||||
|
||||
it.each([[' '], [''], [null], [undefined], [42], [{}]])(
|
||||
'returns null for %p so the caller falls back to a default',
|
||||
(value) => {
|
||||
// A name of spaces is a row the customer cannot read, and a non-string is
|
||||
// a client sending something unexpected. Both mean "no name given".
|
||||
expect(readCredentialName(value)).toBeNull();
|
||||
}
|
||||
);
|
||||
|
||||
it('bounds the length, because this is rendered in a table cell', () => {
|
||||
const long = 'a'.repeat(MAX_CREDENTIAL_NAME_LENGTH + 50);
|
||||
|
||||
expect(readCredentialName(long)).toHaveLength(MAX_CREDENTIAL_NAME_LENGTH);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user