diff --git a/src/Api/AdminConsole/Controllers/OrganizationsController.cs b/src/Api/AdminConsole/Controllers/OrganizationsController.cs index 18045178db..8b1a6243c3 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationsController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationsController.cs @@ -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); + } } diff --git a/src/Core/AdminConsole/Entities/Organization.cs b/src/Core/AdminConsole/Entities/Organization.cs index 3f02462501..7933990e74 100644 --- a/src/Core/AdminConsole/Entities/Organization.cs +++ b/src/Core/AdminConsole/Entities/Organization.cs @@ -30,6 +30,7 @@ public class Organization : ITableObject, IStorableSubscriber, IRevisable /// This value is HTML encoded. For display purposes use the method DisplayBusinessName() instead. /// [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, IStorableSubscriber, IRevisable /// /// Returns the business name of the organization, HTML decoded ready for display. /// + /// + [Obsolete("This method has been deprecated. Use the 'DisplayName()' method instead.")] public string? DisplayBusinessName() { return WebUtility.HtmlDecode(BusinessName); diff --git a/src/Core/AdminConsole/Services/IOrganizationService.cs b/src/Core/AdminConsole/Services/IOrganizationService.cs index 6adfc4772f..e54e6fee12 100644 --- a/src/Core/AdminConsole/Services/IOrganizationService.cs +++ b/src/Core/AdminConsole/Services/IOrganizationService.cs @@ -13,7 +13,6 @@ namespace Bit.Core.Services; public interface IOrganizationService { - Task CancelSubscriptionAsync(Guid organizationId, bool? endOfPeriod = null); Task ReinstateSubscriptionAsync(Guid organizationId); Task AdjustStorageAsync(Guid organizationId, short storageAdjustmentGb); Task UpdateSubscription(Guid organizationId, int seatAdjustment, int? maxAutoscaleSeats); diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 41e4f2f618..575cdb0230 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -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 { "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] + }] + }, }); } diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index 923eaae871..f619fed278 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs @@ -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 sutProvider) + { + // Arrange + var organizationRepository = sutProvider.GetDependency(); + var applicationCacheService = sutProvider.GetDependency(); + var stripeAdapter = sutProvider.GetDependency(); + var eventService = sutProvider.GetDependency(); + + 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(id => id == organization.Identifier)); + await stripeAdapter + .Received(1) + .CustomerUpdateAsync( + Arg.Is(id => id == organization.GatewayCustomerId), + Arg.Is(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(org => org == organization)); + await applicationCacheService + .Received(1) + .UpsertOrganizationAbilityAsync(Arg.Is(org => org == organization)); + await eventService + .Received(1) + .LogOrganizationEventAsync(Arg.Is(org => org == organization), + Arg.Is(e => e == EventType.Organization_Updated)); + } + + [Theory, BitAutoData] + public async Task UpdateAsync_WhenValidOrganization_AndUpdateBillingIsFalse_UpdateOrganization(Organization organization, SutProvider sutProvider) + { + // Arrange + var organizationRepository = sutProvider.GetDependency(); + var applicationCacheService = sutProvider.GetDependency(); + var stripeAdapter = sutProvider.GetDependency(); + var eventService = sutProvider.GetDependency(); + + organizationRepository + .GetByIdentifierAsync(organization.Identifier!) + .Returns(organization); + + // Act + await sutProvider.Sut.UpdateAsync(organization, updateBilling: false); + + // Assert + await organizationRepository + .Received(1) + .GetByIdentifierAsync(Arg.Is(id => id == organization.Identifier)); + await stripeAdapter + .DidNotReceiveWithAnyArgs() + .CustomerUpdateAsync(Arg.Any(), Arg.Any()); + await organizationRepository + .Received(1) + .ReplaceAsync(Arg.Is(org => org == organization)); + await applicationCacheService + .Received(1) + .UpsertOrganizationAbilityAsync(Arg.Is(org => org == organization)); + await eventService + .Received(1) + .LogOrganizationEventAsync(Arg.Is(org => org == organization), + Arg.Is(e => e == EventType.Organization_Updated)); + } + + [Theory, BitAutoData] + public async Task UpdateAsync_WhenOrganizationHasNoId_ThrowsApplicationException(Organization organization, SutProvider sutProvider) + { + // Arrange + organization.Id = Guid.Empty; + + // Act/Assert + var exception = await Assert.ThrowsAnyAsync(() => 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 sutProvider) + { + // Arrange + var organizationRepository = sutProvider.GetDependency(); + var differentOrganization = new Organization { Id = Guid.NewGuid() }; + + organizationRepository + .GetByIdentifierAsync(organization.Identifier!) + .Returns(differentOrganization); + + // Act/Assert + var exception = await Assert.ThrowsAnyAsync(() => sutProvider.Sut.UpdateAsync(organization)); + Assert.Equal("Identifier already in use by another organization.", exception.Message); + + await organizationRepository + .Received(1) + .GetByIdentifierAsync(Arg.Is(id => id == organization.Identifier)); + } + // Must set real guids in order for dictionary of guids to not throw aggregate exceptions private void SetupOrgUserRepositoryCreateManyAsyncMock(IOrganizationUserRepository organizationUserRepository) {