1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-11 13:53:34 +00:00

[PM-19215] Fix Firefox extension biometric unlock autoprompt (#14254)

* Remove restriction from account security component

* Add the ability to manage pop-out to the LockComponentService

* Have the Firefox extension pop-out on biometric auto-prompt unlock
This commit is contained in:
Thomas Avery
2025-04-21 14:08:09 -05:00
committed by GitHub
parent 143473927e
commit 3a8045d7d0
11 changed files with 166 additions and 26 deletions

View File

@@ -23,7 +23,7 @@
<bit-form-control <bit-form-control
class="tw-pl-5" class="tw-pl-5"
[disableMargin]="!((pinEnabled$ | async) || this.form.value.pin)" [disableMargin]="!((pinEnabled$ | async) || this.form.value.pin)"
*ngIf="this.form.value.biometric && showAutoPrompt" *ngIf="this.form.value.biometric"
> >
<input <input
bitCheckbox bitCheckbox

View File

@@ -30,7 +30,6 @@ import { getFirstPolicy } from "@bitwarden/common/admin-console/services/policy/
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { DeviceType } from "@bitwarden/common/enums";
import { import {
VaultTimeout, VaultTimeout,
VaultTimeoutAction, VaultTimeoutAction,
@@ -110,7 +109,6 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
hasVaultTimeoutPolicy = false; hasVaultTimeoutPolicy = false;
biometricUnavailabilityReason: string; biometricUnavailabilityReason: string;
showChangeMasterPass = true; showChangeMasterPass = true;
showAutoPrompt = true;
pinEnabled$: Observable<boolean> = of(true); pinEnabled$: Observable<boolean> = of(true);
form = this.formBuilder.group({ form = this.formBuilder.group({
@@ -147,11 +145,6 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
) {} ) {}
async ngOnInit() { async ngOnInit() {
// Firefox popup closes when unfocused by biometrics, blocking all unlock methods
if (this.platformUtilsService.getDevice() === DeviceType.FirefoxExtension) {
this.showAutoPrompt = false;
}
const hasMasterPassword = await this.userVerificationService.hasMasterPassword(); const hasMasterPassword = await this.userVerificationService.hasMasterPassword();
this.showMasterPasswordOnClientRestartOption = hasMasterPassword; this.showMasterPasswordOnClientRestartOption = hasMasterPassword;
const maximumVaultTimeoutPolicy = this.accountService.activeAccount$.pipe( const maximumVaultTimeoutPolicy = this.accountService.activeAccount$.pipe(

View File

@@ -17,6 +17,8 @@ import {
} from "@bitwarden/key-management"; } from "@bitwarden/key-management";
import { UnlockOptions } from "@bitwarden/key-management-ui"; import { UnlockOptions } from "@bitwarden/key-management-ui";
import { BrowserApi } from "../../../platform/browser/browser-api";
import BrowserPopupUtils from "../../../platform/popup/browser-popup-utils";
import { BrowserRouterService } from "../../../platform/popup/services/browser-router.service"; import { BrowserRouterService } from "../../../platform/popup/services/browser-router.service";
import { ExtensionLockComponentService } from "./extension-lock-component.service"; import { ExtensionLockComponentService } from "./extension-lock-component.service";
@@ -117,6 +119,62 @@ describe("ExtensionLockComponentService", () => {
}); });
}); });
describe("popOutBrowserExtension", () => {
let openPopoutSpy: jest.SpyInstance;
beforeEach(() => {
jest.resetAllMocks();
openPopoutSpy = jest
.spyOn(BrowserPopupUtils, "openCurrentPagePopout")
.mockResolvedValue(undefined);
});
it("opens pop-out when the current window is neither a pop-out nor a sidebar", async () => {
jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(false);
jest.spyOn(BrowserPopupUtils, "inSidebar").mockReturnValue(false);
await service.popOutBrowserExtension();
expect(openPopoutSpy).toHaveBeenCalledWith(global.window);
});
test.each([
[true, false],
[false, true],
[true, true],
])("should not open pop-out under other conditions.", async (inPopout, inSidebar) => {
jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(inPopout);
jest.spyOn(BrowserPopupUtils, "inSidebar").mockReturnValue(inSidebar);
await service.popOutBrowserExtension();
expect(openPopoutSpy).not.toHaveBeenCalled();
});
});
describe("closeBrowserExtensionPopout", () => {
let closePopupSpy: jest.SpyInstance;
beforeEach(() => {
jest.resetAllMocks();
closePopupSpy = jest.spyOn(BrowserApi, "closePopup").mockReturnValue();
});
it("closes pop-out when in pop-out", () => {
jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(true);
service.closeBrowserExtensionPopout();
expect(closePopupSpy).toHaveBeenCalledWith(global.window);
});
it("doesn't close pop-out when not in pop-out", () => {
jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(false);
service.closeBrowserExtensionPopout();
expect(closePopupSpy).not.toHaveBeenCalled();
});
});
describe("isWindowVisible", () => { describe("isWindowVisible", () => {
it("throws an error", async () => { it("throws an error", async () => {
await expect(service.isWindowVisible()).rejects.toThrow("Method not implemented."); await expect(service.isWindowVisible()).rejects.toThrow("Method not implemented.");

View File

@@ -14,6 +14,8 @@ import {
import { LockComponentService, UnlockOptions } from "@bitwarden/key-management-ui"; import { LockComponentService, UnlockOptions } from "@bitwarden/key-management-ui";
import { BiometricErrors, BiometricErrorTypes } from "../../../models/biometricErrors"; import { BiometricErrors, BiometricErrorTypes } from "../../../models/biometricErrors";
import { BrowserApi } from "../../../platform/browser/browser-api";
import BrowserPopupUtils from "../../../platform/popup/browser-popup-utils";
import { BrowserRouterService } from "../../../platform/popup/services/browser-router.service"; import { BrowserRouterService } from "../../../platform/popup/services/browser-router.service";
export class ExtensionLockComponentService implements LockComponentService { export class ExtensionLockComponentService implements LockComponentService {
@@ -37,6 +39,18 @@ export class ExtensionLockComponentService implements LockComponentService {
return biometricsError.description; return biometricsError.description;
} }
async popOutBrowserExtension(): Promise<void> {
if (!BrowserPopupUtils.inPopout(global.window) && !BrowserPopupUtils.inSidebar(global.window)) {
await BrowserPopupUtils.openCurrentPagePopout(global.window);
}
}
closeBrowserExtensionPopout(): void {
if (BrowserPopupUtils.inPopout(global.window)) {
BrowserApi.closePopup(global.window);
}
}
async isWindowVisible(): Promise<boolean> { async isWindowVisible(): Promise<boolean> {
throw new Error("Method not implemented."); throw new Error("Method not implemented.");
} }

View File

@@ -104,6 +104,22 @@ describe("DesktopLockComponentService", () => {
}); });
}); });
describe("popOutBrowserExtension", () => {
it("throws platform not supported error", () => {
expect(() => service.popOutBrowserExtension()).toThrow(
"Method not supported on this platform.",
);
});
});
describe("closeBrowserExtensionPopout", () => {
it("throws platform not supported error", () => {
expect(() => service.closeBrowserExtensionPopout()).toThrow(
"Method not supported on this platform.",
);
});
});
describe("isWindowVisible", () => { describe("isWindowVisible", () => {
it("returns the window visibility", async () => { it("returns the window visibility", async () => {
isWindowVisibleMock.mockReturnValue(true); isWindowVisibleMock.mockReturnValue(true);

View File

@@ -27,6 +27,14 @@ export class DesktopLockComponentService implements LockComponentService {
return null; return null;
} }
popOutBrowserExtension(): Promise<void> {
throw new Error("Method not supported on this platform.");
}
closeBrowserExtensionPopout(): void {
throw new Error("Method not supported on this platform.");
}
async isWindowVisible(): Promise<boolean> { async isWindowVisible(): Promise<boolean> {
return ipc.platform.isWindowVisible(); return ipc.platform.isWindowVisible();
} }

View File

@@ -52,6 +52,22 @@ describe("WebLockComponentService", () => {
}); });
}); });
describe("popOutBrowserExtension", () => {
it("throws platform not supported error", () => {
expect(() => service.popOutBrowserExtension()).toThrow(
"Method not supported on this platform.",
);
});
});
describe("closeBrowserExtensionPopout", () => {
it("throws platform not supported error", () => {
expect(() => service.closeBrowserExtensionPopout()).toThrow(
"Method not supported on this platform.",
);
});
});
describe("isWindowVisible", () => { describe("isWindowVisible", () => {
it("throws an error", async () => { it("throws an error", async () => {
await expect(service.isWindowVisible()).rejects.toThrow("Method not implemented."); await expect(service.isWindowVisible()).rejects.toThrow("Method not implemented.");

View File

@@ -24,6 +24,14 @@ export class WebLockComponentService implements LockComponentService {
return null; return null;
} }
popOutBrowserExtension(): Promise<void> {
throw new Error("Method not supported on this platform.");
}
closeBrowserExtensionPopout(): void {
throw new Error("Method not supported on this platform.");
}
async isWindowVisible(): Promise<boolean> { async isWindowVisible(): Promise<boolean> {
throw new Error("Method not implemented."); throw new Error("Method not implemented.");
} }

View File

@@ -1,10 +1,10 @@
<ng-template #loading> <ng-template #spinner>
<div class="tw-flex tw-items-center tw-justify-center" *ngIf="loading"> <div class="tw-flex tw-items-center tw-justify-center">
<i class="bwi bwi-spinner bwi-spin bwi-3x" aria-hidden="true"></i> <i class="bwi bwi-spinner bwi-spin bwi-3x" aria-hidden="true"></i>
</div> </div>
</ng-template> </ng-template>
<ng-container *ngIf="unlockOptions; else loading"> <ng-container *ngIf="unlockOptions && !loading; else spinner">
<!-- Biometrics Unlock --> <!-- Biometrics Unlock -->
<ng-container *ngIf="activeUnlockOption === UnlockOption.Biometrics"> <ng-container *ngIf="activeUnlockOption === UnlockOption.Biometrics">
<button <button

View File

@@ -4,6 +4,7 @@ import { FormBuilder, FormGroup, ReactiveFormsModule, Validators } from "@angula
import { Router } from "@angular/router"; import { Router } from "@angular/router";
import { import {
BehaviorSubject, BehaviorSubject,
filter,
firstValueFrom, firstValueFrom,
interval, interval,
mergeMap, mergeMap,
@@ -11,6 +12,7 @@ import {
switchMap, switchMap,
take, take,
takeUntil, takeUntil,
tap,
} from "rxjs"; } from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module"; import { JslibModule } from "@bitwarden/angular/jslib.module";
@@ -88,6 +90,7 @@ const AUTOPROMPT_BIOMETRICS_PROCESS_RELOAD_DELAY = 5000;
}) })
export class LockComponent implements OnInit, OnDestroy { export class LockComponent implements OnInit, OnDestroy {
private destroy$ = new Subject<void>(); private destroy$ = new Subject<void>();
protected loading = true;
activeAccount: Account | null = null; activeAccount: Account | null = null;
@@ -122,6 +125,9 @@ export class LockComponent implements OnInit, OnDestroy {
formGroup: FormGroup | null = null; formGroup: FormGroup | null = null;
// Browser extension properties:
private shouldClosePopout = false;
// Desktop properties: // Desktop properties:
private deferFocus: boolean | null = null; private deferFocus: boolean | null = null;
private biometricAsked = false; private biometricAsked = false;
@@ -228,22 +234,22 @@ export class LockComponent implements OnInit, OnDestroy {
private listenForActiveAccountChanges() { private listenForActiveAccountChanges() {
this.accountService.activeAccount$ this.accountService.activeAccount$
.pipe( .pipe(
switchMap((account) => { tap((account) => {
return this.handleActiveAccountChange(account); this.loading = true;
this.activeAccount = account;
this.resetDataOnActiveAccountChange();
}),
filter((account): account is Account => account != null),
switchMap(async (account) => {
await this.handleActiveAccountChange(account);
this.loading = false;
}), }),
takeUntil(this.destroy$), takeUntil(this.destroy$),
) )
.subscribe(); .subscribe();
} }
private async handleActiveAccountChange(activeAccount: Account | null) { private async handleActiveAccountChange(activeAccount: Account) {
this.activeAccount = activeAccount;
this.resetDataOnActiveAccountChange();
if (activeAccount == null) {
return;
}
// this account may be unlocked, prevent any prompts so we can redirect to vault // this account may be unlocked, prevent any prompts so we can redirect to vault
if (await this.keyService.hasUserKeyInMemory(activeAccount.id)) { if (await this.keyService.hasUserKeyInMemory(activeAccount.id)) {
return; return;
@@ -300,16 +306,12 @@ export class LockComponent implements OnInit, OnDestroy {
// desktop and extension. // desktop and extension.
if (this.clientType === "desktop") { if (this.clientType === "desktop") {
if (autoPromptBiometrics) { if (autoPromptBiometrics) {
this.loading = false;
await this.desktopAutoPromptBiometrics(); await this.desktopAutoPromptBiometrics();
} }
} }
if (this.clientType === "browser") { if (this.clientType === "browser") {
// Firefox closes the popup when unfocused, so this would block all unlock methods
if (this.platformUtilsService.getDevice() === DeviceType.FirefoxExtension) {
return;
}
if ( if (
this.unlockOptions?.biometrics.enabled && this.unlockOptions?.biometrics.enabled &&
autoPromptBiometrics && autoPromptBiometrics &&
@@ -323,6 +325,12 @@ export class LockComponent implements OnInit, OnDestroy {
isNaN(lastProcessReload.getTime()) || isNaN(lastProcessReload.getTime()) ||
Date.now() - lastProcessReload.getTime() > AUTOPROMPT_BIOMETRICS_PROCESS_RELOAD_DELAY Date.now() - lastProcessReload.getTime() > AUTOPROMPT_BIOMETRICS_PROCESS_RELOAD_DELAY
) { ) {
// Firefox extension closes the popup when unfocused during biometric unlock, pop out the window to prevent infinite loop.
if (this.platformUtilsService.getDevice() === DeviceType.FirefoxExtension) {
await this.lockComponentService.popOutBrowserExtension();
this.shouldClosePopout = true;
}
this.loading = false;
await this.unlockViaBiometrics(); await this.unlockViaBiometrics();
} }
} }
@@ -637,6 +645,13 @@ export class LockComponent implements OnInit, OnDestroy {
const successRoute = clientTypeToSuccessRouteRecord[this.clientType]; const successRoute = clientTypeToSuccessRouteRecord[this.clientType];
await this.router.navigate([successRoute]); await this.router.navigate([successRoute]);
} }
if (
this.shouldClosePopout &&
this.platformUtilsService.getDevice() === DeviceType.FirefoxExtension
) {
this.lockComponentService.closeBrowserExtensionPopout();
}
} }
/** /**

View File

@@ -33,6 +33,18 @@ export abstract class LockComponentService {
// Extension // Extension
abstract getBiometricsError(error: any): string | null; abstract getBiometricsError(error: any): string | null;
abstract getPreviousUrl(): string | null; abstract getPreviousUrl(): string | null;
/**
* Opens the current page in a popout window if not already in a popout or the sidebar.
* If already in a popout or sidebar, does nothing.
* @throws Error if execution context is not a browser extension.
*/
abstract popOutBrowserExtension(): Promise<void>;
/**
* Closes the current popout window if in a popout.
* If not in a popout, does nothing.
* @throws Error if execution context is not a browser extension.
*/
abstract closeBrowserExtensionPopout(): void;
// Desktop only // Desktop only
abstract isWindowVisible(): Promise<boolean>; abstract isWindowVisible(): Promise<boolean>;