From d91fdad0118d66a3c54ea0839f98b59a04b09f88 Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Thu, 23 Oct 2025 11:54:20 -0400 Subject: [PATCH 1/2] [PM-24650] Resolve sign in button disappearing from ADP login form (#16901) * ensure autofillInsertActions execution order is preserved * don't fill a field if it already has the value that is going to be filled * update tests --- .../insert-autofill-content.service.spec.ts | 54 ++++++++++++++----- .../insert-autofill-content.service.ts | 9 +++- 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts b/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts index 9edcdbb3a95..07fdfb9db79 100644 --- a/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts +++ b/apps/browser/src/autofill/services/insert-autofill-content.service.spec.ts @@ -103,7 +103,7 @@ describe("InsertAutofillContentService", () => { delay_between_operations: 20, }, metadata: {}, - autosubmit: null, + autosubmit: [], savedUrls: ["https://bitwarden.com"], untrustedIframe: false, itemType: "login", @@ -218,28 +218,21 @@ describe("InsertAutofillContentService", () => { await insertAutofillContentService.fillForm(fillScript); - expect(insertAutofillContentService["userCancelledInsecureUrlAutofill"]).toHaveBeenCalled(); - expect( - insertAutofillContentService["userCancelledUntrustedIframeAutofill"], - ).toHaveBeenCalled(); expect(insertAutofillContentService["runFillScriptAction"]).toHaveBeenCalledTimes(3); expect(insertAutofillContentService["runFillScriptAction"]).toHaveBeenNthCalledWith( 1, fillScript.script[0], 0, - fillScript.script, ); expect(insertAutofillContentService["runFillScriptAction"]).toHaveBeenNthCalledWith( 2, fillScript.script[1], 1, - fillScript.script, ); expect(insertAutofillContentService["runFillScriptAction"]).toHaveBeenNthCalledWith( 3, fillScript.script[2], 2, - fillScript.script, ); }); }); @@ -623,14 +616,12 @@ describe("InsertAutofillContentService", () => { }); }); - it("will set the `value` attribute of any passed input or textarea elements", () => { - document.body.innerHTML = ``; + it("will set the `value` attribute of any passed input or textarea elements if the value differs", () => { + document.body.innerHTML = ``; const value1 = "test"; const value2 = "test2"; const textInputElement = document.getElementById("username") as HTMLInputElement; - textInputElement.value = value1; const textareaElement = document.getElementById("bio") as HTMLTextAreaElement; - textareaElement.value = value2; jest.spyOn(insertAutofillContentService as any, "handleInsertValueAndTriggerSimulatedEvents"); insertAutofillContentService["insertValueIntoField"](textInputElement, value1); @@ -647,6 +638,45 @@ describe("InsertAutofillContentService", () => { insertAutofillContentService["handleInsertValueAndTriggerSimulatedEvents"], ).toHaveBeenCalledWith(textareaElement, expect.any(Function)); }); + + it("will NOT set the `value` attribute of any passed input or textarea elements if they already have values matching the passed value", () => { + document.body.innerHTML = ``; + const value1 = "test"; + const value2 = "test2"; + const textInputElement = document.getElementById("username") as HTMLInputElement; + textInputElement.value = value1; + const textareaElement = document.getElementById("bio") as HTMLTextAreaElement; + textareaElement.value = value2; + jest.spyOn(insertAutofillContentService as any, "handleInsertValueAndTriggerSimulatedEvents"); + + insertAutofillContentService["insertValueIntoField"](textInputElement, value1); + + expect(textInputElement.value).toBe(value1); + expect( + insertAutofillContentService["handleInsertValueAndTriggerSimulatedEvents"], + ).not.toHaveBeenCalled(); + + insertAutofillContentService["insertValueIntoField"](textareaElement, value2); + + expect(textareaElement.value).toBe(value2); + expect( + insertAutofillContentService["handleInsertValueAndTriggerSimulatedEvents"], + ).not.toHaveBeenCalled(); + }); + + it("skips filling when the field already has the target value", () => { + const value = "test"; + document.body.innerHTML = ``; + const element = document.getElementById("username") as FillableFormFieldElement; + jest.spyOn(insertAutofillContentService as any, "handleInsertValueAndTriggerSimulatedEvents"); + + insertAutofillContentService["insertValueIntoField"](element, value); + + expect( + insertAutofillContentService["handleInsertValueAndTriggerSimulatedEvents"], + ).not.toHaveBeenCalled(); + expect(element.value).toBe(value); + }); }); describe("handleInsertValueAndTriggerSimulatedEvents", () => { diff --git a/apps/browser/src/autofill/services/insert-autofill-content.service.ts b/apps/browser/src/autofill/services/insert-autofill-content.service.ts index 6034563a947..9ddbcdc005d 100644 --- a/apps/browser/src/autofill/services/insert-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/insert-autofill-content.service.ts @@ -49,8 +49,9 @@ class InsertAutofillContentService implements InsertAutofillContentServiceInterf return; } - const fillActionPromises = fillScript.script.map(this.runFillScriptAction); - await Promise.all(fillActionPromises); + for (let index = 0; index < fillScript.script.length; index++) { + await this.runFillScriptAction(fillScript.script[index], index); + } } /** @@ -189,10 +190,14 @@ class InsertAutofillContentService implements InsertAutofillContentServiceInterf const elementCanBeReadonly = elementIsInputElement(element) || elementIsTextAreaElement(element); const elementCanBeFilled = elementCanBeReadonly || elementIsSelectElement(element); + const elementValue = (element as HTMLInputElement)?.value || element?.innerText || ""; + + const elementAlreadyHasTheValue = !!(elementValue?.length && elementValue === value); if ( !element || !value || + elementAlreadyHasTheValue || (elementCanBeReadonly && element.readOnly) || (elementCanBeFilled && element.disabled) ) { From 660e452ba1de0c0c6dcea5665b859b690c79bd7e Mon Sep 17 00:00:00 2001 From: cyprain-okeke <108260115+cyprain-okeke@users.noreply.github.com> Date: Thu, 23 Oct 2025 16:59:57 +0100 Subject: [PATCH 2/2] [PM-25858]Organization warnings endpoint should not be called from self-hosted instances (#16781) * ensure that getWarnings from server is not called for selfhost * Refactor the code * move the selfhost check to getWarning message * Fix the failing test --- .../organization-warnings.service.spec.ts | 58 +++++++++++++++++++ .../services/organization-warnings.service.ts | 12 +++- 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/apps/web/src/app/billing/organizations/warnings/services/organization-warnings.service.spec.ts b/apps/web/src/app/billing/organizations/warnings/services/organization-warnings.service.spec.ts index 8c2a7634264..9466e813e4d 100644 --- a/apps/web/src/app/billing/organizations/warnings/services/organization-warnings.service.spec.ts +++ b/apps/web/src/app/billing/organizations/warnings/services/organization-warnings.service.spec.ts @@ -16,6 +16,7 @@ import { Organization } from "@bitwarden/common/admin-console/models/domain/orga import { ProductTierType } from "@bitwarden/common/billing/enums"; import { OrganizationSubscriptionResponse } from "@bitwarden/common/billing/models/response/organization-subscription.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { DialogRef, DialogService } from "@bitwarden/components"; import { OrganizationBillingClient } from "@bitwarden/web-vault/app/billing/clients"; import { @@ -37,6 +38,7 @@ describe("OrganizationWarningsService", () => { let i18nService: MockProxy; let organizationApiService: MockProxy; let organizationBillingClient: MockProxy; + let platformUtilsService: MockProxy; let router: MockProxy; const organization = { @@ -58,10 +60,13 @@ describe("OrganizationWarningsService", () => { i18nService = mock(); organizationApiService = mock(); organizationBillingClient = mock(); + platformUtilsService = mock(); router = mock(); (openChangePlanDialog as jest.Mock).mockReset(); + platformUtilsService.isSelfHost.mockReturnValue(false); + i18nService.t.mockImplementation((key: string, ...args: any[]) => { switch (key) { case "freeTrialEndPromptCount": @@ -94,6 +99,7 @@ describe("OrganizationWarningsService", () => { { provide: I18nService, useValue: i18nService }, { provide: OrganizationApiServiceAbstraction, useValue: organizationApiService }, { provide: OrganizationBillingClient, useValue: organizationBillingClient }, + { provide: PlatformUtilsService, useValue: platformUtilsService }, { provide: Router, useValue: router }, ], }); @@ -111,6 +117,16 @@ describe("OrganizationWarningsService", () => { }); }); + it("should return null when platform is self-hosted", (done) => { + platformUtilsService.isSelfHost.mockReturnValue(true); + + service.getFreeTrialWarning$(organization).subscribe((result) => { + expect(result).toBeNull(); + expect(organizationBillingClient.getWarnings).not.toHaveBeenCalled(); + done(); + }); + }); + it("should return warning with count message when remaining trial days >= 2", (done) => { const warning = { remainingTrialDays: 5 }; organizationBillingClient.getWarnings.mockResolvedValue({ @@ -206,6 +222,16 @@ describe("OrganizationWarningsService", () => { }); }); + it("should return null when platform is self-hosted", (done) => { + platformUtilsService.isSelfHost.mockReturnValue(true); + + service.getResellerRenewalWarning$(organization).subscribe((result) => { + expect(result).toBeNull(); + expect(organizationBillingClient.getWarnings).not.toHaveBeenCalled(); + done(); + }); + }); + it("should return upcoming warning with correct type and message", (done) => { const renewalDate = new Date(2024, 11, 31); const warning = { @@ -298,6 +324,16 @@ describe("OrganizationWarningsService", () => { }); }); + it("should return null when platform is self-hosted", (done) => { + platformUtilsService.isSelfHost.mockReturnValue(true); + + service.getTaxIdWarning$(organization).subscribe((result) => { + expect(result).toBeNull(); + expect(organizationBillingClient.getWarnings).not.toHaveBeenCalled(); + done(); + }); + }); + it("should return tax_id_missing type when tax ID is missing", (done) => { const warning = { type: TaxIdWarningTypes.Missing }; organizationBillingClient.getWarnings.mockResolvedValue({ @@ -427,6 +463,16 @@ describe("OrganizationWarningsService", () => { }); }); + it("should not show dialog when platform is self-hosted", (done) => { + platformUtilsService.isSelfHost.mockReturnValue(true); + + service.showInactiveSubscriptionDialog$(organization).subscribe(() => { + expect(dialogService.openSimpleDialog).not.toHaveBeenCalled(); + expect(organizationBillingClient.getWarnings).not.toHaveBeenCalled(); + done(); + }); + }); + it("should show contact provider dialog for contact_provider resolution", (done) => { const warning = { resolution: "contact_provider" }; organizationBillingClient.getWarnings.mockResolvedValue({ @@ -570,6 +616,18 @@ describe("OrganizationWarningsService", () => { }); }); + it("should not show dialog when platform is self-hosted", (done) => { + platformUtilsService.isSelfHost.mockReturnValue(true); + + service.showSubscribeBeforeFreeTrialEndsDialog$(organization).subscribe({ + complete: () => { + expect(organizationApiService.getSubscription).not.toHaveBeenCalled(); + expect(organizationBillingClient.getWarnings).not.toHaveBeenCalled(); + done(); + }, + }); + }); + it("should open trial payment dialog when free trial warning exists", (done) => { const warning = { remainingTrialDays: 2 }; const subscription = { id: "sub-123" } as OrganizationSubscriptionResponse; diff --git a/apps/web/src/app/billing/organizations/warnings/services/organization-warnings.service.ts b/apps/web/src/app/billing/organizations/warnings/services/organization-warnings.service.ts index 8bec7acffe1..a34533bcada 100644 --- a/apps/web/src/app/billing/organizations/warnings/services/organization-warnings.service.ts +++ b/apps/web/src/app/billing/organizations/warnings/services/organization-warnings.service.ts @@ -8,6 +8,7 @@ import { map, merge, Observable, + of, Subject, switchMap, tap, @@ -17,6 +18,7 @@ import { take } from "rxjs/operators"; import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { OrganizationId } from "@bitwarden/common/types/guid"; import { DialogService } from "@bitwarden/components"; import { OrganizationBillingClient } from "@bitwarden/web-vault/app/billing/clients"; @@ -56,6 +58,7 @@ export class OrganizationWarningsService { private i18nService: I18nService, private organizationApiService: OrganizationApiServiceAbstraction, private organizationBillingClient: OrganizationBillingClient, + private platformUtilsService: PlatformUtilsService, private router: Router, ) {} @@ -281,12 +284,17 @@ export class OrganizationWarningsService { organization: Organization, extract: (response: OrganizationWarningsResponse) => T | null | undefined, bypassCache: boolean = false, - ): Observable => - this.readThroughWarnings$(organization, bypassCache).pipe( + ): Observable => { + if (this.platformUtilsService.isSelfHost()) { + return of(null); + } + + return this.readThroughWarnings$(organization, bypassCache).pipe( map((response) => { const value = extract(response); return value ? value : null; }), take(1), ); + }; }