From 197059d4fa876a1b88c4281cac409142d4612bbd Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 3 Nov 2023 18:27:55 +0100 Subject: [PATCH] [PM-4688] Automatically fallback on passkey retrieval if no passkeys are found (#6787) * [PM-4688] feat: auto-fallback when credential not found * [PM-4688] fix: don't show popup unless needed --- apps/browser/src/background/main.background.ts | 3 ++- .../fido2/browser-fido2-user-interface.service.ts | 15 +++++++++++++-- .../fido2/fido2-authenticator.service.spec.ts | 15 ++++++++++++++- .../services/fido2/fido2-authenticator.service.ts | 6 ++++++ .../vault/services/fido2/fido2-client.service.ts | 5 +++++ 5 files changed, 40 insertions(+), 4 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 8e4b9fe83b..e82a09ba00 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -586,7 +586,8 @@ export default class MainBackground { this.browserPopoutWindowService = new BrowserPopoutWindowService(); this.fido2UserInterfaceService = new BrowserFido2UserInterfaceService( - this.browserPopoutWindowService + this.browserPopoutWindowService, + this.authService ); this.fido2AuthenticatorService = new Fido2AuthenticatorService( this.cipherService, diff --git a/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts b/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts index 20121adb27..5eab5849c4 100644 --- a/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts +++ b/apps/browser/src/vault/fido2/browser-fido2-user-interface.service.ts @@ -17,6 +17,8 @@ import { throwError, } from "rxjs"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { UserRequestedFallbackAbortReason } from "@bitwarden/common/vault/abstractions/fido2/fido2-client.service.abstraction"; import { @@ -114,7 +116,10 @@ export type BrowserFido2Message = { sessionId: string } & ( * The user interface is implemented as a popout and the service uses the browser's messaging API to communicate with it. */ export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServiceAbstraction { - constructor(private browserPopoutWindowService: BrowserPopoutWindowService) {} + constructor( + private browserPopoutWindowService: BrowserPopoutWindowService, + private authService: AuthService + ) {} async newSession( fallbackSupported: boolean, @@ -123,6 +128,7 @@ export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServi ): Promise { return await BrowserFido2UserInterfaceSession.create( this.browserPopoutWindowService, + this.authService, fallbackSupported, tab, abortController @@ -133,12 +139,14 @@ export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServi export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSession { static async create( browserPopoutWindowService: BrowserPopoutWindowService, + authService: AuthService, fallbackSupported: boolean, tab: chrome.tabs.Tab, abortController?: AbortController ): Promise { return new BrowserFido2UserInterfaceSession( browserPopoutWindowService, + authService, fallbackSupported, tab, abortController @@ -176,6 +184,7 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi private constructor( private readonly browserPopoutWindowService: BrowserPopoutWindowService, + private readonly authService: AuthService, private readonly fallbackSupported: boolean, private readonly tab: chrome.tabs.Tab, readonly abortController = new AbortController(), @@ -278,7 +287,9 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi } async ensureUnlockedVault(): Promise { - await this.connect(); + if ((await this.authService.getAuthStatus()) !== AuthenticationStatus.Unlocked) { + await this.connect(); + } } async informCredentialNotFound(): Promise { diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts index ecf6f03d7e..39a77152c7 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.spec.ts @@ -9,6 +9,7 @@ import { Fido2AuthenticatorGetAssertionParams, Fido2AuthenticatorMakeCredentialsParams, } from "../../abstractions/fido2/fido2-authenticator.service.abstraction"; +import { FallbackRequestedError } from "../../abstractions/fido2/fido2-client.service.abstraction"; import { Fido2UserInterfaceService, Fido2UserInterfaceSession, @@ -469,7 +470,8 @@ describe("FidoAuthenticatorService", () => { * 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 () => { + it("should inform user if no credential exists when fallback is not supported", async () => { + params.fallbackSupported = false; cipherService.getAllDecrypted.mockResolvedValue([]); userInterfaceSession.informCredentialNotFound.mockResolvedValue(); @@ -481,6 +483,17 @@ describe("FidoAuthenticatorService", () => { expect(userInterfaceSession.informCredentialNotFound).toHaveBeenCalled(); }); + it("should automatically fallback if no credential exists when fallback is supported", async () => { + params.fallbackSupported = true; + cipherService.getAllDecrypted.mockResolvedValue([]); + userInterfaceSession.informCredentialNotFound.mockResolvedValue(); + + const result = async () => await authenticator.getAssertion(params, tab); + + await expect(result).rejects.toThrowError(FallbackRequestedError); + expect(userInterfaceSession.informCredentialNotFound).not.toHaveBeenCalled(); + }); + it("should inform user if credential exists but rpId does not match", async () => { const cipher = await createCipherView({ type: CipherType.Login }); cipher.login.fido2Credentials[0].credentialId = credentialId; diff --git a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts index b04e6de562..292413b3bd 100644 --- a/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-authenticator.service.ts @@ -12,6 +12,7 @@ import { Fido2AuthenticatorService as Fido2AuthenticatorServiceAbstraction, PublicKeyCredentialDescriptor, } from "../../abstractions/fido2/fido2-authenticator.service.abstraction"; +import { FallbackRequestedError } from "../../abstractions/fido2/fido2-client.service.abstraction"; import { Fido2UserInterfaceService } from "../../abstractions/fido2/fido2-user-interface.service.abstraction"; import { SyncService } from "../../abstractions/sync/sync.service.abstraction"; import { CipherRepromptType } from "../../enums/cipher-reprompt-type"; @@ -221,6 +222,11 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr this.logService?.info( `[Fido2Authenticator] Aborting because no matching credentials were found in the vault.` ); + + if (params.fallbackSupported) { + throw new FallbackRequestedError(); + } + await userInterfaceSession.informCredentialNotFound(); throw new Fido2AuthenticatorError(Fido2AuthenticatorErrorCode.NotAllowed); } diff --git a/libs/common/src/vault/services/fido2/fido2-client.service.ts b/libs/common/src/vault/services/fido2/fido2-client.service.ts index 76f51c1f71..cdc8a88fd2 100644 --- a/libs/common/src/vault/services/fido2/fido2-client.service.ts +++ b/libs/common/src/vault/services/fido2/fido2-client.service.ts @@ -267,6 +267,11 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { abortController ); } catch (error) { + if (error instanceof FallbackRequestedError) { + this.logService?.info(`[Fido2Client] Aborting because of auto fallback`); + throw error; + } + if ( abortController.signal.aborted && abortController.signal.reason === UserRequestedFallbackAbortReason