1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-24 12:13:39 +00:00

[PM-27225] Fix nothing showing when biometrics unavailable (#17209)

* Fix nothing showing when biometrics unavailable

* Cleanup

* Switch to tooltip

* Fix type error

* Fix type check

* Fix includes

* Fix types

* Fix tests

* Add missing return

* Add DesktopDisconnected to canUseBiometrics

* Apply suggestions

* Move comment

* Cleanup

* Fix typing for null value

* Add tests

* Fix QA bugs
This commit is contained in:
Bernd Schoolmann
2025-12-11 12:47:00 +01:00
committed by GitHub
parent 267e488390
commit 404e07b6bd
6 changed files with 305 additions and 6 deletions

View File

@@ -48,7 +48,11 @@ export class ForegroundBrowserBiometricsService extends BiometricsService {
result: BiometricsStatus;
error: string;
}>(BiometricsCommands.GetBiometricsStatusForUser, { userId: id });
return response.result;
if (response != null) {
return response.result;
} else {
return BiometricsStatus.DesktopDisconnected;
}
}
async getShouldAutopromptNow(): Promise<boolean> {

View File

@@ -51,6 +51,7 @@ import {
} from "@bitwarden/common/auth/abstractions/auth.service";
import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction";
import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction";
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions";
import { ClientType } from "@bitwarden/common/enums";
@@ -167,12 +168,12 @@ const safeProviders: SafeProvider[] = [
safeProvider({
provide: BiometricsService,
useClass: RendererBiometricsService,
deps: [],
deps: [TokenService],
}),
safeProvider({
provide: DesktopBiometricsService,
useClass: RendererBiometricsService,
deps: [],
deps: [TokenService],
}),
safeProvider(NativeMessagingService),
safeProvider(BiometricMessageHandlerService),

View File

@@ -1,5 +1,7 @@
import { Injectable } from "@angular/core";
import { firstValueFrom } from "rxjs";
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import { UserId } from "@bitwarden/common/types/guid";
import { UserKey } from "@bitwarden/common/types/key";
@@ -13,6 +15,10 @@ import { DesktopBiometricsService } from "./desktop.biometrics.service";
*/
@Injectable()
export class RendererBiometricsService extends DesktopBiometricsService {
constructor(private tokenService: TokenService) {
super();
}
async authenticateWithBiometrics(): Promise<boolean> {
return await ipc.keyManagement.biometric.authenticateWithBiometrics();
}
@@ -31,6 +37,10 @@ export class RendererBiometricsService extends DesktopBiometricsService {
}
async getBiometricsStatusForUser(id: UserId): Promise<BiometricsStatus> {
if ((await firstValueFrom(this.tokenService.hasAccessToken$(id))) === false) {
return BiometricsStatus.NotEnabledInConnectedDesktopApp;
}
return await ipc.keyManagement.biometric.getBiometricsStatusForUser(id);
}

View File

@@ -16,6 +16,7 @@
[disabled]="unlockingViaBiometrics || !biometricsAvailable"
[loading]="unlockingViaBiometrics"
block
[bitTooltip]="biometricUnavailabilityReason"
(click)="unlockViaBiometrics()"
>
<span> {{ biometricUnlockBtnText | i18n }}</span>

View File

@@ -11,7 +11,7 @@ import { JslibModule } from "@bitwarden/angular/jslib.module";
import { LogoutService } from "@bitwarden/auth/common";
import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/models/domain/master-password-policy-options";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { VerificationType } from "@bitwarden/common/auth/enums/verification-type";
import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason";
@@ -57,6 +57,7 @@ import {
import {
LockComponentService,
UnlockOption,
UnlockOptionValue,
UnlockOptions,
} from "../services/lock-component.service";
@@ -878,4 +879,250 @@ describe("LockComponent", () => {
expect(mockRouter.navigate).not.toHaveBeenCalled();
});
});
describe("setDefaultActiveUnlockOption", () => {
it.each([
[
"biometrics enabled",
{
biometrics: { enabled: true, biometricsStatus: BiometricsStatus.Available },
pin: { enabled: false },
masterPassword: { enabled: false },
} as UnlockOptions,
UnlockOption.Biometrics,
],
[
"biometrics disabled, pin enabled",
{
biometrics: { enabled: false, biometricsStatus: BiometricsStatus.NotEnabledLocally },
pin: { enabled: true },
masterPassword: { enabled: false },
} as UnlockOptions,
UnlockOption.Pin,
],
[
"biometrics and pin disabled, masterPassword enabled",
{
biometrics: { enabled: false, biometricsStatus: BiometricsStatus.NotEnabledLocally },
pin: { enabled: false },
masterPassword: { enabled: true },
} as UnlockOptions,
UnlockOption.MasterPassword,
],
[
"hardware unavailable, no other options",
{
biometrics: { enabled: false, biometricsStatus: BiometricsStatus.HardwareUnavailable },
pin: { enabled: false },
masterPassword: { enabled: false },
} as UnlockOptions,
UnlockOption.Biometrics,
],
[
"desktop disconnected, no other options",
{
biometrics: { enabled: false, biometricsStatus: BiometricsStatus.DesktopDisconnected },
pin: { enabled: false },
masterPassword: { enabled: false },
} as UnlockOptions,
UnlockOption.Biometrics,
],
[
"not enabled in connected desktop app, no other options",
{
biometrics: {
enabled: false,
biometricsStatus: BiometricsStatus.NotEnabledInConnectedDesktopApp,
},
pin: { enabled: false },
masterPassword: { enabled: false },
} as UnlockOptions,
UnlockOption.Biometrics,
],
[
"biometrics over pin priority",
{
biometrics: { enabled: true, biometricsStatus: BiometricsStatus.Available },
pin: { enabled: true },
masterPassword: { enabled: false },
} as UnlockOptions,
UnlockOption.Biometrics,
],
[
"biometrics over masterPassword priority",
{
biometrics: { enabled: true, biometricsStatus: BiometricsStatus.Available },
pin: { enabled: false },
masterPassword: { enabled: true },
} as UnlockOptions,
UnlockOption.Biometrics,
],
[
"pin over masterPassword priority",
{
biometrics: { enabled: false, biometricsStatus: BiometricsStatus.NotEnabledLocally },
pin: { enabled: true },
masterPassword: { enabled: true },
} as UnlockOptions,
UnlockOption.Pin,
],
[
"all options enabled",
{
biometrics: { enabled: true, biometricsStatus: BiometricsStatus.Available },
pin: { enabled: true },
masterPassword: { enabled: true },
} as UnlockOptions,
UnlockOption.Biometrics,
],
])(
"should set active unlock option to $1 when %s",
async (
description: string,
unlockOptions: UnlockOptions,
expectedOption: UnlockOptionValue,
) => {
await component["setDefaultActiveUnlockOption"](unlockOptions);
expect(component.activeUnlockOption).toBe(expectedOption);
},
);
});
describe("handleActiveAccountChange", () => {
const mockActiveAccount: Account = {
id: userId,
email: "test@example.com",
name: "Test User",
} as Account;
beforeEach(async () => {
component.activeAccount = mockActiveAccount;
});
it("should return early when account already has user key", async () => {
mockKeyService.hasUserKey.mockResolvedValue(true);
await component["handleActiveAccountChange"](mockActiveAccount);
expect(mockKeyService.hasUserKey).toHaveBeenCalledWith(userId);
expect(mockAnonLayoutWrapperDataService.setAnonLayoutWrapperData).not.toHaveBeenCalled();
});
it("should set email as page subtitle when account is unlocked", async () => {
mockKeyService.hasUserKey.mockResolvedValue(false);
mockLockComponentService.getAvailableUnlockOptions$.mockReturnValue(
of({
biometrics: { enabled: true, biometricsStatus: BiometricsStatus.Available },
pin: { enabled: false },
masterPassword: { enabled: false },
} as UnlockOptions),
);
mockBiometricService.getBiometricsStatusForUser.mockResolvedValue(BiometricsStatus.Available);
await component["handleActiveAccountChange"](mockActiveAccount);
expect(mockAnonLayoutWrapperDataService.setAnonLayoutWrapperData).toHaveBeenCalledWith({
pageSubtitle: mockActiveAccount.email,
});
});
it("should logout user when no unlock options are available", async () => {
mockKeyService.hasUserKey.mockResolvedValue(false);
mockLockComponentService.getAvailableUnlockOptions$.mockReturnValue(
of({
biometrics: { enabled: false, biometricsStatus: BiometricsStatus.UnlockNeeded },
pin: { enabled: false },
masterPassword: { enabled: false },
} as UnlockOptions),
);
mockBiometricService.getBiometricsStatusForUser.mockResolvedValue(
BiometricsStatus.UnlockNeeded,
);
await component["handleActiveAccountChange"](mockActiveAccount);
expect(mockLogService.warning).toHaveBeenCalledWith(
"[LockComponent] User cannot unlock again. Logging out!",
);
expect(mockLogoutService.logout).toHaveBeenCalledWith(userId);
});
it("should not logout when master password is enabled", async () => {
mockKeyService.hasUserKey.mockResolvedValue(false);
mockLockComponentService.getAvailableUnlockOptions$.mockReturnValue(
of({
biometrics: { enabled: false, biometricsStatus: BiometricsStatus.UnlockNeeded },
pin: { enabled: false },
masterPassword: { enabled: true },
} as UnlockOptions),
);
mockBiometricService.getBiometricsStatusForUser.mockResolvedValue(
BiometricsStatus.UnlockNeeded,
);
await component["handleActiveAccountChange"](mockActiveAccount);
expect(mockLogoutService.logout).not.toHaveBeenCalled();
expect(component.activeUnlockOption).toBe(UnlockOption.MasterPassword);
});
it("should not logout when pin is enabled", async () => {
mockKeyService.hasUserKey.mockResolvedValue(false);
mockLockComponentService.getAvailableUnlockOptions$.mockReturnValue(
of({
biometrics: { enabled: false, biometricsStatus: BiometricsStatus.UnlockNeeded },
pin: { enabled: true },
masterPassword: { enabled: false },
} as UnlockOptions),
);
mockBiometricService.getBiometricsStatusForUser.mockResolvedValue(
BiometricsStatus.UnlockNeeded,
);
await component["handleActiveAccountChange"](mockActiveAccount);
expect(mockLogoutService.logout).not.toHaveBeenCalled();
expect(component.activeUnlockOption).toBe(UnlockOption.Pin);
});
it("should not logout when biometrics is available", async () => {
mockKeyService.hasUserKey.mockResolvedValue(false);
mockLockComponentService.getAvailableUnlockOptions$.mockReturnValue(
of({
biometrics: { enabled: true, biometricsStatus: BiometricsStatus.Available },
pin: { enabled: false },
masterPassword: { enabled: false },
} as UnlockOptions),
);
mockBiometricService.getBiometricsStatusForUser.mockResolvedValue(BiometricsStatus.Available);
await component["handleActiveAccountChange"](mockActiveAccount);
expect(mockLogoutService.logout).not.toHaveBeenCalled();
expect(component.activeUnlockOption).toBe(UnlockOption.Biometrics);
});
it("should not logout when biometrics is temporarily unavailable but no other options", async () => {
mockKeyService.hasUserKey.mockResolvedValue(false);
mockLockComponentService.getAvailableUnlockOptions$.mockReturnValue(
of({
biometrics: {
enabled: false,
biometricsStatus: BiometricsStatus.HardwareUnavailable,
},
pin: { enabled: false },
masterPassword: { enabled: false },
} as UnlockOptions),
);
mockBiometricService.getBiometricsStatusForUser.mockResolvedValue(
BiometricsStatus.HardwareUnavailable,
);
await component["handleActiveAccountChange"](mockActiveAccount);
expect(mockLogoutService.logout).not.toHaveBeenCalled();
expect(component.activeUnlockOption).toBe(UnlockOption.Biometrics);
});
});
});

