diff --git a/libs/common/src/webauthn/abstractions/fido2-user-interface.service.abstraction.ts b/libs/common/src/webauthn/abstractions/fido2-user-interface.service.abstraction.ts index f3f304bdc1..68d372d2fa 100644 --- a/libs/common/src/webauthn/abstractions/fido2-user-interface.service.abstraction.ts +++ b/libs/common/src/webauthn/abstractions/fido2-user-interface.service.abstraction.ts @@ -10,9 +10,9 @@ export abstract class Fido2UserInterfaceService { params: NewCredentialParams, abortController?: AbortController ) => Promise; - confirmDuplicateCredential: ( + informExcludedCredential: ( existingCipherIds: string[], newCredential: NewCredentialParams, abortController?: AbortController - ) => Promise; + ) => Promise; } diff --git a/libs/common/src/webauthn/services/fido2-authenticator.service.spec.ts b/libs/common/src/webauthn/services/fido2-authenticator.service.spec.ts index 40be587253..b0f3441631 100644 --- a/libs/common/src/webauthn/services/fido2-authenticator.service.spec.ts +++ b/libs/common/src/webauthn/services/fido2-authenticator.service.spec.ts @@ -79,7 +79,7 @@ describe("FidoAuthenticatorService", () => { }); it("should not request confirmation from user", async () => { - userInterface.confirmDuplicateCredential.mockResolvedValue(true); + userInterface.confirmNewCredential.mockResolvedValue(true); const invalidParams = await createInvalidParams(); for (const p of Object.values(invalidParams)) { @@ -115,17 +115,20 @@ describe("FidoAuthenticatorService", () => { }); /** Spec: wait for user presence */ - it("should request confirmation from user", async () => { - userInterface.confirmDuplicateCredential.mockResolvedValue(true); + it("should inform user", async () => { + userInterface.informExcludedCredential.mockResolvedValue(); - await authenticator.makeCredential(params); + try { + await authenticator.makeCredential(params); + // eslint-disable-next-line no-empty + } catch {} - expect(userInterface.confirmDuplicateCredential).toHaveBeenCalled(); + expect(userInterface.informExcludedCredential).toHaveBeenCalled(); }); /** Spec: then terminate this procedure and return error code */ - it("should throw error if user denies duplication", async () => { - userInterface.confirmDuplicateCredential.mockResolvedValue(false); + it("should throw error", async () => { + userInterface.informExcludedCredential.mockResolvedValue(); const result = async () => await authenticator.makeCredential(params); @@ -135,8 +138,8 @@ describe("FidoAuthenticatorService", () => { }); /** Departure from spec: Check duplication last instead of first */ - it("should not request confirmation from user when input data does not pass checks", async () => { - userInterface.confirmDuplicateCredential.mockResolvedValue(true); + it("should not inform user of duplication when input data does not pass checks", async () => { + userInterface.informExcludedCredential.mockResolvedValue(); const invalidParams = await createInvalidParams(); for (const p of Object.values(invalidParams)) { @@ -145,7 +148,7 @@ describe("FidoAuthenticatorService", () => { // eslint-disable-next-line no-empty } catch {} } - expect(userInterface.confirmDuplicateCredential).not.toHaveBeenCalled(); + expect(userInterface.informExcludedCredential).not.toHaveBeenCalled(); }); }); diff --git a/libs/common/src/webauthn/services/fido2-authenticator.service.ts b/libs/common/src/webauthn/services/fido2-authenticator.service.ts index 98f8d68a94..6c9897f472 100644 --- a/libs/common/src/webauthn/services/fido2-authenticator.service.ts +++ b/libs/common/src/webauthn/services/fido2-authenticator.service.ts @@ -41,33 +41,28 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_PIN_AUTH_INVALID); } - // In the spec the `excludeList` is checked first. - // We deviate from this because we allow duplicates to be created if the user confirms it, - // and we don't want to ask the user for confirmation if the input params haven't already - // been verified. const isExcluded = await this.vaultContainsId( params.excludeList.map((key) => Fido2Utils.bufferToString(key.id)) ); - let userVerification = false; if (isExcluded) { - userVerification = await this.userInterface.confirmDuplicateCredential( + await this.userInterface.informExcludedCredential( [Fido2Utils.bufferToString(params.excludeList[0].id)], { credentialName: params.rp.name, userName: params.user.name, } ); - } else { - userVerification = await this.userInterface.confirmNewCredential({ - credentialName: params.rp.name, - userName: params.user.name, - }); + + throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_CREDENTIAL_EXCLUDED); } - if (!userVerification && isExcluded) { - throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_CREDENTIAL_EXCLUDED); - } else if (!userVerification && !isExcluded) { + const userVerification = await this.userInterface.confirmNewCredential({ + credentialName: params.rp.name, + userName: params.user.name, + }); + + if (!userVerification) { throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.CTAP2_ERR_OPERATION_DENIED); }