Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,44 @@ await stripeAdapter.SubscriptionUpdateAsync(provider.GatewaySubscriptionId,
}
}

public async Task UpdateProviderNameAndEmail(Provider provider)
{
if (string.IsNullOrWhiteSpace(provider.GatewayCustomerId))
{
logger.LogWarning(
"Provider ({ProviderId}) has no Stripe customer to update",
provider.Id);
return;
}

var newDisplayName = provider.DisplayName();

// Provider.DisplayName() can return null - handle gracefully
if (string.IsNullOrWhiteSpace(newDisplayName))
{
logger.LogWarning(
"Provider ({ProviderId}) has no name to update in Stripe",
provider.Id);
return;
}

await stripeAdapter.CustomerUpdateAsync(provider.GatewayCustomerId,
new CustomerUpdateOptions
{
Email = provider.BillingEmail,
Description = newDisplayName,
InvoiceSettings = new CustomerInvoiceSettingsOptions
{
CustomFields = [
new CustomerInvoiceSettingsCustomFieldOptions
{
Name = provider.SubscriberType(),
Value = newDisplayName
}]
},
});
}

private Func<int, Task> CurrySeatScalingUpdate(
Provider provider,
ProviderPlan providerPlan,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2151,4 +2151,151 @@ await stripeAdapter.Received(1).SubscriptionUpdateAsync(provider.GatewaySubscrip
}

#endregion

#region UpdateProviderNameAndEmail

[Theory, BitAutoData]
public async Task UpdateProviderNameAndEmail_NullGatewayCustomerId_LogsWarningAndReturns(
Provider provider,
SutProvider<ProviderBillingService> sutProvider)
{
// Arrange
provider.GatewayCustomerId = null;
var stripeAdapter = sutProvider.GetDependency<IStripeAdapter>();

// Act
await sutProvider.Sut.UpdateProviderNameAndEmail(provider);

// Assert
await stripeAdapter.DidNotReceive().CustomerUpdateAsync(
Arg.Any<string>(),
Arg.Any<CustomerUpdateOptions>());
}

[Theory, BitAutoData]
public async Task UpdateProviderNameAndEmail_EmptyGatewayCustomerId_LogsWarningAndReturns(
Provider provider,
SutProvider<ProviderBillingService> sutProvider)
{
// Arrange
provider.GatewayCustomerId = "";
var stripeAdapter = sutProvider.GetDependency<IStripeAdapter>();

// Act
await sutProvider.Sut.UpdateProviderNameAndEmail(provider);

// Assert
await stripeAdapter.DidNotReceive().CustomerUpdateAsync(
Arg.Any<string>(),
Arg.Any<CustomerUpdateOptions>());
}

[Theory, BitAutoData]
public async Task UpdateProviderNameAndEmail_NullProviderName_LogsWarningAndReturns(
Provider provider,
SutProvider<ProviderBillingService> sutProvider)
{
// Arrange
provider.Name = null;
provider.GatewayCustomerId = "cus_test123";
var stripeAdapter = sutProvider.GetDependency<IStripeAdapter>();

// Act
await sutProvider.Sut.UpdateProviderNameAndEmail(provider);

// Assert
await stripeAdapter.DidNotReceive().CustomerUpdateAsync(
Arg.Any<string>(),
Arg.Any<CustomerUpdateOptions>());
}

[Theory, BitAutoData]
public async Task UpdateProviderNameAndEmail_EmptyProviderName_LogsWarningAndReturns(
Provider provider,
SutProvider<ProviderBillingService> sutProvider)
{
// Arrange
provider.Name = "";
provider.GatewayCustomerId = "cus_test123";
var stripeAdapter = sutProvider.GetDependency<IStripeAdapter>();

// Act
await sutProvider.Sut.UpdateProviderNameAndEmail(provider);

// Assert
await stripeAdapter.DidNotReceive().CustomerUpdateAsync(
Arg.Any<string>(),
Arg.Any<CustomerUpdateOptions>());
}

[Theory, BitAutoData]
public async Task UpdateProviderNameAndEmail_ValidProvider_CallsStripeWithCorrectParameters(
Provider provider,
SutProvider<ProviderBillingService> sutProvider)
{
// Arrange
provider.Name = "Test Provider";
provider.BillingEmail = "[email protected]";
provider.GatewayCustomerId = "cus_test123";
var stripeAdapter = sutProvider.GetDependency<IStripeAdapter>();

// Act
await sutProvider.Sut.UpdateProviderNameAndEmail(provider);

// Assert
await stripeAdapter.Received(1).CustomerUpdateAsync(
provider.GatewayCustomerId,
Arg.Is<CustomerUpdateOptions>(options =>
options.Email == provider.BillingEmail &&
options.Description == provider.Name &&
options.InvoiceSettings.CustomFields.Count == 1 &&
options.InvoiceSettings.CustomFields[0].Name == "Provider" &&
options.InvoiceSettings.CustomFields[0].Value == provider.Name));
}

[Theory, BitAutoData]
public async Task UpdateProviderNameAndEmail_LongProviderName_UsesFullName(
Provider provider,
SutProvider<ProviderBillingService> sutProvider)
{
// Arrange
var longName = new string('A', 50); // 50 characters
provider.Name = longName;
provider.BillingEmail = "[email protected]";
provider.GatewayCustomerId = "cus_test123";
var stripeAdapter = sutProvider.GetDependency<IStripeAdapter>();

// Act
await sutProvider.Sut.UpdateProviderNameAndEmail(provider);

// Assert
await stripeAdapter.Received(1).CustomerUpdateAsync(
provider.GatewayCustomerId,
Arg.Is<CustomerUpdateOptions>(options =>
options.InvoiceSettings.CustomFields[0].Value == longName));
}

[Theory, BitAutoData]
public async Task UpdateProviderNameAndEmail_NullBillingEmail_UpdatesWithNull(
Provider provider,
SutProvider<ProviderBillingService> sutProvider)
{
// Arrange
provider.Name = "Test Provider";
provider.BillingEmail = null;
provider.GatewayCustomerId = "cus_test123";
var stripeAdapter = sutProvider.GetDependency<IStripeAdapter>();

// Act
await sutProvider.Sut.UpdateProviderNameAndEmail(provider);

// Assert
await stripeAdapter.Received(1).CustomerUpdateAsync(
provider.GatewayCustomerId,
Arg.Is<CustomerUpdateOptions>(options =>
options.Email == null &&
options.Description == provider.Name));
}

#endregion
}
25 changes: 24 additions & 1 deletion src/Admin/AdminConsole/Controllers/OrganizationsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Billing.Enums;
using Bit.Core.Billing.Extensions;
using Bit.Core.Billing.Organizations.Services;
using Bit.Core.Billing.Pricing;
using Bit.Core.Billing.Providers.Services;
using Bit.Core.Enums;
Expand Down Expand Up @@ -56,6 +57,7 @@ public class OrganizationsController : Controller
private readonly IOrganizationInitiateDeleteCommand _organizationInitiateDeleteCommand;
private readonly IPricingClient _pricingClient;
private readonly IResendOrganizationInviteCommand _resendOrganizationInviteCommand;
private readonly IOrganizationBillingService _organizationBillingService;

