Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AC-2471] Prevent calls to Stripe when unlinking client org has no Stripe objects #3999

Merged
merged 10 commits into from
May 9, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Bit.Core.AdminConsole.Entities.Provider;
using Bit.Core.AdminConsole.Providers.Interfaces;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Billing.Extensions;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Repositories;
Expand Down Expand Up @@ -67,32 +68,49 @@ public class RemoveOrganizationFromProviderCommand : IRemoveOrganizationFromProv

await _organizationRepository.ReplaceAsync(organization);

var customerUpdateOptions = new CustomerUpdateOptions
{
Coupon = string.Empty,
Email = organization.BillingEmail
};
await ResetOrganizationBillingAsync(organization, provider.Name, organizationOwnerEmails);

await _providerOrganizationRepository.DeleteAsync(providerOrganization);

await _stripeAdapter.CustomerUpdateAsync(organization.GatewayCustomerId, customerUpdateOptions);
await _eventService.LogProviderOrganizationEventAsync(
providerOrganization,
EventType.ProviderOrganization_Removed);
}

var subscriptionUpdateOptions = new SubscriptionUpdateOptions
/// <summary>
/// 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.
/// </summary>
private async Task ResetOrganizationBillingAsync(
Organization organization,
string providerName,
IEnumerable<string> organizationOwnerEmails)
{
if (organization.IsStripeEnabled())
{
CollectionMethod = "send_invoice",
DaysUntilDue = 30
};
var customerUpdateOptions = new CustomerUpdateOptions
{
Coupon = string.Empty,
Email = organization.BillingEmail
};

await _stripeAdapter.SubscriptionUpdateAsync(organization.GatewaySubscriptionId, subscriptionUpdateOptions);
await _stripeAdapter.CustomerUpdateAsync(organization.GatewayCustomerId, customerUpdateOptions);

await _mailService.SendProviderUpdatePaymentMethod(
organization.Id,
organization.Name,
provider.Name,
organizationOwnerEmails);
var subscriptionUpdateOptions = new SubscriptionUpdateOptions
{
CollectionMethod = "send_invoice",
DaysUntilDue = 30
};

await _providerOrganizationRepository.DeleteAsync(providerOrganization);
await _stripeAdapter.SubscriptionUpdateAsync(organization.GatewaySubscriptionId, subscriptionUpdateOptions);

await _eventService.LogProviderOrganizationEventAsync(
providerOrganization,
EventType.ProviderOrganization_Removed);
await _mailService.SendProviderUpdatePaymentMethod(
organization.Id,
organization.Name,
providerName,
organizationOwnerEmails);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using NSubstitute;
using Stripe;
using Xunit;
using IMailService = Bit.Core.Services.IMailService;

namespace Bit.Commercial.Core.Test.AdminConsole.ProviderFeatures;

Expand Down Expand Up @@ -80,6 +81,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<RemoveOrganizationFromProviderCommand> sutProvider)
{
organization.GatewayCustomerId = null;
organization.GatewaySubscriptionId = null;

providerOrganization.ProviderId = provider.Id;

var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();

sutProvider.GetDependency<IOrganizationService>().HasConfirmedOwnersExceptAsync(
providerOrganization.OrganizationId,
Array.Empty<Guid>(),
includeProvider: false)
.Returns(true);

var organizationOwnerEmails = new List<string> { "[email protected]", "[email protected]" };

organizationRepository.GetOwnerEmailAddressesById(organization.Id).Returns(organizationOwnerEmails);

await sutProvider.Sut.RemoveOrganizationFromProvider(provider, providerOrganization, organization);

await organizationRepository.Received(1).ReplaceAsync(Arg.Is<Organization>(
org => org.Id == organization.Id && org.BillingEmail == "[email protected]"));

var stripeAdapter = sutProvider.GetDependency<IStripeAdapter>();

await stripeAdapter.DidNotReceiveWithAnyArgs().CustomerUpdateAsync(Arg.Any<string>(), Arg.Any<CustomerUpdateOptions>());

await stripeAdapter.DidNotReceiveWithAnyArgs().SubscriptionUpdateAsync(Arg.Any<string>(), Arg.Any<SubscriptionUpdateOptions>());

await sutProvider.GetDependency<IMailService>().DidNotReceiveWithAnyArgs().SendProviderUpdatePaymentMethod(
Arg.Any<Guid>(),
Arg.Any<string>(),
Arg.Any<string>(),
Arg.Any<IEnumerable<string>>());

await sutProvider.GetDependency<IProviderOrganizationRepository>().Received(1)
.DeleteAsync(providerOrganization);

await sutProvider.GetDependency<IEventService>().Received(1).LogProviderOrganizationEventAsync(
providerOrganization,
EventType.ProviderOrganization_Removed);
}

[Theory, BitAutoData]
public async Task RemoveOrganizationFromProvider_MakesCorrectInvocations(
Provider provider,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@
providerOrganization,
organization);

await _removePaymentMethodCommand.RemovePaymentMethod(organization);
if (!string.IsNullOrEmpty(organization.GatewayCustomerId) &&
!string.IsNullOrEmpty(organization.GatewaySubscriptionId))
{
await _removePaymentMethodCommand.RemovePaymentMethod(organization);
}

Check warning on line 67 in src/Admin/AdminConsole/Controllers/ProviderOrganizationsController.cs

View check run for this annotation

Codecov / codecov/patch

src/Admin/AdminConsole/Controllers/ProviderOrganizationsController.cs#L64-L67

Added lines #L64 - L67 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be part of the command? Or are you trying to avoid calling a command from within a command?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, originally I had RemoveOrganizationFromProviderCommand invoke RemovePaymentMethodCommand, but I was told not to do so when this was originally implemented.


return Json(null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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;
Expand Down Expand Up @@ -112,6 +113,9 @@
providerOrganization,
organization);

await _removePaymentMethodCommand.RemovePaymentMethod(organization);
if (organization.IsStripeEnabled())
{
await _removePaymentMethodCommand.RemovePaymentMethod(organization);
}

Check warning on line 119 in src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs#L117-L119

Added lines #L117 - L119 were not covered by tests
}
}
4 changes: 4 additions & 0 deletions src/Core/Billing/Extensions/BillingExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ public static bool IsValidClient(this Organization organization)
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;
}