diff --git a/.github/workflows/version-auto-bump.yml b/.github/workflows/version-auto-bump.yml index d0d028e570f..f10abee300d 100644 --- a/.github/workflows/version-auto-bump.yml +++ b/.github/workflows/version-auto-bump.yml @@ -27,9 +27,9 @@ jobs: env: GH_TOKEN: ${{ steps.retrieve-bot-secrets.outputs.github-pat-bitwarden-devops-bot-repo-scope }} run: | - echo '{"cut_rc_branch": "false", \ - "bump_browser": "false", \ - "bump_cli": "false", \ - "bump_desktop": "true", \ + echo '{"cut_rc_branch": "false", + "bump_browser": "false", + "bump_cli": "false", + "bump_desktop": "true", "bump_web": "false"}' | \ gh workflow run version-bump.yml --json --repo bitwarden/clients diff --git a/apps/browser/src/autofill/popup/settings/autofill.component.ts b/apps/browser/src/autofill/popup/settings/autofill.component.ts index 67cc25f2276..1c6583331f4 100644 --- a/apps/browser/src/autofill/popup/settings/autofill.component.ts +++ b/apps/browser/src/autofill/popup/settings/autofill.component.ts @@ -105,11 +105,7 @@ export class AutofillComponent implements OnInit { } async updateAutoFillOverlayVisibility() { - const previousAutoFillOverlayVisibility = await firstValueFrom( - this.autofillSettingsService.inlineMenuVisibility$, - ); await this.autofillSettingsService.setInlineMenuVisibility(this.autoFillOverlayVisibility); - await this.handleUpdatingAutofillOverlayContentScripts(previousAutoFillOverlayVisibility); await this.requestPrivacyPermission(); } @@ -181,27 +177,6 @@ export class AutofillComponent implements OnInit { BrowserApi.createNewTab(this.disablePasswordManagerLink); } - private async handleUpdatingAutofillOverlayContentScripts( - previousAutoFillOverlayVisibility: number, - ) { - const autofillOverlayPreviouslyDisabled = - previousAutoFillOverlayVisibility === AutofillOverlayVisibility.Off; - const autofillOverlayCurrentlyDisabled = - this.autoFillOverlayVisibility === AutofillOverlayVisibility.Off; - - if (!autofillOverlayPreviouslyDisabled && !autofillOverlayCurrentlyDisabled) { - const tabs = await BrowserApi.tabsQuery({}); - tabs.forEach((tab) => - BrowserApi.tabSendMessageData(tab, "updateAutofillOverlayVisibility", { - autofillOverlayVisibility: this.autoFillOverlayVisibility, - }), - ); - return; - } - - await this.autofillService.reloadAutofillScripts(); - } - async requestPrivacyPermission() { if ( this.autoFillOverlayVisibility === AutofillOverlayVisibility.Off || diff --git a/apps/browser/src/autofill/services/autofill.service.spec.ts b/apps/browser/src/autofill/services/autofill.service.spec.ts index 158fde6a567..24b1c90b3fa 100644 --- a/apps/browser/src/autofill/services/autofill.service.spec.ts +++ b/apps/browser/src/autofill/services/autofill.service.spec.ts @@ -1,5 +1,5 @@ -import { mock, mockReset } from "jest-mock-extended"; -import { of } from "rxjs"; +import { mock, mockReset, MockProxy } from "jest-mock-extended"; +import { BehaviorSubject, of } from "rxjs"; import { UserVerificationService } from "@bitwarden/common/auth/services/user-verification/user-verification.service"; import { AutofillOverlayVisibility } from "@bitwarden/common/autofill/constants"; @@ -8,6 +8,7 @@ import { DefaultDomainSettingsService, DomainSettingsService, } from "@bitwarden/common/autofill/services/domain-settings.service"; +import { InlineMenuVisibilitySetting } from "@bitwarden/common/autofill/types"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EventType } from "@bitwarden/common/enums"; import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service"; @@ -45,7 +46,7 @@ import { createChromeTabMock, createGenerateFillScriptOptionsMock, } from "../spec/autofill-mocks"; -import { triggerTestFailure } from "../spec/testing-utils"; +import { flushPromises, triggerTestFailure } from "../spec/testing-utils"; import { AutoFillOptions, @@ -64,7 +65,8 @@ const mockEquivalentDomains = [ describe("AutofillService", () => { let autofillService: AutofillService; const cipherService = mock(); - const autofillSettingsService = mock(); + let inlineMenuVisibilityMock$!: BehaviorSubject; + let autofillSettingsService: MockProxy; const mockUserId = Utils.newGuid() as UserId; const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); const fakeStateProvider: FakeStateProvider = new FakeStateProvider(accountService); @@ -79,6 +81,9 @@ describe("AutofillService", () => { beforeEach(() => { scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService); + inlineMenuVisibilityMock$ = new BehaviorSubject(AutofillOverlayVisibility.OnFieldFocus); + autofillSettingsService = mock(); + (autofillSettingsService as any).inlineMenuVisibility$ = inlineMenuVisibilityMock$; autofillService = new AutofillService( cipherService, autofillSettingsService, @@ -142,17 +147,92 @@ describe("AutofillService", () => { // eslint-disable-next-line no-restricted-syntax expect(chrome.runtime.onConnect.addListener).toHaveBeenCalledWith(expect.any(Function)); }); + + describe("handle inline menu visibility change", () => { + beforeEach(async () => { + await autofillService.loadAutofillScriptsOnInstall(); + jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([tab1, tab2]); + jest.spyOn(BrowserApi, "tabSendMessageData").mockImplementation(); + jest.spyOn(autofillService, "reloadAutofillScripts").mockImplementation(); + }); + + it("returns early if the setting is being initialized", async () => { + await flushPromises(); + + expect(BrowserApi.tabsQuery).toHaveBeenCalledTimes(1); + expect(BrowserApi.tabSendMessageData).not.toHaveBeenCalled(); + }); + + it("returns early if the previous setting is equivalent to the new setting", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + await flushPromises(); + + expect(BrowserApi.tabsQuery).toHaveBeenCalledTimes(1); + expect(BrowserApi.tabSendMessageData).not.toHaveBeenCalled(); + }); + + describe("updates the inline menu visibility setting", () => { + it("when changing the inline menu from on focus of field to on button click", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick); + await flushPromises(); + + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab1, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnButtonClick }, + ); + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab2, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnButtonClick }, + ); + }); + + it("when changing the inline menu from button click to field focus", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnButtonClick); + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + await flushPromises(); + + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab1, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnFieldFocus }, + ); + expect(BrowserApi.tabSendMessageData).toHaveBeenCalledWith( + tab2, + "updateAutofillOverlayVisibility", + { autofillOverlayVisibility: AutofillOverlayVisibility.OnFieldFocus }, + ); + }); + }); + + describe("reloads the autofill scripts", () => { + it("when changing the inline menu from a disabled setting to an enabled setting", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.Off); + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + await flushPromises(); + + expect(autofillService.reloadAutofillScripts).toHaveBeenCalled(); + }); + + it("when changing the inline menu from a enabled setting to a disabled setting", async () => { + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.OnFieldFocus); + inlineMenuVisibilityMock$.next(AutofillOverlayVisibility.Off); + await flushPromises(); + + expect(autofillService.reloadAutofillScripts).toHaveBeenCalled(); + }); + }); + }); }); describe("reloadAutofillScripts", () => { - it("disconnects and removes all autofill script ports", () => { - const port1 = mock({ - disconnect: jest.fn(), - }); - const port2 = mock({ - disconnect: jest.fn(), - }); + it("re-injects the autofill scripts in all tabs and disconnects all connected ports", () => { + const port1 = mock(); + const port2 = mock(); autofillService["autofillScriptPortsSet"] = new Set([port1, port2]); + jest.spyOn(autofillService as any, "injectAutofillScriptsInAllTabs"); + jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true); // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises @@ -161,17 +241,6 @@ describe("AutofillService", () => { expect(port1.disconnect).toHaveBeenCalled(); expect(port2.disconnect).toHaveBeenCalled(); expect(autofillService["autofillScriptPortsSet"].size).toBe(0); - }); - - it("re-injects the autofill scripts in all tabs", () => { - autofillService["autofillScriptPortsSet"] = new Set([mock()]); - jest.spyOn(autofillService as any, "injectAutofillScriptsInAllTabs"); - jest.spyOn(autofillService, "getAutofillOnPageLoad").mockResolvedValue(true); - - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - autofillService.reloadAutofillScripts(); - expect(autofillService["injectAutofillScriptsInAllTabs"]).toHaveBeenCalled(); }); }); diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index 80829ee7141..5348ca5b9ab 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -1,4 +1,5 @@ -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, startWith } from "rxjs"; +import { pairwise } from "rxjs/operators"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -70,10 +71,12 @@ export default class AutofillService implements AutofillServiceInterface { */ async loadAutofillScriptsOnInstall() { BrowserApi.addListener(chrome.runtime.onConnect, this.handleInjectedScriptPortConnection); - - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.injectAutofillScriptsInAllTabs(); + void this.injectAutofillScriptsInAllTabs(); + this.autofillSettingsService.inlineMenuVisibility$ + .pipe(startWith(undefined), pairwise()) + .subscribe(([previousSetting, currentSetting]) => + this.handleInlineMenuVisibilityChange(previousSetting, currentSetting), + ); } /** @@ -2090,4 +2093,34 @@ export default class AutofillService implements AutofillServiceInterface { } } } + + /** + * Updates the autofill inline menu visibility setting in all active tabs + * when the InlineMenuVisibilitySetting observable is updated. + * + * @param previousSetting - The previous setting value + * @param currentSetting - The current setting value + */ + private async handleInlineMenuVisibilityChange( + previousSetting: InlineMenuVisibilitySetting, + currentSetting: InlineMenuVisibilitySetting, + ) { + if (previousSetting === undefined || previousSetting === currentSetting) { + return; + } + + const inlineMenuPreviouslyDisabled = previousSetting === AutofillOverlayVisibility.Off; + const inlineMenuCurrentlyDisabled = currentSetting === AutofillOverlayVisibility.Off; + if (!inlineMenuPreviouslyDisabled && !inlineMenuCurrentlyDisabled) { + const tabs = await BrowserApi.tabsQuery({}); + tabs.forEach((tab) => + BrowserApi.tabSendMessageData(tab, "updateAutofillOverlayVisibility", { + autofillOverlayVisibility: currentSetting, + }), + ); + return; + } + + await this.reloadAutofillScripts(); + } } diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.ts index 7c49a3d9881..909d8584304 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.ts @@ -1349,6 +1349,10 @@ class CollectAutofillContentService implements CollectAutofillContentServiceInte } const cachedAutofillFieldElement = this.autofillFieldElements.get(formFieldElement); + if (!cachedAutofillFieldElement) { + continue; + } + cachedAutofillFieldElement.viewable = true; void this.autofillOverlayContentService?.setupAutofillOverlayListenerOnField( diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index 5473664e3b0..388060d88c9 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -188,6 +188,7 @@ export default class RuntimeBackground { if (msg.command === "loggedIn") { await this.sendBwInstalledMessageToVault(); + await this.autofillService.reloadAutofillScripts(); } if (this.lockedVaultPendingNotifications?.length > 0) { diff --git a/apps/web/src/app/admin-console/organizations/policies/policies.component.ts b/apps/web/src/app/admin-console/organizations/policies/policies.component.ts index 6f8bb27671f..782db231c69 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policies.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policies.component.ts @@ -1,18 +1,19 @@ import { Component, OnInit, ViewChild, ViewContainerRef } from "@angular/core"; -import { ActivatedRoute, Router } from "@angular/router"; +import { ActivatedRoute } from "@angular/router"; +import { lastValueFrom } from "rxjs"; import { first } from "rxjs/operators"; -import { ModalService } from "@bitwarden/angular/services/modal.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { PolicyResponse } from "@bitwarden/common/admin-console/models/response/policy.response"; +import { DialogService } from "@bitwarden/components"; import { PolicyListService } from "../../core/policy-list.service"; import { BasePolicy } from "../policies"; -import { PolicyEditComponent } from "./policy-edit.component"; +import { PolicyEditComponent, PolicyEditDialogResult } from "./policy-edit.component"; @Component({ selector: "app-org-policies", @@ -33,11 +34,10 @@ export class PoliciesComponent implements OnInit { constructor( private route: ActivatedRoute, - private modalService: ModalService, private organizationService: OrganizationService, private policyApiService: PolicyApiServiceAbstraction, private policyListService: PolicyListService, - private router: Router, + private dialogService: DialogService, ) {} async ngOnInit() { @@ -83,21 +83,17 @@ export class PoliciesComponent implements OnInit { } async edit(policy: BasePolicy) { - const [modal] = await this.modalService.openViewRef( - PolicyEditComponent, - this.editModalRef, - (comp) => { - comp.policy = policy; - comp.organizationId = this.organizationId; - comp.policiesEnabledMap = this.policiesEnabledMap; - // eslint-disable-next-line rxjs-angular/prefer-takeuntil - comp.onSavedPolicy.subscribe(() => { - modal.close(); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.load(); - }); + const dialogRef = PolicyEditComponent.open(this.dialogService, { + data: { + policy: policy, + organizationId: this.organizationId, + policiesEnabledMap: this.policiesEnabledMap, }, - ); + }); + + const result = await lastValueFrom(dialogRef.closed); + if (result === PolicyEditDialogResult.Saved) { + await this.load(); + } } } diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit.component.html b/apps/web/src/app/admin-console/organizations/policies/policy-edit.component.html index 5f5dc1d85ef..063a8759e6f 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit.component.html +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit.component.html @@ -1,49 +1,28 @@ -