From 69e36b972bf4f56c61f9675318d89b6a828102d5 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 17 Apr 2023 11:20:25 +0200 Subject: [PATCH] [EC-598] feat: inform user when no credentials are found --- .../fido2/popup/fido2/fido2.component.html | 8 ++++++ .../browser-fido2-user-interface.service.ts | 15 ++++++++++- ...ido2-user-interface.service.abstraction.ts | 1 + .../fido2-authenticator.service.spec.ts | 27 +++++++++++++------ .../services/fido2-authenticator.service.ts | 1 + 5 files changed, 43 insertions(+), 9 deletions(-) diff --git a/apps/browser/src/fido2/popup/fido2/fido2.component.html b/apps/browser/src/fido2/popup/fido2/fido2.component.html index 10e3d7bdbce..09864a0c348 100644 --- a/apps/browser/src/fido2/popup/fido2/fido2.component.html +++ b/apps/browser/src/fido2/popup/fido2/fido2.component.html @@ -46,6 +46,14 @@ + + You do not have a matching login for this site. +
+
+ +
+
+ diff --git a/apps/browser/src/services/fido2/browser-fido2-user-interface.service.ts b/apps/browser/src/services/fido2/browser-fido2-user-interface.service.ts index dd390bb60f9..5fb321f13f5 100644 --- a/apps/browser/src/services/fido2/browser-fido2-user-interface.service.ts +++ b/apps/browser/src/services/fido2/browser-fido2-user-interface.service.ts @@ -10,13 +10,13 @@ import { takeUntil, } from "rxjs"; -import { Utils } from "@bitwarden/common/misc/utils"; import { UserRequestedFallbackAbortReason } from "@bitwarden/common/fido2/abstractions/fido2-client.service.abstraction"; import { Fido2UserInterfaceService as Fido2UserInterfaceServiceAbstraction, Fido2UserInterfaceSession, NewCredentialParams, } from "@bitwarden/common/fido2/abstractions/fido2-user-interface.service.abstraction"; +import { Utils } from "@bitwarden/common/misc/utils"; import { BrowserApi } from "../../browser/browserApi"; import { PopupUtilsService } from "../../popup/services/popup-utils.service"; @@ -79,6 +79,9 @@ export type BrowserFido2Message = { sessionId: string } & ( type: "InformExcludedCredentialRequest"; existingCipherIds: string[]; } + | { + type: "InformCredentialNotFoundRequest"; + } | { type: "AbortRequest"; } @@ -240,6 +243,16 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi await this.receive("AbortResponse"); } + async informCredentialNotFound(): Promise { + const data: BrowserFido2Message = { + type: "InformCredentialNotFoundRequest", + sessionId: this.sessionId, + }; + + await this.send(data); + await this.receive("AbortResponse"); + } + async close() { await this.send({ type: "CloseRequest", sessionId: this.sessionId }); this.closed = true; diff --git a/libs/common/src/fido2/abstractions/fido2-user-interface.service.abstraction.ts b/libs/common/src/fido2/abstractions/fido2-user-interface.service.abstraction.ts index 4486cc5bbfc..994533142af 100644 --- a/libs/common/src/fido2/abstractions/fido2-user-interface.service.abstraction.ts +++ b/libs/common/src/fido2/abstractions/fido2-user-interface.service.abstraction.ts @@ -25,5 +25,6 @@ export abstract class Fido2UserInterfaceSession { existingCipherIds: string[], abortController?: AbortController ) => Promise; + informCredentialNotFound: (abortController?: AbortController) => Promise; close: () => void; } diff --git a/libs/common/src/fido2/services/fido2-authenticator.service.spec.ts b/libs/common/src/fido2/services/fido2-authenticator.service.spec.ts index 8c25857656c..8ccf26aae7e 100644 --- a/libs/common/src/fido2/services/fido2-authenticator.service.spec.ts +++ b/libs/common/src/fido2/services/fido2-authenticator.service.spec.ts @@ -218,7 +218,7 @@ describe("FidoAuthenticatorService", () => { }); /** Devation: Organization ciphers are not checked against excluded credentials, even if the user has access to them. */ - it.only("should not inform user of duplication when the excluded credential belongs to an organization", async () => { + it("should not inform user of duplication when the excluded credential belongs to an organization", async () => { userInterfaceSession.informExcludedCredential.mockResolvedValue(); excludedCipherView.organizationId = "someOrganizationId"; @@ -597,24 +597,35 @@ describe("FidoAuthenticatorService", () => { }); }); - /** Spec: If credentialOptions is now empty, return an error code equivalent to "NotAllowedError" and terminate the operation. */ - it("should throw error if no credential exists", async () => { + /** + * Spec: If credentialOptions is now empty, return an error code equivalent to "NotAllowedError" and terminate the operation. + * Deviation: We do not throw error but instead inform the user and allow the user to fallback to browser implementation. + **/ + it("should inform user if no credential exists", async () => { cipherService.getAllDecrypted.mockResolvedValue([]); + userInterfaceSession.informCredentialNotFound.mockResolvedValue(); - const result = async () => await authenticator.getAssertion(params); + try { + await authenticator.getAssertion(params); + // eslint-disable-next-line no-empty + } catch {} - await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); + expect(userInterfaceSession.informCredentialNotFound).toHaveBeenCalled(); }); - it("should throw error if credential exists but rpId does not match", async () => { + it("should inform user if credential exists but rpId does not match", async () => { const cipher = await createCipherView({ type: CipherType.Login }); cipher.login.fido2Key.nonDiscoverableId = credentialId; cipher.login.fido2Key.rpId = "mismatch-rpid"; cipherService.getAllDecrypted.mockResolvedValue([cipher]); + userInterfaceSession.informCredentialNotFound.mockResolvedValue(); - const result = async () => await authenticator.getAssertion(params); + try { + await authenticator.getAssertion(params); + // eslint-disable-next-line no-empty + } catch {} - await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.NotAllowed); + expect(userInterfaceSession.informCredentialNotFound).toHaveBeenCalled(); }); }); diff --git a/libs/common/src/fido2/services/fido2-authenticator.service.ts b/libs/common/src/fido2/services/fido2-authenticator.service.ts index cdd26739ba0..00df0dcbf41 100644 --- a/libs/common/src/fido2/services/fido2-authenticator.service.ts +++ b/libs/common/src/fido2/services/fido2-authenticator.service.ts @@ -191,6 +191,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr } if (cipherOptions.length === 0) { + await userInterfaceSession.informCredentialNotFound(); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); }