Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
39666ad
Fix nothing showing when biometrics unavailable
quexten Nov 4, 2025
5c000d7
Cleanup
quexten Nov 4, 2025
a85d804
Switch to tooltip
quexten Nov 4, 2025
8e79ec2
Fix type error
quexten Nov 4, 2025
89f2d51
Fix type check
quexten Nov 4, 2025
8aa5b7f
Fix includes
quexten Nov 4, 2025
d28f549
Merge branch 'main' into km/fix-biometrics-tde-unavailable
quexten Nov 4, 2025
d8d7a0c
Fix types
quexten Nov 4, 2025
00b1bc1
Merge branch 'km/fix-biometrics-tde-unavailable' of github.com:bitwar…
quexten Nov 4, 2025
d1b11ed
Fix tests
quexten Nov 5, 2025
8667729
Add missing return
quexten Nov 5, 2025
3617369
Add DesktopDisconnected to canUseBiometrics
quexten Nov 5, 2025
5107570
Apply suggestions
quexten Nov 5, 2025
6b6b851
Move comment
quexten Nov 5, 2025
fc79ba6
Cleanup
quexten Nov 5, 2025
f5a53b3
Fix typing for null value
quexten Nov 5, 2025
214b4e3
Merge branch 'main' into km/fix-biometrics-tde-unavailable
quexten Nov 5, 2025
dc6548e
Add tests
quexten Nov 5, 2025
a1ddda7
Merge github.com:bitwarden/clients into km/fix-biometrics-tde-unavail…
quexten Nov 5, 2025
5e29c93
Merge branch 'km/fix-biometrics-tde-unavailable' of github.com:bitwar…
quexten Nov 5, 2025
a6b2435
Merge branch 'main' into km/fix-biometrics-tde-unavailable
quexten Nov 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions libs/key-management-ui/src/lock/components/lock.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
[disabled]="unlockingViaBiometrics || !biometricsAvailable"
[loading]="unlockingViaBiometrics"
block
[bitTooltip]="biometricUnavailabilityReason"
(click)="unlockViaBiometrics()"
>
<span> {{ biometricUnlockBtnText | i18n }}</span>
Expand Down
249 changes: 248 additions & 1 deletion libs/key-management-ui/src/lock/components/lock.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -56,6 +56,7 @@ import {
import {
LockComponentService,
UnlockOption,
UnlockOptionValue,
UnlockOptions,
} from "../services/lock-component.service";

Expand Down Expand Up @@ -805,4 +806,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: "[email protected]",
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);
});
});
});
36 changes: 34 additions & 2 deletions libs/key-management-ui/src/lock/components/lock.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,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,
Expand Down Expand Up @@ -86,6 +87,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({
Expand All @@ -100,6 +107,7 @@ const AUTOPROMPT_BIOMETRICS_PROCESS_RELOAD_DELAY = 5000;
AsyncActionsModule,
IconButtonModule,
MasterPasswordLockComponent,
TooltipDirective,
],
})
export class LockComponent implements OnInit, OnDestroy {
Expand Down Expand Up @@ -276,7 +284,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));
Comment on lines +287 to +290
Copy link

@claude claude bot Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Race condition: This makes a separate call to getBiometricsStatusForUser() after already fetching unlockOptions, which could result in inconsistent state.

As mzieniukbw suggested, use the status from unlockOptions directly:

const canUseBiometrics = [
  BiometricsStatus.Available,
  ...BIOMETRIC_UNLOCK_TEMPORARY_UNAVAILABLE_STATUSES,
].includes(this.unlockOptions.biometrics.biometricsStatus);

This ensures the biometric status check uses the same data already fetched in unlockOptions.

Copy link

@claude claude bot Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Inconsistent data source: This calls getBiometricsStatusForUser() directly after getAvailableUnlockOptions$() already fetched biometric status.

The biometric status is already available in this.unlockOptions.biometrics.biometricsStatus (line 283-285). Using a separate call could result in inconsistent state if the biometric status changes between the two calls.

Suggested change:

const canUseBiometrics = [
  BiometricsStatus.Available,
  ...BIOMETRIC_UNLOCK_TEMPORARY_UNAVAILABLE_STATUSES,
].includes(this.unlockOptions?.biometrics.biometricsStatus ?? BiometricsStatus.PlatformUnsupported);

This was flagged in a previous review and appears unresolved.

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();
Expand All @@ -299,14 +322,23 @@ 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;
} else if (unlockOptions?.pin.enabled) {
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;
}
}

Expand Down
Loading