diff --git a/app/javascript/packages/webauthn/enroll-webauthn-device.spec.ts b/app/javascript/packages/webauthn/enroll-webauthn-device.spec.ts index b6ba622a824..1996503b1d1 100644 --- a/app/javascript/packages/webauthn/enroll-webauthn-device.spec.ts +++ b/app/javascript/packages/webauthn/enroll-webauthn-device.spec.ts @@ -24,7 +24,13 @@ describe('enrollWebauthnDevice', () => { 225, 190, 13, 223, 243, 75, 174, 252, 212, 215, 183, 9, ]).buffer; - beforeEach(() => { + function defineNavigatorCredentials({ + getAuthenticatorData, + getTransports, + }: { + getAuthenticatorData?: AuthenticatorAttestationResponse['getAuthenticatorData']; + getTransports?: AuthenticatorAttestationResponse['getTransports']; + }) { defineProperty(navigator, 'credentials', { configurable: true, value: { @@ -34,100 +40,149 @@ describe('enrollWebauthnDevice', () => { response: { attestationObject: Buffer.from('attest', 'utf-8'), clientDataJSON: Buffer.from('json', 'utf-8'), - getAuthenticatorData: () => authenticatorData, - getTransports: () => ['usb'], + getAuthenticatorData, + getTransports, }, }), }, }); - }); + } - it('enrolls a device using the proper create options', async () => { - const result = await enrollWebauthnDevice({ - user, - challenge, - excludeCredentials, - authenticatorAttachment: 'cross-platform', + context('fully supported AuthenticatorAttestationResponse', () => { + beforeEach(() => { + defineNavigatorCredentials({ + getAuthenticatorData: () => authenticatorData, + getTransports: () => ['usb'], + }); }); - expect(navigator.credentials.create).to.have.been.calledWith({ - publicKey: { - challenge: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]), - rp: { name: 'example.test' }, - user: { - id: new Uint8Array([123, 0, 0, 0, 0, 0, 0, 0]), - name: 'test@test.com', - displayName: 'test@test.com', - }, - pubKeyCredParams: [ - { type: 'public-key', alg: -7 }, - { type: 'public-key', alg: -35 }, - { type: 'public-key', alg: -36 }, - { type: 'public-key', alg: -37 }, - { type: 'public-key', alg: -38 }, - { type: 'public-key', alg: -39 }, - { type: 'public-key', alg: -257 }, - ], - timeout: 800000, - attestation: 'none', - authenticatorSelection: { - authenticatorAttachment: 'cross-platform', - userVerification: 'discouraged', - }, - excludeCredentials: [ - { - id: new TextEncoder().encode('credential123').buffer, - type: 'public-key', + it('enrolls a device using the proper create options', async () => { + const result = await enrollWebauthnDevice({ + user, + challenge, + excludeCredentials, + authenticatorAttachment: 'cross-platform', + }); + + expect(navigator.credentials.create).to.have.been.calledWith({ + publicKey: { + challenge: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]), + rp: { name: 'example.test' }, + user: { + id: new Uint8Array([123, 0, 0, 0, 0, 0, 0, 0]), + name: 'test@test.com', + displayName: 'test@test.com', }, - { - id: new TextEncoder().encode('credential456').buffer, - type: 'public-key', + pubKeyCredParams: [ + { type: 'public-key', alg: -7 }, + { type: 'public-key', alg: -35 }, + { type: 'public-key', alg: -36 }, + { type: 'public-key', alg: -37 }, + { type: 'public-key', alg: -38 }, + { type: 'public-key', alg: -39 }, + { type: 'public-key', alg: -257 }, + ], + timeout: 800000, + attestation: 'none', + authenticatorSelection: { + authenticatorAttachment: 'cross-platform', + userVerification: 'discouraged', }, - ], - }, + excludeCredentials: [ + { + id: new TextEncoder().encode('credential123').buffer, + type: 'public-key', + }, + { + id: new TextEncoder().encode('credential456').buffer, + type: 'public-key', + }, + ], + }, + }); + + expect(result).to.deep.equal({ + webauthnId: btoa('123'), + webauthnPublicKey: '123', + attestationObject: btoa('attest'), + clientDataJSON: btoa('json'), + authenticatorDataFlagsValue: 65, + transports: ['usb'], + }); + }); + + it('forwards errors from the webauthn api', async () => { + const dummyError = new Error('dummy error'); + navigator.credentials.create = () => Promise.reject(dummyError); + + let didCatch; + try { + await enrollWebauthnDevice({ user, challenge, excludeCredentials }); + } catch (error) { + expect(error).to.equal(dummyError); + didCatch = true; + } + + expect(didCatch).to.be.true(); }); - expect(result).to.deep.equal({ - webauthnId: btoa('123'), - webauthnPublicKey: '123', - attestationObject: btoa('attest'), - clientDataJSON: btoa('json'), - authenticatorDataValue: 65, - transports: ['usb'], + context('platform authenticator', () => { + it('enrolls a device with correct authenticatorAttachment', async () => { + await enrollWebauthnDevice({ + user, + challenge, + excludeCredentials, + authenticatorAttachment: 'platform', + }); + + expect(navigator.credentials.create).to.have.been.calledWithMatch({ + publicKey: { + authenticatorSelection: { + authenticatorAttachment: 'platform', + }, + }, + }); + }); }); }); - it('forwards errors from the webauthn api', async () => { - const dummyError = new Error('dummy error'); - navigator.credentials.create = () => Promise.reject(dummyError); + context('AuthenticatorAttestationResponse#getTransports unsupported', () => { + beforeEach(() => { + defineNavigatorCredentials({ + getAuthenticatorData: () => authenticatorData, + getTransports: undefined, + }); + }); - let didCatch; - try { - await enrollWebauthnDevice({ user, challenge, excludeCredentials }); - } catch (error) { - expect(error).to.equal(dummyError); - didCatch = true; - } + it('enrolls a device with a blank transports result', async () => { + const result = await enrollWebauthnDevice({ + user, + challenge, + excludeCredentials, + authenticatorAttachment: 'cross-platform', + }); - expect(didCatch).to.be.true(); + expect(result.transports).to.equal(undefined); + }); }); - context('platform authenticator', () => { - it('enrolls a device with correct authenticatorAttachment', async () => { - await enrollWebauthnDevice({ + context('AuthenticatorAttestationResponse#getAuthenticatorData unsupported', () => { + beforeEach(() => { + defineNavigatorCredentials({ + getAuthenticatorData: undefined, + getTransports: () => ['usb'], + }); + }); + + it('enrolls a device with a blank authenticatorDataFlagsValue result', async () => { + const result = await enrollWebauthnDevice({ user, challenge, excludeCredentials, - authenticatorAttachment: 'platform', + authenticatorAttachment: 'cross-platform', }); - expect(navigator.credentials.create).to.have.been.calledWithMatch({ - publicKey: { - authenticatorSelection: { - authenticatorAttachment: 'platform', - }, - }, - }); + expect(result.authenticatorDataFlagsValue).to.equal(undefined); }); }); }); diff --git a/app/javascript/packages/webauthn/enroll-webauthn-device.ts b/app/javascript/packages/webauthn/enroll-webauthn-device.ts index aadd4b411cf..29401b9baed 100644 --- a/app/javascript/packages/webauthn/enroll-webauthn-device.ts +++ b/app/javascript/packages/webauthn/enroll-webauthn-device.ts @@ -1,5 +1,20 @@ import { arrayBufferToBase64 } from './converters'; +/** + * Response object with properties as possibly undefined where browser support varies. + * + * As of writing, Firefox does not implement getTransports or getAuthenticatorData. Remove this if + * and when support changes. + * + * @see https://developer.mozilla.org/en-US/docs/Web/API/AuthenticatorAttestationResponse/getTransports#browser_compatibility + * @see https://developer.mozilla.org/en-US/docs/Web/API/AuthenticatorAttestationResponse/getAuthenticatorData#browser_compatibility + */ +interface AuthenticatorAttestationResponseBrowserSupport + extends Omit { + getTransports: AuthenticatorAttestationResponse['getTransports'] | undefined; + getAuthenticatorData: AuthenticatorAttestationResponse['getAuthenticatorData'] | undefined; +} + interface EnrollOptions { user: PublicKeyCredentialUserEntity; @@ -19,9 +34,9 @@ interface EnrollResult { clientDataJSON: string; - authenticatorDataValue: number; + authenticatorDataFlagsValue?: number; - transports: string[]; + transports?: string[]; } async function enrollWebauthnDevice({ @@ -76,15 +91,19 @@ async function enrollWebauthnDevice({ }, })) as PublicKeyCredential; - const response = credential.response as AuthenticatorAttestationResponse; - const authenticatorDataValue = new Uint8Array(response.getAuthenticatorData())[32]; + const response = credential.response as AuthenticatorAttestationResponseBrowserSupport; + const authenticatorData = response.getAuthenticatorData?.(); + const authenticatorDataFlagsValue = authenticatorData + ? new Uint8Array(authenticatorData)[32] + : undefined; + return { webauthnId: arrayBufferToBase64(credential.rawId), webauthnPublicKey: credential.id, attestationObject: arrayBufferToBase64(response.attestationObject), clientDataJSON: arrayBufferToBase64(response.clientDataJSON), - authenticatorDataValue, - transports: response.getTransports(), + authenticatorDataFlagsValue, + transports: response.getTransports?.(), }; } diff --git a/app/javascript/packs/webauthn-setup.ts b/app/javascript/packs/webauthn-setup.ts index 4f93824cd1d..582d3d28e46 100644 --- a/app/javascript/packs/webauthn-setup.ts +++ b/app/javascript/packs/webauthn-setup.ts @@ -66,11 +66,15 @@ function webauthn() { result.attestationObject; (document.getElementById('client_data_json') as HTMLInputElement).value = result.clientDataJSON; - ( - document.getElementById('authenticator_data_value') as HTMLInputElement - ).value = `${result.authenticatorDataValue}`; - (document.getElementById('transports') as HTMLInputElement).value = - result.transports.join(); + if (result.authenticatorDataFlagsValue) { + ( + document.getElementById('authenticator_data_value') as HTMLInputElement + ).value = `${result.authenticatorDataFlagsValue}`; + } + if (result.transports) { + (document.getElementById('transports') as HTMLInputElement).value = + result.transports.join(); + } (document.getElementById('webauthn_form') as HTMLFormElement).submit(); }) .catch((error: Error) => { diff --git a/spec/forms/webauthn_setup_form_spec.rb b/spec/forms/webauthn_setup_form_spec.rb index 67c3118c4da..235307a57b9 100644 --- a/spec/forms/webauthn_setup_form_spec.rb +++ b/spec/forms/webauthn_setup_form_spec.rb @@ -77,62 +77,39 @@ end context 'with non backed up option data flags' do - let(:params) do - { - attestation_object: attestation_object, - client_data_json: setup_client_data_json, - name: 'mykey', - platform_authenticator: false, - transports: 'usb', - authenticator_data_value: '65', - } - end + let(:params) { super().merge(authenticator_data_value: '65') } it 'includes data flags with bs set as false ' do result = subject.submit(protocol, params) - expect(result.to_h).to eq( - success: true, - errors: {}, - enabled_mfa_methods_count: 1, - mfa_method_counts: { webauthn: 1 }, - multi_factor_auth_method: 'webauthn', - authenticator_data_flags: { - up: true, - uv: false, - be: false, - bs: false, - at: true, - ed: false, - }, - pii_like_keypaths: [[:mfa_method_counts, :phone]], + expect(result.to_h[:authenticator_data_flags]).to eq( + up: true, + uv: false, + be: false, + bs: false, + at: true, + ed: false, ) end end - context 'when user enters in a non number value' do - let(:params) do - { - attestation_object: attestation_object, - client_data_json: setup_client_data_json, - name: 'mykey', - platform_authenticator: false, - transports: 'usb', - authenticator_data_value: 'bad_error', - } + context 'when authenticator_data_value is not a number' do + let(:params) { super().merge(authenticator_data_value: 'bad_error') } + + it 'should not include authenticator data flag' do + result = subject.submit(protocol, params) + + expect(result.to_h[:authenticator_data_flags]).to be_nil end + end + + context 'when authenticator_data_value is missing' do + let(:params) { super().merge(authenticator_data_value: nil) } it 'should not include authenticator data flag' do result = subject.submit(protocol, params) - expect(result.to_h).to eq( - success: true, - errors: {}, - enabled_mfa_methods_count: 1, - mfa_method_counts: { webauthn: 1 }, - multi_factor_auth_method: 'webauthn', - pii_like_keypaths: [[:mfa_method_counts, :phone]], - ) + expect(result.to_h[:authenticator_data_flags]).to be_nil end end end