public OrganizationsController(
IOrganizationRepository organizationRepository,
Expand All @@ -80,7 +82,8 @@ public OrganizationsController(
IProviderBillingService providerBillingService,
IOrganizationInitiateDeleteCommand organizationInitiateDeleteCommand,
IPricingClient pricingClient,
IResendOrganizationInviteCommand resendOrganizationInviteCommand)
IResendOrganizationInviteCommand resendOrganizationInviteCommand,
IOrganizationBillingService organizationBillingService)
{
_organizationRepository = organizationRepository;
_organizationUserRepository = organizationUserRepository;
Expand All @@ -105,6 +108,7 @@ public OrganizationsController(
_organizationInitiateDeleteCommand = organizationInitiateDeleteCommand;
_pricingClient = pricingClient;
_resendOrganizationInviteCommand = resendOrganizationInviteCommand;
_organizationBillingService = organizationBillingService;
}

[RequirePermission(Permission.Org_List_View)]
Expand Down Expand Up @@ -241,6 +245,8 @@ public async Task<IActionResult> Edit(Guid id, OrganizationEditModel model)
var existingOrganizationData = new Organization
{
Id = organization.Id,
Name = organization.Name,
BillingEmail = organization.BillingEmail,
Status = organization.Status,
PlanType = organization.PlanType,
Seats = organization.Seats
Expand Down Expand Up @@ -286,6 +292,23 @@ await HandlePotentialProviderSeatScalingAsync(

await _applicationCacheService.UpsertOrganizationAbilityAsync(organization);

// Sync name/email changes to Stripe
if (!string.IsNullOrEmpty(organization.GatewayCustomerId) &&
(existingOrganizationData.Name != organization.Name || existingOrganizationData.BillingEmail != organization.BillingEmail))
{
try
{
await _organizationBillingService.UpdateOrganizationNameAndEmail(organization);
}
catch (Exception ex)
{
_logger.LogError(ex,
"Failed to update Stripe customer for organization {OrganizationId}. Database was updated successfully.",
organization.Id);
TempData["Warning"] = "Organization updated successfully, but Stripe customer name/email synchronization failed.";
}
}

return RedirectToAction("Edit", new { id });
}

Expand Down
24 changes: 23 additions & 1 deletion src/Admin/AdminConsole/Controllers/ProvidersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class ProvidersController : Controller
private readonly IStripeAdapter _stripeAdapter;
private readonly IAccessControlService _accessControlService;
private readonly ISubscriberService _subscriberService;
private readonly ILogger<ProvidersController> _logger;

public ProvidersController(IOrganizationRepository organizationRepository,
IResellerClientOrganizationSignUpCommand resellerClientOrganizationSignUpCommand,
Expand All @@ -72,7 +73,8 @@ public ProvidersController(IOrganizationRepository organizationRepository,
IPricingClient pricingClient,
IStripeAdapter stripeAdapter,
IAccessControlService accessControlService,
ISubscriberService subscriberService)
ISubscriberService subscriberService,
ILogger<ProvidersController> logger)
{
_organizationRepository = organizationRepository;
_resellerClientOrganizationSignUpCommand = resellerClientOrganizationSignUpCommand;
Expand All @@ -92,6 +94,7 @@ public ProvidersController(IOrganizationRepository organizationRepository,
_braintreeMerchantUrl = webHostEnvironment.GetBraintreeMerchantUrl();
_braintreeMerchantId = globalSettings.Braintree.MerchantId;
_subscriberService = subscriberService;
_logger = logger;
}

[RequirePermission(Permission.Provider_List_View)]
Expand Down Expand Up @@ -296,6 +299,9 @@ public async Task<IActionResult> Edit(Guid id, ProviderEditModel model)

var originalProviderStatus = provider.Enabled;

// Capture original billing email before modifications for Stripe sync
var originalBillingEmail = provider.BillingEmail;

model.ToProvider(provider);

// validate the stripe ids to prevent saving a bad one
Expand All @@ -321,6 +327,22 @@ public async Task<IActionResult> Edit(Guid id, ProviderEditModel model)
await _providerService.UpdateAsync(provider);
await _applicationCacheService.UpsertProviderAbilityAsync(provider);

// Sync billing email changes to Stripe
if (!string.IsNullOrEmpty(provider.GatewayCustomerId) && originalBillingEmail != provider.BillingEmail)
{
try
{
await _providerBillingService.UpdateProviderNameAndEmail(provider);
}
catch (Exception ex)
{
_logger.LogError(ex,
"Failed to update Stripe customer for provider {ProviderId}. Database was updated successfully.",
provider.Id);
TempData["Warning"] = "Provider updated successfully, but Stripe customer email synchronization failed.";
}
}

if (!provider.IsBillable())
{
return RedirectToAction("Edit", new { id });
Expand Down
29 changes: 28 additions & 1 deletion src/Api/AdminConsole/Controllers/ProvidersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Bit.Api.AdminConsole.Models.Response.Providers;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.AdminConsole.Services;
using Bit.Core.Billing.Providers.Services;
using Bit.Core.Context;
using Bit.Core.Exceptions;
using Bit.Core.Services;
Expand All @@ -23,15 +24,20 @@ public class ProvidersController : Controller
private readonly IProviderService _providerService;
private readonly ICurrentContext _currentContext;
private readonly GlobalSettings _globalSettings;
private readonly IProviderBillingService _providerBillingService;
private readonly ILogger<ProvidersController> _logger;

public ProvidersController(IUserService userService, IProviderRepository providerRepository,
IProviderService providerService, ICurrentContext currentContext, GlobalSettings globalSettings)
IProviderService providerService, ICurrentContext currentContext, GlobalSettings globalSettings,
IProviderBillingService providerBillingService, ILogger<ProvidersController> logger)
{
_userService = userService;
_providerRepository = providerRepository;
_providerService = providerService;
_currentContext = currentContext;
_globalSettings = globalSettings;
_providerBillingService = providerBillingService;
_logger = logger;
}

[HttpGet("{id:guid}")]
Expand Down Expand Up @@ -65,7 +71,28 @@ public async Task<ProviderResponseModel> Put(Guid id, [FromBody] ProviderUpdateR
throw new NotFoundException();
}

// Capture original values before modifications for Stripe sync
var originalName = provider.Name;
var originalBillingEmail = provider.BillingEmail;

await _providerService.UpdateAsync(model.ToProvider(provider, _globalSettings));

// Sync name/email changes to Stripe
if (!string.IsNullOrEmpty(provider.GatewayCustomerId) &&
(originalName != provider.Name || originalBillingEmail != provider.BillingEmail))
Copy link
Member

Choose a reason for hiding this comment

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

๐ŸŽจ (non-blocking) This bit of boilerplate is repeated a few times. Is there a way to simplify things for the callers?

  • the BillingServices no longer throw if the GatewayCustomerId is null. Can we remove that check from the callers? (That has another benefit, which is that callers don't have to know about the significance of this property, which is effectively billing metadata.)
  • (less sure about this) is there a better way to track / handle the "has the name or billing email changed" logic? It's a shame that cloning objects is so difficult in C#.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the Customer ID check from the callers here: 8982bcb

With regards to your second bullet, I didn't see a way to do it that would constitute any meaningful difference. Could use a new DTO to capture previous name / email prior to the database update, but that's the same process with a different flavor.

{
try
{
await _providerBillingService.UpdateProviderNameAndEmail(provider);
}
catch (Exception ex)
{
_logger.LogError(ex,
"Failed to update Stripe customer for provider {ProviderId}. Database was updated successfully.",
provider.Id);
}
}

return new ProviderResponseModel(provider);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ Task UpdatePaymentMethod(
/// Updates the organization name and email on the Stripe customer entry.
/// This only updates Stripe, not the Bitwarden database.
/// </summary>
/// <remarks>
/// The caller should ensure that the organization has a GatewayCustomerId before calling this method.
/// </remarks>
/// <param name="organization">The organization to update in Stripe.</param>
/// <exception cref="BillingException">Thrown when the organization does not have a GatewayCustomerId.</exception>
Task UpdateOrganizationNameAndEmail(Organization organization);
}
Loading
Loading