From e14e2627ad7409b5a538bf928a9abdf5858957fe Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Tue, 28 May 2024 14:05:24 -0500 Subject: [PATCH] [PM-8447] Autofill On Load Causes Extension to Hang After Browser Startup (#9400) --- .../services/autofill.service.spec.ts | 20 +++++++++++++++++++ .../src/autofill/services/autofill.service.ts | 7 ++++++- .../browser/src/background/main.background.ts | 1 + .../src/popup/services/services.module.ts | 1 + 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/apps/browser/src/autofill/services/autofill.service.spec.ts b/apps/browser/src/autofill/services/autofill.service.spec.ts index 24b1c90b3f..23f690544d 100644 --- a/apps/browser/src/autofill/services/autofill.service.spec.ts +++ b/apps/browser/src/autofill/services/autofill.service.spec.ts @@ -1,6 +1,8 @@ import { mock, mockReset, MockProxy } from "jest-mock-extended"; import { BehaviorSubject, of } from "rxjs"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { UserVerificationService } from "@bitwarden/common/auth/services/user-verification/user-verification.service"; import { AutofillOverlayVisibility } from "@bitwarden/common/autofill/constants"; import { AutofillSettingsService } from "@bitwarden/common/autofill/services/autofill-settings.service"; @@ -78,12 +80,17 @@ describe("AutofillService", () => { const userVerificationService = mock(); const billingAccountProfileStateService = mock(); const platformUtilsService = mock(); + let activeAccountStatusMock$: BehaviorSubject; + let authService: MockProxy; beforeEach(() => { scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService); inlineMenuVisibilityMock$ = new BehaviorSubject(AutofillOverlayVisibility.OnFieldFocus); autofillSettingsService = mock(); (autofillSettingsService as any).inlineMenuVisibility$ = inlineMenuVisibilityMock$; + activeAccountStatusMock$ = new BehaviorSubject(AuthenticationStatus.Unlocked); + authService = mock(); + authService.activeAccountStatus$ = activeAccountStatusMock$; autofillService = new AutofillService( cipherService, autofillSettingsService, @@ -95,6 +102,7 @@ describe("AutofillService", () => { billingAccountProfileStateService, scriptInjectorService, accountService, + authService, ); domainSettingsService = new DefaultDomainSettingsService(fakeStateProvider); @@ -287,6 +295,18 @@ describe("AutofillService", () => { }); }); + it("skips injecting the autofiller script when the user's account is not unlocked", async () => { + activeAccountStatusMock$.next(AuthenticationStatus.Locked); + + await autofillService.injectAutofillScripts(sender.tab, sender.frameId, true); + + expect(BrowserApi.executeScriptInTab).not.toHaveBeenCalledWith(tabMock.id, { + file: "content/autofiller.js", + frameId: sender.frameId, + ...defaultExecuteScriptOptions, + }); + }); + it("will inject the bootstrap-autofill-overlay script if the user has the autofill overlay enabled", async () => { await autofillService.injectAutofillScripts(sender.tab, sender.frameId); diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index 5348ca5b9a..9ec2052381 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -3,7 +3,9 @@ import { pairwise } from "rxjs/operators"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AutofillOverlayVisibility } from "@bitwarden/common/autofill/constants"; import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service"; import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service"; @@ -61,6 +63,7 @@ export default class AutofillService implements AutofillServiceInterface { private billingAccountProfileStateService: BillingAccountProfileStateService, private scriptInjectorService: ScriptInjectorService, private accountService: AccountService, + private authService: AuthService, ) {} /** @@ -113,6 +116,8 @@ export default class AutofillService implements AutofillServiceInterface { // Autofill user settings loaded from state can await the active account state indefinitely // if not guarded by an active account check (e.g. the user is logged in) const activeAccount = await firstValueFrom(this.accountService.activeAccount$); + const authStatus = await firstValueFrom(this.authService.activeAccountStatus$); + const accountIsUnlocked = authStatus === AuthenticationStatus.Unlocked; let overlayVisibility: InlineMenuVisibilitySetting = AutofillOverlayVisibility.Off; let autoFillOnPageLoadIsEnabled = false; @@ -126,7 +131,7 @@ export default class AutofillService implements AutofillServiceInterface { const injectedScripts = [mainAutofillScript]; - if (activeAccount) { + if (activeAccount && accountIsUnlocked) { autoFillOnPageLoadIsEnabled = await this.getAutofillOnPageLoad(); } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index a382a76781..2072f8396e 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -872,6 +872,7 @@ export default class MainBackground { this.billingAccountProfileStateService, this.scriptInjectorService, this.accountService, + this.authService, ); this.auditService = new AuditService(this.cryptoFunctionService, this.apiService); diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index d260b4ec7b..342b3d26a6 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -341,6 +341,7 @@ const safeProviders: SafeProvider[] = [ BillingAccountProfileStateService, ScriptInjectorService, AccountServiceAbstraction, + AuthService, ], }), safeProvider({