From dff44b02e2aaa2ce229530869276bbd88213a883 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 21 May 2024 10:46:46 -0400 Subject: [PATCH] Retrieve auth status when updating overlay ciphers. (#9282) * Retrieve auth status when updating overlay ciphers. We are experiencing a hang due to https://github.com/bitwarden/clients/blob/e8ed4f38f48aa644df57dfc8a3d4913dcf8b5573/apps/browser/src/background/main.background.ts#L1218 and the fact that the auth status is not updated during account switch for this service. Ideally, the service would just use latest everywhere, but this is sufficient for this bug fix. Account-switcher changes avoid multiple updates for the same event. * Avoid loop * Test fixes Co-authored-by: Cesar Gonzalez --------- Co-authored-by: Cesar Gonzalez --- .../services/account-switcher.service.spec.ts | 4 ++-- .../services/account-switcher.service.ts | 1 - .../autofill/background/overlay.background.spec.ts | 12 ++++++++---- .../src/autofill/background/overlay.background.ts | 5 +++-- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts index be12d249441..6865adca393 100644 --- a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts @@ -153,7 +153,7 @@ describe("AccountSwitcherService", () => { await selectAccountPromise; - expect(accountService.switchAccount).toBeCalledWith(null); + expect(messagingService.send).toHaveBeenCalledWith("switchAccount", { userId: null }); expect(removeListenerSpy).toBeCalledTimes(1); }); @@ -176,7 +176,7 @@ describe("AccountSwitcherService", () => { await selectAccountPromise; - expect(accountService.switchAccount).toBeCalledWith("1"); + expect(messagingService.send).toHaveBeenCalledWith("switchAccount", { userId: "1" }); expect(messagingService.send).toBeCalledWith( "switchAccount", matches((payload) => { diff --git a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts index 2650c2db4e4..d60b0dfaebc 100644 --- a/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts +++ b/apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts @@ -134,7 +134,6 @@ export class AccountSwitcherService { const switchAccountFinishedPromise = this.listenForSwitchAccountFinish(userId); // Initiate the actions required to make account switching happen - await this.accountService.switchAccount(userId); this.messagingService.send("switchAccount", { userId }); // This message should cause switchAccountFinish to be sent // Wait until we receive the switchAccountFinished message diff --git a/apps/browser/src/autofill/background/overlay.background.spec.ts b/apps/browser/src/autofill/background/overlay.background.spec.ts index e65397a62b1..9f4da8f21f3 100644 --- a/apps/browser/src/autofill/background/overlay.background.spec.ts +++ b/apps/browser/src/autofill/background/overlay.background.spec.ts @@ -1,4 +1,4 @@ -import { mock, mockReset } from "jest-mock-extended"; +import { mock, MockProxy, mockReset } from "jest-mock-extended"; import { BehaviorSubject, of } from "rxjs"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -62,7 +62,8 @@ describe("OverlayBackground", () => { let overlayBackground: OverlayBackground; const cipherService = mock(); const autofillService = mock(); - const authService = mock(); + let activeAccountStatusMock$: BehaviorSubject; + let authService: MockProxy; const environmentService = mock(); environmentService.environment$ = new BehaviorSubject( @@ -94,6 +95,9 @@ describe("OverlayBackground", () => { beforeEach(() => { domainSettingsService = new DefaultDomainSettingsService(fakeStateProvider); + activeAccountStatusMock$ = new BehaviorSubject(AuthenticationStatus.Unlocked); + authService = mock(); + authService.activeAccountStatus$ = activeAccountStatusMock$; overlayBackground = new OverlayBackground( cipherService, autofillService, @@ -166,11 +170,11 @@ describe("OverlayBackground", () => { }); beforeEach(() => { - overlayBackground["userAuthStatus"] = AuthenticationStatus.Unlocked; + activeAccountStatusMock$.next(AuthenticationStatus.Unlocked); }); it("ignores updating the overlay ciphers if the user's auth status is not unlocked", async () => { - overlayBackground["userAuthStatus"] = AuthenticationStatus.Locked; + activeAccountStatusMock$.next(AuthenticationStatus.Locked); jest.spyOn(BrowserApi, "getTabFromCurrentWindowId"); jest.spyOn(cipherService, "getAllDecryptedForUrl"); diff --git a/apps/browser/src/autofill/background/overlay.background.ts b/apps/browser/src/autofill/background/overlay.background.ts index 551263525e9..0e4abcd82d0 100644 --- a/apps/browser/src/autofill/background/overlay.background.ts +++ b/apps/browser/src/autofill/background/overlay.background.ts @@ -136,7 +136,8 @@ class OverlayBackground implements OverlayBackgroundInterface { * list of ciphers if the extension is not unlocked. */ async updateOverlayCiphers() { - if (this.userAuthStatus !== AuthenticationStatus.Unlocked) { + const authStatus = await firstValueFrom(this.authService.activeAccountStatus$); + if (authStatus !== AuthenticationStatus.Unlocked) { return; } @@ -167,7 +168,7 @@ class OverlayBackground implements OverlayBackgroundInterface { private async getOverlayCipherData(): Promise { const showFavicons = await firstValueFrom(this.domainSettingsService.showFavicons$); const overlayCiphersArray = Array.from(this.overlayLoginCiphers); - const overlayCipherData = []; + const overlayCipherData: OverlayCipherData[] = []; let loginCipherIcon: WebsiteIconData; for (let cipherIndex = 0; cipherIndex < overlayCiphersArray.length; cipherIndex++) {