Skip to content

Commit

Permalink
[PM-17121/17204] Fix fingerprint dialogs and disabled active biometri…
Browse files Browse the repository at this point in the history
…c lock component (#12928)

* Fix biometrics unlock window being empty

* Add trust on sensitive action

* Add dialog for outdated desktop app and fix spelling

* Use updated fingerprint method

* Refactor connected app trust

* Move connected apps to ephemeral value store and show error on outdated browser

* Move trust logic to only occur when fingerprint setting is enabled

* Add more tests

* Simplify code

* Update ephemeral value list call to "listEphemeralValueKeys"

* Fix trust being ignored

(cherry picked from commit ef20ca8)
  • Loading branch information
quexten committed Jan 21, 2025
1 parent b4e6496 commit 06b60c6
Show file tree
Hide file tree
Showing 11 changed files with 508 additions and 69 deletions.
6 changes: 6 additions & 0 deletions apps/browser/src/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -4903,5 +4903,11 @@
},
"extraWide": {
"message": "Extra wide"
},
"updateDesktopAppOrDisableFingerprintDialogTitle": {
"message": "Please update your desktop application"
},
"updateDesktopAppOrDisableFingerprintDialogMessage": {
"message": "To use biometric unlock, please update your desktop application, or disable fingerprint unlock in the desktop settings."
}
}
38 changes: 20 additions & 18 deletions apps/browser/src/background/nativeMessaging.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,6 @@ export class NativeMessagingBackground {
HashAlgorithmForEncryption,
);

if (this.validatingFingerprint) {
this.validatingFingerprint = false;
await this.biometricStateService.setFingerprintValidated(true);
}
this.sharedSecret = new SymmetricCryptoKey(decrypted);
this.logService.info("[Native Messaging IPC] Secure channel established");

Expand Down Expand Up @@ -200,15 +196,24 @@ export class NativeMessagingBackground {
}
return;
case "verifyFingerprint": {
if (this.sharedSecret == null) {
this.logService.info(
"[Native Messaging IPC] Desktop app requested trust verification by fingerprint.",
);
this.validatingFingerprint = true;
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.showFingerprintDialog();
}
this.logService.info("[Native Messaging IPC] Legacy app is requesting fingerprint");
this.messagingService.send("showUpdateDesktopAppOrDisableFingerprintDialog", {});
break;
}
case "verifyDesktopIPCFingerprint": {
this.logService.info(
"[Native Messaging IPC] Desktop app requested trust verification by fingerprint.",
);
await this.showFingerprintDialog();
break;
}
case "verifiedDesktopIPCFingerprint": {
await this.biometricStateService.setFingerprintValidated(true);
this.messagingService.send("hideNativeMessagingFingerprintDialog", {});
break;
}
case "rejectedDesktopIPCFingerprint": {
this.messagingService.send("hideNativeMessagingFingerprintDialog", {});
break;
}
case "wrongUserId":
Expand Down Expand Up @@ -426,12 +431,9 @@ export class NativeMessagingBackground {
}

