From d63a9a2d9f268c1d1cf024be5ee9da5d35dd2107 Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Mon, 29 May 2023 00:37:47 -0400 Subject: [PATCH] Defect/PM-2403 - Fix non-locally reproducible Master Password Invalid issue in Safari after SSO + 2FA login (#5531) * PM-2403 - (1) Fix issue with Safari's tab extension not closing after successful SSO Login + 2FA continue which sidesteps the inconsistent MP invalid issue (due to invalid KDF config settings on reload) -- Firefox + sidebar, opera + sidebar, chrome, safari, and edge tested (2) Refactor reload logic to exempt the current window from the reload logic as we really only need to reload sidebars so they end up on the lock screen vs staying on the login screen and we are just going to close the open extension running in the tab. (3) Added comments * PM-2403 - Per PR feedback, update reloadOpenWindows exemptCurrentHref filter to properly work * PM-2403 - Per PR feedback, remove unneeded new method and replace with existing method closeBitwardenExtensionTab after testing on all browsers (apparently the chrome.tabs namespace is supported in all major browsers that we support now) * PM-2403 - Refactor sync to be kicked off earlier in the process b/c to make sure that closing the window does not abort it --- .../src/auth/popup/two-factor.component.ts | 18 ++++++++++++++---- apps/browser/src/browser/browserApi.ts | 4 +++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/apps/browser/src/auth/popup/two-factor.component.ts b/apps/browser/src/auth/popup/two-factor.component.ts index 678675ba38..99a850d3c0 100644 --- a/apps/browser/src/auth/popup/two-factor.component.ts +++ b/apps/browser/src/auth/popup/two-factor.component.ts @@ -118,10 +118,20 @@ export class TwoFactorComponent extends BaseTwoFactorComponent { this.route.queryParams.pipe(first()).subscribe(async (qParams) => { if (qParams.sso === "true") { super.onSuccessfulLogin = () => { - BrowserApi.reloadOpenWindows(); - const thisWindow = window.open("", "_self"); - thisWindow.close(); - return this.syncService.fullSync(true); + // This is not awaited so we don't pause the application while the sync is happening. + // This call is executed by the service that lives in the background script so it will continue + // the sync even if this tab closes. + const syncPromise = this.syncService.fullSync(true); + + // Force sidebars (FF && Opera) to reload while exempting current window + // because we are just going to close the current window. + BrowserApi.reloadOpenWindows(true); + + // We don't need this window anymore because the intent is for the user to be left + // on the web vault screen which tells them to continue in the browser extension (sidebar or popup) + BrowserApi.closeBitwardenExtensionTab(); + + return syncPromise; }; } }); diff --git a/apps/browser/src/browser/browserApi.ts b/apps/browser/src/browser/browserApi.ts index b1313ebda6..b69cf388de 100644 --- a/apps/browser/src/browser/browserApi.ts +++ b/apps/browser/src/browser/browserApi.ts @@ -227,10 +227,12 @@ export class BrowserApi { } } - static reloadOpenWindows() { + static reloadOpenWindows(exemptCurrentHref = false) { + const currentHref = window.location.href; const views = chrome.extension.getViews() as Window[]; views .filter((w) => w.location.href != null && !w.location.href.includes("background.html")) + .filter((w) => !exemptCurrentHref || w.location.href !== currentHref) .forEach((w) => { w.location.reload(); });