From a559a113d3eb5c770c84b54d36f4a496f590f73c Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 13 Aug 2024 11:15:37 +0200 Subject: [PATCH] Move dialogs out of native messaging (#10411) --- .../settings/account-security.component.ts | 99 ++++++++------ .../background/nativeMessaging.background.ts | 125 +++++------------- apps/browser/src/models/biometricErrors.ts | 34 ++++- 3 files changed, 130 insertions(+), 128 deletions(-) diff --git a/apps/browser/src/auth/popup/settings/account-security.component.ts b/apps/browser/src/auth/popup/settings/account-security.component.ts index 820cdf11cdc..076c03801aa 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.ts @@ -1,3 +1,4 @@ +import { DialogRef } from "@angular/cdk/dialog"; import { ChangeDetectorRef, Component, OnDestroy, OnInit } from "@angular/core"; import { FormBuilder } from "@angular/forms"; import { @@ -382,49 +383,73 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { return; } - const awaitDesktopDialogRef = AwaitDesktopDialogComponent.open(this.dialogService); - const awaitDesktopDialogClosed = firstValueFrom(awaitDesktopDialogRef.closed); + let awaitDesktopDialogRef: DialogRef | undefined; + let biometricsResponseReceived = false; await this.cryptoService.refreshAdditionalKeys(); - await Promise.race([ - awaitDesktopDialogClosed.then(async (result) => { - if (result !== true) { - this.form.controls.biometric.setValue(false); - } - }), - this.platformUtilsService - .authenticateBiometric() - .then((result) => { - this.form.controls.biometric.setValue(result); - if (!result) { - this.platformUtilsService.showToast( - "error", - this.i18nService.t("errorEnableBiometricTitle"), - this.i18nService.t("errorEnableBiometricDesc"), - ); - } - }) - .catch((e) => { - // Handle connection errors - this.form.controls.biometric.setValue(false); + const waitForUserDialogPromise = async () => { + // only show waiting dialog if we have waited for 200 msec to prevent double dialog + // the os will respond instantly if the dialog shows successfully, and the desktop app will respond instantly if something is wrong + await new Promise((resolve) => setTimeout(resolve, 200)); + if (biometricsResponseReceived) { + return; + } - const error = BiometricErrors[e.message as BiometricErrorTypes]; + awaitDesktopDialogRef = AwaitDesktopDialogComponent.open(this.dialogService); + const result = await firstValueFrom(awaitDesktopDialogRef.closed); + if (result !== true) { + this.form.controls.biometric.setValue(false); + } + }; - // 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.dialogService.openSimpleDialog({ - title: { key: error.title }, - content: { key: error.description }, - acceptButtonText: { key: "ok" }, - cancelButtonText: null, - type: "danger", - }); - }) - .finally(() => { + const biometricsPromise = async () => { + try { + const result = await this.platformUtilsService.authenticateBiometric(); + + // prevent duplicate dialog + biometricsResponseReceived = true; + if (awaitDesktopDialogRef) { awaitDesktopDialogRef.close(true); - }), - ]); + } + + this.form.controls.biometric.setValue(result); + if (!result) { + this.platformUtilsService.showToast( + "error", + this.i18nService.t("errorEnableBiometricTitle"), + this.i18nService.t("errorEnableBiometricDesc"), + ); + } + } catch (e) { + // prevent duplicate dialog + biometricsResponseReceived = true; + if (awaitDesktopDialogRef) { + awaitDesktopDialogRef.close(true); + } + + this.form.controls.biometric.setValue(false); + + if (e.message == "canceled") { + return; + } + + const error = BiometricErrors[e.message as BiometricErrorTypes]; + await this.dialogService.openSimpleDialog({ + title: { key: error.title }, + content: { key: error.description }, + acceptButtonText: { key: "ok" }, + cancelButtonText: null, + type: "danger", + }); + } finally { + if (awaitDesktopDialogRef) { + awaitDesktopDialogRef.close(true); + } + } + }; + + await Promise.race([waitForUserDialogPromise(), biometricsPromise()]); } else { await this.biometricStateService.setBiometricUnlockEnabled(false); await this.biometricStateService.setFingerprintValidated(false); diff --git a/apps/browser/src/background/nativeMessaging.background.ts b/apps/browser/src/background/nativeMessaging.background.ts index e19485c7118..19c55f9f696 100644 --- a/apps/browser/src/background/nativeMessaging.background.ts +++ b/apps/browser/src/background/nativeMessaging.background.ts @@ -21,7 +21,7 @@ import { BrowserApi } from "../platform/browser/browser-api"; import RuntimeBackground from "./runtime.background"; const MessageValidTimeout = 10 * 1000; -const EncryptionAlgorithm = "sha1"; +const HashAlgorithmForEncryption = "sha1"; type Message = { command: string; @@ -64,6 +64,7 @@ export class NativeMessagingBackground { private port: browser.runtime.Port | chrome.runtime.Port; private resolver: any = null; + private rejecter: any = null; private privateKey: Uint8Array = null; private publicKey: Uint8Array = null; private secureSetupResolve: any = null; @@ -137,7 +138,7 @@ export class NativeMessagingBackground { const decrypted = await this.cryptoFunctionService.rsaDecrypt( encrypted, this.privateKey, - EncryptionAlgorithm, + HashAlgorithmForEncryption, ); if (this.validatingFingerprint) { @@ -158,19 +159,10 @@ export class NativeMessagingBackground { this.privateKey = null; this.connected = false; - this.messagingService.send("showDialog", { - title: { key: "nativeMessagingInvalidEncryptionTitle" }, - content: { key: "nativeMessagingInvalidEncryptionDesc" }, - acceptButtonText: { key: "ok" }, - cancelButtonText: null, - type: "danger", + this.rejecter({ + message: "invalidateEncryption", }); - - if (this.resolver) { - this.resolver(message); - } - - break; + return; case "verifyFingerprint": { if (this.sharedSecret == null) { this.validatingFingerprint = true; @@ -181,8 +173,10 @@ export class NativeMessagingBackground { break; } case "wrongUserId": - this.showWrongUserDialog(); - break; + this.rejecter({ + message: "wrongUserId", + }); + return; default: // Ignore since it belongs to another device if (!this.platformUtilsService.isSafari() && message.appId !== this.appId) { @@ -215,26 +209,6 @@ export class NativeMessagingBackground { }); } - showWrongUserDialog() { - this.messagingService.send("showDialog", { - title: { key: "nativeMessagingWrongUserTitle" }, - content: { key: "nativeMessagingWrongUserDesc" }, - acceptButtonText: { key: "ok" }, - cancelButtonText: null, - type: "danger", - }); - } - - showIncorrectUserKeyDialog() { - this.messagingService.send("showDialog", { - title: { key: "nativeMessagingWrongUserKeyTitle" }, - content: { key: "nativeMessagingWrongUserKeyDesc" }, - acceptButtonText: { key: "ok" }, - cancelButtonText: null, - type: "danger", - }); - } - async send(message: Message) { if (!this.connected) { await this.connect(); @@ -260,7 +234,14 @@ export class NativeMessagingBackground { getResponse(): Promise { return new Promise((resolve, reject) => { - this.resolver = resolve; + this.resolver = function (response: any) { + resolve(response); + }; + this.rejecter = function (resp: any) { + reject({ + message: resp, + }); + }; }); } @@ -286,13 +267,7 @@ export class NativeMessagingBackground { this.privateKey = null; this.connected = false; - this.messagingService.send("showDialog", { - title: { key: "nativeMessagingInvalidEncryptionTitle" }, - content: { key: "nativeMessagingInvalidEncryptionDesc" }, - acceptButtonText: { key: "ok" }, - cancelButtonText: null, - type: "danger", - }); + this.rejecter("invalidateEncryption"); } } @@ -311,35 +286,11 @@ export class NativeMessagingBackground { switch (message.command) { case "biometricUnlock": { - if (message.response === "not enabled") { - this.messagingService.send("showDialog", { - title: { key: "biometricsNotEnabledTitle" }, - content: { key: "biometricsNotEnabledDesc" }, - acceptButtonText: { key: "ok" }, - cancelButtonText: null, - type: "danger", - }); - break; - } else if (message.response === "not supported") { - this.messagingService.send("showDialog", { - title: { key: "biometricsNotSupportedTitle" }, - content: { key: "biometricsNotSupportedDesc" }, - acceptButtonText: { key: "ok" }, - cancelButtonText: null, - type: "danger", - }); - break; - } else if (message.response === "not unlocked") { - this.messagingService.send("showDialog", { - title: { key: "biometricsNotUnlockedTitle" }, - content: { key: "biometricsNotUnlockedDesc" }, - acceptButtonText: { key: "ok" }, - cancelButtonText: null, - type: "danger", - }); - break; - } else if (message.response === "canceled") { - break; + if ( + ["not enabled", "not supported", "not unlocked", "canceled"].includes(message.response) + ) { + this.rejecter(message.response); + return; } // Check for initial setup of biometric unlock @@ -374,12 +325,7 @@ export class NativeMessagingBackground { } else { this.logService.error("Unable to verify biometric unlocked userkey"); await this.cryptoService.clearKeys(activeUserId); - this.showIncorrectUserKeyDialog(); - - // Exit early - if (this.resolver) { - this.resolver(message); - } + this.rejecter("userkey wrong"); return; } } else { @@ -387,18 +333,17 @@ export class NativeMessagingBackground { } } catch (e) { this.logService.error("Unable to set key: " + e); - this.messagingService.send("showDialog", { - title: { key: "biometricsFailedTitle" }, - content: { key: "biometricsFailedDesc" }, - acceptButtonText: { key: "ok" }, - cancelButtonText: null, - type: "danger", - }); + this.rejecter("userkey wrong"); + return; + } - // Exit early - if (this.resolver) { - this.resolver(message); - } + // Verify key is correct by attempting to decrypt a secret + try { + await this.cryptoService.getFingerprint(await this.stateService.getUserId()); + } catch (e) { + this.logService.error("Unable to verify key: " + e); + await this.cryptoService.clearKeys(); + this.rejecter("userkey wrong"); return; } diff --git a/apps/browser/src/models/biometricErrors.ts b/apps/browser/src/models/biometricErrors.ts index 822a5c16f42..570c776f563 100644 --- a/apps/browser/src/models/biometricErrors.ts +++ b/apps/browser/src/models/biometricErrors.ts @@ -3,7 +3,15 @@ type BiometricError = { description: string; }; -export type BiometricErrorTypes = "startDesktop" | "desktopIntegrationDisabled"; +export type BiometricErrorTypes = + | "startDesktop" + | "desktopIntegrationDisabled" + | "not enabled" + | "not supported" + | "not unlocked" + | "invalidateEncryption" + | "userkey wrong" + | "wrongUserId"; export const BiometricErrors: Record = { startDesktop: { @@ -14,4 +22,28 @@ export const BiometricErrors: Record = { title: "desktopIntegrationDisabledTitle", description: "desktopIntegrationDisabledDesc", }, + "not enabled": { + title: "biometricsNotEnabledTitle", + description: "biometricsNotEnabledDesc", + }, + "not supported": { + title: "biometricsNotSupportedTitle", + description: "biometricsNotSupportedDesc", + }, + "not unlocked": { + title: "biometricsUnlockNotUnlockedTitle", + description: "biometricsUnlockNotUnlockedDesc", + }, + invalidateEncryption: { + title: "nativeMessagingInvalidEncryptionTitle", + description: "nativeMessagingInvalidEncryptionDesc", + }, + "userkey wrong": { + title: "nativeMessagingWrongUserKeyTitle", + description: "nativeMessagingWrongUserKeyDesc", + }, + wrongUserId: { + title: "biometricsWrongUserTitle", + description: "biometricsWrongUserDesc", + }, };