View File

@@ -44,6 +44,7 @@ import { SyncService } from "@bitwarden/common/platform/sync";
import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength";
import { UserKey } from "@bitwarden/common/types/key";
import {
TooltipDirective,
AsyncActionsModule,
AnonLayoutWrapperDataService,
ButtonModule,
@@ -87,6 +88,12 @@ type AfterUnlockActions = {
/// Fixes safari autoprompt behavior
const AUTOPROMPT_BIOMETRICS_PROCESS_RELOAD_DELAY = 5000;
const BIOMETRIC_UNLOCK_TEMPORARY_UNAVAILABLE_STATUSES = [
BiometricsStatus.HardwareUnavailable,
BiometricsStatus.DesktopDisconnected,
BiometricsStatus.NotEnabledInConnectedDesktopApp,
];
// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
@Component({
@@ -101,6 +108,7 @@ const AUTOPROMPT_BIOMETRICS_PROCESS_RELOAD_DELAY = 5000;
AsyncActionsModule,
IconButtonModule,
MasterPasswordLockComponent,
TooltipDirective,
],
})
export class LockComponent implements OnInit, OnDestroy {
@@ -212,6 +220,10 @@ export class LockComponent implements OnInit, OnDestroy {
this.unlockOptions = await firstValueFrom(
this.lockComponentService.getAvailableUnlockOptions$(this.activeAccount.id),
);
if (this.activeUnlockOption == null) {
this.loading = false;
await this.setDefaultActiveUnlockOption(this.unlockOptions);
}
}
}),
takeUntil(this.destroy$),
@@ -280,7 +292,22 @@ export class LockComponent implements OnInit, OnDestroy {
this.lockComponentService.getAvailableUnlockOptions$(activeAccount.id),
);
this.setDefaultActiveUnlockOption(this.unlockOptions);
const canUseBiometrics = [
BiometricsStatus.Available,
...BIOMETRIC_UNLOCK_TEMPORARY_UNAVAILABLE_STATUSES,
].includes(await this.biometricService.getBiometricsStatusForUser(activeAccount.id));
if (
!this.unlockOptions?.masterPassword.enabled &&
!this.unlockOptions?.pin.enabled &&
!canUseBiometrics
) {
// User has no available unlock options, force logout. This happens for TDE users without a masterpassword, that don't have a persistent unlock method set.
this.logService.warning("[LockComponent] User cannot unlock again. Logging out!");
await this.logoutService.logout(activeAccount.id);
return;
}
await this.setDefaultActiveUnlockOption(this.unlockOptions);
if (this.unlockOptions?.biometrics.enabled) {
await this.handleBiometricsUnlockEnabled();
@@ -303,7 +330,7 @@ export class LockComponent implements OnInit, OnDestroy {
});
}
private setDefaultActiveUnlockOption(unlockOptions: UnlockOptions | null) {
private async setDefaultActiveUnlockOption(unlockOptions: UnlockOptions | null) {
// Priorities should be Biometrics > Pin > Master Password for speed
if (unlockOptions?.biometrics.enabled) {
this.activeUnlockOption = UnlockOption.Biometrics;
@@ -311,6 +338,15 @@ export class LockComponent implements OnInit, OnDestroy {
this.activeUnlockOption = UnlockOption.Pin;
} else if (unlockOptions?.masterPassword.enabled) {
this.activeUnlockOption = UnlockOption.MasterPassword;
} else if (
unlockOptions != null &&
BIOMETRIC_UNLOCK_TEMPORARY_UNAVAILABLE_STATUSES.includes(
unlockOptions.biometrics.biometricsStatus,
)
) {
// If biometrics is temporarily unavailable for masterpassword-less users, but they have biometrics configured,
// then show the biometrics screen so the user knows why they can't unlock, and to give them the option to log out.
this.activeUnlockOption = UnlockOption.Biometrics;
}
}