From b1b6d33c8c07f445b63f9c29cc98c47db65767d7 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 11 Apr 2024 18:20:57 -0700 Subject: [PATCH 01/12] Add method to generate user IDs --- .../server/src/helpers/generateUserID.test.ts | 16 ++++++++++++++ packages/server/src/helpers/generateUserID.ts | 21 +++++++++++++++++++ packages/server/src/helpers/index.ts | 9 +++++++- 3 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 packages/server/src/helpers/generateUserID.test.ts create mode 100644 packages/server/src/helpers/generateUserID.ts diff --git a/packages/server/src/helpers/generateUserID.test.ts b/packages/server/src/helpers/generateUserID.test.ts new file mode 100644 index 00000000..b15cab84 --- /dev/null +++ b/packages/server/src/helpers/generateUserID.test.ts @@ -0,0 +1,16 @@ +import { assert, assertNotEquals } from 'https://deno.land/std@0.198.0/assert/mod.ts'; + +import { generateUserID } from './generateUserID.ts'; + +Deno.test('should return a buffer of 32 bytes', async () => { + const userID = await generateUserID(); + + assert(userID.byteLength === 32); +}); + +Deno.test('should return random bytes on each execution', async () => { + const userID1 = await generateUserID(); + const userID2 = await generateUserID(); + + assertNotEquals(userID1, userID2); +}); diff --git a/packages/server/src/helpers/generateUserID.ts b/packages/server/src/helpers/generateUserID.ts new file mode 100644 index 00000000..eaf9bb05 --- /dev/null +++ b/packages/server/src/helpers/generateUserID.ts @@ -0,0 +1,21 @@ +import { isoCrypto } from './iso/index.ts'; + +/** + * Generate a suitably random value to be used as user ID + */ +export async function generateUserID(): Promise { + /** + * WebAuthn spec says user.id has a max length of 64 bytes. I prefer how 32 random bytes look + * after they're base64url-encoded so I'm choosing to go with that here. + */ + const newUserID = new Uint8Array(32); + + await isoCrypto.getRandomValues(newUserID); + + return _generateUserIDInternals.stubThis(newUserID); +} + +// Make it possible to stub the return value during testing +export const _generateUserIDInternals = { + stubThis: (value: Uint8Array) => value, +}; diff --git a/packages/server/src/helpers/index.ts b/packages/server/src/helpers/index.ts index 30cf8676..09b2f33b 100644 --- a/packages/server/src/helpers/index.ts +++ b/packages/server/src/helpers/index.ts @@ -5,6 +5,7 @@ import { decodeAttestationObject } from './decodeAttestationObject.ts'; import { decodeClientDataJSON } from './decodeClientDataJSON.ts'; import { decodeCredentialPublicKey } from './decodeCredentialPublicKey.ts'; import { generateChallenge } from './generateChallenge.ts'; +import { generateUserID } from './generateUserID.ts'; import { getCertificateInfo } from './getCertificateInfo.ts'; import { isCertRevoked } from './isCertRevoked.ts'; import { parseAuthenticatorData } from './parseAuthenticatorData.ts'; @@ -23,6 +24,7 @@ export { decodeClientDataJSON, decodeCredentialPublicKey, generateChallenge, + generateUserID, getCertificateInfo, isCertRevoked, isoBase64URL, @@ -42,7 +44,12 @@ import type { } from './decodeAttestationObject.ts'; import type { CertificateInfo } from './getCertificateInfo.ts'; import type { ClientDataJSON } from './decodeClientDataJSON.ts'; -import type { COSEPublicKey, COSEPublicKeyEC2, COSEPublicKeyOKP, COSEPublicKeyRSA } from './cose.ts'; +import type { + COSEPublicKey, + COSEPublicKeyEC2, + COSEPublicKeyOKP, + COSEPublicKeyRSA, +} from './cose.ts'; import type { ParsedAuthenticatorData } from './parseAuthenticatorData.ts'; export type { From 4c3f6934817913dc065c6400b956e36b301ec5fe Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 11 Apr 2024 21:40:36 -0700 Subject: [PATCH 02/12] Generate user IDs by default --- .../src/registration/generateRegistrationOptions.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/server/src/registration/generateRegistrationOptions.ts b/packages/server/src/registration/generateRegistrationOptions.ts index 887452b7..c8d7ea8e 100644 --- a/packages/server/src/registration/generateRegistrationOptions.ts +++ b/packages/server/src/registration/generateRegistrationOptions.ts @@ -9,13 +9,14 @@ import type { PublicKeyCredentialParameters, } from '../deps.ts'; import { generateChallenge } from '../helpers/generateChallenge.ts'; +import { generateUserID } from '../helpers/generateUserID.ts'; import { isoBase64URL, isoUint8Array } from '../helpers/iso/index.ts'; export type GenerateRegistrationOptionsOpts = { rpName: string; rpID: string; - userID: string; userName: string; + userID?: Uint8Array; challenge?: string | Uint8Array; userDisplayName?: string; timeout?: number; @@ -164,6 +165,14 @@ export async function generateRegistrationOptions( _challenge = isoUint8Array.fromUTF8String(_challenge); } + /** + * Generate a user ID if one is not provided + */ + let _userID = userID; + if (_userID === undefined) { + _userID = await generateUserID(); + } + return { challenge: isoBase64URL.fromBuffer(_challenge), rp: { @@ -171,7 +180,7 @@ export async function generateRegistrationOptions( id: rpID, }, user: { - id: userID, + id: isoBase64URL.fromBuffer(_userID), name: userName, displayName: userDisplayName, }, From 0156c639cabdc44392aa58cb5d2f55bf201fd8ab Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 11 Apr 2024 21:42:05 -0700 Subject: [PATCH 03/12] Treat user.id as base64url string during reg --- .../src/methods/startRegistration.test.ts | 27 +++++++++---------- .../browser/src/methods/startRegistration.ts | 3 +-- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/packages/browser/src/methods/startRegistration.test.ts b/packages/browser/src/methods/startRegistration.test.ts index e6bdcb7d..6b62a795 100644 --- a/packages/browser/src/methods/startRegistration.test.ts +++ b/packages/browser/src/methods/startRegistration.test.ts @@ -6,12 +6,10 @@ import { } from '@simplewebauthn/types'; import { generateCustomError } from '../helpers/__jest__/generateCustomError'; import { browserSupportsWebAuthn } from '../helpers/browserSupportsWebAuthn'; -import { bufferToBase64URLString } from '../helpers/bufferToBase64URLString'; +import { base64URLStringToBuffer } from '../helpers/base64URLStringToBuffer'; import { WebAuthnError } from '../helpers/webAuthnError'; import { WebAuthnAbortService } from '../helpers/webAuthnAbortService'; -import { utf8StringToBuffer } from '../helpers/utf8StringToBuffer'; - import { startRegistration } from './startRegistration'; jest.mock('../helpers/browserSupportsWebAuthn'); @@ -23,7 +21,7 @@ const mockAttestationObject = 'mockAtte'; const mockClientDataJSON = 'mockClie'; const goodOpts1: PublicKeyCredentialCreationOptionsJSON = { - challenge: bufferToBase64URLString(utf8StringToBuffer('fizz')), + challenge: '1T6uHri4OAQ', attestation: 'direct', pubKeyCredParams: [ { @@ -36,7 +34,7 @@ const goodOpts1: PublicKeyCredentialCreationOptionsJSON = { name: 'SimpleWebAuthn', }, user: { - id: '5678', + id: 'f4pdy3fpA34', displayName: 'username', name: 'username', }, @@ -77,10 +75,10 @@ test('should convert options before passing to navigator.credentials.create(...) // Make sure challenge and user.id are converted to Buffers expect(new Uint8Array(argsPublicKey.challenge)).toEqual( - new Uint8Array([102, 105, 122, 122]), + new Uint8Array([213, 62, 174, 30, 184, 184, 56, 4]), ); expect(new Uint8Array(argsPublicKey.user.id)).toEqual( - new Uint8Array([53, 54, 55, 56]), + new Uint8Array([127, 138, 93, 203, 119, 233, 3, 126]), ); // Confirm construction of excludeCredentials array @@ -95,8 +93,8 @@ test('should return base64url-encoded response values', async () => { (): Promise => { return new Promise((resolve) => { resolve({ - id: 'foobar', - rawId: utf8StringToBuffer('foobar'), + id: '6mUg8GzxDxs', + rawId: base64URLStringToBuffer('6mUg8GzxDxs'), response: { attestationObject: Buffer.from(mockAttestationObject, 'ascii'), clientDataJSON: Buffer.from(mockClientDataJSON, 'ascii'), @@ -115,12 +113,12 @@ test('should return base64url-encoded response values', async () => { const response = await startRegistration(goodOpts1); - expect(response.rawId).toEqual('Zm9vYmFy'); + expect(response.rawId).toEqual('6mUg8GzxDxs'); expect(response.response.attestationObject).toEqual('bW9ja0F0dGU'); expect(response.response.clientDataJSON).toEqual('bW9ja0NsaWU'); }); -test('should throw error if WebAuthn isn\'t supported', async () => { +test("should throw error if WebAuthn isn't supported", async () => { mockSupportsWebauthn.mockReturnValue(false); await expect(startRegistration(goodOpts1)).rejects.toThrow( @@ -216,8 +214,8 @@ test('should support "cable" transport in excludeCredentials', async () => { test('should return "cable" transport from response', async () => { mockNavigatorCreate.mockResolvedValue({ - id: 'foobar', - rawId: utf8StringToBuffer('foobar'), + id: '6mUg8GzxDxs', + rawId: base64URLStringToBuffer('6mUg8GzxDxs'), response: { attestationObject: Buffer.from(mockAttestationObject, 'ascii'), clientDataJSON: Buffer.from(mockClientDataJSON, 'ascii'), @@ -589,7 +587,8 @@ describe('WebAuthnError', () => { ...goodOpts1, user: { ...goodOpts1.user, - id: Array(65).fill('a').join(''), + // A base64url string 100 characters long should decode to ~70 bytes + id: Array(100).fill('a').join(''), }, }; diff --git a/packages/browser/src/methods/startRegistration.ts b/packages/browser/src/methods/startRegistration.ts index 09117c36..73c899e5 100644 --- a/packages/browser/src/methods/startRegistration.ts +++ b/packages/browser/src/methods/startRegistration.ts @@ -5,7 +5,6 @@ import { RegistrationResponseJSON, } from '@simplewebauthn/types'; -import { utf8StringToBuffer } from '../helpers/utf8StringToBuffer'; import { bufferToBase64URLString } from '../helpers/bufferToBase64URLString'; import { base64URLStringToBuffer } from '../helpers/base64URLStringToBuffer'; import { browserSupportsWebAuthn } from '../helpers/browserSupportsWebAuthn'; @@ -32,7 +31,7 @@ export async function startRegistration( challenge: base64URLStringToBuffer(creationOptionsJSON.challenge), user: { ...creationOptionsJSON.user, - id: utf8StringToBuffer(creationOptionsJSON.user.id), + id: base64URLStringToBuffer(creationOptionsJSON.user.id), }, excludeCredentials: creationOptionsJSON.excludeCredentials?.map( toPublicKeyCredentialDescriptor, From 0eef7420a1bf5fe6940344bebe8c3375e318133d Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 11 Apr 2024 21:42:46 -0700 Subject: [PATCH 04/12] Treat userHandle as base64url string during auth --- .../src/methods/startAuthentication.test.ts | 20 +++++++++---------- .../src/methods/startAuthentication.ts | 3 +-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/packages/browser/src/methods/startAuthentication.test.ts b/packages/browser/src/methods/startAuthentication.test.ts index 5450e164..d2dd4b77 100644 --- a/packages/browser/src/methods/startAuthentication.test.ts +++ b/packages/browser/src/methods/startAuthentication.test.ts @@ -7,7 +7,7 @@ import { import { browserSupportsWebAuthn } from '../helpers/browserSupportsWebAuthn'; import { browserSupportsWebAuthnAutofill } from '../helpers/browserSupportsWebAuthnAutofill'; -import { utf8StringToBuffer } from '../helpers/utf8StringToBuffer'; +import { base64URLStringToBuffer } from '../helpers/base64URLStringToBuffer'; import { bufferToBase64URLString } from '../helpers/bufferToBase64URLString'; import { WebAuthnError } from '../helpers/webAuthnError'; import { generateCustomError } from '../helpers/__jest__/generateCustomError'; @@ -25,11 +25,11 @@ const mockSupportsAutofill = browserSupportsWebAuthnAutofill as jest.Mock; const mockAuthenticatorData = 'mockAuthenticatorData'; const mockClientDataJSON = 'mockClientDataJSON'; const mockSignature = 'mockSignature'; -const mockUserHandle = 'mockUserHandle'; +const mockUserHandle = 'f4pdy3fpA34'; // With ASCII challenge const goodOpts1: PublicKeyCredentialRequestOptionsJSON = { - challenge: bufferToBase64URLString(utf8StringToBuffer('fizz')), + challenge: '1T6uHri4OAQ', allowCredentials: [ { id: 'C0VGlvYFratUdAV1iCw-ULpUW8E-exHPXQChBfyVeJZCMfjMFcwDmOFgoMUz39LoMtCJUBW8WPlLkGT6q8qTCg', @@ -42,7 +42,7 @@ const goodOpts1: PublicKeyCredentialRequestOptionsJSON = { // With UTF-8 challenge const goodOpts2UTF8: PublicKeyCredentialRequestOptionsJSON = { - challenge: bufferToBase64URLString(utf8StringToBuffer('やれやれだぜ')), + challenge: bufferToBase64URLString(new TextEncoder().encode('やれやれだぜ')), allowCredentials: [], timeout: 1, }; @@ -78,7 +78,7 @@ test('should convert options before passing to navigator.credentials.get(...)', const credId = argsPublicKey.allowCredentials[0].id; expect(new Uint8Array(argsPublicKey.challenge)).toEqual( - new Uint8Array([102, 105, 122, 122]), + new Uint8Array([213, 62, 174, 30, 184, 184, 56, 4]), ); // Make sure the credential ID is an ArrayBuffer with a length of 64 expect(credId instanceof ArrayBuffer).toEqual(true); @@ -87,7 +87,7 @@ test('should convert options before passing to navigator.credentials.get(...)', test('should support optional allowCredential', async () => { await startAuthentication({ - challenge: bufferToBase64URLString(utf8StringToBuffer('fizz')), + challenge: '1T6uHri4OAQ', timeout: 1, }); @@ -96,7 +96,7 @@ test('should support optional allowCredential', async () => { test('should convert allow allowCredential to undefined when empty', async () => { await startAuthentication({ - challenge: bufferToBase64URLString(utf8StringToBuffer('fizz')), + challenge: '1T6uHri4OAQ', timeout: 1, allowCredentials: [], }); @@ -113,7 +113,7 @@ test('should return base64url-encoded response values', async () => { authenticatorData: Buffer.from(mockAuthenticatorData, 'ascii'), clientDataJSON: Buffer.from(mockClientDataJSON, 'ascii'), signature: Buffer.from(mockSignature, 'ascii'), - userHandle: Buffer.from(mockUserHandle, 'ascii'), + userHandle: base64URLStringToBuffer(mockUserHandle), }, getClientExtensionResults: () => ({}), type: 'public-key', @@ -130,10 +130,10 @@ test('should return base64url-encoded response values', async () => { ); expect(response.response.clientDataJSON).toEqual('bW9ja0NsaWVudERhdGFKU09O'); expect(response.response.signature).toEqual('bW9ja1NpZ25hdHVyZQ'); - expect(response.response.userHandle).toEqual('mockUserHandle'); + expect(response.response.userHandle).toEqual('f4pdy3fpA34'); }); -test('should throw error if WebAuthn isn\'t supported', async () => { +test("should throw error if WebAuthn isn't supported", async () => { mockSupportsWebAuthn.mockReturnValue(false); await expect(startAuthentication(goodOpts1)).rejects.toThrow( diff --git a/packages/browser/src/methods/startAuthentication.ts b/packages/browser/src/methods/startAuthentication.ts index 390b6ef4..a63eed97 100644 --- a/packages/browser/src/methods/startAuthentication.ts +++ b/packages/browser/src/methods/startAuthentication.ts @@ -6,7 +6,6 @@ import { import { bufferToBase64URLString } from '../helpers/bufferToBase64URLString'; import { base64URLStringToBuffer } from '../helpers/base64URLStringToBuffer'; -import { bufferToUTF8String } from '../helpers/bufferToUTF8String'; import { browserSupportsWebAuthn } from '../helpers/browserSupportsWebAuthn'; import { browserSupportsWebAuthnAutofill } from '../helpers/browserSupportsWebAuthnAutofill'; import { toPublicKeyCredentialDescriptor } from '../helpers/toPublicKeyCredentialDescriptor'; @@ -97,7 +96,7 @@ export async function startAuthentication( let userHandle = undefined; if (response.userHandle) { - userHandle = bufferToUTF8String(response.userHandle); + userHandle = bufferToBase64URLString(response.userHandle); } // Convert values to base64 to make it easier to send back to the server From 6af30185938addf9a12c6076dd460f5f0c5146f9 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 11 Apr 2024 21:42:59 -0700 Subject: [PATCH 05/12] Remove UTF-8 string helpers from browser --- packages/browser/src/helpers/bufferToUTF8String.ts | 7 ------- packages/browser/src/helpers/utf8StringToBuffer.ts | 7 ------- 2 files changed, 14 deletions(-) delete mode 100644 packages/browser/src/helpers/bufferToUTF8String.ts delete mode 100644 packages/browser/src/helpers/utf8StringToBuffer.ts diff --git a/packages/browser/src/helpers/bufferToUTF8String.ts b/packages/browser/src/helpers/bufferToUTF8String.ts deleted file mode 100644 index 0da3246a..00000000 --- a/packages/browser/src/helpers/bufferToUTF8String.ts +++ /dev/null @@ -1,7 +0,0 @@ -/** - * A helper method to convert an arbitrary ArrayBuffer, returned from an authenticator, to a UTF-8 - * string. - */ -export function bufferToUTF8String(value: ArrayBuffer): string { - return new TextDecoder('utf-8').decode(value); -} diff --git a/packages/browser/src/helpers/utf8StringToBuffer.ts b/packages/browser/src/helpers/utf8StringToBuffer.ts deleted file mode 100644 index b15e5fe8..00000000 --- a/packages/browser/src/helpers/utf8StringToBuffer.ts +++ /dev/null @@ -1,7 +0,0 @@ -/** - * A helper method to convert an arbitrary string sent from the server to an ArrayBuffer the - * authenticator will expect. - */ -export function utf8StringToBuffer(value: string): ArrayBuffer { - return new TextEncoder().encode(value); -} From 9ebd9ec6199d0fb716b6dfea80dbcffee8f88e92 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 11 Apr 2024 23:10:11 -0700 Subject: [PATCH 06/12] Update generateRegistrationOptions tests --- .../generateRegistrationOptions.test.ts | 23 +++---------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/packages/server/src/registration/generateRegistrationOptions.test.ts b/packages/server/src/registration/generateRegistrationOptions.test.ts index 4c5e0c1d..fb164b38 100644 --- a/packages/server/src/registration/generateRegistrationOptions.test.ts +++ b/packages/server/src/registration/generateRegistrationOptions.test.ts @@ -3,12 +3,13 @@ import { returnsNext, stub } from 'https://deno.land/std@0.198.0/testing/mock.ts import { generateRegistrationOptions } from './generateRegistrationOptions.ts'; import { _generateChallengeInternals } from '../helpers/generateChallenge.ts'; +import { isoBase64URL, isoUint8Array } from '../helpers/index.ts'; Deno.test('should generate credential request options suitable for sending via JSON', async () => { const rpName = 'SimpleWebAuthn'; const rpID = 'not.real'; const challenge = 'totallyrandomvalue'; - const userID = '1234'; + const userID = isoUint8Array.fromUTF8String('1234'); const userName = 'usernameHere'; const timeout = 1; const attestationType = 'indirect'; @@ -35,7 +36,7 @@ Deno.test('should generate credential request options suitable for sending via J id: rpID, }, user: { - id: userID, + id: isoBase64URL.fromBuffer(userID), name: userName, displayName: userDisplayName, }, @@ -64,7 +65,6 @@ Deno.test('should map excluded credential IDs if specified', async () => { rpName: 'SimpleWebAuthn', rpID: 'not.real', challenge: 'totallyrandomvalue', - userID: '1234', userName: 'usernameHere', excludeCredentials: [ { @@ -91,7 +91,6 @@ Deno.test('defaults to 60 seconds if no timeout is specified', async () => { rpName: 'SimpleWebAuthn', rpID: 'not.real', challenge: 'totallyrandomvalue', - userID: '1234', userName: 'usernameHere', }); @@ -103,7 +102,6 @@ Deno.test('defaults to none attestation if no attestation type is specified', as rpName: 'SimpleWebAuthn', rpID: 'not.real', challenge: 'totallyrandomvalue', - userID: '1234', userName: 'usernameHere', }); @@ -115,7 +113,6 @@ Deno.test('defaults to empty string for displayName if no userDisplayName is spe rpName: 'SimpleWebAuthn', rpID: 'not.real', challenge: 'totallyrandomvalue', - userID: '1234', userName: 'usernameHere', }); @@ -127,7 +124,6 @@ Deno.test('should set authenticatorSelection if specified', async () => { rpName: 'SimpleWebAuthn', rpID: 'not.real', challenge: 'totallyrandomvalue', - userID: '1234', userName: 'usernameHere', authenticatorSelection: { authenticatorAttachment: 'cross-platform', @@ -151,7 +147,6 @@ Deno.test('should set extensions if specified', async () => { rpName: 'SimpleWebAuthn', rpID: 'not.real', challenge: 'totallyrandomvalue', - userID: '1234', userName: 'usernameHere', extensions: { appid: 'simplewebauthn' }, }); @@ -163,7 +158,6 @@ Deno.test('should include credProps if extensions are not provided', async () => const options = await generateRegistrationOptions({ rpName: 'SimpleWebAuthn', rpID: 'not.real', - userID: '1234', userName: 'usernameHere', }); @@ -174,7 +168,6 @@ Deno.test('should include credProps if extensions are provided', async () => { const options = await generateRegistrationOptions({ rpName: 'SimpleWebAuthn', rpID: 'not.real', - userID: '1234', userName: 'usernameHere', extensions: { appid: 'simplewebauthn' }, }); @@ -194,7 +187,6 @@ Deno.test('should generate a challenge if one is not provided', async () => { const options = await generateRegistrationOptions({ rpID: 'not.real', rpName: 'SimpleWebAuthn', - userID: '1234', userName: 'usernameHere', }); @@ -208,7 +200,6 @@ Deno.test('should treat string challenges as UTF-8 strings', async () => { const options = await generateRegistrationOptions({ rpID: 'not.real', rpName: 'SimpleWebAuthn', - userID: '1234', userName: 'usernameHere', challenge: 'こんにちは', }); @@ -223,7 +214,6 @@ Deno.test('should use custom supported algorithm IDs as-is when provided', async const options = await generateRegistrationOptions({ rpID: 'not.real', rpName: 'SimpleWebAuthn', - userID: '1234', userName: 'usernameHere', supportedAlgorithmIDs: [-7, -8, -65535], }); @@ -242,7 +232,6 @@ Deno.test('should require resident key if residentKey option is absent but requi const options = await generateRegistrationOptions({ rpID: 'not.real', rpName: 'SimpleWebAuthn', - userID: '1234', userName: 'usernameHere', authenticatorSelection: { requireResidentKey: true, @@ -257,7 +246,6 @@ Deno.test('should discourage resident key if residentKey option is absent but re const options = await generateRegistrationOptions({ rpID: 'not.real', rpName: 'SimpleWebAuthn', - userID: '1234', userName: 'usernameHere', authenticatorSelection: { requireResidentKey: false, @@ -272,7 +260,6 @@ Deno.test('should prefer resident key if both residentKey and requireResidentKey const options = await generateRegistrationOptions({ rpID: 'not.real', rpName: 'SimpleWebAuthn', - userID: '1234', userName: 'usernameHere', }); @@ -284,7 +271,6 @@ Deno.test('should set requireResidentKey to true if residentKey if set to requir const options = await generateRegistrationOptions({ rpID: 'not.real', rpName: 'SimpleWebAuthn', - userID: '1234', userName: 'usernameHere', authenticatorSelection: { residentKey: 'required', @@ -299,7 +285,6 @@ Deno.test('should set requireResidentKey to false if residentKey if set to prefe const options = await generateRegistrationOptions({ rpID: 'not.real', rpName: 'SimpleWebAuthn', - userID: '1234', userName: 'usernameHere', authenticatorSelection: { residentKey: 'preferred', @@ -314,7 +299,6 @@ Deno.test('should set requireResidentKey to false if residentKey if set to disco const options = await generateRegistrationOptions({ rpID: 'not.real', rpName: 'SimpleWebAuthn', - userID: '1234', userName: 'usernameHere', authenticatorSelection: { residentKey: 'discouraged', @@ -330,7 +314,6 @@ Deno.test('should prefer Ed25519 in pubKeyCredParams', async () => { rpName: 'SimpleWebAuthn', rpID: 'not.real', challenge: 'totallyrandomvalue', - userID: '1234', userName: 'usernameHere', }); From 6e780a49d702c0f52dc17820ce85a64fa9df346f Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 12 Apr 2024 13:04:48 -0700 Subject: [PATCH 07/12] Rename options JSON args in browser methods --- .../browser/src/methods/startAuthentication.ts | 17 ++++++++--------- .../browser/src/methods/startRegistration.ts | 14 +++++++------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/packages/browser/src/methods/startAuthentication.ts b/packages/browser/src/methods/startAuthentication.ts index a63eed97..8b2e02d8 100644 --- a/packages/browser/src/methods/startAuthentication.ts +++ b/packages/browser/src/methods/startAuthentication.ts @@ -16,12 +16,11 @@ import { toAuthenticatorAttachment } from '../helpers/toAuthenticatorAttachment' /** * Begin authenticator "login" via WebAuthn assertion * - * @param requestOptionsJSON Output from **@simplewebauthn/server**'s `generateAuthenticationOptions()` - * @param useBrowserAutofill Initialize conditional UI to enable logging in via browser - * autofill prompts + * @param optionsJSON Output from **@simplewebauthn/server**'s `generateAuthenticationOptions()` + * @param useBrowserAutofill (Optional) Initialize conditional UI to enable logging in via browser autofill prompts. Defaults to `false`. */ export async function startAuthentication( - requestOptionsJSON: PublicKeyCredentialRequestOptionsJSON, + optionsJSON: PublicKeyCredentialRequestOptionsJSON, useBrowserAutofill = false, ): Promise { if (!browserSupportsWebAuthn()) { @@ -31,16 +30,16 @@ export async function startAuthentication( // We need to avoid passing empty array to avoid blocking retrieval // of public key let allowCredentials; - if (requestOptionsJSON.allowCredentials?.length !== 0) { - allowCredentials = requestOptionsJSON.allowCredentials?.map( + if (optionsJSON.allowCredentials?.length !== 0) { + allowCredentials = optionsJSON.allowCredentials?.map( toPublicKeyCredentialDescriptor, ); } // We need to convert some values to Uint8Arrays before passing the credentials to the navigator const publicKey: PublicKeyCredentialRequestOptions = { - ...requestOptionsJSON, - challenge: base64URLStringToBuffer(requestOptionsJSON.challenge), + ...optionsJSON, + challenge: base64URLStringToBuffer(optionsJSON.challenge), allowCredentials, }; @@ -58,7 +57,7 @@ export async function startAuthentication( // Check for an with "webauthn" in its `autocomplete` attribute const eligibleInputs = document.querySelectorAll( - 'input[autocomplete$=\'webauthn\']', + "input[autocomplete$='webauthn']", ); // WebAuthn autofill requires at least one valid input diff --git a/packages/browser/src/methods/startRegistration.ts b/packages/browser/src/methods/startRegistration.ts index 73c899e5..b661de7e 100644 --- a/packages/browser/src/methods/startRegistration.ts +++ b/packages/browser/src/methods/startRegistration.ts @@ -16,10 +16,10 @@ import { toAuthenticatorAttachment } from '../helpers/toAuthenticatorAttachment' /** * Begin authenticator "registration" via WebAuthn attestation * - * @param creationOptionsJSON Output from **@simplewebauthn/server**'s `generateRegistrationOptions()` + * @param optionsJSON Output from **@simplewebauthn/server**'s `generateRegistrationOptions()` */ export async function startRegistration( - creationOptionsJSON: PublicKeyCredentialCreationOptionsJSON, + optionsJSON: PublicKeyCredentialCreationOptionsJSON, ): Promise { if (!browserSupportsWebAuthn()) { throw new Error('WebAuthn is not supported in this browser'); @@ -27,13 +27,13 @@ export async function startRegistration( // We need to convert some values to Uint8Arrays before passing the credentials to the navigator const publicKey: PublicKeyCredentialCreationOptions = { - ...creationOptionsJSON, - challenge: base64URLStringToBuffer(creationOptionsJSON.challenge), + ...optionsJSON, + challenge: base64URLStringToBuffer(optionsJSON.challenge), user: { - ...creationOptionsJSON.user, - id: base64URLStringToBuffer(creationOptionsJSON.user.id), + ...optionsJSON.user, + id: base64URLStringToBuffer(optionsJSON.user.id), }, - excludeCredentials: creationOptionsJSON.excludeCredentials?.map( + excludeCredentials: optionsJSON.excludeCredentials?.map( toPublicKeyCredentialDescriptor, ), }; From d470a1c2cd8451dbfc491af962f5b304409c585d Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 12 Apr 2024 13:05:32 -0700 Subject: [PATCH 08/12] Explicitly disallow string userIDs --- .../generateRegistrationOptions.test.ts | 17 ++++++++++++++++- .../registration/generateRegistrationOptions.ts | 14 ++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/packages/server/src/registration/generateRegistrationOptions.test.ts b/packages/server/src/registration/generateRegistrationOptions.test.ts index fb164b38..1dc866d9 100644 --- a/packages/server/src/registration/generateRegistrationOptions.test.ts +++ b/packages/server/src/registration/generateRegistrationOptions.test.ts @@ -1,4 +1,4 @@ -import { assertEquals } from 'https://deno.land/std@0.198.0/assert/mod.ts'; +import { assertEquals, assertRejects } from 'https://deno.land/std@0.198.0/assert/mod.ts'; import { returnsNext, stub } from 'https://deno.land/std@0.198.0/testing/mock.ts'; import { generateRegistrationOptions } from './generateRegistrationOptions.ts'; @@ -319,3 +319,18 @@ Deno.test('should prefer Ed25519 in pubKeyCredParams', async () => { assertEquals(options.pubKeyCredParams[0].alg, -8); }); + +Deno.test('should raise if string is specified for userID', async () => { + await assertRejects( + () => + generateRegistrationOptions({ + rpName: 'SimpleWebAuthn', + rpID: 'not.real', + userName: 'usernameHere', + // @ts-ignore: Pretending a dev missed a refactor between v9 and v10 + userID: 'customUserID', + }), + Error, + 'String values for `userID` are no longer supported. See https://simplewebauthn.dev/docs/advanced/server/custom-user-ids)', + ); +}); diff --git a/packages/server/src/registration/generateRegistrationOptions.ts b/packages/server/src/registration/generateRegistrationOptions.ts index c8d7ea8e..39a0b18d 100644 --- a/packages/server/src/registration/generateRegistrationOptions.ts +++ b/packages/server/src/registration/generateRegistrationOptions.ts @@ -105,8 +105,8 @@ export async function generateRegistrationOptions( const { rpName, rpID, - userID, userName, + userID, challenge = await generateChallenge(), userDisplayName = '', timeout = 60000, @@ -165,11 +165,21 @@ export async function generateRegistrationOptions( _challenge = isoUint8Array.fromUTF8String(_challenge); } + /** + * Explicitly disallow use of strings for userID anymore because `isoBase64URL.fromBuffer()` below + * will return an empty string if one gets through! + */ + if (typeof userID === 'string') { + throw new Error( + `String values for \`userID\` are no longer supported. See https://simplewebauthn.dev/docs/advanced/server/custom-user-ids)`, + ); + } + /** * Generate a user ID if one is not provided */ let _userID = userID; - if (_userID === undefined) { + if (!_userID) { _userID = await generateUserID(); } From 1cff4ceaba6fc468e6d00146dbb706c6318fcc1a Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 12 Apr 2024 13:05:37 -0700 Subject: [PATCH 09/12] Tweak userHandle type --- packages/types/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index b7694fc1..9d0cfe40 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -168,7 +168,7 @@ export interface AuthenticatorAssertionResponseJSON { clientDataJSON: Base64URLString; authenticatorData: Base64URLString; signature: Base64URLString; - userHandle?: string; + userHandle?: Base64URLString; } /** From a95489efbbfbaca48322bb728daee4f2c782a2a2 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 12 Apr 2024 13:18:15 -0700 Subject: [PATCH 10/12] Remove trailing close parenthesis --- .../server/src/registration/generateRegistrationOptions.test.ts | 2 +- packages/server/src/registration/generateRegistrationOptions.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/registration/generateRegistrationOptions.test.ts b/packages/server/src/registration/generateRegistrationOptions.test.ts index 1dc866d9..6f8282e1 100644 --- a/packages/server/src/registration/generateRegistrationOptions.test.ts +++ b/packages/server/src/registration/generateRegistrationOptions.test.ts @@ -331,6 +331,6 @@ Deno.test('should raise if string is specified for userID', async () => { userID: 'customUserID', }), Error, - 'String values for `userID` are no longer supported. See https://simplewebauthn.dev/docs/advanced/server/custom-user-ids)', + 'String values for `userID` are no longer supported. See https://simplewebauthn.dev/docs/advanced/server/custom-user-ids', ); }); diff --git a/packages/server/src/registration/generateRegistrationOptions.ts b/packages/server/src/registration/generateRegistrationOptions.ts index 39a0b18d..18283509 100644 --- a/packages/server/src/registration/generateRegistrationOptions.ts +++ b/packages/server/src/registration/generateRegistrationOptions.ts @@ -171,7 +171,7 @@ export async function generateRegistrationOptions( */ if (typeof userID === 'string') { throw new Error( - `String values for \`userID\` are no longer supported. See https://simplewebauthn.dev/docs/advanced/server/custom-user-ids)`, + `String values for \`userID\` are no longer supported. See https://simplewebauthn.dev/docs/advanced/server/custom-user-ids`, ); } From 84a2ea53dd2fbc413d0c4381ebf0bbfaefcd5e82 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 12 Apr 2024 13:25:16 -0700 Subject: [PATCH 11/12] Clarify string encoding on isoBase64URL methods --- packages/server/src/helpers/iso/isoBase64URL.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/server/src/helpers/iso/isoBase64URL.ts b/packages/server/src/helpers/iso/isoBase64URL.ts index 99bf82a5..5c2388d4 100644 --- a/packages/server/src/helpers/iso/isoBase64URL.ts +++ b/packages/server/src/helpers/iso/isoBase64URL.ts @@ -41,16 +41,16 @@ export function toBase64(base64urlString: string): string { } /** - * Encode a string to base64url + * Encode a UTF-8 string to base64url */ -export function fromString(ascii: string): string { - return base64.fromString(ascii, true); +export function fromUTF8String(utf8String: string): string { + return base64.fromString(utf8String, true); } /** - * Decode a base64url string into its original string + * Decode a base64url string into its original UTF-8 string */ -export function toString(base64urlString: string): string { +export function toUTF8String(base64urlString: string): string { return base64.toString(base64urlString, true); } From b316c3f6de77824680c8e153e9124aeaf9c10d4f Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 12 Apr 2024 13:25:40 -0700 Subject: [PATCH 12/12] Update uses of base64url string methods --- .../verifyAuthenticationResponse.test.ts | 8 ++++---- packages/server/src/helpers/decodeClientDataJSON.ts | 2 +- packages/server/src/metadata/parseJWT.ts | 4 ++-- .../verifyAttestationAndroidSafetyNet.ts | 4 ++-- .../registration/verifyRegistrationResponse.test.ts | 12 ++++++------ 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/server/src/authentication/verifyAuthenticationResponse.test.ts b/packages/server/src/authentication/verifyAuthenticationResponse.test.ts index 844726f6..f4046a6a 100644 --- a/packages/server/src/authentication/verifyAuthenticationResponse.test.ts +++ b/packages/server/src/authentication/verifyAuthenticationResponse.test.ts @@ -390,7 +390,7 @@ Deno.test('should pass verification if custom challenge verifier returns true', actualChallenge: string; arbitraryData: string; } = JSON.parse( - isoBase64URL.toString(challenge), + isoBase64URL.toUTF8String(challenge), ); return parsedChallenge.actualChallenge === 'K3QxOjnVJLiGlnVEp5va5QJeMVWNf_7PYgutgbAtAUA'; @@ -448,7 +448,7 @@ Deno.test('should pass verification if custom challenge verifier returns a Promi actualChallenge: string; arbitraryData: string; } = JSON.parse( - isoBase64URL.toString(challenge), + isoBase64URL.toUTF8String(challenge), ); return Promise.resolve( parsedChallenge.actualChallenge === @@ -597,7 +597,7 @@ const assertionResponse: AuthenticationResponseJSON = { clientExtensionResults: {}, type: 'public-key', }; -const assertionChallenge = isoBase64URL.fromString( +const assertionChallenge = isoBase64URL.fromUTF8String( 'totallyUniqueValueEveryTime', ); const assertionOrigin = 'https://dev.dontneeda.pw'; @@ -627,7 +627,7 @@ const assertionFirstTimeUsedResponse: AuthenticationResponseJSON = { type: 'public-key', clientExtensionResults: {}, }; -const assertionFirstTimeUsedChallenge = isoBase64URL.fromString( +const assertionFirstTimeUsedChallenge = isoBase64URL.fromUTF8String( 'totallyUniqueValueEveryAssertion', ); const assertionFirstTimeUsedOrigin = 'https://dev.dontneeda.pw'; diff --git a/packages/server/src/helpers/decodeClientDataJSON.ts b/packages/server/src/helpers/decodeClientDataJSON.ts index aeb42511..8230f42b 100644 --- a/packages/server/src/helpers/decodeClientDataJSON.ts +++ b/packages/server/src/helpers/decodeClientDataJSON.ts @@ -5,7 +5,7 @@ import type { Base64URLString } from '../deps.ts'; * Decode an authenticator's base64url-encoded clientDataJSON to JSON */ export function decodeClientDataJSON(data: Base64URLString): ClientDataJSON { - const toString = isoBase64URL.toString(data); + const toString = isoBase64URL.toUTF8String(data); const clientData: ClientDataJSON = JSON.parse(toString); return _decodeClientDataJSONInternals.stubThis(clientData); diff --git a/packages/server/src/metadata/parseJWT.ts b/packages/server/src/metadata/parseJWT.ts index a86dacde..3b04aeac 100644 --- a/packages/server/src/metadata/parseJWT.ts +++ b/packages/server/src/metadata/parseJWT.ts @@ -6,8 +6,8 @@ import { isoBase64URL } from '../helpers/iso/index.ts'; export function parseJWT(jwt: string): [T1, T2, string] { const parts = jwt.split('.'); return [ - JSON.parse(isoBase64URL.toString(parts[0])) as T1, - JSON.parse(isoBase64URL.toString(parts[1])) as T2, + JSON.parse(isoBase64URL.toUTF8String(parts[0])) as T1, + JSON.parse(isoBase64URL.toUTF8String(parts[1])) as T2, parts[2], ]; } diff --git a/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts b/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts index 5862cc54..29a20f1a 100644 --- a/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts +++ b/packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.ts @@ -43,10 +43,10 @@ export async function verifyAttestationAndroidSafetyNet( const jwtParts = jwt.split('.'); const HEADER: SafetyNetJWTHeader = JSON.parse( - isoBase64URL.toString(jwtParts[0]), + isoBase64URL.toUTF8String(jwtParts[0]), ); const PAYLOAD: SafetyNetJWTPayload = JSON.parse( - isoBase64URL.toString(jwtParts[1]), + isoBase64URL.toUTF8String(jwtParts[1]), ); const SIGNATURE: SafetyNetJWTSignature = jwtParts[2]; diff --git a/packages/server/src/registration/verifyRegistrationResponse.test.ts b/packages/server/src/registration/verifyRegistrationResponse.test.ts index 89b4694c..09f21231 100644 --- a/packages/server/src/registration/verifyRegistrationResponse.test.ts +++ b/packages/server/src/registration/verifyRegistrationResponse.test.ts @@ -775,7 +775,7 @@ Deno.test('should pass verification if custom challenge verifier returns true', actualChallenge: string; arbitraryData: string; } = JSON.parse( - isoBase64URL.toString(challenge), + isoBase64URL.toUTF8String(challenge), ); return parsedChallenge.actualChallenge === 'xRsYdCQv5WZOqmxReiZl6C9q5SfrZne4lNSr9QVtPig'; @@ -823,7 +823,7 @@ Deno.test('should pass verification if custom challenge verifier returns a Promi actualChallenge: string; arbitraryData: string; } = JSON.parse( - isoBase64URL.toString(challenge), + isoBase64URL.toUTF8String(challenge), ); return Promise.resolve( parsedChallenge.actualChallenge === @@ -1011,7 +1011,7 @@ const attestationFIDOU2F: RegistrationResponseJSON = { type: 'public-key', clientExtensionResults: {}, }; -const attestationFIDOU2FChallenge = isoBase64URL.fromString( +const attestationFIDOU2FChallenge = isoBase64URL.fromUTF8String( 'totallyUniqueValueEveryAttestation', ); @@ -1033,7 +1033,7 @@ const attestationPacked: RegistrationResponseJSON = { clientExtensionResults: {}, type: 'public-key', }; -const attestationPackedChallenge = isoBase64URL.fromString( +const attestationPackedChallenge = isoBase64URL.fromUTF8String( 's6PIbBnPPnrGNSBxNdtDrT7UrVYJK9HM', ); @@ -1065,7 +1065,7 @@ const attestationPackedX5C: RegistrationResponseJSON = { type: 'public-key', clientExtensionResults: {}, }; -const attestationPackedX5CChallenge = isoBase64URL.fromString( +const attestationPackedX5CChallenge = isoBase64URL.fromUTF8String( 'totallyUniqueValueEveryTime', ); @@ -1085,6 +1085,6 @@ const attestationNone: RegistrationResponseJSON = { type: 'public-key', clientExtensionResults: {}, }; -const attestationNoneChallenge = isoBase64URL.fromString( +const attestationNoneChallenge = isoBase64URL.fromUTF8String( 'hEccPWuziP00H0p5gxh2_u5_PC4NeYgd', );