From 395d6e845cbad8de04b9d6b2a41fabe83296da1f Mon Sep 17 00:00:00 2001 From: Conner Turnbull <133619638+cturnbull-bitwarden@users.noreply.github.com> Date: Mon, 3 Jun 2024 13:18:46 -0400 Subject: [PATCH] [AC-2678] Enterprise to Families Sponsorship Bugs (#4118) * Removed prorationDate as it wasn't used, and wasn't needed * Fixed logic to detect if a subscription was sponsored * Moved OrganizationSponsorshipsController.cs to Billing folder --- .../src/Sso/Controllers/AccountController.cs | 5 +- .../OrganizationSponsorshipsController.cs | 2 +- src/Billing/Controllers/StripeController.cs | 4 +- .../Services/IOrganizationService.cs | 4 +- .../Implementations/OrganizationService.cs | 25 +++--- .../SecretsManagerSubscriptionUpdate.cs | 5 -- ...UpdateSecretsManagerSubscriptionCommand.cs | 4 +- src/Core/Services/IPaymentService.cs | 17 ++-- .../Implementations/StripePaymentService.cs | 82 ++++++++++--------- ...OrganizationSponsorshipsControllerTests.cs | 4 +- 10 files changed, 75 insertions(+), 77 deletions(-) rename src/Api/{ => Billing}/Controllers/OrganizationSponsorshipsController.cs (99%) rename test/Api.Test/{AdminConsole => Billing}/Controllers/OrganizationSponsorshipsControllerTests.cs (98%) diff --git a/bitwarden_license/src/Sso/Controllers/AccountController.cs b/bitwarden_license/src/Sso/Controllers/AccountController.cs index bbd4143c3..ddf2553a8 100644 --- a/bitwarden_license/src/Sso/Controllers/AccountController.cs +++ b/bitwarden_license/src/Sso/Controllers/AccountController.cs @@ -497,7 +497,6 @@ public class AccountController : Controller var occupiedSeats = await _organizationUserRepository.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id); var initialSeatCount = organization.Seats.Value; var availableSeats = initialSeatCount - occupiedSeats; - var prorationDate = DateTime.UtcNow; if (availableSeats < 1) { try @@ -507,13 +506,13 @@ public class AccountController : Controller throw new Exception("Cannot autoscale on self-hosted instance."); } - await _organizationService.AutoAddSeatsAsync(organization, 1, prorationDate); + await _organizationService.AutoAddSeatsAsync(organization, 1); } catch (Exception e) { if (organization.Seats.Value != initialSeatCount) { - await _organizationService.AdjustSeatsAsync(orgId, initialSeatCount - organization.Seats.Value, prorationDate); + await _organizationService.AdjustSeatsAsync(orgId, initialSeatCount - organization.Seats.Value); } _logger.LogInformation(e, "SSO auto provisioning failed"); throw new Exception(_i18nService.T("NoSeatsAvailable", organization.DisplayName())); diff --git a/src/Api/Controllers/OrganizationSponsorshipsController.cs b/src/Api/Billing/Controllers/OrganizationSponsorshipsController.cs similarity index 99% rename from src/Api/Controllers/OrganizationSponsorshipsController.cs rename to src/Api/Billing/Controllers/OrganizationSponsorshipsController.cs index c01ec101d..7485f8da8 100644 --- a/src/Api/Controllers/OrganizationSponsorshipsController.cs +++ b/src/Api/Billing/Controllers/OrganizationSponsorshipsController.cs @@ -13,7 +13,7 @@ using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; -namespace Bit.Api.Controllers; +namespace Bit.Api.Billing.Controllers; [Route("organization/sponsorship")] public class OrganizationSponsorshipsController : Controller diff --git a/src/Billing/Controllers/StripeController.cs b/src/Billing/Controllers/StripeController.cs index a2cc06854..bde61e809 100644 --- a/src/Billing/Controllers/StripeController.cs +++ b/src/Billing/Controllers/StripeController.cs @@ -1199,7 +1199,9 @@ public class StripeController : Controller } private static bool IsSponsoredSubscription(Subscription subscription) => - StaticStore.SponsoredPlans.Any(p => p.StripePlanId == subscription.Id); + StaticStore.SponsoredPlans + .Any(p => subscription.Items + .Any(i => i.Plan.Id == p.StripePlanId)); /// /// Handles the event type from Stripe. diff --git a/src/Core/AdminConsole/Services/IOrganizationService.cs b/src/Core/AdminConsole/Services/IOrganizationService.cs index 53f912287..d26ed901b 100644 --- a/src/Core/AdminConsole/Services/IOrganizationService.cs +++ b/src/Core/AdminConsole/Services/IOrganizationService.cs @@ -17,8 +17,8 @@ public interface IOrganizationService Task ReinstateSubscriptionAsync(Guid organizationId); Task AdjustStorageAsync(Guid organizationId, short storageAdjustmentGb); Task UpdateSubscription(Guid organizationId, int seatAdjustment, int? maxAutoscaleSeats); - Task AutoAddSeatsAsync(Organization organization, int seatsToAdd, DateTime? prorationDate = null); - Task AdjustSeatsAsync(Guid organizationId, int seatAdjustment, DateTime? prorationDate = null); + Task AutoAddSeatsAsync(Organization organization, int seatsToAdd); + Task AdjustSeatsAsync(Guid organizationId, int seatAdjustment); Task VerifyBankAsync(Guid organizationId, int amount1, int amount2); /// /// Create a new organization in a cloud environment diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 55bb223ad..aec2178f7 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -282,7 +282,7 @@ public class OrganizationService : IOrganizationService await ReplaceAndUpdateCacheAsync(organization); } - public async Task AdjustSeatsAsync(Guid organizationId, int seatAdjustment, DateTime? prorationDate = null) + public async Task AdjustSeatsAsync(Guid organizationId, int seatAdjustment) { var organization = await GetOrgById(organizationId); if (organization == null) @@ -290,10 +290,10 @@ public class OrganizationService : IOrganizationService throw new NotFoundException(); } - return await AdjustSeatsAsync(organization, seatAdjustment, prorationDate); + return await AdjustSeatsAsync(organization, seatAdjustment); } - private async Task AdjustSeatsAsync(Organization organization, int seatAdjustment, DateTime? prorationDate = null, IEnumerable ownerEmails = null) + private async Task AdjustSeatsAsync(Organization organization, int seatAdjustment, IEnumerable ownerEmails = null) { if (organization.Seats == null) { @@ -349,7 +349,7 @@ public class OrganizationService : IOrganizationService } } - var paymentIntentClientSecret = await _paymentService.AdjustSeatsAsync(organization, plan, additionalSeats, prorationDate); + var paymentIntentClientSecret = await _paymentService.AdjustSeatsAsync(organization, plan, additionalSeats); await _referenceEventService.RaiseEventAsync( new ReferenceEvent(ReferenceEventType.AdjustSeats, organization, _currentContext) { @@ -1161,7 +1161,6 @@ public class OrganizationService : IOrganizationService throw new AggregateException("One or more errors occurred while inviting users.", exceptions); } - var prorationDate = DateTime.UtcNow; try { await _organizationUserRepository.CreateManyAsync(orgUsers); @@ -1180,11 +1179,10 @@ public class OrganizationService : IOrganizationService throw new BadRequestException("Cannot add seats. Cannot manage organization users."); } - await AutoAddSeatsAsync(organization, newSeatsRequired, prorationDate); + await AutoAddSeatsAsync(organization, newSeatsRequired); if (additionalSmSeatsRequired > 0) { - smSubscriptionUpdate.ProrationDate = prorationDate; await _updateSecretsManagerSubscriptionCommand.UpdateSubscriptionAsync(smSubscriptionUpdate); } @@ -1206,7 +1204,7 @@ public class OrganizationService : IOrganizationService // Revert autoscaling if (initialSeatCount.HasValue && currentOrganization.Seats.HasValue && currentOrganization.Seats.Value != initialSeatCount.Value) { - await AdjustSeatsAsync(organization, initialSeatCount.Value - currentOrganization.Seats.Value, prorationDate); + await AdjustSeatsAsync(organization, initialSeatCount.Value - currentOrganization.Seats.Value); } // Revert SmSeat autoscaling @@ -1215,8 +1213,7 @@ public class OrganizationService : IOrganizationService { var smSubscriptionUpdateRevert = new SecretsManagerSubscriptionUpdate(currentOrganization, false) { - SmSeats = initialSmSeatCount.Value, - ProrationDate = prorationDate + SmSeats = initialSmSeatCount.Value }; await _updateSecretsManagerSubscriptionCommand.UpdateSubscriptionAsync(smSubscriptionUpdateRevert); } @@ -1457,7 +1454,7 @@ public class OrganizationService : IOrganizationService return (true, failureReason); } - public async Task AutoAddSeatsAsync(Organization organization, int seatsToAdd, DateTime? prorationDate = null) + public async Task AutoAddSeatsAsync(Organization organization, int seatsToAdd) { if (seatsToAdd < 1 || !organization.Seats.HasValue) { @@ -1485,7 +1482,7 @@ public class OrganizationService : IOrganizationService } var initialSeatCount = organization.Seats.Value; - await AdjustSeatsAsync(organization, seatsToAdd, prorationDate, ownerEmails); + await AdjustSeatsAsync(organization, seatsToAdd, ownerEmails); if (!organization.OwnersNotifiedOfAutoscaling.HasValue) { @@ -2364,7 +2361,7 @@ public class OrganizationService : IOrganizationService var availableSeats = organization.Seats.GetValueOrDefault(0) - occupiedSeats; if (availableSeats < 1) { - await AutoAddSeatsAsync(organization, 1, DateTime.UtcNow); + await AutoAddSeatsAsync(organization, 1); } await CheckPoliciesBeforeRestoreAsync(organizationUser, userService); @@ -2391,7 +2388,7 @@ public class OrganizationService : IOrganizationService var occupiedSeats = await _organizationUserRepository.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id); var availableSeats = organization.Seats.GetValueOrDefault(0) - occupiedSeats; var newSeatsRequired = organizationUserIds.Count() - availableSeats; - await AutoAddSeatsAsync(organization, newSeatsRequired, DateTime.UtcNow); + await AutoAddSeatsAsync(organization, newSeatsRequired); var deletingUserIsOwner = false; if (restoringUserId.HasValue) diff --git a/src/Core/Models/Business/SecretsManagerSubscriptionUpdate.cs b/src/Core/Models/Business/SecretsManagerSubscriptionUpdate.cs index e731377b7..9a4fcac03 100644 --- a/src/Core/Models/Business/SecretsManagerSubscriptionUpdate.cs +++ b/src/Core/Models/Business/SecretsManagerSubscriptionUpdate.cs @@ -29,11 +29,6 @@ public class SecretsManagerSubscriptionUpdate /// public int? MaxAutoscaleSmServiceAccounts { get; set; } - /// - /// The proration date for the subscription update (optional) - /// - public DateTime? ProrationDate { get; set; } - /// /// Whether the subscription update is a result of autoscaling /// diff --git a/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs b/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs index 287132099..6dccf7c81 100644 --- a/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs @@ -66,7 +66,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs { if (update.SmSeatsChanged) { - await _paymentService.AdjustSmSeatsAsync(update.Organization, update.Plan, update.SmSeatsExcludingBase, update.ProrationDate); + await _paymentService.AdjustSmSeatsAsync(update.Organization, update.Plan, update.SmSeatsExcludingBase); // TODO: call ReferenceEventService - see AC-1481 } @@ -74,7 +74,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs if (update.SmServiceAccountsChanged) { await _paymentService.AdjustServiceAccountsAsync(update.Organization, update.Plan, - update.SmServiceAccountsExcludingBase, update.ProrationDate); + update.SmServiceAccountsExcludingBase); // TODO: call ReferenceEventService - see AC-1481 } diff --git a/src/Core/Services/IPaymentService.cs b/src/Core/Services/IPaymentService.cs index 3c78c585f..e3146c439 100644 --- a/src/Core/Services/IPaymentService.cs +++ b/src/Core/Services/IPaymentService.cs @@ -26,20 +26,17 @@ public interface IPaymentService bool subscribedToSecretsManager, int? newlyPurchasedSecretsManagerSeats, int? newlyPurchasedAdditionalSecretsManagerServiceAccounts, - int newlyPurchasedAdditionalStorage, - DateTime? prorationDate = null); - Task AdjustSeatsAsync(Organization organization, Plan plan, int additionalSeats, DateTime? prorationDate = null); + int newlyPurchasedAdditionalStorage); + Task AdjustSeatsAsync(Organization organization, Plan plan, int additionalSeats); Task AdjustSeats( Provider provider, Plan plan, int currentlySubscribedSeats, - int newlySubscribedSeats, - DateTime? prorationDate = null); - Task AdjustSmSeatsAsync(Organization organization, Plan plan, int additionalSeats, DateTime? prorationDate = null); - Task AdjustStorageAsync(IStorableSubscriber storableSubscriber, int additionalStorage, string storagePlanId, DateTime? prorationDate = null); + int newlySubscribedSeats); + Task AdjustSmSeatsAsync(Organization organization, Plan plan, int additionalSeats); + Task AdjustStorageAsync(IStorableSubscriber storableSubscriber, int additionalStorage, string storagePlanId); - Task AdjustServiceAccountsAsync(Organization organization, Plan plan, int additionalServiceAccounts, - DateTime? prorationDate = null); + Task AdjustServiceAccountsAsync(Organization organization, Plan plan, int additionalServiceAccounts); Task CancelSubscriptionAsync(ISubscriber subscriber, bool endOfPeriod = false); Task ReinstateSubscriptionAsync(ISubscriber subscriber); Task UpdatePaymentMethodAsync(ISubscriber subscriber, PaymentMethodType paymentMethodType, @@ -55,7 +52,7 @@ public interface IPaymentService Task UpdateTaxRateAsync(TaxRate taxRate); Task ArchiveTaxRateAsync(TaxRate taxRate); Task AddSecretsManagerToSubscription(Organization org, Plan plan, int additionalSmSeats, - int additionalServiceAccount, DateTime? prorationDate = null); + int additionalServiceAccount); Task RisksSubscriptionFailure(Organization organization); Task HasSecretsManagerStandalone(Organization organization); } diff --git a/src/Core/Services/Implementations/StripePaymentService.cs b/src/Core/Services/Implementations/StripePaymentService.cs index cc2bee06b..ad89e20c2 100644 --- a/src/Core/Services/Implementations/StripePaymentService.cs +++ b/src/Core/Services/Implementations/StripePaymentService.cs @@ -226,7 +226,10 @@ public class StripePaymentService : IPaymentService } } - private async Task ChangeOrganizationSponsorship(Organization org, OrganizationSponsorship sponsorship, bool applySponsorship) + private async Task ChangeOrganizationSponsorship( + Organization org, + OrganizationSponsorship sponsorship, + bool applySponsorship) { var existingPlan = Utilities.StaticStore.GetPlan(org.PlanType); var sponsoredPlan = sponsorship != null ? @@ -234,7 +237,7 @@ public class StripePaymentService : IPaymentService null; var subscriptionUpdate = new SponsorOrganizationSubscriptionUpdate(existingPlan, sponsoredPlan, applySponsorship); - await FinalizeSubscriptionChangeAsync(org, subscriptionUpdate, DateTime.UtcNow, true); + await FinalizeSubscriptionChangeAsync(org, subscriptionUpdate, true); var sub = await _stripeAdapter.SubscriptionGetAsync(org.GatewaySubscriptionId); org.ExpirationDate = sub.CurrentPeriodEnd; @@ -759,7 +762,7 @@ public class StripePaymentService : IPaymentService } private async Task FinalizeSubscriptionChangeAsync(ISubscriber subscriber, - SubscriptionUpdate subscriptionUpdate, DateTime? prorationDate, bool invoiceNow = false) + SubscriptionUpdate subscriptionUpdate, bool invoiceNow = false) { // remember, when in doubt, throw var subGetOptions = new SubscriptionGetOptions(); @@ -771,7 +774,6 @@ public class StripePaymentService : IPaymentService throw new GatewayException("Subscription not found."); } - prorationDate ??= DateTime.UtcNow; var collectionMethod = sub.CollectionMethod; var daysUntilDue = sub.DaysUntilDue; var chargeNow = collectionMethod == "charge_automatically"; @@ -786,8 +788,7 @@ public class StripePaymentService : IPaymentService ? Constants.AlwaysInvoice : Constants.CreateProrations, DaysUntilDue = daysUntilDue ?? 1, - CollectionMethod = "send_invoice", - ProrationDate = prorationDate, + CollectionMethod = "send_invoice" }; if (!invoiceNow && isAnnualPlan && isPm5864DollarThresholdEnabled && sub.Status.Trim() != "trialing") { @@ -907,9 +908,8 @@ public class StripePaymentService : IPaymentService bool subscribedToSecretsManager, int? newlyPurchasedSecretsManagerSeats, int? newlyPurchasedAdditionalSecretsManagerServiceAccounts, - int newlyPurchasedAdditionalStorage, - DateTime? prorationDate = null) - => FinalizeSubscriptionChangeAsync( + int newlyPurchasedAdditionalStorage) => + FinalizeSubscriptionChangeAsync( organization, new CompleteSubscriptionUpdate( organization, @@ -919,41 +919,47 @@ public class StripePaymentService : IPaymentService PurchasedPasswordManagerSeats = newlyPurchasedPasswordManagerSeats, SubscribedToSecretsManager = subscribedToSecretsManager, PurchasedSecretsManagerSeats = newlyPurchasedSecretsManagerSeats, - PurchasedAdditionalSecretsManagerServiceAccounts = newlyPurchasedAdditionalSecretsManagerServiceAccounts, + PurchasedAdditionalSecretsManagerServiceAccounts = + newlyPurchasedAdditionalSecretsManagerServiceAccounts, PurchasedAdditionalStorage = newlyPurchasedAdditionalStorage - }), - prorationDate, true); + }), true); - public Task AdjustSeatsAsync(Organization organization, StaticStore.Plan plan, int additionalSeats, DateTime? prorationDate = null) - { - return FinalizeSubscriptionChangeAsync(organization, new SeatSubscriptionUpdate(organization, plan, additionalSeats), prorationDate); - } + public Task AdjustSeatsAsync(Organization organization, StaticStore.Plan plan, int additionalSeats) => + FinalizeSubscriptionChangeAsync(organization, new SeatSubscriptionUpdate(organization, plan, additionalSeats)); public Task AdjustSeats( Provider provider, StaticStore.Plan plan, int currentlySubscribedSeats, - int newlySubscribedSeats, - DateTime? prorationDate = null) + int newlySubscribedSeats) => FinalizeSubscriptionChangeAsync( provider, - new ProviderSubscriptionUpdate(plan.Type, currentlySubscribedSeats, newlySubscribedSeats), - prorationDate); + new ProviderSubscriptionUpdate( + plan.Type, + currentlySubscribedSeats, + newlySubscribedSeats)); - public Task AdjustSmSeatsAsync(Organization organization, StaticStore.Plan plan, int additionalSeats, DateTime? prorationDate = null) - { - return FinalizeSubscriptionChangeAsync(organization, new SmSeatSubscriptionUpdate(organization, plan, additionalSeats), prorationDate); - } + public Task AdjustSmSeatsAsync(Organization organization, StaticStore.Plan plan, int additionalSeats) => + FinalizeSubscriptionChangeAsync( + organization, + new SmSeatSubscriptionUpdate(organization, plan, additionalSeats)); - public Task AdjustServiceAccountsAsync(Organization organization, StaticStore.Plan plan, int additionalServiceAccounts, DateTime? prorationDate = null) - { - return FinalizeSubscriptionChangeAsync(organization, new ServiceAccountSubscriptionUpdate(organization, plan, additionalServiceAccounts), prorationDate); - } + public Task AdjustServiceAccountsAsync( + Organization organization, + StaticStore.Plan plan, + int additionalServiceAccounts) => + FinalizeSubscriptionChangeAsync( + organization, + new ServiceAccountSubscriptionUpdate(organization, plan, additionalServiceAccounts)); - public Task AdjustStorageAsync(IStorableSubscriber storableSubscriber, int additionalStorage, - string storagePlanId, DateTime? prorationDate = null) + public Task AdjustStorageAsync( + IStorableSubscriber storableSubscriber, + int additionalStorage, + string storagePlanId) { - return FinalizeSubscriptionChangeAsync(storableSubscriber, new StorageSubscriptionUpdate(storagePlanId, additionalStorage), prorationDate); + return FinalizeSubscriptionChangeAsync( + storableSubscriber, + new StorageSubscriptionUpdate(storagePlanId, additionalStorage)); } public async Task CancelAndRecoverChargesAsync(ISubscriber subscriber) @@ -1771,13 +1777,15 @@ public class StripePaymentService : IPaymentService } } - public async Task AddSecretsManagerToSubscription(Organization org, StaticStore.Plan plan, int additionalSmSeats, - int additionalServiceAccount, DateTime? prorationDate = null) - { - return await FinalizeSubscriptionChangeAsync(org, - new SecretsManagerSubscribeUpdate(org, plan, additionalSmSeats, additionalServiceAccount), prorationDate, + public async Task AddSecretsManagerToSubscription( + Organization org, + StaticStore.Plan plan, + int additionalSmSeats, + int additionalServiceAccount) => + await FinalizeSubscriptionChangeAsync( + org, + new SecretsManagerSubscribeUpdate(org, plan, additionalSmSeats, additionalServiceAccount), true); - } public async Task RisksSubscriptionFailure(Organization organization) { diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationSponsorshipsControllerTests.cs b/test/Api.Test/Billing/Controllers/OrganizationSponsorshipsControllerTests.cs similarity index 98% rename from test/Api.Test/AdminConsole/Controllers/OrganizationSponsorshipsControllerTests.cs rename to test/Api.Test/Billing/Controllers/OrganizationSponsorshipsControllerTests.cs index 8d9b10fde..2f0dfa49d 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationSponsorshipsControllerTests.cs +++ b/test/Api.Test/Billing/Controllers/OrganizationSponsorshipsControllerTests.cs @@ -1,4 +1,4 @@ -using Bit.Api.Controllers; +using Bit.Api.Billing.Controllers; using Bit.Api.Models.Request.Organizations; using Bit.Core.AdminConsole.Entities; using Bit.Core.Context; @@ -14,7 +14,7 @@ using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; using Xunit; -namespace Bit.Api.Test.AdminConsole.Controllers; +namespace Bit.Api.Test.Billing.Controllers; [ControllerCustomize(typeof(OrganizationSponsorshipsController))] [SutProviderCustomize]