Browse Source

Retrieve auth status when updating overlay ciphers. (#9282)

* Retrieve auth status when updating overlay ciphers.

We are experiencing a hang due to e8ed4f38f4/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 <cesar.a.gonzalezcs@gmail.com>

---------

Co-authored-by: Cesar Gonzalez <cesar.a.gonzalezcs@gmail.com>
pull/9148/head
Matt Gibson 2 years ago committed by GitHub
parent
commit
dff44b02e2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 4
      apps/browser/src/auth/popup/account-switching/services/account-switcher.service.spec.ts
  2. 1
      apps/browser/src/auth/popup/account-switching/services/account-switcher.service.ts
  3. 12
      apps/browser/src/autofill/background/overlay.background.spec.ts
  4. 5
      apps/browser/src/autofill/background/overlay.background.ts

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

@ -153,7 +153,7 @@ describe("AccountSwitcherService", () => { @@ -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", () => { @@ -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) => {

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

@ -134,7 +134,6 @@ export class AccountSwitcherService { @@ -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

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

@ -1,4 +1,4 @@ @@ -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", () => { @@ -62,7 +62,8 @@ describe("OverlayBackground", () => {
let overlayBackground: OverlayBackground;
const cipherService = mock<CipherService>();
const autofillService = mock<AutofillService>();
const authService = mock<AuthService>();
let activeAccountStatusMock$: BehaviorSubject<AuthenticationStatus>;
let authService: MockProxy<AuthService>;
const environmentService = mock<EnvironmentService>();
environmentService.environment$ = new BehaviorSubject(
@ -94,6 +95,9 @@ describe("OverlayBackground", () => { @@ -94,6 +95,9 @@ describe("OverlayBackground", () => {
beforeEach(() => {
domainSettingsService = new DefaultDomainSettingsService(fakeStateProvider);
activeAccountStatusMock$ = new BehaviorSubject(AuthenticationStatus.Unlocked);
authService = mock<AuthService>();
authService.activeAccountStatus$ = activeAccountStatusMock$;
overlayBackground = new OverlayBackground(
cipherService,
autofillService,
@ -166,11 +170,11 @@ describe("OverlayBackground", () => { @@ -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");

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

@ -136,7 +136,8 @@ class OverlayBackground implements OverlayBackgroundInterface { @@ -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 { @@ -167,7 +168,7 @@ class OverlayBackground implements OverlayBackgroundInterface {
private async getOverlayCipherData(): Promise<OverlayCipherData[]> {
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++) {

Loading…
Cancel
Save