From 58eae7a22019a203f38126849757733fbe784a18 Mon Sep 17 00:00:00 2001 From: Kyle Denney <4227399+kdenney@users.noreply.github.com> Date: Wed, 20 Aug 2025 14:11:15 -0500 Subject: [PATCH] [PM-24552] - Remove code for pm-19956-require-provider-payment-method-during-setup (#6196) * [PM-24552] - remove code for feature flag * pr gate: removing unused and redundant usings/qualifiers --- .../AdminConsole/Services/ProviderService.cs | 5 +- .../Services/ProviderBillingService.cs | 127 ++++++++---------- .../Services/ProviderServiceTests.cs | 5 +- .../Services/ProviderBillingServiceTests.cs | 79 ++--------- .../Implementations/ProviderMigrator.cs | 7 +- .../Services/IProviderBillingService.cs | 2 +- src/Core/Constants.cs | 1 - 7 files changed, 80 insertions(+), 146 deletions(-) diff --git a/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs b/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs index 3300b05531..aa19ad5382 100644 --- a/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs +++ b/bitwarden_license/src/Commercial.Core/AdminConsole/Services/ProviderService.cs @@ -120,10 +120,7 @@ public class ProviderService : IProviderService throw new BadRequestException("Both address and postal code are required to set up your provider."); } - var requireProviderPaymentMethodDuringSetup = - _featureService.IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup); - - if (requireProviderPaymentMethodDuringSetup && tokenizedPaymentSource is not + if (tokenizedPaymentSource is not { Type: PaymentMethodType.BankAccount or PaymentMethodType.Card or PaymentMethodType.PayPal, Token: not null and not "" diff --git a/bitwarden_license/src/Commercial.Core/Billing/Providers/Services/ProviderBillingService.cs b/bitwarden_license/src/Commercial.Core/Billing/Providers/Services/ProviderBillingService.cs index e02b52cd46..49bcf193b4 100644 --- a/bitwarden_license/src/Commercial.Core/Billing/Providers/Services/ProviderBillingService.cs +++ b/bitwarden_license/src/Commercial.Core/Billing/Providers/Services/ProviderBillingService.cs @@ -483,8 +483,10 @@ public class ProviderBillingService( public async Task SetupCustomer( Provider provider, TaxInfo taxInfo, - TokenizedPaymentSource tokenizedPaymentSource = null) + TokenizedPaymentSource tokenizedPaymentSource) { + ArgumentNullException.ThrowIfNull(tokenizedPaymentSource); + if (taxInfo is not { BillingAddressCountry: not null and not "", @@ -569,56 +571,50 @@ public class ProviderBillingService( options.Coupon = provider.DiscountId; } - var requireProviderPaymentMethodDuringSetup = - featureService.IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup); - var braintreeCustomerId = ""; - if (requireProviderPaymentMethodDuringSetup) + if (tokenizedPaymentSource is not + { + Type: PaymentMethodType.BankAccount or PaymentMethodType.Card or PaymentMethodType.PayPal, + Token: not null and not "" + }) { - if (tokenizedPaymentSource is not + logger.LogError("Cannot create customer for provider ({ProviderID}) with invalid payment method", provider.Id); + throw new BillingException(); + } + + var (type, token) = tokenizedPaymentSource; + + // ReSharper disable once SwitchStatementMissingSomeEnumCasesNoDefault + switch (type) + { + case PaymentMethodType.BankAccount: { - Type: PaymentMethodType.BankAccount or PaymentMethodType.Card or PaymentMethodType.PayPal, - Token: not null and not "" - }) - { - logger.LogError("Cannot create customer for provider ({ProviderID}) without a payment method", provider.Id); - throw new BillingException(); - } + var setupIntent = + (await stripeAdapter.SetupIntentList(new SetupIntentListOptions { PaymentMethod = token })) + .FirstOrDefault(); - var (type, token) = tokenizedPaymentSource; - - // ReSharper disable once SwitchStatementMissingSomeEnumCasesNoDefault - switch (type) - { - case PaymentMethodType.BankAccount: + if (setupIntent == null) { - var setupIntent = - (await stripeAdapter.SetupIntentList(new SetupIntentListOptions { PaymentMethod = token })) - .FirstOrDefault(); + logger.LogError("Cannot create customer for provider ({ProviderID}) without a setup intent for their bank account", provider.Id); + throw new BillingException(); + } - if (setupIntent == null) - { - logger.LogError("Cannot create customer for provider ({ProviderID}) without a setup intent for their bank account", provider.Id); - throw new BillingException(); - } - - await setupIntentCache.Set(provider.Id, setupIntent.Id); - break; - } - case PaymentMethodType.Card: - { - options.PaymentMethod = token; - options.InvoiceSettings.DefaultPaymentMethod = token; - break; - } - case PaymentMethodType.PayPal: - { - braintreeCustomerId = await subscriberService.CreateBraintreeCustomer(provider, token); - options.Metadata[BraintreeCustomerIdKey] = braintreeCustomerId; - break; - } - } + await setupIntentCache.Set(provider.Id, setupIntent.Id); + break; + } + case PaymentMethodType.Card: + { + options.PaymentMethod = token; + options.InvoiceSettings.DefaultPaymentMethod = token; + break; + } + case PaymentMethodType.PayPal: + { + braintreeCustomerId = await subscriberService.CreateBraintreeCustomer(provider, token); + options.Metadata[BraintreeCustomerIdKey] = braintreeCustomerId; + break; + } } try @@ -640,25 +636,22 @@ public class ProviderBillingService( async Task Revert() { - if (requireProviderPaymentMethodDuringSetup && tokenizedPaymentSource != null) + // ReSharper disable once SwitchStatementMissingSomeEnumCasesNoDefault + switch (tokenizedPaymentSource.Type) { - // ReSharper disable once SwitchStatementMissingSomeEnumCasesNoDefault - switch (tokenizedPaymentSource.Type) - { - case PaymentMethodType.BankAccount: - { - var setupIntentId = await setupIntentCache.Get(provider.Id); - await stripeAdapter.SetupIntentCancel(setupIntentId, - new SetupIntentCancelOptions { CancellationReason = "abandoned" }); - await setupIntentCache.Remove(provider.Id); - break; - } - case PaymentMethodType.PayPal when !string.IsNullOrEmpty(braintreeCustomerId): - { - await braintreeGateway.Customer.DeleteAsync(braintreeCustomerId); - break; - } - } + case PaymentMethodType.BankAccount: + { + var setupIntentId = await setupIntentCache.Get(provider.Id); + await stripeAdapter.SetupIntentCancel(setupIntentId, + new SetupIntentCancelOptions { CancellationReason = "abandoned" }); + await setupIntentCache.Remove(provider.Id); + break; + } + case PaymentMethodType.PayPal when !string.IsNullOrEmpty(braintreeCustomerId): + { + await braintreeGateway.Customer.DeleteAsync(braintreeCustomerId); + break; + } } } } @@ -701,9 +694,6 @@ public class ProviderBillingService( }); } - var requireProviderPaymentMethodDuringSetup = - featureService.IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup); - var setupIntentId = await setupIntentCache.Get(provider.Id); var setupIntent = !string.IsNullOrEmpty(setupIntentId) @@ -714,10 +704,9 @@ public class ProviderBillingService( : null; var usePaymentMethod = - requireProviderPaymentMethodDuringSetup && - (!string.IsNullOrEmpty(customer.InvoiceSettings.DefaultPaymentMethodId) || - customer.Metadata.ContainsKey(BraintreeCustomerIdKey) || - setupIntent.IsUnverifiedBankAccount()); + !string.IsNullOrEmpty(customer.InvoiceSettings?.DefaultPaymentMethodId) || + (customer.Metadata?.ContainsKey(BraintreeCustomerIdKey) == true) || + (setupIntent?.IsUnverifiedBankAccount() == true); int? trialPeriodDays = provider.Type switch { diff --git a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs index 608b4b3034..f2ba2fab8f 100644 --- a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/Services/ProviderServiceTests.cs @@ -1,6 +1,5 @@ using Bit.Commercial.Core.AdminConsole.Services; using Bit.Commercial.Core.Test.AdminConsole.AutoFixture; -using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.AdminConsole.Enums.Provider; @@ -120,8 +119,6 @@ public class ProviderServiceTests var token = protector.Protect($"ProviderSetupInvite {provider.Id} {user.Email} {CoreHelpers.ToEpocMilliseconds(DateTime.UtcNow)}"); - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); tokenizedPaymentSource = tokenizedPaymentSource with { Type = PaymentMethodType.BitPay }; @@ -1194,7 +1191,7 @@ public class ProviderServiceTests private static SubscriptionUpdateOptions SubscriptionUpdateRequest(string expectedPlanId, Subscription subscriptionItem) => new() { - Items = new List + Items = new List { new() { Id = subscriptionItem.Id, Price = expectedPlanId }, } diff --git a/bitwarden_license/test/Commercial.Core.Test/Billing/Providers/Services/ProviderBillingServiceTests.cs b/bitwarden_license/test/Commercial.Core.Test/Billing/Providers/Services/ProviderBillingServiceTests.cs index 9af9a71cce..c5b34e45bb 100644 --- a/bitwarden_license/test/Commercial.Core.Test/Billing/Providers/Services/ProviderBillingServiceTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/Billing/Providers/Services/ProviderBillingServiceTests.cs @@ -901,11 +901,12 @@ public class ProviderBillingServiceTests public async Task SetupCustomer_MissingCountry_ContactSupport( SutProvider sutProvider, Provider provider, - TaxInfo taxInfo) + TaxInfo taxInfo, + TokenizedPaymentSource tokenizedPaymentSource) { taxInfo.BillingAddressCountry = null; - await ThrowsBillingExceptionAsync(() => sutProvider.Sut.SetupCustomer(provider, taxInfo)); + await ThrowsBillingExceptionAsync(() => sutProvider.Sut.SetupCustomer(provider, taxInfo, tokenizedPaymentSource)); await sutProvider.GetDependency() .DidNotReceiveWithAnyArgs() @@ -916,60 +917,27 @@ public class ProviderBillingServiceTests public async Task SetupCustomer_MissingPostalCode_ContactSupport( SutProvider sutProvider, Provider provider, - TaxInfo taxInfo) + TaxInfo taxInfo, + TokenizedPaymentSource tokenizedPaymentSource) { taxInfo.BillingAddressCountry = null; - await ThrowsBillingExceptionAsync(() => sutProvider.Sut.SetupCustomer(provider, taxInfo)); + await ThrowsBillingExceptionAsync(() => sutProvider.Sut.SetupCustomer(provider, taxInfo, tokenizedPaymentSource)); await sutProvider.GetDependency() .DidNotReceiveWithAnyArgs() .CustomerGetAsync(Arg.Any(), Arg.Any()); } + [Theory, BitAutoData] - public async Task SetupCustomer_NoPaymentMethod_Success( + public async Task SetupCustomer_NullPaymentSource_ThrowsArgumentNullException( SutProvider sutProvider, Provider provider, TaxInfo taxInfo) { - provider.Name = "MSP"; - - sutProvider.GetDependency() - .GetStripeTaxCode(Arg.Is( - p => p == taxInfo.BillingAddressCountry), - Arg.Is(p => p == taxInfo.TaxIdNumber)) - .Returns(taxInfo.TaxIdType); - - taxInfo.BillingAddressCountry = "AD"; - - var stripeAdapter = sutProvider.GetDependency(); - - var expected = new Customer - { - Id = "customer_id", - Tax = new CustomerTax { AutomaticTax = StripeConstants.AutomaticTaxStatus.Supported } - }; - - stripeAdapter.CustomerCreateAsync(Arg.Is(o => - o.Address.Country == taxInfo.BillingAddressCountry && - o.Address.PostalCode == taxInfo.BillingAddressPostalCode && - o.Address.Line1 == taxInfo.BillingAddressLine1 && - o.Address.Line2 == taxInfo.BillingAddressLine2 && - o.Address.City == taxInfo.BillingAddressCity && - o.Address.State == taxInfo.BillingAddressState && - o.Description == WebUtility.HtmlDecode(provider.BusinessName) && - o.Email == provider.BillingEmail && - o.InvoiceSettings.CustomFields.FirstOrDefault().Name == "Provider" && - o.InvoiceSettings.CustomFields.FirstOrDefault().Value == "MSP" && - o.Metadata["region"] == "" && - o.TaxIdData.FirstOrDefault().Type == taxInfo.TaxIdType && - o.TaxIdData.FirstOrDefault().Value == taxInfo.TaxIdNumber)) - .Returns(expected); - - var actual = await sutProvider.Sut.SetupCustomer(provider, taxInfo); - - Assert.Equivalent(expected, actual); + await Assert.ThrowsAsync(() => + sutProvider.Sut.SetupCustomer(provider, taxInfo, null)); } [Theory, BitAutoData] @@ -989,8 +957,6 @@ public class ProviderBillingServiceTests taxInfo.BillingAddressCountry = "AD"; - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); tokenizedPaymentSource = tokenizedPaymentSource with { Type = PaymentMethodType.BitPay }; @@ -1018,8 +984,6 @@ public class ProviderBillingServiceTests var tokenizedPaymentSource = new TokenizedPaymentSource(PaymentMethodType.BankAccount, "token"); - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); stripeAdapter.SetupIntentList(Arg.Is(options => options.PaymentMethod == tokenizedPaymentSource.Token)).Returns([ @@ -1075,8 +1039,6 @@ public class ProviderBillingServiceTests var tokenizedPaymentSource = new TokenizedPaymentSource(PaymentMethodType.PayPal, "token"); - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); sutProvider.GetDependency().CreateBraintreeCustomer(provider, tokenizedPaymentSource.Token) .Returns("braintree_customer_id"); @@ -1130,8 +1092,6 @@ public class ProviderBillingServiceTests var tokenizedPaymentSource = new TokenizedPaymentSource(PaymentMethodType.BankAccount, "token"); - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); stripeAdapter.SetupIntentList(Arg.Is(options => options.PaymentMethod == tokenizedPaymentSource.Token)).Returns([ @@ -1187,8 +1147,6 @@ public class ProviderBillingServiceTests var tokenizedPaymentSource = new TokenizedPaymentSource(PaymentMethodType.PayPal, "token"); - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); sutProvider.GetDependency().CreateBraintreeCustomer(provider, tokenizedPaymentSource.Token) .Returns("braintree_customer_id"); @@ -1241,8 +1199,6 @@ public class ProviderBillingServiceTests var tokenizedPaymentSource = new TokenizedPaymentSource(PaymentMethodType.Card, "token"); - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); stripeAdapter.CustomerCreateAsync(Arg.Is(o => o.Address.Country == taxInfo.BillingAddressCountry && @@ -1293,8 +1249,6 @@ public class ProviderBillingServiceTests var tokenizedPaymentSource = new TokenizedPaymentSource(PaymentMethodType.Card, "token"); - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); sutProvider.GetDependency() .IsEnabled(FeatureFlagKeys.PM21092_SetNonUSBusinessUseToReverseCharge).Returns(true); @@ -1327,7 +1281,8 @@ public class ProviderBillingServiceTests public async Task SetupCustomer_Throws_BadRequestException_WhenTaxIdIsInvalid( SutProvider sutProvider, Provider provider, - TaxInfo taxInfo) + TaxInfo taxInfo, + TokenizedPaymentSource tokenizedPaymentSource) { provider.Name = "MSP"; @@ -1340,7 +1295,7 @@ public class ProviderBillingServiceTests .Returns((string)null); var actual = await Assert.ThrowsAsync(async () => - await sutProvider.Sut.SetupCustomer(provider, taxInfo)); + await sutProvider.Sut.SetupCustomer(provider, taxInfo, tokenizedPaymentSource)); Assert.IsType(actual); Assert.Equal("billingTaxIdTypeInferenceError", actual.Message); @@ -1616,8 +1571,6 @@ public class ProviderBillingServiceTests var expected = new Subscription { Id = "subscription_id", Status = StripeConstants.SubscriptionStatus.Active }; - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); sutProvider.GetDependency().SubscriptionCreateAsync(Arg.Is( sub => @@ -1694,8 +1647,6 @@ public class ProviderBillingServiceTests var expected = new Subscription { Id = "subscription_id", Status = StripeConstants.SubscriptionStatus.Active }; - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); const string setupIntentId = "seti_123"; @@ -1797,8 +1748,6 @@ public class ProviderBillingServiceTests var expected = new Subscription { Id = "subscription_id", Status = StripeConstants.SubscriptionStatus.Active }; - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); sutProvider.GetDependency().SubscriptionCreateAsync(Arg.Is( sub => @@ -1877,8 +1826,6 @@ public class ProviderBillingServiceTests var expected = new Subscription { Id = "subscription_id", Status = StripeConstants.SubscriptionStatus.Active }; - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PM19956_RequireProviderPaymentMethodDuringSetup).Returns(true); sutProvider.GetDependency() .IsEnabled(FeatureFlagKeys.PM21092_SetNonUSBusinessUseToReverseCharge).Returns(true); diff --git a/src/Core/Billing/Providers/Migration/Services/Implementations/ProviderMigrator.cs b/src/Core/Billing/Providers/Migration/Services/Implementations/ProviderMigrator.cs index 3a33f96dab..07a057d40c 100644 --- a/src/Core/Billing/Providers/Migration/Services/Implementations/ProviderMigrator.cs +++ b/src/Core/Billing/Providers/Migration/Services/Implementations/ProviderMigrator.cs @@ -7,11 +7,13 @@ using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Constants; using Bit.Core.Billing.Enums; +using Bit.Core.Billing.Models; using Bit.Core.Billing.Providers.Entities; using Bit.Core.Billing.Providers.Migration.Models; using Bit.Core.Billing.Providers.Models; using Bit.Core.Billing.Providers.Repositories; using Bit.Core.Billing.Providers.Services; +using Bit.Core.Enums; using Bit.Core.Repositories; using Bit.Core.Services; using Microsoft.Extensions.Logging; @@ -253,7 +255,10 @@ public class ProviderMigrator( var taxInfo = await paymentService.GetTaxInfoAsync(sampleOrganization); - var customer = await providerBillingService.SetupCustomer(provider, taxInfo); + // Create dummy payment source for legacy migration - this migrator is deprecated and will be removed + var dummyPaymentSource = new TokenizedPaymentSource(PaymentMethodType.Card, "migration_dummy_token"); + + var customer = await providerBillingService.SetupCustomer(provider, taxInfo, dummyPaymentSource); await stripeAdapter.CustomerUpdateAsync(customer.Id, new CustomerUpdateOptions { diff --git a/src/Core/Billing/Providers/Services/IProviderBillingService.cs b/src/Core/Billing/Providers/Services/IProviderBillingService.cs index 518fa1ba98..173249f79f 100644 --- a/src/Core/Billing/Providers/Services/IProviderBillingService.cs +++ b/src/Core/Billing/Providers/Services/IProviderBillingService.cs @@ -88,7 +88,7 @@ public interface IProviderBillingService Task SetupCustomer( Provider provider, TaxInfo taxInfo, - TokenizedPaymentSource tokenizedPaymentSource = null); + TokenizedPaymentSource tokenizedPaymentSource); /// /// For use during the provider setup process, this method starts a Stripe for the given . diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 2f66df9d22..1636faf86f 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -153,7 +153,6 @@ public static class FeatureFlagKeys public const string PM12276Breadcrumbing = "pm-12276-breadcrumbing-for-business-features"; public const string PM19422_AllowAutomaticTaxUpdates = "pm-19422-allow-automatic-tax-updates"; public const string PM199566_UpdateMSPToChargeAutomatically = "pm-199566-update-msp-to-charge-automatically"; - public const string PM19956_RequireProviderPaymentMethodDuringSetup = "pm-19956-require-provider-payment-method-during-setup"; public const string UseOrganizationWarningsService = "use-organization-warnings-service"; public const string PM20322_AllowTrialLength0 = "pm-20322-allow-trial-length-0"; public const string PM21092_SetNonUSBusinessUseToReverseCharge = "pm-21092-set-non-us-business-use-to-reverse-charge";