[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
This commit is contained in:
Andreas Coroiu 2023-11-03 18:27:55 +01:00 committed by GitHub
parent 62e1e165c4
commit 197059d4fa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 40 additions and 4 deletions

View File

@ -586,7 +586,8 @@ export default class MainBackground {
this.browserPopoutWindowService = new BrowserPopoutWindowService(); this.browserPopoutWindowService = new BrowserPopoutWindowService();
this.fido2UserInterfaceService = new BrowserFido2UserInterfaceService( this.fido2UserInterfaceService = new BrowserFido2UserInterfaceService(
this.browserPopoutWindowService this.browserPopoutWindowService,
this.authService
); );
this.fido2AuthenticatorService = new Fido2AuthenticatorService( this.fido2AuthenticatorService = new Fido2AuthenticatorService(
this.cipherService, this.cipherService,

View File

@ -17,6 +17,8 @@ import {
throwError, throwError,
} from "rxjs"; } 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 { Utils } from "@bitwarden/common/platform/misc/utils";
import { UserRequestedFallbackAbortReason } from "@bitwarden/common/vault/abstractions/fido2/fido2-client.service.abstraction"; import { UserRequestedFallbackAbortReason } from "@bitwarden/common/vault/abstractions/fido2/fido2-client.service.abstraction";
import { 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. * 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 { export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServiceAbstraction {
constructor(private browserPopoutWindowService: BrowserPopoutWindowService) {} constructor(
private browserPopoutWindowService: BrowserPopoutWindowService,
private authService: AuthService
) {}
async newSession( async newSession(
fallbackSupported: boolean, fallbackSupported: boolean,
@ -123,6 +128,7 @@ export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServi
): Promise<Fido2UserInterfaceSession> { ): Promise<Fido2UserInterfaceSession> {
return await BrowserFido2UserInterfaceSession.create( return await BrowserFido2UserInterfaceSession.create(
this.browserPopoutWindowService, this.browserPopoutWindowService,
this.authService,
fallbackSupported, fallbackSupported,
tab, tab,
abortController abortController
@ -133,12 +139,14 @@ export class BrowserFido2UserInterfaceService implements Fido2UserInterfaceServi
export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSession { export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSession {
static async create( static async create(
browserPopoutWindowService: BrowserPopoutWindowService, browserPopoutWindowService: BrowserPopoutWindowService,
authService: AuthService,
fallbackSupported: boolean, fallbackSupported: boolean,
tab: chrome.tabs.Tab, tab: chrome.tabs.Tab,
abortController?: AbortController abortController?: AbortController
): Promise<BrowserFido2UserInterfaceSession> { ): Promise<BrowserFido2UserInterfaceSession> {
return new BrowserFido2UserInterfaceSession( return new BrowserFido2UserInterfaceSession(
browserPopoutWindowService, browserPopoutWindowService,
authService,
fallbackSupported, fallbackSupported,
tab, tab,
abortController abortController
@ -176,6 +184,7 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi
private constructor( private constructor(
private readonly browserPopoutWindowService: BrowserPopoutWindowService, private readonly browserPopoutWindowService: BrowserPopoutWindowService,
private readonly authService: AuthService,
private readonly fallbackSupported: boolean, private readonly fallbackSupported: boolean,
private readonly tab: chrome.tabs.Tab, private readonly tab: chrome.tabs.Tab,
readonly abortController = new AbortController(), readonly abortController = new AbortController(),
@ -278,8 +287,10 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi
} }
async ensureUnlockedVault(): Promise<void> { async ensureUnlockedVault(): Promise<void> {
if ((await this.authService.getAuthStatus()) !== AuthenticationStatus.Unlocked) {
await this.connect(); await this.connect();
} }
}
async informCredentialNotFound(): Promise<void> { async informCredentialNotFound(): Promise<void> {
const data: BrowserFido2Message = { const data: BrowserFido2Message = {

View File

@ -9,6 +9,7 @@ import {
Fido2AuthenticatorGetAssertionParams, Fido2AuthenticatorGetAssertionParams,
Fido2AuthenticatorMakeCredentialsParams, Fido2AuthenticatorMakeCredentialsParams,
} from "../../abstractions/fido2/fido2-authenticator.service.abstraction"; } from "../../abstractions/fido2/fido2-authenticator.service.abstraction";
import { FallbackRequestedError } from "../../abstractions/fido2/fido2-client.service.abstraction";
import { import {
Fido2UserInterfaceService, Fido2UserInterfaceService,
Fido2UserInterfaceSession, Fido2UserInterfaceSession,
@ -469,7 +470,8 @@ describe("FidoAuthenticatorService", () => {
* Spec: If credentialOptions is now empty, return an error code equivalent to "NotAllowedError" and terminate the operation. * 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. * 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([]); cipherService.getAllDecrypted.mockResolvedValue([]);
userInterfaceSession.informCredentialNotFound.mockResolvedValue(); userInterfaceSession.informCredentialNotFound.mockResolvedValue();
@ -481,6 +483,17 @@ describe("FidoAuthenticatorService", () => {
expect(userInterfaceSession.informCredentialNotFound).toHaveBeenCalled(); 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 () => { it("should inform user if credential exists but rpId does not match", async () => {
const cipher = await createCipherView({ type: CipherType.Login }); const cipher = await createCipherView({ type: CipherType.Login });
cipher.login.fido2Credentials[0].credentialId = credentialId; cipher.login.fido2Credentials[0].credentialId = credentialId;

View File

@ -12,6 +12,7 @@ import {
Fido2AuthenticatorService as Fido2AuthenticatorServiceAbstraction, Fido2AuthenticatorService as Fido2AuthenticatorServiceAbstraction,
PublicKeyCredentialDescriptor, PublicKeyCredentialDescriptor,
} from "../../abstractions/fido2/fido2-authenticator.service.abstraction"; } 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 { Fido2UserInterfaceService } from "../../abstractions/fido2/fido2-user-interface.service.abstraction";
import { SyncService } from "../../abstractions/sync/sync.service.abstraction"; import { SyncService } from "../../abstractions/sync/sync.service.abstraction";
import { CipherRepromptType } from "../../enums/cipher-reprompt-type"; import { CipherRepromptType } from "../../enums/cipher-reprompt-type";
@ -221,6 +222,11 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
this.logService?.info( this.logService?.info(
`[Fido2Authenticator] Aborting because no matching credentials were found in the vault.` `[Fido2Authenticator] Aborting because no matching credentials were found in the vault.`
); );
if (params.fallbackSupported) {
throw new FallbackRequestedError();
}
await userInterfaceSession.informCredentialNotFound(); await userInterfaceSession.informCredentialNotFound();
throw new Fido2AuthenticatorError(Fido2AuthenticatorErrorCode.NotAllowed); throw new Fido2AuthenticatorError(Fido2AuthenticatorErrorCode.NotAllowed);
} }

View File

@ -267,6 +267,11 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
abortController abortController
); );
} catch (error) { } catch (error) {
if (error instanceof FallbackRequestedError) {
this.logService?.info(`[Fido2Client] Aborting because of auto fallback`);
throw error;
}
if ( if (
abortController.signal.aborted && abortController.signal.aborted &&
abortController.signal.reason === UserRequestedFallbackAbortReason abortController.signal.reason === UserRequestedFallbackAbortReason