From 5f508216dc3ee38a7f17c038b367e8575ab14929 Mon Sep 17 00:00:00 2001 From: Antoine Cormouls Date: Fri, 4 Dec 2020 15:01:35 +0100 Subject: Optional Allow Credential --- .../browser/src/methods/startAssertion.test.ts | 25 ++++++++++++++++------ packages/browser/src/methods/startAssertion.ts | 7 +++++- 2 files changed, 24 insertions(+), 8 deletions(-) (limited to 'packages/browser/src') diff --git a/packages/browser/src/methods/startAssertion.test.ts b/packages/browser/src/methods/startAssertion.test.ts index 996f66a..656c419 100644 --- a/packages/browser/src/methods/startAssertion.test.ts +++ b/packages/browser/src/methods/startAssertion.test.ts @@ -39,6 +39,12 @@ const goodOpts2UTF8: PublicKeyCredentialRequestOptionsJSON = { timeout: 1, }; +// Without allow credentials +const goodOpts3: PublicKeyCredentialRequestOptionsJSON = { + challenge: bufferToBase64URLString(toUint8Array('fizz')), + timeout: 1, +}; + beforeEach(() => { mockNavigatorGet.mockReset(); mockSupportsWebauthn.mockReset(); @@ -59,15 +65,20 @@ test('should convert options before passing to navigator.credentials.get(...)', }, ); - await startAssertion(goodOpts1); + const checkWithOpts = async (opts: PublicKeyCredentialRequestOptionsJSON) => { + await startAssertion(opts); - const argsPublicKey = mockNavigatorGet.mock.calls[0][0].publicKey; - const credId = argsPublicKey.allowCredentials[0].id; + const argsPublicKey = mockNavigatorGet.mock.calls[0][0].publicKey; + const credId = argsPublicKey.allowCredentials[0].id; + + expect(new Uint8Array(argsPublicKey.challenge)).toEqual(new Uint8Array([102, 105, 122, 122])); + // Make sure the credential ID is an ArrayBuffer with a length of 64 + expect(credId instanceof ArrayBuffer).toEqual(true); + expect(credId.byteLength).toEqual(64); + }; - expect(new Uint8Array(argsPublicKey.challenge)).toEqual(new Uint8Array([102, 105, 122, 122])); - // Make sure the credential ID is an ArrayBuffer with a length of 64 - expect(credId instanceof ArrayBuffer).toEqual(true); - expect(credId.byteLength).toEqual(64); + await checkWithOpts(goodOpts1); + await checkWithOpts(goodOpts3); done(); }); diff --git a/packages/browser/src/methods/startAssertion.ts b/packages/browser/src/methods/startAssertion.ts index 09e416f..fbc6f59 100644 --- a/packages/browser/src/methods/startAssertion.ts +++ b/packages/browser/src/methods/startAssertion.ts @@ -25,7 +25,12 @@ export default async function startAssertion( const publicKey: PublicKeyCredentialRequestOptions = { ...requestOptionsJSON, challenge: base64URLStringToBuffer(requestOptionsJSON.challenge), - allowCredentials: requestOptionsJSON.allowCredentials.map(toPublicKeyCredentialDescriptor), + // We need to avoid passing empty array to avoid blocking retrieval + // of public key + allowCredentials: + requestOptionsJSON.allowCredentials?.length === 0 + ? undefined + : requestOptionsJSON.allowCredentials?.map(toPublicKeyCredentialDescriptor), }; // Wait for the user to complete assertion -- cgit v1.2.3 From 106e05f4294aabed133a26efb31252628fffec93 Mon Sep 17 00:00:00 2001 From: Antoine Cormouls Date: Mon, 7 Dec 2020 09:11:40 +0100 Subject: review fixes --- .../browser/src/methods/startAssertion.test.ts | 50 ++++++++++++++++------ packages/browser/src/methods/startAssertion.ts | 14 +++--- 2 files changed, 46 insertions(+), 18 deletions(-) (limited to 'packages/browser/src') diff --git a/packages/browser/src/methods/startAssertion.test.ts b/packages/browser/src/methods/startAssertion.test.ts index 656c419..0a8a16f 100644 --- a/packages/browser/src/methods/startAssertion.test.ts +++ b/packages/browser/src/methods/startAssertion.test.ts @@ -45,6 +45,12 @@ const goodOpts3: PublicKeyCredentialRequestOptionsJSON = { timeout: 1, }; +const goodOpts4: PublicKeyCredentialRequestOptionsJSON = { + challenge: bufferToBase64URLString(toUint8Array('fizz')), + timeout: 1, + allowCredentials: [], +}; + beforeEach(() => { mockNavigatorGet.mockReset(); mockSupportsWebauthn.mockReset(); @@ -65,24 +71,44 @@ test('should convert options before passing to navigator.credentials.get(...)', }, ); - const checkWithOpts = async (opts: PublicKeyCredentialRequestOptionsJSON) => { - await startAssertion(opts); - - const argsPublicKey = mockNavigatorGet.mock.calls[0][0].publicKey; - const credId = argsPublicKey.allowCredentials[0].id; + await startAssertion(goodOpts1); - expect(new Uint8Array(argsPublicKey.challenge)).toEqual(new Uint8Array([102, 105, 122, 122])); - // Make sure the credential ID is an ArrayBuffer with a length of 64 - expect(credId instanceof ArrayBuffer).toEqual(true); - expect(credId.byteLength).toEqual(64); - }; + const argsPublicKey = mockNavigatorGet.mock.calls[0][0].publicKey; + const credId = argsPublicKey.allowCredentials[0].id; - await checkWithOpts(goodOpts1); - await checkWithOpts(goodOpts3); + expect(new Uint8Array(argsPublicKey.challenge)).toEqual(new Uint8Array([102, 105, 122, 122])); + // Make sure the credential ID is an ArrayBuffer with a length of 64 + expect(credId instanceof ArrayBuffer).toEqual(true); + expect(credId.byteLength).toEqual(64); done(); }); +test('should support optional allowCredential', async () => { + mockSupportsWebauthn.mockReturnValue(true); + + // Stub out a response so the method won't throw + mockNavigatorGet.mockImplementation( + (): Promise => { + return new Promise(resolve => { + resolve({ + response: {}, + getClientExtensionResults: () => ({}), + }); + }); + }, + ); + + await startAssertion(goodOpts3); + let allowCredentials = mockNavigatorGet.mock.calls[0][0].allowCredentials; + expect(allowCredentials).toEqual(undefined); + + // Should convert empty array to undefined + await startAssertion(goodOpts4); + allowCredentials = mockNavigatorGet.mock.calls[1][0].allowCredentials; + expect(allowCredentials).toEqual(undefined); +}); + test('should return base64url-encoded response values', async done => { mockSupportsWebauthn.mockReturnValue(true); diff --git a/packages/browser/src/methods/startAssertion.ts b/packages/browser/src/methods/startAssertion.ts index fbc6f59..e02e577 100644 --- a/packages/browser/src/methods/startAssertion.ts +++ b/packages/browser/src/methods/startAssertion.ts @@ -21,16 +21,18 @@ export default async function startAssertion( throw new Error('WebAuthn is not supported in this browser'); } + // 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(toPublicKeyCredentialDescriptor); + } + // We need to convert some values to Uint8Arrays before passing the credentials to the navigator const publicKey: PublicKeyCredentialRequestOptions = { ...requestOptionsJSON, challenge: base64URLStringToBuffer(requestOptionsJSON.challenge), - // We need to avoid passing empty array to avoid blocking retrieval - // of public key - allowCredentials: - requestOptionsJSON.allowCredentials?.length === 0 - ? undefined - : requestOptionsJSON.allowCredentials?.map(toPublicKeyCredentialDescriptor), + allowCredentials, }; // Wait for the user to complete assertion -- cgit v1.2.3 From 87f337f82b1054c55456be834ae00f3ae9dd75f0 Mon Sep 17 00:00:00 2001 From: Antoine Cormouls Date: Mon, 7 Dec 2020 17:12:12 +0100 Subject: review fxes --- package.json | 4 +- .../browser/src/methods/startAssertion.test.ts | 47 +++++++++++++--------- 2 files changed, 31 insertions(+), 20 deletions(-) (limited to 'packages/browser/src') diff --git a/package.json b/package.json index c3618f1..cb57499 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,9 @@ "test": "lerna run test", "build:types": "lerna bootstrap --scope=@simplewebauthn/typescript-types", "build:browser": "lerna bootstrap --scope=@simplewebauthn/browser", - "build:server": "lerna bootstrap --scope=@simplewebauthn/server" + "build:server": "lerna bootstrap --scope=@simplewebauthn/server", + "dev:server": "lerna exec npm run test:watch --scope=@simplewebauthn/server", + "dev:browser": "lerna exec npm run test:watch --scope=@simplewebauthn/browser" }, "devDependencies": { "@simplewebauthn/typescript-types": "^0.10.0", diff --git a/packages/browser/src/methods/startAssertion.test.ts b/packages/browser/src/methods/startAssertion.test.ts index 0a8a16f..c74b88f 100644 --- a/packages/browser/src/methods/startAssertion.test.ts +++ b/packages/browser/src/methods/startAssertion.test.ts @@ -39,18 +39,6 @@ const goodOpts2UTF8: PublicKeyCredentialRequestOptionsJSON = { timeout: 1, }; -// Without allow credentials -const goodOpts3: PublicKeyCredentialRequestOptionsJSON = { - challenge: bufferToBase64URLString(toUint8Array('fizz')), - timeout: 1, -}; - -const goodOpts4: PublicKeyCredentialRequestOptionsJSON = { - challenge: bufferToBase64URLString(toUint8Array('fizz')), - timeout: 1, - allowCredentials: [], -}; - beforeEach(() => { mockNavigatorGet.mockReset(); mockSupportsWebauthn.mockReset(); @@ -99,14 +87,35 @@ test('should support optional allowCredential', async () => { }, ); - await startAssertion(goodOpts3); - let allowCredentials = mockNavigatorGet.mock.calls[0][0].allowCredentials; - expect(allowCredentials).toEqual(undefined); + await startAssertion({ + challenge: bufferToBase64URLString(toUint8Array('fizz')), + timeout: 1, + }); + + expect(mockNavigatorGet.mock.calls[0][0].allowCredentials).toEqual(undefined); +}); + +test('should convert allow allowCredential to undefined when empty', async () => { + mockSupportsWebauthn.mockReturnValue(true); + + // Stub out a response so the method won't throw + mockNavigatorGet.mockImplementation( + (): Promise => { + return new Promise(resolve => { + resolve({ + response: {}, + getClientExtensionResults: () => ({}), + }); + }); + }, + ); - // Should convert empty array to undefined - await startAssertion(goodOpts4); - allowCredentials = mockNavigatorGet.mock.calls[1][0].allowCredentials; - expect(allowCredentials).toEqual(undefined); + await startAssertion({ + challenge: bufferToBase64URLString(toUint8Array('fizz')), + timeout: 1, + allowCredentials: [], + }); + expect(mockNavigatorGet.mock.calls[0][0].allowCredentials).toEqual(undefined); }); test('should return base64url-encoded response values', async done => { -- cgit v1.2.3