From ad9d9e432ae28d909c51afdfe52073bbd65cef5c Mon Sep 17 00:00:00 2001 From: Jason Ng Date: Wed, 27 Sep 2023 09:51:54 -0400 Subject: [PATCH] PM-2559 Messaging Rework for Passkey Bug (#6282) * [PM-2559] Messaging Rework - Update browser-api messageListener removing promises to fix Firefox bug Co-authored-by: Cesar Gonzalez --- .../background/context-menus.background.ts | 15 ++++++------- .../background/notification.background.ts | 4 ++-- .../src/background/commands.background.ts | 14 ++++-------- .../src/background/runtime.background.ts | 4 +--- .../src/platform/browser/browser-api.ts | 22 ++++++++----------- .../session-sync-observable/session-syncer.ts | 7 +++--- apps/browser/src/popup/app.component.ts | 19 ++++++---------- 7 files changed, 33 insertions(+), 52 deletions(-) diff --git a/apps/browser/src/autofill/background/context-menus.background.ts b/apps/browser/src/autofill/background/context-menus.background.ts index bc26353cbd9..add392f3293 100644 --- a/apps/browser/src/autofill/background/context-menus.background.ts +++ b/apps/browser/src/autofill/background/context-menus.background.ts @@ -20,17 +20,16 @@ export default class ContextMenusBackground { BrowserApi.messageListener( "contextmenus.background", - async ( + ( msg: { command: string; data: LockedVaultPendingNotificationsItem }, - sender: chrome.runtime.MessageSender, - sendResponse: any + sender: chrome.runtime.MessageSender ) => { if (msg.command === "unlockCompleted" && msg.data.target === "contextmenus.background") { - await this.contextMenuClickedHandler.cipherAction( - msg.data.commandToRetry.msg.data, - msg.data.commandToRetry.sender.tab - ); - await BrowserApi.tabSendMessageData(sender.tab, "closeNotificationBar"); + this.contextMenuClickedHandler + .cipherAction(msg.data.commandToRetry.msg.data, msg.data.commandToRetry.sender.tab) + .then(() => { + BrowserApi.tabSendMessageData(sender.tab, "closeNotificationBar"); + }); } } ); diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 1cb006fa3a2..3a1f06a3b6e 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -47,8 +47,8 @@ export default class NotificationBackground { BrowserApi.messageListener( "notification.background", - async (msg: any, sender: chrome.runtime.MessageSender) => { - await this.processMessage(msg, sender); + (msg: any, sender: chrome.runtime.MessageSender) => { + this.processMessage(msg, sender); } ); diff --git a/apps/browser/src/background/commands.background.ts b/apps/browser/src/background/commands.background.ts index 0cbf91c6666..77300272d26 100644 --- a/apps/browser/src/background/commands.background.ts +++ b/apps/browser/src/background/commands.background.ts @@ -25,17 +25,11 @@ export default class CommandsBackground { } async init() { - BrowserApi.messageListener( - "commands.background", - async (msg: any, sender: chrome.runtime.MessageSender, sendResponse: any) => { - if (msg.command === "unlockCompleted" && msg.data.target === "commands.background") { - await this.processCommand( - msg.data.commandToRetry.msg.command, - msg.data.commandToRetry.sender - ); - } + BrowserApi.messageListener("commands.background", (msg: any) => { + if (msg.command === "unlockCompleted" && msg.data.target === "commands.background") { + this.processCommand(msg.data.commandToRetry.msg.command, msg.data.commandToRetry.sender); } - ); + }); if (chrome && chrome.commands) { chrome.commands.onCommand.addListener(async (command: string) => { diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index 2b1814372a0..d169e63e9d1 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -72,9 +72,7 @@ export default class RuntimeBackground { return false; }; - BrowserApi.messageListener("runtime.background", (msg, sender, sendResponse) => { - return backgroundMessageListener(msg, sender, sendResponse); - }); + BrowserApi.messageListener("runtime.background", backgroundMessageListener); if (this.main.popupOnlyContext) { (window as any).bitwardenBackgroundMessageListener = backgroundMessageListener; } diff --git a/apps/browser/src/platform/browser/browser-api.ts b/apps/browser/src/platform/browser/browser-api.ts index 5ad5ea17317..1295eeb6aee 100644 --- a/apps/browser/src/platform/browser/browser-api.ts +++ b/apps/browser/src/platform/browser/browser-api.ts @@ -220,20 +220,16 @@ export class BrowserApi { static messageListener( name: string, - callback: (message: any, sender: chrome.runtime.MessageSender, response: any) => unknown + callback: ( + message: any, + sender: chrome.runtime.MessageSender, + sendResponse: any + ) => boolean | void ) { - chrome.runtime.onMessage.addListener( - (msg: any, sender: chrome.runtime.MessageSender, sendResponse: any) => { - const messageResponse = callback(msg, sender, sendResponse); - - if (!messageResponse) { - return false; - } - - Promise.resolve(messageResponse); - return true; - } - ); + // updated to pass synchronous callbacks to addListener. + // Will not pass async methods because they will default return a Promoise + // this causes race conditions in Firefox when a runtime.sendMessage listener receives undefined + chrome.runtime.onMessage.addListener(callback); if (BrowserApi.isSafariApi && !BrowserApi.isBackgroundPage(window)) { BrowserApi.registeredMessageListeners.push(callback); diff --git a/apps/browser/src/platform/decorators/session-sync-observable/session-syncer.ts b/apps/browser/src/platform/decorators/session-sync-observable/session-syncer.ts index 001c546b9c6..120c4b8b58c 100644 --- a/apps/browser/src/platform/decorators/session-sync-observable/session-syncer.ts +++ b/apps/browser/src/platform/decorators/session-sync-observable/session-syncer.ts @@ -73,10 +73,9 @@ export class SessionSyncer { private listenForUpdates() { // This is an unawaited promise, but it will be executed asynchronously in the background. - BrowserApi.messageListener( - this.updateMessageCommand, - async (message) => await this.updateFromMessage(message) - ); + BrowserApi.messageListener(this.updateMessageCommand, (message) => { + this.updateFromMessage(message); + }); } async updateFromMessage(message: any) { diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 93f824a7dd1..16ef77ed8f5 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -80,11 +80,7 @@ export class AppComponent implements OnInit, OnDestroy { window.onkeypress = () => this.recordActivity(); }); - (window as any).bitwardenPopupMainMessageListener = async ( - msg: any, - sender: any, - sendResponse: any - ) => { + const bitwardenPopupMainMessageListener = (msg: any, sender: any) => { if (msg.command === "doneLoggingOut") { this.authService.logOut(async () => { if (msg.expired) { @@ -102,15 +98,13 @@ export class AppComponent implements OnInit, OnDestroy { this.changeDetectorRef.detectChanges(); } else if (msg.command === "authBlocked") { this.router.navigate(["home"]); - } else if (msg.command === "locked") { - if (msg.userId == null || msg.userId === (await this.stateService.getUserId())) { - this.router.navigate(["lock"]); - } + } else if (msg.command === "locked" && msg.userId == null) { + this.router.navigate(["lock"]); } else if (msg.command === "showDialog") { - await this.ngZone.run(() => this.showDialog(msg)); + this.showDialog(msg); } else if (msg.command === "showNativeMessagingFinterprintDialog") { // TODO: Should be refactored to live in another service. - await this.ngZone.run(() => this.showNativeMessagingFingerprintDialog(msg)); + this.showNativeMessagingFingerprintDialog(msg); } else if (msg.command === "showToast") { this.showToast(msg); } else if (msg.command === "reloadProcess") { @@ -133,7 +127,8 @@ export class AppComponent implements OnInit, OnDestroy { } }; - BrowserApi.messageListener("app.component", (window as any).bitwardenPopupMainMessageListener); + (window as any).bitwardenPopupMainMessageListener = bitwardenPopupMainMessageListener; + BrowserApi.messageListener("app.component", bitwardenPopupMainMessageListener); // eslint-disable-next-line rxjs/no-async-subscribe this.router.events.pipe(takeUntil(this.destroy$)).subscribe(async (event) => {