From c8dfc70999e50031de6bae6e6e094c1a95193503 Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Wed, 19 Nov 2025 12:57:00 -0600 Subject: [PATCH] [PM-27925] Refactor `StripeService` to allow more than one instance (#17467) * Refactor StripeService to allow more than one instance per scope * Fix linting issue * Claude's feedback --- .../settings/create-organization.component.ts | 15 +- .../enter-payment-method.component.ts | 154 ++-- .../billing/services/stripe.service.spec.ts | 797 ++++++++++++++++++ .../app/billing/services/stripe.service.ts | 259 ++++-- 4 files changed, 1088 insertions(+), 137 deletions(-) create mode 100644 apps/web/src/app/billing/services/stripe.service.spec.ts diff --git a/apps/web/src/app/admin-console/settings/create-organization.component.ts b/apps/web/src/app/admin-console/settings/create-organization.component.ts index 45ce89c0e3d..78398fd8897 100644 --- a/apps/web/src/app/admin-console/settings/create-organization.component.ts +++ b/apps/web/src/app/admin-console/settings/create-organization.component.ts @@ -1,8 +1,8 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { Component, OnInit } from "@angular/core"; -import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; +import { Component, OnDestroy, OnInit } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; +import { Subject, takeUntil } from "rxjs"; import { first } from "rxjs/operators"; import { PlanType, ProductTierType, ProductType } from "@bitwarden/common/billing/enums"; @@ -19,7 +19,7 @@ import { SharedModule } from "../../shared"; templateUrl: "create-organization.component.html", imports: [SharedModule, OrganizationPlansComponent, HeaderModule], }) -export class CreateOrganizationComponent implements OnInit { +export class CreateOrganizationComponent implements OnInit, OnDestroy { protected secretsManager = false; protected plan: PlanType = PlanType.Free; protected productTier: ProductTierType = ProductTierType.Free; @@ -29,6 +29,8 @@ export class CreateOrganizationComponent implements OnInit { private configService: ConfigService, ) {} + private destroy$ = new Subject(); + async ngOnInit(): Promise { const milestone3FeatureEnabled = await this.configService.getFeatureFlag( FeatureFlag.PM26462_Milestone_3, @@ -37,7 +39,7 @@ export class CreateOrganizationComponent implements OnInit { ? PlanType.FamiliesAnnually : PlanType.FamiliesAnnually2025; - this.route.queryParams.pipe(first(), takeUntilDestroyed()).subscribe((qParams) => { + this.route.queryParams.pipe(first(), takeUntil(this.destroy$)).subscribe((qParams) => { if (qParams.plan === "families" || qParams.productTier == ProductTierType.Families) { this.plan = familyPlan; this.productTier = ProductTierType.Families; @@ -61,4 +63,9 @@ export class CreateOrganizationComponent implements OnInit { this.secretsManager = qParams.product == ProductType.SecretsManager; }); } + + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); + } } diff --git a/apps/web/src/app/billing/payment/components/enter-payment-method.component.ts b/apps/web/src/app/billing/payment/components/enter-payment-method.component.ts index 5448f03aa56..9e7b870579d 100644 --- a/apps/web/src/app/billing/payment/components/enter-payment-method.component.ts +++ b/apps/web/src/app/billing/payment/components/enter-payment-method.component.ts @@ -1,9 +1,10 @@ -import { Component, Input, OnInit } from "@angular/core"; +import { ChangeDetectionStrategy, Component, input, OnDestroy, OnInit } from "@angular/core"; import { FormControl, FormGroup, Validators } from "@angular/forms"; import { map, Observable, of, startWith, Subject, takeUntil } from "rxjs"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; import { PopoverModule, ToastService } from "@bitwarden/components"; import { SharedModule } from "../../../shared"; @@ -34,18 +35,17 @@ type PaymentMethodFormGroup = FormGroup<{ }>; }>; -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "app-enter-payment-method", + changeDetection: ChangeDetectionStrategy.OnPush, template: ` - @let showBillingDetails = includeBillingAddress && selected !== "payPal"; -
+ @let showBillingDetails = includeBillingAddress() && selected !== "payPal"; + @if (showBillingDetails) {
{{ "paymentMethod" | i18n }}
}
- + @@ -60,7 +60,7 @@ type PaymentMethodFormGroup = FormGroup<{ } - @if (showPayPal) { + @if (showPayPal()) { @@ -68,7 +68,7 @@ type PaymentMethodFormGroup = FormGroup<{ } - @if (showAccountCredit) { + @if (showAccountCredit()) { @@ -82,10 +82,10 @@ type PaymentMethodFormGroup = FormGroup<{ @case ("card") {
- + {{ "cardNumberLabel" | i18n }} -
+
- + {{ "expiration" | i18n }} -
+
- + {{ "securityCodeSlashCVV" | i18n }}
} @@ -131,7 +131,7 @@ type PaymentMethodFormGroup = FormGroup<{ bitInput id="routingNumber" type="text" - [formControl]="group.controls.bankAccount.controls.routingNumber" + [formControl]="group().controls.bankAccount.controls.routingNumber" required /> @@ -141,7 +141,7 @@ type PaymentMethodFormGroup = FormGroup<{ bitInput id="accountNumber" type="text" - [formControl]="group.controls.bankAccount.controls.accountNumber" + [formControl]="group().controls.bankAccount.controls.accountNumber" required /> @@ -151,7 +151,7 @@ type PaymentMethodFormGroup = FormGroup<{ id="accountHolderName" bitInput type="text" - [formControl]="group.controls.bankAccount.controls.accountHolderName" + [formControl]="group().controls.bankAccount.controls.accountHolderName" required /> @@ -159,7 +159,7 @@ type PaymentMethodFormGroup = FormGroup<{ {{ "bankAccountType" | i18n }} @@ -186,7 +186,7 @@ type PaymentMethodFormGroup = FormGroup<{ } @case ("accountCredit") { - @if (hasEnoughAccountCredit) { + @if (hasEnoughAccountCredit()) { {{ "makeSureEnoughCredit" | i18n }} @@ -204,7 +204,7 @@ type PaymentMethodFormGroup = FormGroup<{
{{ "country" | i18n }} - + @for (selectableCountry of selectableCountries; track selectableCountry.value) { @@ -233,26 +233,15 @@ type PaymentMethodFormGroup = FormGroup<{ standalone: true, imports: [BillingServicesModule, PaymentLabelComponent, PopoverModule, SharedModule], }) -export class EnterPaymentMethodComponent implements OnInit { - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input({ required: true }) group!: PaymentMethodFormGroup; +export class EnterPaymentMethodComponent implements OnInit, OnDestroy { + protected readonly instanceId = Utils.newGuid(); - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() private showBankAccount = true; - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() showPayPal = true; - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() showAccountCredit = false; - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() hasEnoughAccountCredit = true; - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() includeBillingAddress = false; + readonly group = input.required(); + protected readonly showBankAccount = input(true); + readonly showPayPal = input(true); + readonly showAccountCredit = input(false); + readonly hasEnoughAccountCredit = input(true); + readonly includeBillingAddress = input(false); protected showBankAccount$!: Observable; protected selectableCountries = selectableCountries; @@ -269,57 +258,62 @@ export class EnterPaymentMethodComponent implements OnInit { ngOnInit() { this.stripeService.loadStripe( + this.instanceId, { - cardNumber: "#stripe-card-number", - cardExpiry: "#stripe-card-expiry", - cardCvc: "#stripe-card-cvc", + cardNumber: `#stripe-card-number-${this.instanceId}`, + cardExpiry: `#stripe-card-expiry-${this.instanceId}`, + cardCvc: `#stripe-card-cvc-${this.instanceId}`, }, true, ); - if (this.showPayPal) { + if (this.showPayPal()) { this.braintreeService.loadBraintree("#braintree-container", false); } - if (!this.includeBillingAddress) { - this.showBankAccount$ = of(this.showBankAccount); - this.group.controls.billingAddress.disable(); + if (!this.includeBillingAddress()) { + this.showBankAccount$ = of(this.showBankAccount()); + this.group().controls.billingAddress.disable(); } else { - this.group.controls.billingAddress.patchValue({ + this.group().controls.billingAddress.patchValue({ country: "US", }); - this.showBankAccount$ = this.group.controls.billingAddress.controls.country.valueChanges.pipe( - startWith(this.group.controls.billingAddress.controls.country.value), - map((country) => this.showBankAccount && country === "US"), - ); + this.showBankAccount$ = + this.group().controls.billingAddress.controls.country.valueChanges.pipe( + startWith(this.group().controls.billingAddress.controls.country.value), + map((country) => this.showBankAccount() && country === "US"), + ); } - this.group.controls.type.valueChanges - .pipe(startWith(this.group.controls.type.value), takeUntil(this.destroy$)) + this.group() + .controls.type.valueChanges.pipe( + startWith(this.group().controls.type.value), + takeUntil(this.destroy$), + ) .subscribe((selected) => { if (selected === "bankAccount") { - this.group.controls.bankAccount.enable(); - if (this.includeBillingAddress) { - this.group.controls.billingAddress.enable(); + this.group().controls.bankAccount.enable(); + if (this.includeBillingAddress()) { + this.group().controls.billingAddress.enable(); } } else { switch (selected) { case "card": { - this.stripeService.mountElements(); - if (this.includeBillingAddress) { - this.group.controls.billingAddress.enable(); + this.stripeService.mountElements(this.instanceId); + if (this.includeBillingAddress()) { + this.group().controls.billingAddress.enable(); } break; } case "payPal": { this.braintreeService.createDropin(); - if (this.includeBillingAddress) { - this.group.controls.billingAddress.disable(); + if (this.includeBillingAddress()) { + this.group().controls.billingAddress.disable(); } break; } } - this.group.controls.bankAccount.disable(); + this.group().controls.bankAccount.disable(); } }); @@ -330,22 +324,28 @@ export class EnterPaymentMethodComponent implements OnInit { }); } + ngOnDestroy() { + this.stripeService.unloadStripe(this.instanceId); + this.destroy$.next(); + this.destroy$.complete(); + } + select = (paymentMethod: PaymentMethodOption) => - this.group.controls.type.patchValue(paymentMethod); + this.group().controls.type.patchValue(paymentMethod); tokenize = async (): Promise => { const exchange = async (paymentMethod: TokenizablePaymentMethod) => { switch (paymentMethod) { case "bankAccount": { - this.group.controls.bankAccount.markAllAsTouched(); - if (!this.group.controls.bankAccount.valid) { + this.group().controls.bankAccount.markAllAsTouched(); + if (!this.group().controls.bankAccount.valid) { throw new Error("Attempted to tokenize invalid bank account information."); } - const bankAccount = this.group.controls.bankAccount.getRawValue(); + const bankAccount = this.group().controls.bankAccount.getRawValue(); const clientSecret = await this.stripeService.createSetupIntent("bankAccount"); - const billingDetails = this.group.controls.billingAddress.enabled - ? this.group.controls.billingAddress.getRawValue() + const billingDetails = this.group().controls.billingAddress.enabled + ? this.group().controls.billingAddress.getRawValue() : undefined; return await this.stripeService.setupBankAccountPaymentMethod( clientSecret, @@ -355,10 +355,14 @@ export class EnterPaymentMethodComponent implements OnInit { } case "card": { const clientSecret = await this.stripeService.createSetupIntent("card"); - const billingDetails = this.group.controls.billingAddress.enabled - ? this.group.controls.billingAddress.getRawValue() + const billingDetails = this.group().controls.billingAddress.enabled + ? this.group().controls.billingAddress.getRawValue() : undefined; - return this.stripeService.setupCardPaymentMethod(clientSecret, billingDetails); + return this.stripeService.setupCardPaymentMethod( + this.instanceId, + clientSecret, + billingDetails, + ); } case "payPal": { return this.braintreeService.requestPaymentMethod(); @@ -410,15 +414,15 @@ export class EnterPaymentMethodComponent implements OnInit { validate = (): boolean => { if (this.selected === "bankAccount") { - this.group.controls.bankAccount.markAllAsTouched(); - return this.group.controls.bankAccount.valid; + this.group().controls.bankAccount.markAllAsTouched(); + return this.group().controls.bankAccount.valid; } return true; }; get selected(): PaymentMethodOption { - return this.group.value.type!; + return this.group().value.type!; } static getFormGroup = (): PaymentMethodFormGroup => diff --git a/apps/web/src/app/billing/services/stripe.service.spec.ts b/apps/web/src/app/billing/services/stripe.service.spec.ts new file mode 100644 index 00000000000..983aeb266ae --- /dev/null +++ b/apps/web/src/app/billing/services/stripe.service.spec.ts @@ -0,0 +1,797 @@ +import { mock, MockProxy } from "jest-mock-extended"; + +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { BankAccount } from "@bitwarden/common/billing/models/domain"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; + +import { StripeService } from "./stripe.service"; + +// Extend Window interface to include Stripe +declare global { + interface Window { + Stripe: any; + } +} + +describe("StripeService", () => { + let service: StripeService; + let apiService: MockProxy; + let logService: MockProxy; + + // Stripe SDK mocks + let mockStripeInstance: any; + let mockElements: any; + let mockCardNumber: any; + let mockCardExpiry: any; + let mockCardCvc: any; + + // DOM mocks + let mockScript: HTMLScriptElement; + let mockIframe: HTMLIFrameElement; + + beforeEach(() => { + jest.useFakeTimers(); + + // Setup service dependency mocks + apiService = mock(); + logService = mock(); + + // Setup Stripe element mocks + mockCardNumber = { + mount: jest.fn(), + unmount: jest.fn(), + }; + mockCardExpiry = { + mount: jest.fn(), + unmount: jest.fn(), + }; + mockCardCvc = { + mount: jest.fn(), + unmount: jest.fn(), + }; + + // Setup Stripe Elements mock + mockElements = { + create: jest.fn((type: string) => { + switch (type) { + case "cardNumber": + return mockCardNumber; + case "cardExpiry": + return mockCardExpiry; + case "cardCvc": + return mockCardCvc; + default: + return null; + } + }), + getElement: jest.fn((type: string) => { + switch (type) { + case "cardNumber": + return mockCardNumber; + case "cardExpiry": + return mockCardExpiry; + case "cardCvc": + return mockCardCvc; + default: + return null; + } + }), + }; + + // Setup Stripe instance mock + mockStripeInstance = { + elements: jest.fn(() => mockElements), + confirmCardSetup: jest.fn(), + confirmUsBankAccountSetup: jest.fn(), + }; + + // Setup window.Stripe mock + window.Stripe = jest.fn(() => mockStripeInstance); + + // Setup DOM mocks + mockScript = { + id: "", + src: "", + onload: null, + onerror: null, + } as any; + + mockIframe = { + src: "https://js.stripe.com/v3/", + remove: jest.fn(), + } as any; + + jest.spyOn(window.document, "createElement").mockReturnValue(mockScript); + jest.spyOn(window.document, "getElementById").mockReturnValue(null); + jest.spyOn(window.document.head, "appendChild").mockReturnValue(mockScript); + jest.spyOn(window.document.head, "removeChild").mockImplementation(() => mockScript); + jest.spyOn(window.document, "querySelectorAll").mockReturnValue([mockIframe] as any); + + // Mock getComputedStyle + jest.spyOn(window, "getComputedStyle").mockReturnValue({ + getPropertyValue: (prop: string) => { + const props: Record = { + "--color-text-main": "0, 0, 0", + "--color-text-muted": "128, 128, 128", + "--color-danger-600": "220, 38, 38", + }; + return props[prop] || ""; + }, + } as any); + + // Create service instance + service = new StripeService(apiService, logService); + }); + + afterEach(() => { + jest.clearAllTimers(); + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + // Helper function to trigger script load + const triggerScriptLoad = () => { + if (mockScript.onload) { + mockScript.onload(new Event("load")); + } + }; + + // Helper function to advance timers and flush promises + const advanceTimersAndFlush = async (ms: number) => { + jest.advanceTimersByTime(ms); + await Promise.resolve(); + }; + + describe("createSetupIntent", () => { + it("should call API with correct path for card payment", async () => { + apiService.send.mockResolvedValue("client_secret_card_123"); + + const result = await service.createSetupIntent("card"); + + expect(apiService.send).toHaveBeenCalledWith("POST", "/setup-intent/card", null, true, true); + expect(result).toBe("client_secret_card_123"); + }); + + it("should call API with correct path for bank account payment", async () => { + apiService.send.mockResolvedValue("client_secret_bank_456"); + + const result = await service.createSetupIntent("bankAccount"); + + expect(apiService.send).toHaveBeenCalledWith( + "POST", + "/setup-intent/bank-account", + null, + true, + true, + ); + expect(result).toBe("client_secret_bank_456"); + }); + + it("should return client secret from API response", async () => { + const expectedSecret = "seti_1234567890_secret_abcdefg"; + apiService.send.mockResolvedValue(expectedSecret); + + const result = await service.createSetupIntent("card"); + + expect(result).toBe(expectedSecret); + }); + + it("should propagate API errors", async () => { + const error = new Error("API error"); + apiService.send.mockRejectedValue(error); + + await expect(service.createSetupIntent("card")).rejects.toThrow("API error"); + }); + }); + + describe("loadStripe - initial load", () => { + const instanceId = "test-instance-1"; + const elementIds = { + cardNumber: "#card-number", + cardExpiry: "#card-expiry", + cardCvc: "#card-cvc", + }; + + it("should create script element with correct attributes", () => { + service.loadStripe(instanceId, elementIds, false); + + expect(window.document.createElement).toHaveBeenCalledWith("script"); + expect(mockScript.id).toBe("stripe-script"); + expect(mockScript.src).toBe("https://js.stripe.com/v3?advancedFraudSignals=false"); + }); + + it("should append script to document head", () => { + service.loadStripe(instanceId, elementIds, false); + + expect(window.document.head.appendChild).toHaveBeenCalledWith(mockScript); + }); + + it("should initialize Stripe client on script load", async () => { + service.loadStripe(instanceId, elementIds, false); + + triggerScriptLoad(); + await advanceTimersAndFlush(0); + + expect(window.Stripe).toHaveBeenCalledWith(process.env.STRIPE_KEY); + }); + + it("should create Elements instance and store in Map", async () => { + service.loadStripe(instanceId, elementIds, false); + + triggerScriptLoad(); + await advanceTimersAndFlush(50); + + expect(mockStripeInstance.elements).toHaveBeenCalled(); + expect(service["instances"].size).toBe(1); + expect(service["instances"].get(instanceId)).toBeDefined(); + }); + + it("should increment instanceCount", async () => { + service.loadStripe(instanceId, elementIds, false); + + triggerScriptLoad(); + await advanceTimersAndFlush(50); + + expect(service["instanceCount"]).toBe(1); + }); + }); + + describe("loadStripe - already loaded", () => { + const instanceId1 = "instance-1"; + const instanceId2 = "instance-2"; + const elementIds = { + cardNumber: "#card-number", + cardExpiry: "#card-expiry", + cardCvc: "#card-cvc", + }; + + beforeEach(async () => { + // Load first instance to initialize Stripe + service.loadStripe(instanceId1, elementIds, false); + triggerScriptLoad(); + await advanceTimersAndFlush(100); + }); + + it("should not create new script if already loaded", () => { + jest.clearAllMocks(); + + service.loadStripe(instanceId2, elementIds, false); + + expect(window.document.createElement).not.toHaveBeenCalled(); + expect(window.document.head.appendChild).not.toHaveBeenCalled(); + }); + + it("should immediately initialize instance when script loaded", async () => { + service.loadStripe(instanceId2, elementIds, false); + await advanceTimersAndFlush(50); + + expect(service["instances"].size).toBe(2); + expect(service["instances"].get(instanceId2)).toBeDefined(); + }); + + it("should increment instanceCount correctly", async () => { + expect(service["instanceCount"]).toBe(1); + + service.loadStripe(instanceId2, elementIds, false); + await advanceTimersAndFlush(50); + + expect(service["instanceCount"]).toBe(2); + }); + }); + + describe("loadStripe - concurrent calls", () => { + const elementIds = { + cardNumber: "#card-number", + cardExpiry: "#card-expiry", + cardCvc: "#card-cvc", + }; + + it("should handle multiple loadStripe calls sequentially", async () => { + // Test practical scenario: load instances one after another + service.loadStripe("instance-1", elementIds, false); + triggerScriptLoad(); + await advanceTimersAndFlush(100); + + service.loadStripe("instance-2", elementIds, false); + await advanceTimersAndFlush(100); + + service.loadStripe("instance-3", elementIds, false); + await advanceTimersAndFlush(100); + + // All instances should be initialized + expect(service["instances"].size).toBe(3); + expect(service["instanceCount"]).toBe(3); + expect(service["instances"].get("instance-1")).toBeDefined(); + expect(service["instances"].get("instance-2")).toBeDefined(); + expect(service["instances"].get("instance-3")).toBeDefined(); + }); + + it("should share Stripe client across instances", async () => { + // Load first instance + service.loadStripe("instance-1", elementIds, false); + triggerScriptLoad(); + await advanceTimersAndFlush(100); + + const stripeClientAfterFirst = service["stripe"]; + expect(stripeClientAfterFirst).toBeDefined(); + + // Load second instance + service.loadStripe("instance-2", elementIds, false); + await advanceTimersAndFlush(100); + + // Should reuse the same Stripe client + expect(service["stripe"]).toBe(stripeClientAfterFirst); + expect(service["instances"].size).toBe(2); + }); + }); + + describe("mountElements - success path", () => { + const instanceId = "mount-test-instance"; + const elementIds = { + cardNumber: "#card-number-mount", + cardExpiry: "#card-expiry-mount", + cardCvc: "#card-cvc-mount", + }; + + beforeEach(async () => { + service.loadStripe(instanceId, elementIds, false); + triggerScriptLoad(); + await advanceTimersAndFlush(100); + }); + + it("should mount all three card elements to DOM", async () => { + service.mountElements(instanceId); + await advanceTimersAndFlush(100); + + expect(mockCardNumber.mount).toHaveBeenCalledWith("#card-number-mount"); + expect(mockCardExpiry.mount).toHaveBeenCalledWith("#card-expiry-mount"); + expect(mockCardCvc.mount).toHaveBeenCalledWith("#card-cvc-mount"); + }); + + it("should use correct element IDs from instance", async () => { + const customIds = { + cardNumber: "#custom-card", + cardExpiry: "#custom-expiry", + cardCvc: "#custom-cvc", + }; + + service.loadStripe("custom-instance", customIds, false); + await advanceTimersAndFlush(100); + + service.mountElements("custom-instance"); + await advanceTimersAndFlush(100); + + expect(mockCardNumber.mount).toHaveBeenCalledWith("#custom-card"); + expect(mockCardExpiry.mount).toHaveBeenCalledWith("#custom-expiry"); + expect(mockCardCvc.mount).toHaveBeenCalledWith("#custom-cvc"); + }); + + it("should handle autoMount flag correctly", async () => { + const autoMountId = "auto-mount-instance"; + jest.clearAllMocks(); + + service.loadStripe(autoMountId, elementIds, true); + triggerScriptLoad(); + await advanceTimersAndFlush(150); + + // Should auto-mount without explicit call + expect(mockCardNumber.mount).toHaveBeenCalled(); + expect(mockCardExpiry.mount).toHaveBeenCalled(); + expect(mockCardCvc.mount).toHaveBeenCalled(); + }); + }); + + describe("mountElements - retry logic", () => { + const elementIds = { + cardNumber: "#card-number", + cardExpiry: "#card-expiry", + cardCvc: "#card-cvc", + }; + + it("should retry if instance not found", async () => { + service.mountElements("non-existent-instance"); + await advanceTimersAndFlush(100); + + expect(logService.warning).toHaveBeenCalledWith( + expect.stringContaining("Stripe instance non-existent-instance not found"), + ); + }); + + it("should log error after 10 failed attempts", async () => { + service.mountElements("non-existent-instance"); + + for (let i = 0; i < 10; i++) { + await advanceTimersAndFlush(100); + } + + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("not found after 10 attempts"), + ); + }); + + it("should retry if elements not ready", async () => { + const instanceId = "retry-elements-instance"; + service.loadStripe(instanceId, elementIds, false); + triggerScriptLoad(); + await advanceTimersAndFlush(100); + + // Make elements temporarily unavailable + mockElements.getElement.mockReturnValueOnce(null); + mockElements.getElement.mockReturnValueOnce(null); + mockElements.getElement.mockReturnValueOnce(null); + + service.mountElements(instanceId); + await advanceTimersAndFlush(100); + + expect(logService.warning).toHaveBeenCalledWith( + expect.stringContaining("Some Stripe card elements"), + ); + }); + }); + + describe("setupCardPaymentMethod", () => { + const instanceId = "card-setup-instance"; + const clientSecret = "seti_card_secret_123"; + const elementIds = { + cardNumber: "#card-number", + cardExpiry: "#card-expiry", + cardCvc: "#card-cvc", + }; + + beforeEach(async () => { + service.loadStripe(instanceId, elementIds, false); + triggerScriptLoad(); + await advanceTimersAndFlush(100); + }); + + it("should call Stripe confirmCardSetup with correct parameters", async () => { + mockStripeInstance.confirmCardSetup.mockResolvedValue({ + setupIntent: { status: "succeeded", payment_method: "pm_card_123" }, + }); + + await service.setupCardPaymentMethod(instanceId, clientSecret); + + expect(mockStripeInstance.confirmCardSetup).toHaveBeenCalledWith(clientSecret, { + payment_method: { + card: mockCardNumber, + }, + }); + }); + + it("should include billing details when provided", async () => { + mockStripeInstance.confirmCardSetup.mockResolvedValue({ + setupIntent: { status: "succeeded", payment_method: "pm_card_123" }, + }); + + const billingDetails = { country: "US", postalCode: "12345" }; + await service.setupCardPaymentMethod(instanceId, clientSecret, billingDetails); + + expect(mockStripeInstance.confirmCardSetup).toHaveBeenCalledWith(clientSecret, { + payment_method: { + card: mockCardNumber, + billing_details: { + address: { + country: "US", + postal_code: "12345", + }, + }, + }, + }); + }); + + it("should throw error if instance not found", async () => { + await expect(service.setupCardPaymentMethod("non-existent", clientSecret)).rejects.toThrow( + "Payment method initialization failed. Please try again.", + ); + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Stripe instance non-existent not found"), + ); + }); + + it("should throw error if setup fails", async () => { + const error = { message: "Card declined" }; + mockStripeInstance.confirmCardSetup.mockResolvedValue({ error }); + + await expect(service.setupCardPaymentMethod(instanceId, clientSecret)).rejects.toEqual(error); + expect(logService.error).toHaveBeenCalledWith(error); + }); + + it("should throw error if status is not succeeded", async () => { + const error = { message: "Invalid status" }; + mockStripeInstance.confirmCardSetup.mockResolvedValue({ + setupIntent: { status: "requires_action" }, + error, + }); + + await expect(service.setupCardPaymentMethod(instanceId, clientSecret)).rejects.toEqual(error); + }); + + it("should return payment method ID on success", async () => { + mockStripeInstance.confirmCardSetup.mockResolvedValue({ + setupIntent: { status: "succeeded", payment_method: "pm_card_success_123" }, + }); + + const result = await service.setupCardPaymentMethod(instanceId, clientSecret); + + expect(result).toBe("pm_card_success_123"); + }); + }); + + describe("setupBankAccountPaymentMethod", () => { + const clientSecret = "seti_bank_secret_456"; + const bankAccount: BankAccount = { + accountHolderName: "John Doe", + routingNumber: "110000000", + accountNumber: "000123456789", + accountHolderType: "individual", + }; + + beforeEach(async () => { + // Initialize Stripe instance for bank account tests + service.loadStripe( + "bank-test-instance", + { + cardNumber: "#card", + cardExpiry: "#expiry", + cardCvc: "#cvc", + }, + false, + ); + triggerScriptLoad(); + await advanceTimersAndFlush(100); + }); + + it("should call Stripe confirmUsBankAccountSetup with bank details", async () => { + mockStripeInstance.confirmUsBankAccountSetup.mockResolvedValue({ + setupIntent: { status: "requires_action", payment_method: "pm_bank_123" }, + }); + + await service.setupBankAccountPaymentMethod(clientSecret, bankAccount); + + expect(mockStripeInstance.confirmUsBankAccountSetup).toHaveBeenCalledWith(clientSecret, { + payment_method: { + us_bank_account: { + routing_number: "110000000", + account_number: "000123456789", + account_holder_type: "individual", + }, + billing_details: { + name: "John Doe", + }, + }, + }); + }); + + it("should include billing address when provided", async () => { + mockStripeInstance.confirmUsBankAccountSetup.mockResolvedValue({ + setupIntent: { status: "requires_action", payment_method: "pm_bank_123" }, + }); + + const billingDetails = { country: "US", postalCode: "90210" }; + await service.setupBankAccountPaymentMethod(clientSecret, bankAccount, billingDetails); + + expect(mockStripeInstance.confirmUsBankAccountSetup).toHaveBeenCalledWith(clientSecret, { + payment_method: { + us_bank_account: { + routing_number: "110000000", + account_number: "000123456789", + account_holder_type: "individual", + }, + billing_details: { + name: "John Doe", + address: { + country: "US", + postal_code: "90210", + }, + }, + }, + }); + }); + + it("should omit billing address when not provided", async () => { + mockStripeInstance.confirmUsBankAccountSetup.mockResolvedValue({ + setupIntent: { status: "requires_action", payment_method: "pm_bank_123" }, + }); + + await service.setupBankAccountPaymentMethod(clientSecret, bankAccount); + + const call = mockStripeInstance.confirmUsBankAccountSetup.mock.calls[0][1]; + expect(call.payment_method.billing_details.address).toBeUndefined(); + }); + + it("should validate status is requires_action", async () => { + const error = { message: "Invalid status" }; + mockStripeInstance.confirmUsBankAccountSetup.mockResolvedValue({ + setupIntent: { status: "succeeded" }, + error, + }); + + await expect( + service.setupBankAccountPaymentMethod(clientSecret, bankAccount), + ).rejects.toEqual(error); + }); + + it("should return payment method ID on success", async () => { + mockStripeInstance.confirmUsBankAccountSetup.mockResolvedValue({ + setupIntent: { status: "requires_action", payment_method: "pm_bank_success_456" }, + }); + + const result = await service.setupBankAccountPaymentMethod(clientSecret, bankAccount); + + expect(result).toBe("pm_bank_success_456"); + }); + }); + + describe("unloadStripe - single instance", () => { + const instanceId = "unload-test-instance"; + const elementIds = { + cardNumber: "#card-number", + cardExpiry: "#card-expiry", + cardCvc: "#card-cvc", + }; + + beforeEach(async () => { + service.loadStripe(instanceId, elementIds, false); + triggerScriptLoad(); + await advanceTimersAndFlush(100); + }); + + it("should unmount all card elements", () => { + service.unloadStripe(instanceId); + + expect(mockCardNumber.unmount).toHaveBeenCalled(); + expect(mockCardExpiry.unmount).toHaveBeenCalled(); + expect(mockCardCvc.unmount).toHaveBeenCalled(); + }); + + it("should remove instance from Map", () => { + expect(service["instances"].has(instanceId)).toBe(true); + + service.unloadStripe(instanceId); + + expect(service["instances"].has(instanceId)).toBe(false); + }); + + it("should decrement instanceCount", () => { + expect(service["instanceCount"]).toBe(1); + + service.unloadStripe(instanceId); + + expect(service["instanceCount"]).toBe(0); + }); + + it("should remove script when last instance unloaded", () => { + jest.spyOn(window.document, "getElementById").mockReturnValue(mockScript); + + service.unloadStripe(instanceId); + + expect(window.document.head.removeChild).toHaveBeenCalledWith(mockScript); + }); + + it("should remove Stripe iframes after cleanup delay", async () => { + service.unloadStripe(instanceId); + + await advanceTimersAndFlush(500); + + expect(window.document.querySelectorAll).toHaveBeenCalledWith("iframe"); + expect(mockIframe.remove).toHaveBeenCalled(); + }); + }); + + describe("unloadStripe - multiple instances", () => { + const elementIds = { + cardNumber: "#card-number", + cardExpiry: "#card-expiry", + cardCvc: "#card-cvc", + }; + + beforeEach(async () => { + // Load first instance + service.loadStripe("instance-1", elementIds, false); + triggerScriptLoad(); + await advanceTimersAndFlush(100); + + // Load second instance (script already loaded) + service.loadStripe("instance-2", elementIds, false); + await advanceTimersAndFlush(100); + }); + + it("should not remove script when other instances exist", () => { + expect(service["instanceCount"]).toBe(2); + + service.unloadStripe("instance-1"); + + expect(service["instanceCount"]).toBe(1); + expect(window.document.head.removeChild).not.toHaveBeenCalled(); + }); + + it("should only cleanup specific instance", () => { + service.unloadStripe("instance-1"); + + expect(service["instances"].has("instance-1")).toBe(false); + expect(service["instances"].has("instance-2")).toBe(true); + }); + + it("should handle reference counting correctly", () => { + expect(service["instanceCount"]).toBe(2); + + service.unloadStripe("instance-1"); + expect(service["instanceCount"]).toBe(1); + + service.unloadStripe("instance-2"); + expect(service["instanceCount"]).toBe(0); + }); + }); + + describe("unloadStripe - edge cases", () => { + it("should handle unload of non-existent instance gracefully", () => { + expect(() => service.unloadStripe("non-existent")).not.toThrow(); + expect(service["instanceCount"]).toBe(0); + }); + + it("should handle duplicate unload calls", async () => { + const instanceId = "duplicate-unload"; + const elementIds = { + cardNumber: "#card-number", + cardExpiry: "#card-expiry", + cardCvc: "#card-cvc", + }; + + service.loadStripe(instanceId, elementIds, false); + triggerScriptLoad(); + await advanceTimersAndFlush(100); + + service.unloadStripe(instanceId); + expect(service["instanceCount"]).toBe(0); + + service.unloadStripe(instanceId); + expect(service["instanceCount"]).toBe(0); // Should not go negative + }); + + it("should catch and log element unmount errors", async () => { + const instanceId = "error-unmount"; + const elementIds = { + cardNumber: "#card-number", + cardExpiry: "#card-expiry", + cardCvc: "#card-cvc", + }; + + service.loadStripe(instanceId, elementIds, false); + triggerScriptLoad(); + await advanceTimersAndFlush(100); + + const unmountError = new Error("Unmount failed"); + mockCardNumber.unmount.mockImplementation(() => { + throw unmountError; + }); + + service.unloadStripe(instanceId); + + expect(logService.error).toHaveBeenCalledWith( + expect.stringContaining("Error unmounting Stripe elements"), + unmountError, + ); + }); + }); + + describe("element styling", () => { + it("should apply correct CSS custom properties", () => { + const options = service["getElementOptions"]("cardNumber"); + + expect(options.style.base.color).toBe("rgb(0, 0, 0)"); + expect(options.style.base["::placeholder"].color).toBe("rgb(128, 128, 128)"); + expect(options.style.invalid.color).toBe("rgb(0, 0, 0)"); + expect(options.style.invalid.borderColor).toBe("rgb(220, 38, 38)"); + }); + + it("should remove placeholder for cardNumber and cardCvc", () => { + const cardNumberOptions = service["getElementOptions"]("cardNumber"); + const cardCvcOptions = service["getElementOptions"]("cardCvc"); + const cardExpiryOptions = service["getElementOptions"]("cardExpiry"); + + expect(cardNumberOptions.placeholder).toBe(""); + expect(cardCvcOptions.placeholder).toBe(""); + expect(cardExpiryOptions.placeholder).toBeUndefined(); + }); + }); +}); diff --git a/apps/web/src/app/billing/services/stripe.service.ts b/apps/web/src/app/billing/services/stripe.service.ts index a2eb7cd98f2..9aabab9beb0 100644 --- a/apps/web/src/app/billing/services/stripe.service.ts +++ b/apps/web/src/app/billing/services/stripe.service.ts @@ -8,8 +8,6 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" import { BankAccountPaymentMethod, CardPaymentMethod } from "../payment/types"; -import { BillingServicesModule } from "./billing-services.module"; - type SetupBankAccountRequest = { payment_method: { us_bank_account: { @@ -39,15 +37,21 @@ type SetupCardRequest = { }; }; -@Injectable({ providedIn: BillingServicesModule }) +@Injectable({ providedIn: "root" }) export class StripeService { - private stripe: any; - private elements: any; - private elementIds: { - cardNumber: string; - cardExpiry: string; - cardCvc: string; - }; + // Shared/Global - One Stripe client for entire application + private stripe: any = null; + private stripeScriptLoaded = false; + private instanceCount = 0; + + // Per-Instance - Isolated Elements for each component + private instances = new Map< + string, + { + elements: any; + elementIds: { cardNumber: string; cardExpiry: string; cardCvc: string }; + } + >(); constructor( private apiService: ApiService, @@ -76,53 +80,121 @@ export class StripeService { * Loads [Stripe JS]{@link https://docs.stripe.com/js} in the element of the current page and mounts * Stripe credit card [elements]{@link https://docs.stripe.com/js/elements_object/create} into the HTML elements with the provided element IDS. * We do this to avoid having to load the Stripe JS SDK on every page of the Web Vault given many pages contain sensitive information. + * @param instanceId - Unique identifier for this component instance. * @param elementIds - The ID attributes of the HTML elements used to load the Stripe JS credit card elements. * @param autoMount - A flag indicating whether you want to immediately mount the Stripe credit card elements. */ loadStripe( + instanceId: string, elementIds: { cardNumber: string; cardExpiry: string; cardCvc: string }, autoMount: boolean, ) { - this.elementIds = elementIds; - const script = window.document.createElement("script"); - script.id = "stripe-script"; - script.src = "https://js.stripe.com/v3?advancedFraudSignals=false"; - script.onload = async () => { - const window$ = window as any; - this.stripe = window$.Stripe(process.env.STRIPE_KEY); - this.elements = this.stripe.elements(); - setTimeout(() => { - this.elements.create("cardNumber", this.getElementOptions("cardNumber")); - this.elements.create("cardExpiry", this.getElementOptions("cardExpiry")); - this.elements.create("cardCvc", this.getElementOptions("cardCvc")); - if (autoMount) { - this.mountElements(); - } - }, 50); - }; + // Check if script is already loaded + if (this.stripeScriptLoaded) { + // Script already loaded, initialize this instance immediately + this.initializeInstance(instanceId, elementIds, autoMount); + } else if (!window.document.getElementById("stripe-script")) { + // Script not loaded and not loading, start loading it + const script = window.document.createElement("script"); + script.id = "stripe-script"; + script.src = "https://js.stripe.com/v3?advancedFraudSignals=false"; + script.onload = async () => { + const window$ = window as any; + this.stripe = window$.Stripe(process.env.STRIPE_KEY); + this.stripeScriptLoaded = true; // Mark as loaded after script loads - window.document.head.appendChild(script); + // Initialize this instance after script loads + this.initializeInstance(instanceId, elementIds, autoMount); + }; + window.document.head.appendChild(script); + } else { + // Script is currently loading, wait for it + this.initializeInstance(instanceId, elementIds, autoMount); + } } - mountElements(attempt: number = 1) { - setTimeout(() => { - if (!this.elements) { - this.logService.warning(`Stripe elements are missing, retrying for attempt ${attempt}...`); - this.mountElements(attempt + 1); + private initializeInstance( + instanceId: string, + elementIds: { cardNumber: string; cardExpiry: string; cardCvc: string }, + autoMount: boolean, + attempt: number = 1, + ) { + // Wait for stripe to be available if script just loaded + if (!this.stripe) { + if (attempt < 10) { + this.logService.warning( + `Stripe not yet loaded for instance ${instanceId}, retrying attempt ${attempt}...`, + ); + setTimeout( + () => this.initializeInstance(instanceId, elementIds, autoMount, attempt + 1), + 50, + ); } else { - const cardNumber = this.elements.getElement("cardNumber"); - const cardExpiry = this.elements.getElement("cardExpiry"); - const cardCVC = this.elements.getElement("cardCvc"); + this.logService.error( + `Stripe failed to load for instance ${instanceId} after ${attempt} attempts`, + ); + } + return; + } + + // Create a new Elements instance for this component + const elements = this.stripe.elements(); + + // Store instance data + this.instances.set(instanceId, { elements, elementIds }); + + // Increment instance count now that instance is successfully initialized + this.instanceCount++; + + // Create the card elements + setTimeout(() => { + elements.create("cardNumber", this.getElementOptions("cardNumber")); + elements.create("cardExpiry", this.getElementOptions("cardExpiry")); + elements.create("cardCvc", this.getElementOptions("cardCvc")); + + if (autoMount) { + this.mountElements(instanceId); + } + }, 50); + } + + mountElements(instanceId: string, attempt: number = 1) { + setTimeout(() => { + const instance = this.instances.get(instanceId); + + if (!instance) { + if (attempt < 10) { + this.logService.warning( + `Stripe instance ${instanceId} not found, retrying for attempt ${attempt}...`, + ); + this.mountElements(instanceId, attempt + 1); + } else { + this.logService.error( + `Stripe instance ${instanceId} not found after ${attempt} attempts`, + ); + } + return; + } + + if (!instance.elements) { + this.logService.warning( + `Stripe elements for instance ${instanceId} are missing, retrying for attempt ${attempt}...`, + ); + this.mountElements(instanceId, attempt + 1); + } else { + const cardNumber = instance.elements.getElement("cardNumber"); + const cardExpiry = instance.elements.getElement("cardExpiry"); + const cardCVC = instance.elements.getElement("cardCvc"); if ([cardNumber, cardExpiry, cardCVC].some((element) => !element)) { this.logService.warning( - `Some Stripe card elements are missing, retrying for attempt ${attempt}...`, + `Some Stripe card elements for instance ${instanceId} are missing, retrying for attempt ${attempt}...`, ); - this.mountElements(attempt + 1); + this.mountElements(instanceId, attempt + 1); } else { - cardNumber.mount(this.elementIds.cardNumber); - cardExpiry.mount(this.elementIds.cardExpiry); - cardCVC.mount(this.elementIds.cardCvc); + cardNumber.mount(instance.elementIds.cardNumber); + cardExpiry.mount(instance.elementIds.cardExpiry); + cardCVC.mount(instance.elementIds.cardCvc); } } }, 100); @@ -132,6 +204,9 @@ export class StripeService { * Creates a Stripe [SetupIntent]{@link https://docs.stripe.com/api/setup_intents} and uses the resulting client secret * to invoke the Stripe JS [confirmUsBankAccountSetup]{@link https://docs.stripe.com/js/setup_intents/confirm_us_bank_account_setup} method, * thereby creating and storing a Stripe [PaymentMethod]{@link https://docs.stripe.com/api/payment_methods}. + * @param clientSecret - The client secret from the SetupIntent. + * @param bankAccount - The bank account details. + * @param billingDetails - Optional billing details. * @returns The ID of the newly created PaymentMethod. */ async setupBankAccountPaymentMethod( @@ -171,13 +246,28 @@ export class StripeService { * Creates a Stripe [SetupIntent]{@link https://docs.stripe.com/api/setup_intents} and uses the resulting client secret * to invoke the Stripe JS [confirmCardSetup]{@link https://docs.stripe.com/js/setup_intents/confirm_card_setup} method, * thereby creating and storing a Stripe [PaymentMethod]{@link https://docs.stripe.com/api/payment_methods}. + * @param instanceId - Unique identifier for the component instance. + * @param clientSecret - The client secret from the SetupIntent. + * @param billingDetails - Optional billing details. * @returns The ID of the newly created PaymentMethod. */ async setupCardPaymentMethod( + instanceId: string, clientSecret: string, billingDetails?: { country: string; postalCode: string }, ): Promise { - const cardNumber = this.elements.getElement("cardNumber"); + const instance = this.instances.get(instanceId); + if (!instance) { + const availableInstances = Array.from(this.instances.keys()); + this.logService.error( + `Stripe instance ${instanceId} not found. ` + + `Available instances: [${availableInstances.join(", ")}]. ` + + `This may occur if the component was destroyed during the payment flow.`, + ); + throw new Error("Payment method initialization failed. Please try again."); + } + + const cardNumber = instance.elements.getElement("cardNumber"); const request: SetupCardRequest = { payment_method: { card: cardNumber, @@ -200,24 +290,77 @@ export class StripeService { } /** - * Removes {@link https://docs.stripe.com/js} from the element of the current page as well as all - * Stripe-managed