private async showFingerprintDialog() {
const fingerprint = await this.keyService.getFingerprint(
(await firstValueFrom(this.accountService.activeAccount$))?.id,
this.publicKey,
);
const fingerprint = await this.keyService.getFingerprint(this.appId, this.publicKey);

this.messagingService.send("showNativeMessagingFinterprintDialog", {
this.messagingService.send("showNativeMessagingFingerprintDialog", {
fingerprint: fingerprint,
});
}
Expand Down
9 changes: 8 additions & 1 deletion apps/browser/src/popup/app.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,18 @@ export class AppComponent implements OnInit, OnDestroy {
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.showDialog(msg);
} else if (msg.command === "showNativeMessagingFinterprintDialog") {
} else if (msg.command === "showNativeMessagingFingerprintDialog") {
// TODO: Should be refactored to live in another service.
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.showNativeMessagingFingerprintDialog(msg);
} else if (msg.command === "showUpdateDesktopAppOrDisableFingerprintDialog") {
// TODO: Should be refactored to live in another service.
await this.showDialog({
title: this.i18nService.t("updateDesktopAppOrDisableFingerprintDialogTitle"),
content: this.i18nService.t("updateDesktopAppOrDisableFingerprintDialogMessage"),
type: "warning",
});
} else if (msg.command === "showToast") {
this.toastService._showToast(msg);
} else if (msg.command === "reloadProcess") {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { DIALOG_DATA } from "@angular/cdk/dialog";
import { Component, Inject } from "@angular/core";
import { DIALOG_DATA, DialogRef } from "@angular/cdk/dialog";
import { Component, Inject, OnDestroy, OnInit } from "@angular/core";
import { filter, Subject, takeUntil } from "rxjs";

import { JslibModule } from "@bitwarden/angular/jslib.module";
import { MessageListener } from "@bitwarden/common/platform/messaging";
import { ButtonModule, DialogModule, DialogService } from "@bitwarden/components";

export type DesktopSyncVerificationDialogParams = {
Expand All @@ -13,8 +15,30 @@ export type DesktopSyncVerificationDialogParams = {
standalone: true,
imports: [JslibModule, ButtonModule, DialogModule],
})
export class DesktopSyncVerificationDialogComponent {
constructor(@Inject(DIALOG_DATA) protected params: DesktopSyncVerificationDialogParams) {}
export class DesktopSyncVerificationDialogComponent implements OnDestroy, OnInit {
private destroy$ = new Subject<void>();

constructor(
@Inject(DIALOG_DATA) protected params: DesktopSyncVerificationDialogParams,
private dialogRef: DialogRef<DesktopSyncVerificationDialogComponent>,
private messageListener: MessageListener,
) {}

ngOnDestroy(): void {
this.destroy$.next();
this.destroy$.complete();
}

ngOnInit(): void {
this.messageListener.allMessages$
.pipe(
filter((m) => m.command === "hideNativeMessagingFingerprintDialog"),
takeUntil(this.destroy$),
)
.subscribe(() => {
this.dialogRef.close();
});
}

static open(dialogService: DialogService, data: DesktopSyncVerificationDialogParams) {
return dialogService.open(DesktopSyncVerificationDialogComponent, {
Expand Down
15 changes: 15 additions & 0 deletions apps/desktop/src/locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3474,5 +3474,20 @@
},
"changeAcctEmail": {
"message": "Change account email"
},
"organizationUpgradeRequired": {
"message": "Organization upgrade required"
},
"upgradeOrganization": {
"message": "Upgrade organization"
},
"upgradeOrganizationDesc": {
"message": "This feature is not available for free organizations. Switch to a paid plan to unlock more features."
},
"updateBrowserOrDisableFingerprintDialogTitle": {
"message": "Extension update required"
},
"updateBrowserOrDisableFingerprintDialogMessage": {
"message": "The browser extension you are using is out of date. Please update it or disable browser integration fingerprint validation in the desktop app settings."
}
}
1 change: 1 addition & 0 deletions apps/desktop/src/platform/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ const ephemeralStore = {
getEphemeralValue: (key: string): Promise<string> => ipcRenderer.invoke("getEphemeralValue", key),
removeEphemeralValue: (key: string): Promise<void> =>
ipcRenderer.invoke("deleteEphemeralValue", key),
listEphemeralValueKeys: (): Promise<string[]> => ipcRenderer.invoke("listEphemeralValueKeys"),
};

const localhostCallbackService = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@ export class EphemeralValueStorageService {
ipcMain.handle("deleteEphemeralValue", async (event, key: string) => {
this.ephemeralValues.delete(key);
});
ipcMain.handle("listEphemeralValueKeys", async (event) => {
return Array.from(this.ephemeralValues.keys());
});
}
}
Loading

0 comments on commit 06b60c6

Please sign in to comment.