mirror of
https://github.com/bitwarden/server
synced 2025-12-06 00:03:34 +00:00
[PM-22980] Organization name not updated in Stripe when organization name is changed (#6189)
* tests: add tests for UpdateAsync change * fix: update Stripe customer object update * refactor: replace CustomerService objects with stripeAdapter * refactor: simplify controller logic * fix: mark businessname and it's function obsolete for future use * fix: pr feedback remove business name check * refactor: remove unused functions in organizationservice
This commit is contained in:
@@ -12,6 +12,7 @@ using Bit.Api.Models.Request.Accounts;
|
||||
using Bit.Api.Models.Request.Organizations;
|
||||
using Bit.Api.Models.Response;
|
||||
using Bit.Core;
|
||||
using Bit.Core.AdminConsole.Entities;
|
||||
using Bit.Core.AdminConsole.Enums;
|
||||
using Bit.Core.AdminConsole.Models.Business.Tokenables;
|
||||
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
|
||||
@@ -235,8 +236,7 @@ public class OrganizationsController : Controller
|
||||
throw new NotFoundException();
|
||||
}
|
||||
|
||||
var updateBilling = !_globalSettings.SelfHosted && (model.BusinessName != organization.DisplayBusinessName() ||
|
||||
model.BillingEmail != organization.BillingEmail);
|
||||
var updateBilling = ShouldUpdateBilling(model, organization);
|
||||
|
||||
var hasRequiredPermissions = updateBilling
|
||||
? await _currentContext.EditSubscription(orgIdGuid)
|
||||
@@ -582,4 +582,11 @@ public class OrganizationsController : Controller
|
||||
|
||||
return organization.PlanType;
|
||||
}
|
||||
|
||||
private bool ShouldUpdateBilling(OrganizationUpdateRequestModel model, Organization organization)
|
||||
{
|
||||
var organizationNameChanged = model.Name != organization.Name;
|
||||
var billingEmailChanged = model.BillingEmail != organization.BillingEmail;
|
||||
return !_globalSettings.SelfHosted && (organizationNameChanged || billingEmailChanged);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -30,6 +30,7 @@ public class Organization : ITableObject<Guid>, IStorableSubscriber, IRevisable
|
||||
/// This value is HTML encoded. For display purposes use the method DisplayBusinessName() instead.
|
||||
/// </summary>
|
||||
[MaxLength(50)]
|
||||
[Obsolete("This property has been deprecated. Use the 'Name' property instead.")]
|
||||
public string? BusinessName { get; set; }
|
||||
[MaxLength(50)]
|
||||
public string? BusinessAddress1 { get; set; }
|
||||
@@ -147,6 +148,8 @@ public class Organization : ITableObject<Guid>, IStorableSubscriber, IRevisable
|
||||
/// <summary>
|
||||
/// Returns the business name of the organization, HTML decoded ready for display.
|
||||
/// </summary>
|
||||
///
|
||||
[Obsolete("This method has been deprecated. Use the 'DisplayName()' method instead.")]
|
||||
public string? DisplayBusinessName()
|
||||
{
|
||||
return WebUtility.HtmlDecode(BusinessName);
|
||||
|
||||
@@ -13,7 +13,6 @@ namespace Bit.Core.Services;
|
||||
|
||||
public interface IOrganizationService
|
||||
{
|
||||
Task CancelSubscriptionAsync(Guid organizationId, bool? endOfPeriod = null);
|
||||
Task ReinstateSubscriptionAsync(Guid organizationId);
|
||||
Task<string> AdjustStorageAsync(Guid organizationId, short storageAdjustmentGb);
|
||||
Task UpdateSubscription(Guid organizationId, int seatAdjustment, int? maxAutoscaleSeats);
|
||||
|
||||
@@ -65,6 +65,7 @@ public class OrganizationService : IOrganizationService
|
||||
private readonly IPricingClient _pricingClient;
|
||||
private readonly IPolicyRequirementQuery _policyRequirementQuery;
|
||||
private readonly ISendOrganizationInvitesCommand _sendOrganizationInvitesCommand;
|
||||
private readonly IStripeAdapter _stripeAdapter;
|
||||
|
||||
public OrganizationService(
|
||||
IOrganizationRepository organizationRepository,
|
||||
@@ -90,7 +91,8 @@ public class OrganizationService : IOrganizationService
|
||||
IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery,
|
||||
IPricingClient pricingClient,
|
||||
IPolicyRequirementQuery policyRequirementQuery,
|
||||
ISendOrganizationInvitesCommand sendOrganizationInvitesCommand
|
||||
ISendOrganizationInvitesCommand sendOrganizationInvitesCommand,
|
||||
IStripeAdapter stripeAdapter
|
||||
)
|
||||
{
|
||||
_organizationRepository = organizationRepository;
|
||||
@@ -117,24 +119,7 @@ public class OrganizationService : IOrganizationService
|
||||
_pricingClient = pricingClient;
|
||||
_policyRequirementQuery = policyRequirementQuery;
|
||||
_sendOrganizationInvitesCommand = sendOrganizationInvitesCommand;
|
||||
}
|
||||
|
||||
public async Task CancelSubscriptionAsync(Guid organizationId, bool? endOfPeriod = null)
|
||||
{
|
||||
var organization = await GetOrgById(organizationId);
|
||||
if (organization == null)
|
||||
{
|
||||
throw new NotFoundException();
|
||||
}
|
||||
|
||||
var eop = endOfPeriod.GetValueOrDefault(true);
|
||||
if (!endOfPeriod.HasValue && organization.ExpirationDate.HasValue &&
|
||||
organization.ExpirationDate.Value < DateTime.UtcNow)
|
||||
{
|
||||
eop = false;
|
||||
}
|
||||
|
||||
await _paymentService.CancelSubscriptionAsync(organization, eop);
|
||||
_stripeAdapter = stripeAdapter;
|
||||
}
|
||||
|
||||
public async Task ReinstateSubscriptionAsync(Guid organizationId)
|
||||
@@ -355,8 +340,7 @@ public class OrganizationService : IOrganizationService
|
||||
}
|
||||
|
||||
var bankService = new BankAccountService();
|
||||
var customerService = new CustomerService();
|
||||
var customer = await customerService.GetAsync(organization.GatewayCustomerId,
|
||||
var customer = await _stripeAdapter.CustomerGetAsync(organization.GatewayCustomerId,
|
||||
new CustomerGetOptions { Expand = new List<string> { "sources" } });
|
||||
if (customer == null)
|
||||
{
|
||||
@@ -417,12 +401,25 @@ public class OrganizationService : IOrganizationService
|
||||
|
||||
if (updateBilling && !string.IsNullOrWhiteSpace(organization.GatewayCustomerId))
|
||||
{
|
||||
var customerService = new CustomerService();
|
||||
await customerService.UpdateAsync(organization.GatewayCustomerId,
|
||||
var newDisplayName = organization.DisplayName();
|
||||
|
||||
await _stripeAdapter.CustomerUpdateAsync(organization.GatewayCustomerId,
|
||||
new CustomerUpdateOptions
|
||||
{
|
||||
Email = organization.BillingEmail,
|
||||
Description = organization.DisplayBusinessName()
|
||||
Description = organization.DisplayBusinessName(),
|
||||
InvoiceSettings = new CustomerInvoiceSettingsOptions
|
||||
{
|
||||
// This overwrites the existing custom fields for this organization
|
||||
CustomFields = [
|
||||
new CustomerInvoiceSettingsCustomFieldOptions
|
||||
{
|
||||
Name = organization.SubscriberType(),
|
||||
Value = newDisplayName.Length <= 30
|
||||
? newDisplayName
|
||||
: newDisplayName[..30]
|
||||
}]
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -27,7 +27,9 @@ using Bit.Test.Common.AutoFixture.Attributes;
|
||||
using Bit.Test.Common.Fakes;
|
||||
using NSubstitute;
|
||||
using NSubstitute.ExceptionExtensions;
|
||||
using NSubstitute.ReceivedExtensions;
|
||||
using NSubstitute.ReturnsExtensions;
|
||||
using Stripe;
|
||||
using Xunit;
|
||||
using Organization = Bit.Core.AdminConsole.Entities.Organization;
|
||||
using OrganizationUser = Bit.Core.Entities.OrganizationUser;
|
||||
@@ -1235,6 +1237,130 @@ public class OrganizationServiceTests
|
||||
await sutProvider.Sut.ValidateOrganizationCustomPermissionsEnabledAsync(organization.Id, OrganizationUserType.Custom);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task UpdateAsync_WhenValidOrganization_AndUpdateBillingIsTrue_UpdateStripeCustomerAndOrganization(Organization organization, SutProvider<OrganizationService> sutProvider)
|
||||
{
|
||||
// Arrange
|
||||
var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
|
||||
var applicationCacheService = sutProvider.GetDependency<IApplicationCacheService>();
|
||||
var stripeAdapter = sutProvider.GetDependency<IStripeAdapter>();
|
||||
var eventService = sutProvider.GetDependency<IEventService>();
|
||||
|
||||
var requestOptionsReturned = new CustomerUpdateOptions
|
||||
{
|
||||
Email = organization.BillingEmail,
|
||||
Description = organization.DisplayBusinessName(),
|
||||
InvoiceSettings = new CustomerInvoiceSettingsOptions
|
||||
{
|
||||
// This overwrites the existing custom fields for this organization
|
||||
CustomFields =
|
||||
[
|
||||
new CustomerInvoiceSettingsCustomFieldOptions
|
||||
{
|
||||
Name = organization.SubscriberType(),
|
||||
Value = organization.DisplayName()[..30]
|
||||
}
|
||||
]
|
||||
},
|
||||
};
|
||||
organizationRepository
|
||||
.GetByIdentifierAsync(organization.Identifier!)
|
||||
.Returns(organization);
|
||||
|
||||
// Act
|
||||
await sutProvider.Sut.UpdateAsync(organization, updateBilling: true);
|
||||
|
||||
// Assert
|
||||
await organizationRepository
|
||||
.Received(1)
|
||||
.GetByIdentifierAsync(Arg.Is<string>(id => id == organization.Identifier));
|
||||
await stripeAdapter
|
||||
.Received(1)
|
||||
.CustomerUpdateAsync(
|
||||
Arg.Is<string>(id => id == organization.GatewayCustomerId),
|
||||
Arg.Is<CustomerUpdateOptions>(options => options.Email == requestOptionsReturned.Email
|
||||
&& options.Description == requestOptionsReturned.Description
|
||||
&& options.InvoiceSettings.CustomFields.First().Name == requestOptionsReturned.InvoiceSettings.CustomFields.First().Name
|
||||
&& options.InvoiceSettings.CustomFields.First().Value == requestOptionsReturned.InvoiceSettings.CustomFields.First().Value)); ;
|
||||
await organizationRepository
|
||||
.Received(1)
|
||||
.ReplaceAsync(Arg.Is<Organization>(org => org == organization));
|
||||
await applicationCacheService
|
||||
.Received(1)
|
||||
.UpsertOrganizationAbilityAsync(Arg.Is<Organization>(org => org == organization));
|
||||
await eventService
|
||||
.Received(1)
|
||||
.LogOrganizationEventAsync(Arg.Is<Organization>(org => org == organization),
|
||||
Arg.Is<EventType>(e => e == EventType.Organization_Updated));
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task UpdateAsync_WhenValidOrganization_AndUpdateBillingIsFalse_UpdateOrganization(Organization organization, SutProvider<OrganizationService> sutProvider)
|
||||
{
|
||||
// Arrange
|
||||
var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
|
||||
var applicationCacheService = sutProvider.GetDependency<IApplicationCacheService>();
|
||||
var stripeAdapter = sutProvider.GetDependency<IStripeAdapter>();
|
||||
var eventService = sutProvider.GetDependency<IEventService>();
|
||||
|
||||
organizationRepository
|
||||
.GetByIdentifierAsync(organization.Identifier!)
|
||||
.Returns(organization);
|
||||
|
||||
// Act
|
||||
await sutProvider.Sut.UpdateAsync(organization, updateBilling: false);
|
||||
|
||||
// Assert
|
||||
await organizationRepository
|
||||
.Received(1)
|
||||
.GetByIdentifierAsync(Arg.Is<string>(id => id == organization.Identifier));
|
||||
await stripeAdapter
|
||||
.DidNotReceiveWithAnyArgs()
|
||||
.CustomerUpdateAsync(Arg.Any<string>(), Arg.Any<CustomerUpdateOptions>());
|
||||
await organizationRepository
|
||||
.Received(1)
|
||||
.ReplaceAsync(Arg.Is<Organization>(org => org == organization));
|
||||
await applicationCacheService
|
||||
.Received(1)
|
||||
.UpsertOrganizationAbilityAsync(Arg.Is<Organization>(org => org == organization));
|
||||
await eventService
|
||||
.Received(1)
|
||||
.LogOrganizationEventAsync(Arg.Is<Organization>(org => org == organization),
|
||||
Arg.Is<EventType>(e => e == EventType.Organization_Updated));
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task UpdateAsync_WhenOrganizationHasNoId_ThrowsApplicationException(Organization organization, SutProvider<OrganizationService> sutProvider)
|
||||
{
|
||||
// Arrange
|
||||
organization.Id = Guid.Empty;
|
||||
|
||||
// Act/Assert
|
||||
var exception = await Assert.ThrowsAnyAsync<ApplicationException>(() => sutProvider.Sut.UpdateAsync(organization));
|
||||
Assert.Equal("Cannot create org this way. Call SignUpAsync.", exception.Message);
|
||||
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task UpdateAsync_WhenIdentifierAlreadyExistsForADifferentOrganization_ThrowsBadRequestException(Organization organization, SutProvider<OrganizationService> sutProvider)
|
||||
{
|
||||
// Arrange
|
||||
var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
|
||||
var differentOrganization = new Organization { Id = Guid.NewGuid() };
|
||||
|
||||
organizationRepository
|
||||
.GetByIdentifierAsync(organization.Identifier!)
|
||||
.Returns(differentOrganization);
|
||||
|
||||
// Act/Assert
|
||||
var exception = await Assert.ThrowsAnyAsync<BadRequestException>(() => sutProvider.Sut.UpdateAsync(organization));
|
||||
Assert.Equal("Identifier already in use by another organization.", exception.Message);
|
||||
|
||||
await organizationRepository
|
||||
.Received(1)
|
||||
.GetByIdentifierAsync(Arg.Is<string>(id => id == organization.Identifier));
|
||||
}
|
||||
|
||||
// Must set real guids in order for dictionary of guids to not throw aggregate exceptions
|
||||
private void SetupOrgUserRepositoryCreateManyAsyncMock(IOrganizationUserRepository organizationUserRepository)
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user