Merge pull request #21 from MasterKale/feature/improve-browser

Refactor Megamix 1

9 files changed, 127 insertions, 91 deletions

diff --git a/packages/browser/src/helpers/base64URLStringToBuffer.ts b/packages/browser/src/helpers/base64URLStringToBuffer.ts
new file mode 100644
index 0000000..baa258f
--- /dev/null
+++ b/packages/browser/src/helpers/base64URLStringToBuffer.ts
@@ -0,0 +1,33 @@
+/**
+ * Convert from a Base64URL-encoded string to an Array Buffer. Best used when converting a
+ * credential ID from a JSON string to an ArrayBuffer, like in allowCredentials or
+ * excludeCredentials
+ *
+ * Helper method to compliment `bufferToBase64URLString`
+ */
+export default function base64URLStringToBuffer(base64URLString: string): ArrayBuffer {
+  // Convert from Base64URL to Base64
+  const base64 = base64URLString.replace(/-/g, '+').replace(/_/g, '/');
+  /**
+   * Pad with '=' until it's a multiple of four
+   * (4 - (85 % 4 = 1) = 3) % 4 = 3 padding
+   * (4 - (86 % 4 = 2) = 2) % 4 = 2 padding
+   * (4 - (87 % 4 = 3) = 1) % 4 = 1 padding
+   * (4 - (88 % 4 = 0) = 4) % 4 = 0 padding
+   */
+  const padLength = (4 - (base64.length % 4)) % 4;
+  const padded = base64.padEnd(base64.length + padLength, '=');
+
+  // Convert to a binary string
+  const binary = atob(padded);
+
+  // Convert binary string to buffer
+  const buffer = new ArrayBuffer(binary.length);
+  const bytes = new Uint8Array(buffer);
+
+  for (let i = 0; i < binary.length; i++) {
+    bytes[i] = binary.charCodeAt(i);
+  }
+
+  return buffer;
+}
diff --git a/packages/browser/src/helpers/bufferToBase64URLString.ts b/packages/browser/src/helpers/bufferToBase64URLString.ts
new file mode 100644
index 0000000..954f4b0
--- /dev/null
+++ b/packages/browser/src/helpers/bufferToBase64URLString.ts
@@ -0,0 +1,21 @@
+/**
+ * Convert the given array buffer into a Base64URL-encoded string. Ideal for converting various
+ * credential response ArrayBuffers to string for sending back to the server as JSON.
+ *
+ * Helper method to compliment `base64URLStringToBuffer`
+ */
+export default function bufferToBase64URLString(buffer: ArrayBuffer): string {
+  const bytes = new Uint8Array(buffer);
+  let str = '';
+
+  for (const charCode of bytes) {
+    str += String.fromCharCode(charCode);
+  }
+
+  const base64String = btoa(str);
+
+  return base64String
+    .replace(/\+/g, '-')
+    .replace(/\//g, '_')
+    .replace(/=/g, '');
+}
diff --git a/packages/browser/src/helpers/toBase64String.test.ts b/packages/browser/src/helpers/toBase64String.test.ts
deleted file mode 100644
index bbcb11b..0000000
--- a/packages/browser/src/helpers/toBase64String.test.ts
+++ /dev/null
@@ -1,15 +0,0 @@
-import toBase64String from './toBase64String';
-
-import toUint8Array from './toUint8Array';
-
-test('should convert a Buffer to a string with a length that is a multiple of 4', () => {
-  const base64 = toBase64String(Buffer.from('123456', 'ascii'));
-
-  expect(base64.length % 4).toEqual(0);
-});
-
-test('should convert a Uint8Array to a string with a length that is a multiple of 4', () => {
-  const base64 = toBase64String(toUint8Array('123456'));
-
-  expect(base64.length % 4).toEqual(0);
-});
diff --git a/packages/browser/src/helpers/toBase64String.ts b/packages/browser/src/helpers/toBase64String.ts
deleted file mode 100644
index 3178695..0000000
--- a/packages/browser/src/helpers/toBase64String.ts
+++ /dev/null
@@ -1,6 +0,0 @@
-import base64js from 'base64-js';
-
-export default function toBase64String(buffer: ArrayBuffer): string {
-  // TODO: Make sure converting buffer to Uint8Array() is correct
-  return base64js.fromByteArray(new Uint8Array(buffer)).replace(/\+/g, '-').replace(/\//g, '_');
-}
diff --git a/packages/browser/src/helpers/toPublicKeyCredentialDescriptor.ts b/packages/browser/src/helpers/toPublicKeyCredentialDescriptor.ts
index c37fed0..8fad78b 100644
--- a/packages/browser/src/helpers/toPublicKeyCredentialDescriptor.ts
+++ b/packages/browser/src/helpers/toPublicKeyCredentialDescriptor.ts
@@ -1,16 +1,14 @@
-import base64js from 'base64-js';
 import type { PublicKeyCredentialDescriptorJSON } from '@simplewebauthn/typescript-types';
 
+import base64URLStringToBuffer from './base64URLStringToBuffer';
+
 export default function toPublicKeyCredentialDescriptor(
   descriptor: PublicKeyCredentialDescriptorJSON,
 ): PublicKeyCredentialDescriptor {
-  // Make sure the Base64'd credential ID length is a multiple of 4 or else toByteArray will throw
   const { id } = descriptor;
-  const padLength = 4 - (id.length % 4);
-  const paddedId = id.padEnd(id.length + padLength, '=');
 
   return {
     ...descriptor,
-    id: base64js.toByteArray(paddedId),
+    id: base64URLStringToBuffer(id),
   };
 }
diff --git a/packages/browser/src/methods/startAssertion.test.ts b/packages/browser/src/methods/startAssertion.test.ts
index db401dc..669e8eb 100644
--- a/packages/browser/src/methods/startAssertion.test.ts
+++ b/packages/browser/src/methods/startAssertion.test.ts
@@ -1,13 +1,9 @@
-import base64js from 'base64-js';
-
 import {
   AssertionCredential,
   PublicKeyCredentialRequestOptionsJSON,
 } from '@simplewebauthn/typescript-types';
 
-import toUint8Array from '../helpers/toUint8Array';
 import supportsWebauthn from '../helpers/supportsWebauthn';
-import toBase64String from '../helpers/toBase64String';
 
 import startAssertion from './startAssertion';
 
@@ -16,16 +12,16 @@ jest.mock('../helpers/supportsWebauthn');
 const mockNavigatorGet = window.navigator.credentials.get as jest.Mock;
 const mockSupportsWebauthn = supportsWebauthn as jest.Mock;
 
-const mockAuthenticatorData = toBase64String(toUint8Array('mockAuthenticatorData'));
-const mockClientDataJSON = toBase64String(toUint8Array('mockClientDataJSON'));
-const mockSignature = toBase64String(toUint8Array('mockSignature'));
-const mockUserHandle = toBase64String(toUint8Array('mockUserHandle'));
+const mockAuthenticatorData = 'mockAuthenticatorData';
+const mockClientDataJSON = 'mockClientDataJSON';
+const mockSignature = 'mockSignature';
+const mockUserHandle = 'mockUserHandle';
 
 const goodOpts1: PublicKeyCredentialRequestOptionsJSON = {
   challenge: 'fizz',
   allowCredentials: [
     {
-      id: 'abcdefgfdnsdfunguisdfgs',
+      id: 'C0VGlvYFratUdAV1iCw-ULpUW8E-exHPXQChBfyVeJZCMfjMFcwDmOFgoMUz39LoMtCJUBW8WPlLkGT6q8qTCg',
       type: 'public-key',
       transports: ['nfc'],
     },
@@ -45,7 +41,10 @@ test('should convert options before passing to navigator.credentials.get(...)',
   mockNavigatorGet.mockImplementation(
     (): Promise<any> => {
       return new Promise(resolve => {
-        resolve({ response: {} });
+        resolve({
+          response: {},
+          getClientExtensionResults: () => ({}),
+        });
       });
     },
   );
@@ -53,31 +52,30 @@ test('should convert options before passing to navigator.credentials.get(...)',
   await startAssertion(goodOpts1);
 
   const argsPublicKey = mockNavigatorGet.mock.calls[0][0].publicKey;
-  const credId = base64js.fromByteArray(argsPublicKey.allowCredentials[0].id);
+  const credId = argsPublicKey.allowCredentials[0].id;
 
-  expect(argsPublicKey.challenge).toEqual(toUint8Array(goodOpts1.challenge));
-  // Make sure the credential ID is a proper base64 with a length that's a multiple of 4
-  expect(credId.length % 4).toEqual(0);
+  expect(JSON.stringify(argsPublicKey.challenge)).toEqual('{"0":102,"1":105,"2":122,"3":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 return base64-encoded response values', async done => {
+test('should return base64url-encoded response values', async done => {
   mockSupportsWebauthn.mockReturnValue(true);
 
-  const credentialID = 'foobar';
-
   mockNavigatorGet.mockImplementation(
     (): Promise<AssertionCredential> => {
       return new Promise(resolve => {
         resolve({
           id: 'foobar',
-          rawId: toUint8Array('foobar'),
+          rawId: Buffer.from('foobar', 'ascii'),
           response: {
-            authenticatorData: base64js.toByteArray(mockAuthenticatorData),
-            clientDataJSON: base64js.toByteArray(mockClientDataJSON),
-            signature: base64js.toByteArray(mockSignature),
-            userHandle: base64js.toByteArray(mockUserHandle),
+            authenticatorData: Buffer.from(mockAuthenticatorData, 'ascii'),
+            clientDataJSON: Buffer.from(mockClientDataJSON, 'ascii'),
+            signature: Buffer.from(mockSignature, 'ascii'),
+            userHandle: Buffer.from(mockUserHandle, 'ascii'),
           },
           getClientExtensionResults: () => ({}),
           type: 'webauthn.get',
@@ -88,13 +86,11 @@ test('should return base64-encoded response values', async done => {
 
   const response = await startAssertion(goodOpts1);
 
-  expect(response).toEqual({
-    base64CredentialID: credentialID,
-    base64AuthenticatorData: mockAuthenticatorData,
-    base64ClientDataJSON: mockClientDataJSON,
-    base64Signature: mockSignature,
-    base64UserHandle: mockUserHandle,
-  });
+  expect(response.rawId).toEqual('Zm9vYmFy');
+  expect(response.response.authenticatorData).toEqual('bW9ja0F1dGhlbnRpY2F0b3JEYXRh');
+  expect(response.response.clientDataJSON).toEqual('bW9ja0NsaWVudERhdGFKU09O');
+  expect(response.response.signature).toEqual('bW9ja1NpZ25hdHVyZQ');
+  expect(response.response.userHandle).toEqual('bW9ja1VzZXJIYW5kbGU');
 
   done();
 });
diff --git a/packages/browser/src/methods/startAssertion.ts b/packages/browser/src/methods/startAssertion.ts
index 093cf30..b65325b 100644
--- a/packages/browser/src/methods/startAssertion.ts
+++ b/packages/browser/src/methods/startAssertion.ts
@@ -1,11 +1,11 @@
 import {
   PublicKeyCredentialRequestOptionsJSON,
-  AuthenticatorAssertionResponseJSON,
   AssertionCredential,
+  AssertionCredentialJSON,
 } from '@simplewebauthn/typescript-types';
 
 import toUint8Array from '../helpers/toUint8Array';
-import toBase64String from '../helpers/toBase64String';
+import bufferToBase64URLString from '../helpers/bufferToBase64URLString';
 import supportsWebauthn from '../helpers/supportsWebauthn';
 import toPublicKeyCredentialDescriptor from '../helpers/toPublicKeyCredentialDescriptor';
 
@@ -16,7 +16,7 @@ import toPublicKeyCredentialDescriptor from '../helpers/toPublicKeyCredentialDes
  */
 export default async function startAssertion(
   requestOptionsJSON: PublicKeyCredentialRequestOptionsJSON,
-): Promise<AuthenticatorAssertionResponseJSON> {
+): Promise<AssertionCredentialJSON> {
   if (!supportsWebauthn()) {
     throw new Error('WebAuthn is not supported in this browser');
   }
@@ -31,25 +31,29 @@ export default async function startAssertion(
   };
 
   // Wait for the user to complete assertion
-  const credential = await navigator.credentials.get({ publicKey });
+  const credential = await navigator.credentials.get({ publicKey }) as AssertionCredential;
 
   if (!credential) {
     throw new Error('Assertion was not completed');
   }
 
-  const { response } = credential as AssertionCredential;
+  const { id, rawId, response, type } = credential;
 
-  let base64UserHandle = undefined;
+  let userHandle = undefined;
   if (response.userHandle) {
-    base64UserHandle = toBase64String(response.userHandle);
+    userHandle = bufferToBase64URLString(response.userHandle);
   }
 
   // Convert values to base64 to make it easier to send back to the server
   return {
-    base64CredentialID: credential.id,
-    base64AuthenticatorData: toBase64String(response.authenticatorData),
-    base64ClientDataJSON: toBase64String(response.clientDataJSON),
-    base64Signature: toBase64String(response.signature),
-    base64UserHandle,
+    id,
+    rawId: bufferToBase64URLString(rawId),
+    response: {
+      authenticatorData: bufferToBase64URLString(response.authenticatorData),
+      clientDataJSON: bufferToBase64URLString(response.clientDataJSON),
+      signature: bufferToBase64URLString(response.signature),
+      userHandle,
+    },
+    type,
   };
 }
diff --git a/packages/browser/src/methods/startAttestation.test.ts b/packages/browser/src/methods/startAttestation.test.ts
index ae79235..bf6ab9b 100644
--- a/packages/browser/src/methods/startAttestation.test.ts
+++ b/packages/browser/src/methods/startAttestation.test.ts
@@ -1,5 +1,3 @@
-import base64js from 'base64-js';
-
 import {
   AttestationCredential,
   PublicKeyCredentialCreationOptionsJSON,
@@ -38,7 +36,7 @@ const goodOpts1: PublicKeyCredentialCreationOptionsJSON = {
   },
   timeout: 1,
   excludeCredentials: [{
-    id: 'authIdentifier',
+    id: 'C0VGlvYFratUdAV1iCw-ULpUW8E-exHPXQChBfyVeJZCMfjMFcwDmOFgoMUz39LoMtCJUBW8WPlLkGT6q8qTCg',
     type: 'public-key',
     transports: ['internal'],
   }],
@@ -64,19 +62,22 @@ test('should convert options before passing to navigator.credentials.create(...)
   await startAttestation(goodOpts1);
 
   const argsPublicKey = mockNavigatorCreate.mock.calls[0][0].publicKey;
+  const credId = argsPublicKey.excludeCredentials[0].id;
 
-  expect(argsPublicKey.challenge).toEqual(toUint8Array(goodOpts1.challenge));
-  expect(argsPublicKey.user.id).toEqual(toUint8Array(goodOpts1.user.id));
-  expect(argsPublicKey.excludeCredentials).toEqual([{
-    id: base64js.toByteArray('authIdentifier=='),
-    type: 'public-key',
-    transports: ['internal'],
-  }])
+  // Make sure challenge and user.id are converted to Buffers
+  expect(JSON.stringify(argsPublicKey.challenge)).toEqual('{"0":102,"1":105,"2":122,"3":122}');
+  expect(JSON.stringify(argsPublicKey.user.id)).toEqual('{"0":53,"1":54,"2":55,"3":56}');
+
+  // Confirm construction of excludeCredentials array
+  expect(credId instanceof ArrayBuffer).toEqual(true);
+  expect(credId.byteLength).toEqual(64);
+  expect(argsPublicKey.excludeCredentials[0].type).toEqual('public-key');
+  expect(argsPublicKey.excludeCredentials[0].transports).toEqual(['internal']);
 
   done();
 });
 
-test('should return base64-encoded response values', async done => {
+test('should return base64url-encoded response values', async done => {
   mockSupportsWebauthn.mockReturnValue(true);
 
   mockNavigatorCreate.mockImplementation(
@@ -86,8 +87,8 @@ test('should return base64-encoded response values', async done => {
           id: 'foobar',
           rawId: toUint8Array('foobar'),
           response: {
-            attestationObject: base64js.toByteArray(mockAttestationObject),
-            clientDataJSON: base64js.toByteArray(mockClientDataJSON),
+            attestationObject: Buffer.from(mockAttestationObject, 'ascii'),
+            clientDataJSON: Buffer.from(mockClientDataJSON, 'ascii'),
           },
           getClientExtensionResults: () => ({}),
           type: 'webauthn.create',
@@ -98,10 +99,9 @@ test('should return base64-encoded response values', async done => {
 
   const response = await startAttestation(goodOpts1);
 
-  expect(response).toEqual({
-    base64AttestationObject: mockAttestationObject,
-    base64ClientDataJSON: mockClientDataJSON,
-  });
+  expect(response.rawId).toEqual('Zm9vYmFy');
+  expect(response.response.attestationObject).toEqual('bW9ja0F0dGU');
+  expect(response.response.clientDataJSON).toEqual('bW9ja0NsaWU');
 
   done();
 });
diff --git a/packages/browser/src/methods/startAttestation.ts b/packages/browser/src/methods/startAttestation.ts
index ea05fa8..d5e540f 100644
--- a/packages/browser/src/methods/startAttestation.ts
+++ b/packages/browser/src/methods/startAttestation.ts
@@ -1,11 +1,11 @@
 import {
   PublicKeyCredentialCreationOptionsJSON,
-  AuthenticatorAttestationResponseJSON,
   AttestationCredential,
+  AttestationCredentialJSON,
 } from '@simplewebauthn/typescript-types';
 
 import toUint8Array from '../helpers/toUint8Array';
-import toBase64String from '../helpers/toBase64String';
+import bufferToBase64URLString from '../helpers/bufferToBase64URLString';
 import supportsWebauthn from '../helpers/supportsWebauthn';
 import toPublicKeyCredentialDescriptor from '../helpers/toPublicKeyCredentialDescriptor';
 
@@ -16,7 +16,7 @@ import toPublicKeyCredentialDescriptor from '../helpers/toPublicKeyCredentialDes
  */
 export default async function startAttestation(
   creationOptionsJSON: PublicKeyCredentialCreationOptionsJSON,
-): Promise<AuthenticatorAttestationResponseJSON> {
+): Promise<AttestationCredentialJSON> {
   if (!supportsWebauthn()) {
     throw new Error('WebAuthn is not supported in this browser');
   }
@@ -35,17 +35,22 @@ export default async function startAttestation(
   };
 
   // Wait for the user to complete attestation
-  const credential = await navigator.credentials.create({ publicKey });
+  const credential = await navigator.credentials.create({ publicKey }) as AttestationCredential;
 
   if (!credential) {
     throw new Error('Attestation was not completed');
   }
 
-  const { response } = credential as AttestationCredential;
+  const { id, rawId, response, type } = credential;
 
   // Convert values to base64 to make it easier to send back to the server
   return {
-    base64AttestationObject: toBase64String(response.attestationObject),
-    base64ClientDataJSON: toBase64String(response.clientDataJSON),
+    id,
+    rawId: bufferToBase64URLString(rawId),
+    response: {
+      attestationObject: bufferToBase64URLString(response.attestationObject),
+      clientDataJSON: bufferToBase64URLString(response.clientDataJSON),
+    },
+    type,
   };
 }