diff --git a/.changeset/lazy-pears-repeat.md b/.changeset/lazy-pears-repeat.md new file mode 100644 index 0000000..adf6f19 --- /dev/null +++ b/.changeset/lazy-pears-repeat.md @@ -0,0 +1,28 @@ +--- +'seamless-auth-api': minor +--- + +Ask for exactly the user verification that will be enforced. + +Registration advertised `userVerification: 'preferred'`, telling the +authenticator verification was optional, and then rejected a response that +skipped it, because SimpleWebAuthn's `requireUserVerification` defaults to true +and this server never set it. A user on an authenticator that skips verification +completed the whole ceremony and failed at the last step, having never been asked +to verify. Authentication separately asked for `required`, so the two halves +disagreed. + +`authenticator_policy.userVerification` now drives both, for registration and +authentication, so what the browser is asked for and what the server accepts come +from one value and cannot drift. It accepts `required`, `preferred` or +`discouraged` and defaults to `required`. + +The default does not change what is accepted, since that was already enforced. It +changes what is asked for, so an authenticator is told to verify rather than +being allowed to skip and be rejected afterwards. + +Step-up deliberately still requires verification regardless of the policy. It +exists to re-verify the human, and without verification it is a second signature +from a key the session already proved it holds. + +Requires `@seamless-auth/types` 0.13.0. diff --git a/.env.example b/.env.example index 8155621..eea6b9b 100644 --- a/.env.example +++ b/.env.example @@ -86,7 +86,9 @@ LOCKOUT_POLICY={"enabled":true,"maxFailures":10,"windowSeconds":900,"lockoutSeco # JSON. Which authenticators this deployment will enrol. attachment is any, platform or # cross-platform. "any" offers both built-in authenticators and roaming security keys; # naming one narrows the browser picker and rejects a request asking for the other. -AUTHENTICATOR_POLICY={"attachment":"any"} +# userVerification is required, preferred or discouraged. It drives both what the +# browser is asked for and what the server enforces, so the two cannot disagree. +AUTHENTICATOR_POLICY={"attachment":"any","userVerification":"required"} # SERVICE TOKENS # Required for trusted server adapters and internal bearer validation. diff --git a/docs/configuration.md b/docs/configuration.md index 6994df7..b021f84 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -269,7 +269,7 @@ Validation is enforced by [`systemConfig.schema.ts`](../src/schemas/systemConfig | `passkey_login_fallback_enabled` | boolean | `PASSKEY_LOGIN_FALLBACK_ENABLED` | `true` | | `oauth_providers` | provider[] | `OAUTH_PROVIDERS` | `[]` | | `lockout_policy` | object | `LOCKOUT_POLICY` | `{enabled,maxFailures:10,windowSeconds:900,lockoutSeconds:900}` | -| `authenticator_policy` | object | `AUTHENTICATOR_POLICY` | `{attachment:"any"}` | +| `authenticator_policy` | object | `AUTHENTICATOR_POLICY` | `{attachment:"any",userVerification:"required"}` | | `session_idle_ttl` | string (`\d+[smhd]`) | `SESSION_IDLE_TTL` | `8h` | | `access_token_ttl` | string (`\d+[smhd]`) | `ACCESS_TOKEN_TTL` | - | | `refresh_token_ttl` | string (`\d+[smhd]`) | `REFRESH_TOKEN_TTL` | - | diff --git a/openapi.json b/openapi.json index 0a0cbf0..8a23a6a 100644 --- a/openapi.json +++ b/openapi.json @@ -6333,9 +6333,14 @@ "type": "string", "enum": ["any", "platform", "cross-platform"], "default": "any" + }, + "userVerification": { + "type": "string", + "enum": ["required", "preferred", "discouraged"], + "default": "required" } }, - "default": { "attachment": "any" } + "default": { "attachment": "any", "userVerification": "required" } }, "access_token_ttl": { "type": "string", "pattern": "^\\d+[smhd]$" }, "session_idle_ttl": { @@ -6536,6 +6541,11 @@ "type": "string", "enum": ["any", "platform", "cross-platform"], "default": "any" + }, + "userVerification": { + "type": "string", + "enum": ["required", "preferred", "discouraged"], + "default": "required" } } }, diff --git a/package-lock.json b/package-lock.json index a31dce6..727a67d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,7 @@ "@seamless-auth/messaging": "^0.1.0", "@seamless-auth/messaging-aws": "^0.1.0", "@seamless-auth/messaging-twilio": "^0.1.0", - "@seamless-auth/types": "^0.12.0", + "@seamless-auth/types": "^0.13.0", "@simplewebauthn/server": "^13.1.1", "base64url": "^3.0.1", "bcrypt-ts": "^7.1.0", @@ -3393,9 +3393,9 @@ } }, "node_modules/@seamless-auth/types": { - "version": "0.12.0", - "resolved": "https://registry.npmjs.org/@seamless-auth/types/-/types-0.12.0.tgz", - "integrity": "sha512-FetF+KYZ7HQ1X5TNczRCVRPL3GQ8aV/CpgtwAGntse9LMaLUHe1ttxd/dFjTYkPq0c+NdcCGYkWWykWFWQnD/Q==", + "version": "0.13.0", + "resolved": "https://registry.npmjs.org/@seamless-auth/types/-/types-0.13.0.tgz", + "integrity": "sha512-dkgTljGcMur/U93pGz9aU3I3xMNtSLE5ZECx/AGqVl8Kj3V/2MDV6FLspTb5qN81foilMJUBVMkPSzsE28vt/A==", "license": "AGPL-3.0-only", "dependencies": { "zod": "^4.3.6" diff --git a/package.json b/package.json index da5a388..2327da8 100644 --- a/package.json +++ b/package.json @@ -50,7 +50,7 @@ "@seamless-auth/messaging": "^0.1.0", "@seamless-auth/messaging-aws": "^0.1.0", "@seamless-auth/messaging-twilio": "^0.1.0", - "@seamless-auth/types": "^0.12.0", + "@seamless-auth/types": "^0.13.0", "@simplewebauthn/server": "^13.1.1", "base64url": "^3.0.1", "bcrypt-ts": "^7.1.0", diff --git a/src/config/systemConfig.defaults.ts b/src/config/systemConfig.defaults.ts index aa7c685..1de1e85 100644 --- a/src/config/systemConfig.defaults.ts +++ b/src/config/systemConfig.defaults.ts @@ -17,6 +17,7 @@ export const SYSTEM_CONFIG_DEFAULTS: Partial = { }, authenticator_policy: { attachment: 'any', + userVerification: 'required', }, session_idle_ttl: '8h', passkey_login_fallback_enabled: true, diff --git a/src/controllers/stepUp.ts b/src/controllers/stepUp.ts index 10ce6d1..7eef357 100644 --- a/src/controllers/stepUp.ts +++ b/src/controllers/stepUp.ts @@ -106,6 +106,11 @@ export const startWebAuthnStepUp = async (req: Request, res: Response) => { id: credential.id, transports: credential.transports, })), + // Deliberately not the deployment's authenticator_policy. Step-up exists to + // re-verify the human; without user verification it is a second signature + // from a key the session already proved it holds, which proves nothing + // extra. A deployment that relaxes verification generally should still get + // a real check when elevating. userVerification: 'required', timeout: 60000, rpID: rpid, @@ -199,6 +204,9 @@ export const finishWebAuthnStepUp = async (req: Request, res: Response) => { const { origins, rpid } = await getSystemConfig(); const verification = await verifyAuthenticationResponse({ response: assertionResponse, + // Matches the 'required' asked for above rather than relying on a library + // default, so the ask and the enforcement cannot drift apart. + requireUserVerification: true, expectedChallenge, expectedOrigin: origins, expectedRPID: rpid, diff --git a/src/controllers/webauthn.ts b/src/controllers/webauthn.ts index 90549a2..af0e0f0 100644 --- a/src/controllers/webauthn.ts +++ b/src/controllers/webauthn.ts @@ -126,6 +126,7 @@ const registerWebAuthn = async (req: Request, res: Response) => { }); const { app_name, rpid, authenticator_policy } = await getSystemConfig(); + const userVerification = authenticator_policy.userVerification; const pinnedAttachment = authenticator_policy.attachment === 'any' ? null : authenticator_policy.attachment; @@ -164,7 +165,9 @@ const registerWebAuthn = async (req: Request, res: Response) => { // this to 'platform' hides roaming authenticators from the browser picker // entirely, which makes issued security keys impossible to enrol. authenticatorSelection: { - userVerification: 'preferred', + // The same value is enforced at verification below, so the browser is + // never asked for less than the server will accept. + userVerification, residentKey: 'preferred', ...(effectiveAttachment ? { authenticatorAttachment: effectiveAttachment } : {}), }, @@ -264,13 +267,14 @@ const verifyWebAuthnRegistration = async (req: Request, res: Response) => { let verification; try { - const { origins, rpid } = await getSystemConfig(); + const { origins, rpid, authenticator_policy } = await getSystemConfig(); verification = await verifyRegistrationResponse({ response: attestationResponse, expectedChallenge, expectedOrigin: origins, expectedRPID: rpid, + requireUserVerification: authenticator_policy.userVerification === 'required', // Pinned to the advertised set. The library default here is every // algorithm it knows, which would accept a credential using something // this server never offered. @@ -413,7 +417,8 @@ const generateWebAuthn = async (req: Request, res: Response) => { return res.status(401).send('Credentials not found'); } - const { rpid } = await getSystemConfig(); + const { rpid, authenticator_policy } = await getSystemConfig(); + const userVerification = authenticator_policy.userVerification; const options: PublicKeyCredentialRequestOptionsJSON = await generateAuthenticationOptions({ allowCredentials: assertionCredentials.map((cred) => { @@ -422,7 +427,7 @@ const generateWebAuthn = async (req: Request, res: Response) => { transports: cred.transports, }; }), - userVerification: 'required', + userVerification, timeout: 60000, rpID: rpid, extensions: buildPrfAuthenticationExtensions(prf), @@ -532,12 +537,13 @@ const verifyWebAuthn = async (req: Request, res: Response) => { let verification; try { - const { origins, rpid } = await getSystemConfig(); + const { origins, rpid, authenticator_policy } = await getSystemConfig(); verification = await verifyAuthenticationResponse({ response: assertionResponse, expectedChallenge, expectedOrigin: origins, expectedRPID: rpid, + requireUserVerification: authenticator_policy.userVerification === 'required', credential: { id: cred.id, // @ts-expect-error Needed to work. diff --git a/src/generated/api.ts b/src/generated/api.ts index 409efef..9dc97c3 100644 --- a/src/generated/api.ts +++ b/src/generated/api.ts @@ -6850,7 +6850,8 @@ export interface paths { }; /** * @default { - * "attachment": "any" + * "attachment": "any", + * "userVerification": "required" * } */ authenticator_policy: { @@ -6859,6 +6860,11 @@ export interface paths { * @enum {string} */ attachment: 'any' | 'platform' | 'cross-platform'; + /** + * @default required + * @enum {string} + */ + userVerification: 'required' | 'preferred' | 'discouraged'; }; access_token_ttl: string; /** @default 8h */ @@ -6985,6 +6991,11 @@ export interface paths { * @enum {string} */ attachment?: 'any' | 'platform' | 'cross-platform'; + /** + * @default required + * @enum {string} + */ + userVerification?: 'required' | 'preferred' | 'discouraged'; }; access_token_ttl?: string; session_idle_ttl?: string; diff --git a/tests/integration/webauthn/webauthn.spec.ts b/tests/integration/webauthn/webauthn.spec.ts index 9e06441..63beaac 100644 --- a/tests/integration/webauthn/webauthn.spec.ts +++ b/tests/integration/webauthn/webauthn.spec.ts @@ -76,7 +76,7 @@ beforeEach(() => { access_token_ttl: '15m', refresh_token_ttl: '1h', session_idle_ttl: '8h', - authenticator_policy: { attachment: 'any' }, + authenticator_policy: { attachment: 'any', userVerification: 'required' }, }); (Credential.findAll as any).mockResolvedValue([]); (Credential.findOne as any).mockResolvedValue(null); @@ -88,7 +88,7 @@ describe('GET /webauthn/register/start', () => { (getSystemConfig as any).mockResolvedValue({ app_name: 'SeamlessAuth', rpid: 'localhost', - authenticator_policy: { attachment: 'any' }, + authenticator_policy: { attachment: 'any', userVerification: 'required' }, }); const { generateRegistrationOptions } = await import('@simplewebauthn/server'); @@ -108,7 +108,7 @@ describe('GET /webauthn/register/start', () => { (getSystemConfig as any).mockResolvedValue({ app_name: 'SeamlessAuth', rpid: 'localhost', - authenticator_policy: { attachment: 'any' }, + authenticator_policy: { attachment: 'any', userVerification: 'required' }, }); const { generateRegistrationOptions } = await import('@simplewebauthn/server'); @@ -203,7 +203,7 @@ describe('GET /webauthn/register/start', () => { (getSystemConfig as any).mockResolvedValue({ app_name: 'SeamlessAuth', rpid: 'localhost', - authenticator_policy: { attachment: 'cross-platform' }, + authenticator_policy: { attachment: 'cross-platform', userVerification: 'required' }, }); const { generateRegistrationOptions } = await import('@simplewebauthn/server'); (generateRegistrationOptions as any).mockResolvedValue({ challenge: 'challenge' }); @@ -224,7 +224,7 @@ describe('GET /webauthn/register/start', () => { (getSystemConfig as any).mockResolvedValue({ app_name: 'SeamlessAuth', rpid: 'localhost', - authenticator_policy: { attachment: 'cross-platform' }, + authenticator_policy: { attachment: 'cross-platform', userVerification: 'required' }, }); const { generateRegistrationOptions } = await import('@simplewebauthn/server'); (generateRegistrationOptions as any).mockResolvedValue({ challenge: 'challenge' }); @@ -240,7 +240,7 @@ describe('GET /webauthn/register/start', () => { (getSystemConfig as any).mockResolvedValue({ app_name: 'SeamlessAuth', rpid: 'localhost', - authenticator_policy: { attachment: 'cross-platform' }, + authenticator_policy: { attachment: 'cross-platform', userVerification: 'required' }, }); const { generateRegistrationOptions } = await import('@simplewebauthn/server'); (generateRegistrationOptions as any).mockResolvedValue({ challenge: 'challenge' }); @@ -303,6 +303,38 @@ describe('GET /webauthn/register/start', () => { expect(options.supportedAlgorithmIDs).toEqual([-8, -7, -257, -65535]); }); + // Registration used to ask for 'preferred' while the library default enforced + // 'required', so a user on an authenticator that skips verification completed + // the whole ceremony and was rejected at the last step. + it('asks for exactly the verification it will enforce', async () => { + const { generateRegistrationOptions } = await import('@simplewebauthn/server'); + (generateRegistrationOptions as any).mockResolvedValue({ challenge: 'challenge' }); + + const res = await request(app).get('/webauthn/register/start'); + + expect(res.status).toBe(200); + + const [options] = (generateRegistrationOptions as any).mock.calls.at(-1); + + expect(options.authenticatorSelection.userVerification).toBe('required'); + }); + + it('follows a deployment that relaxes verification', async () => { + (getSystemConfig as any).mockResolvedValue({ + app_name: 'SeamlessAuth', + rpid: 'localhost', + authenticator_policy: { attachment: 'any', userVerification: 'discouraged' }, + }); + const { generateRegistrationOptions } = await import('@simplewebauthn/server'); + (generateRegistrationOptions as any).mockResolvedValue({ challenge: 'challenge' }); + + await request(app).get('/webauthn/register/start'); + + const [options] = (generateRegistrationOptions as any).mock.calls.at(-1); + + expect(options.authenticatorSelection.userVerification).toBe('discouraged'); + }); + it('returns 500 when credential lookup fails', async () => { (Credential.findAll as any).mockRejectedValue(new Error('db down')); @@ -384,6 +416,42 @@ describe('challenge is spent on every login outcome', () => { }); }); +describe('verification policy reaches both verifiers', () => { + it('enforces user verification at registration when the policy requires it', async () => { + (User.findOne as any).mockResolvedValue(buildUser()); + const { verifyRegistrationResponse } = await import('@simplewebauthn/server'); + (verifyRegistrationResponse as any).mockResolvedValue({ verified: false }); + + await request(app) + .post('/webauthn/register/finish') + .send({ attestationResponse: {}, metadata: {} }); + + const [options] = (verifyRegistrationResponse as any).mock.calls.at(-1); + + expect(options.requireUserVerification).toBe(true); + }); + + it('stops enforcing it when the deployment relaxes the policy', async () => { + (getSystemConfig as any).mockResolvedValue({ + app_name: 'SeamlessAuth', + rpid: 'localhost', + origins: ['http://localhost:5137'], + authenticator_policy: { attachment: 'any', userVerification: 'discouraged' }, + }); + (User.findOne as any).mockResolvedValue(buildUser()); + const { verifyRegistrationResponse } = await import('@simplewebauthn/server'); + (verifyRegistrationResponse as any).mockResolvedValue({ verified: false }); + + await request(app) + .post('/webauthn/register/finish') + .send({ attestationResponse: {}, metadata: {} }); + + const [options] = (verifyRegistrationResponse as any).mock.calls.at(-1); + + expect(options.requireUserVerification).toBe(false); + }); +}); + describe('POST /webauthn/register/finish', () => { it('creates credential and session', async () => { const user = buildUser(); @@ -895,6 +963,7 @@ describe('POST /webauthn/login/finish', () => { (getSystemConfig as any).mockResolvedValue({ origins: ['http://localhost:5137'], rpid: 'localhost', + authenticator_policy: { attachment: 'any', userVerification: 'required' }, }); const { verifyAuthenticationResponse } = await import('@simplewebauthn/server'); (verifyAuthenticationResponse as any).mockRejectedValue(new Error('bad assertion')); @@ -953,6 +1022,7 @@ describe('POST /webauthn/login/finish', () => { (getSystemConfig as any).mockResolvedValue({ origins: ['http://localhost:5137'], rpid: 'localhost', + authenticator_policy: { attachment: 'any', userVerification: 'required' }, }); const { verifyAuthenticationResponse } = await import('@simplewebauthn/server'); (verifyAuthenticationResponse as any).mockResolvedValue({