Skip to content

Commit b5896b7

Browse files
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 <[email protected]> --------- Co-authored-by: Cesar Gonzalez <[email protected]> (cherry picked from commit dff44b0)
1 parent e8ed4f3 commit b5896b7

File tree

4 files changed

+13
-9
lines changed

4 files changed

+13
-9
lines changed

apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ describe("AccountSwitcherService", () => {
153153

154154
await selectAccountPromise;
155155

156-
expect(accountService.switchAccount).toBeCalledWith(null);
156+
expect(messagingService.send).toHaveBeenCalledWith("switchAccount", { userId: null });
157157

158158
expect(removeListenerSpy).toBeCalledTimes(1);
159159
});
@@ -176,7 +176,7 @@ describe("AccountSwitcherService", () => {
176176

177177
await selectAccountPromise;
178178

179-
expect(accountService.switchAccount).toBeCalledWith("1");
179+
expect(messagingService.send).toHaveBeenCalledWith("switchAccount", { userId: "1" });
180180
expect(messagingService.send).toBeCalledWith(
181181
"switchAccount",
182182
matches((payload) => {

apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts

-1
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ export class AccountSwitcherService {
134134
const switchAccountFinishedPromise = this.listenForSwitchAccountFinish(userId);
135135

136136
// Initiate the actions required to make account switching happen
137-
await this.accountService.switchAccount(userId);
138137
this.messagingService.send("switchAccount", { userId }); // This message should cause switchAccountFinish to be sent
139138

140139
// Wait until we receive the switchAccountFinished message

apps/browser/src/autofill/background/overlay.background.spec.ts

+8-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { mock, mockReset } from "jest-mock-extended";
1+
import { mock, MockProxy, mockReset } from "jest-mock-extended";
22
import { BehaviorSubject, of } from "rxjs";
33

44
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
@@ -62,7 +62,8 @@ describe("OverlayBackground", () => {
6262
let overlayBackground: OverlayBackground;
6363
const cipherService = mock<CipherService>();
6464
const autofillService = mock<AutofillService>();
65-
const authService = mock<AuthService>();
65+
let activeAccountStatusMock$: BehaviorSubject<AuthenticationStatus>;
66+
let authService: MockProxy<AuthService>;
6667

6768
const environmentService = mock<EnvironmentService>();
6869
environmentService.environment$ = new BehaviorSubject(
@@ -94,6 +95,9 @@ describe("OverlayBackground", () => {
9495

9596
beforeEach(() => {
9697
domainSettingsService = new DefaultDomainSettingsService(fakeStateProvider);
98+
activeAccountStatusMock$ = new BehaviorSubject(AuthenticationStatus.Unlocked);
99+
authService = mock<AuthService>();
100+
authService.activeAccountStatus$ = activeAccountStatusMock$;
97101
overlayBackground = new OverlayBackground(
98102
cipherService,
99103
autofillService,
@@ -166,11 +170,11 @@ describe("OverlayBackground", () => {
166170
});
167171

168172
beforeEach(() => {
169-
overlayBackground["userAuthStatus"] = AuthenticationStatus.Unlocked;
173+
activeAccountStatusMock$.next(AuthenticationStatus.Unlocked);
170174
});
171175

172176
it("ignores updating the overlay ciphers if the user's auth status is not unlocked", async () => {
173-
overlayBackground["userAuthStatus"] = AuthenticationStatus.Locked;
177+
activeAccountStatusMock$.next(AuthenticationStatus.Locked);
174178
jest.spyOn(BrowserApi, "getTabFromCurrentWindowId");
175179
jest.spyOn(cipherService, "getAllDecryptedForUrl");
176180

apps/browser/src/autofill/background/overlay.background.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ class OverlayBackground implements OverlayBackgroundInterface {
136136
* list of ciphers if the extension is not unlocked.
137137
*/
138138
async updateOverlayCiphers() {
139-
if (this.userAuthStatus !== AuthenticationStatus.Unlocked) {
139+
const authStatus = await firstValueFrom(this.authService.activeAccountStatus$);
140+
if (authStatus !== AuthenticationStatus.Unlocked) {
140141
return;
141142
}
142143

@@ -167,7 +168,7 @@ class OverlayBackground implements OverlayBackgroundInterface {
167168
private async getOverlayCipherData(): Promise<OverlayCipherData[]> {
168169
const showFavicons = await firstValueFrom(this.domainSettingsService.showFavicons$);
169170
const overlayCiphersArray = Array.from(this.overlayLoginCiphers);
170-
const overlayCipherData = [];
171+
const overlayCipherData: OverlayCipherData[] = [];
171172
let loginCipherIcon: WebsiteIconData;
172173

173174
for (let cipherIndex = 0; cipherIndex < overlayCiphersArray.length; cipherIndex++) {

0 commit comments

Comments
 (0)