From ac4ccafe194c3c7a1ea77a21f84b604af0f136ad Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Thu, 9 May 2024 09:20:02 -0400 Subject: [PATCH] [AC-2471] Prevent calls to Stripe when unlinking client org has no Stripe objects (#3999) * Prevent calls to Stripe when unlinking client org has no Stripe objects * Thomas' feedback * Check for stripe when org unlinked from org page --------- Co-authored-by: Conner Turnbull --- .../RemoveOrganizationFromProviderCommand.cs | 45 +++++++++++++---- ...oveOrganizationFromProviderCommandTests.cs | 50 +++++++++++++++++++ .../Controllers/OrganizationsController.cs | 5 +- .../ProviderOrganizationsController.cs | 6 ++- .../ProviderOrganizationsController.cs | 6 ++- .../Billing/Extensions/BillingExtensions.cs | 4 ++ 6 files changed, 103 insertions(+), 13 deletions(-) diff --git a/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs b/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs index 9207c64ac..3fb0b0598 100644 --- a/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs +++ b/bitwarden_license/src/Commercial.Core/AdminConsole/Providers/RemoveOrganizationFromProviderCommand.cs @@ -6,6 +6,7 @@ using Bit.Core.AdminConsole.Providers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Commands; using Bit.Core.Billing.Constants; +using Bit.Core.Billing.Extensions; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; @@ -76,6 +77,35 @@ public class RemoveOrganizationFromProviderCommand : IRemoveOrganizationFromProv organization.BillingEmail = organizationOwnerEmails.MinBy(email => email); + await ResetOrganizationBillingAsync(organization, provider, organizationOwnerEmails); + + await _organizationRepository.ReplaceAsync(organization); + + await _providerOrganizationRepository.DeleteAsync(providerOrganization); + + await _eventService.LogProviderOrganizationEventAsync( + providerOrganization, + EventType.ProviderOrganization_Removed); + } + + /// + /// When a client organization is unlinked from a provider, we have to check if they're Stripe-enabled + /// and, if they are, we remove their MSP discount and set their Subscription to `send_invoice`. This is because + /// the provider's payment method will be removed from their Stripe customer causing ensuing charges to fail. Lastly, + /// we email the organization owners letting them know they need to add a new payment method. + /// + private async Task ResetOrganizationBillingAsync( + Organization organization, + Provider provider, + IEnumerable organizationOwnerEmails) + { + if (!organization.IsStripeEnabled()) + { + return; + } + + var isConsolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); + var customerUpdateOptions = new CustomerUpdateOptions { Coupon = string.Empty, @@ -84,11 +114,10 @@ public class RemoveOrganizationFromProviderCommand : IRemoveOrganizationFromProv await _stripeAdapter.CustomerUpdateAsync(organization.GatewayCustomerId, customerUpdateOptions); - var isConsolidatedBillingEnabled = _featureService.IsEnabled(FeatureFlagKeys.EnableConsolidatedBilling); - if (isConsolidatedBillingEnabled && provider.Status == ProviderStatusType.Billable) { var plan = StaticStore.GetPlan(organization.PlanType).PasswordManager; + var subscriptionCreateOptions = new SubscriptionCreateOptions { Customer = organization.GatewayCustomerId, @@ -103,8 +132,11 @@ public class RemoveOrganizationFromProviderCommand : IRemoveOrganizationFromProv ProrationBehavior = StripeConstants.ProrationBehavior.CreateProrations, Items = [new SubscriptionItemOptions { Price = plan.StripeSeatPlanId, Quantity = organization.Seats }] }; + var subscription = await _stripeAdapter.SubscriptionCreateAsync(subscriptionCreateOptions); + organization.GatewaySubscriptionId = subscription.Id; + await _scaleSeatsCommand.ScalePasswordManagerSeats(provider, organization.PlanType, -(organization.Seats ?? 0)); } @@ -115,21 +147,14 @@ public class RemoveOrganizationFromProviderCommand : IRemoveOrganizationFromProv CollectionMethod = "send_invoice", DaysUntilDue = 30 }; + await _stripeAdapter.SubscriptionUpdateAsync(organization.GatewaySubscriptionId, subscriptionUpdateOptions); } - await _organizationRepository.ReplaceAsync(organization); - await _mailService.SendProviderUpdatePaymentMethod( organization.Id, organization.Name, provider.Name, organizationOwnerEmails); - - await _providerOrganizationRepository.DeleteAsync(providerOrganization); - - await _eventService.LogProviderOrganizationEventAsync( - providerOrganization, - EventType.ProviderOrganization_Removed); } } diff --git a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs index e175b653d..e5b0a4e3d 100644 --- a/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/AdminConsole/ProviderFeatures/RemoveOrganizationFromProviderCommandTests.cs @@ -14,6 +14,7 @@ using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; using Stripe; using Xunit; +using IMailService = Bit.Core.Services.IMailService; namespace Bit.Commercial.Core.Test.AdminConsole.ProviderFeatures; @@ -83,6 +84,55 @@ public class RemoveOrganizationFromProviderCommandTests Assert.Equal("Organization must have at least one confirmed owner.", exception.Message); } + [Theory, BitAutoData] + public async Task RemoveOrganizationFromProvider_NoStripeObjects_MakesCorrectInvocations( + Provider provider, + ProviderOrganization providerOrganization, + Organization organization, + SutProvider sutProvider) + { + organization.GatewayCustomerId = null; + organization.GatewaySubscriptionId = null; + + providerOrganization.ProviderId = provider.Id; + + var organizationRepository = sutProvider.GetDependency(); + + sutProvider.GetDependency().HasConfirmedOwnersExceptAsync( + providerOrganization.OrganizationId, + Array.Empty(), + includeProvider: false) + .Returns(true); + + var organizationOwnerEmails = new List { "a@gmail.com", "b@gmail.com" }; + + organizationRepository.GetOwnerEmailAddressesById(organization.Id).Returns(organizationOwnerEmails); + + await sutProvider.Sut.RemoveOrganizationFromProvider(provider, providerOrganization, organization); + + await organizationRepository.Received(1).ReplaceAsync(Arg.Is( + org => org.Id == organization.Id && org.BillingEmail == "a@gmail.com")); + + var stripeAdapter = sutProvider.GetDependency(); + + await stripeAdapter.DidNotReceiveWithAnyArgs().CustomerUpdateAsync(Arg.Any(), Arg.Any()); + + await stripeAdapter.DidNotReceiveWithAnyArgs().SubscriptionUpdateAsync(Arg.Any(), Arg.Any()); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().SendProviderUpdatePaymentMethod( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + + await sutProvider.GetDependency().Received(1) + .DeleteAsync(providerOrganization); + + await sutProvider.GetDependency().Received(1).LogProviderOrganizationEventAsync( + providerOrganization, + EventType.ProviderOrganization_Removed); + } + [Theory, BitAutoData] public async Task RemoveOrganizationFromProvider_MakesCorrectInvocations__FeatureFlagOff( Provider provider, diff --git a/src/Admin/AdminConsole/Controllers/OrganizationsController.cs b/src/Admin/AdminConsole/Controllers/OrganizationsController.cs index 5d89be0c4..19b450686 100644 --- a/src/Admin/AdminConsole/Controllers/OrganizationsController.cs +++ b/src/Admin/AdminConsole/Controllers/OrganizationsController.cs @@ -349,7 +349,10 @@ public class OrganizationsController : Controller providerOrganization, organization); - await _removePaymentMethodCommand.RemovePaymentMethod(organization); + if (organization.IsStripeEnabled()) + { + await _removePaymentMethodCommand.RemovePaymentMethod(organization); + } return Json(null); } diff --git a/src/Admin/AdminConsole/Controllers/ProviderOrganizationsController.cs b/src/Admin/AdminConsole/Controllers/ProviderOrganizationsController.cs index a017b2b43..b143a2c42 100644 --- a/src/Admin/AdminConsole/Controllers/ProviderOrganizationsController.cs +++ b/src/Admin/AdminConsole/Controllers/ProviderOrganizationsController.cs @@ -3,6 +3,7 @@ using Bit.Admin.Utilities; using Bit.Core.AdminConsole.Providers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Commands; +using Bit.Core.Billing.Extensions; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Utilities; @@ -69,7 +70,10 @@ public class ProviderOrganizationsController : Controller } - await _removePaymentMethodCommand.RemovePaymentMethod(organization); + if (organization.IsStripeEnabled()) + { + await _removePaymentMethodCommand.RemovePaymentMethod(organization); + } return Json(null); } diff --git a/src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs b/src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs index 136119848..e2becc9b8 100644 --- a/src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs +++ b/src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs @@ -5,6 +5,7 @@ using Bit.Core.AdminConsole.Providers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; using Bit.Core.Billing.Commands; +using Bit.Core.Billing.Extensions; using Bit.Core.Context; using Bit.Core.Exceptions; using Bit.Core.Repositories; @@ -112,6 +113,9 @@ public class ProviderOrganizationsController : Controller providerOrganization, organization); - await _removePaymentMethodCommand.RemovePaymentMethod(organization); + if (organization.IsStripeEnabled()) + { + await _removePaymentMethodCommand.RemovePaymentMethod(organization); + } } } diff --git a/src/Core/Billing/Extensions/BillingExtensions.cs b/src/Core/Billing/Extensions/BillingExtensions.cs index 9b1bb9e92..f0ee8989c 100644 --- a/src/Core/Billing/Extensions/BillingExtensions.cs +++ b/src/Core/Billing/Extensions/BillingExtensions.cs @@ -22,6 +22,10 @@ public static class BillingExtensions PlanType: PlanType.TeamsMonthly or PlanType.EnterpriseMonthly }; + public static bool IsStripeEnabled(this Organization organization) + => !string.IsNullOrEmpty(organization.GatewayCustomerId) && + !string.IsNullOrEmpty(organization.GatewaySubscriptionId); + public static bool SupportsConsolidatedBilling(this PlanType planType) => planType is PlanType.TeamsMonthly or PlanType.EnterpriseMonthly; }