From 610be2cdcc348d69d6723a675408d910615580d0 Mon Sep 17 00:00:00 2001 From: Daniel James Smith Date: Tue, 31 May 2022 22:55:09 +0200 Subject: [PATCH] [EC-144] Fix stripe revert logic (#2014) * Revert scaling by previous value * Throw is Stripe subscription revert fails * Remove unused property * Add null check to accommodate for not existing storage-gb-xxx subscription item * Use long? instead of Nullable * Remove redundant try/catch * Ensure collectionMethod is changed back, even when revertSub fails Co-authored-by: Matt Gibson --- .../Models/Business/SubscriptionUpdate.cs | 24 +++-- .../Implementations/StripePaymentService.cs | 94 ++++++++++--------- 2 files changed, 68 insertions(+), 50 deletions(-) diff --git a/src/Core/Models/Business/SubscriptionUpdate.cs b/src/Core/Models/Business/SubscriptionUpdate.cs index 24315c723..94a08b6dd 100644 --- a/src/Core/Models/Business/SubscriptionUpdate.cs +++ b/src/Core/Models/Business/SubscriptionUpdate.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; using Bit.Core.Entities; using Stripe; @@ -34,16 +35,17 @@ namespace Bit.Core.Models.Business public class SeatSubscriptionUpdate : SubscriptionUpdate { - private readonly Organization _organization; + private readonly int _previousSeats; private readonly StaticStore.Plan _plan; private readonly long? _additionalSeats; protected override List PlanIds => new() { _plan.StripeSeatPlanId }; + public SeatSubscriptionUpdate(Organization organization, StaticStore.Plan plan, long? additionalSeats) { - _organization = organization; _plan = plan; _additionalSeats = additionalSeats; + _previousSeats = organization.Seats ?? 0; } public override List UpgradeItemsOptions(Subscription subscription) @@ -63,6 +65,7 @@ namespace Bit.Core.Models.Business public override List RevertItemsOptions(Subscription subscription) { + var item = SubscriptionItem(subscription, PlanIds.Single()); return new() { @@ -70,8 +73,8 @@ namespace Bit.Core.Models.Business { Id = item?.Id, Plan = PlanIds.Single(), - Quantity = _organization.Seats, - Deleted = item?.Id != null ? true : (bool?)null, + Quantity = _previousSeats, + Deleted = _previousSeats == 0 ? true : (bool?)null, } }; } @@ -79,6 +82,7 @@ namespace Bit.Core.Models.Business public class StorageSubscriptionUpdate : SubscriptionUpdate { + private long? _prevStorage; private readonly string _plan; private readonly long? _additionalStorage; protected override List PlanIds => new() { _plan }; @@ -92,6 +96,7 @@ namespace Bit.Core.Models.Business public override List UpgradeItemsOptions(Subscription subscription) { var item = SubscriptionItem(subscription, PlanIds.Single()); + _prevStorage = item?.Quantity ?? 0; return new() { new SubscriptionItemOptions @@ -106,6 +111,11 @@ namespace Bit.Core.Models.Business public override List RevertItemsOptions(Subscription subscription) { + if (!_prevStorage.HasValue) + { + throw new Exception("Unknown previous value, must first call UpgradeItemsOptions"); + } + var item = SubscriptionItem(subscription, PlanIds.Single()); return new() { @@ -113,8 +123,8 @@ namespace Bit.Core.Models.Business { Id = item?.Id, Plan = _plan, - Quantity = item?.Quantity ?? 0, - Deleted = item?.Id != null ? true : (bool?)null, + Quantity = _prevStorage.Value, + Deleted = _prevStorage.Value == 0 ? true : (bool?)null, } }; } diff --git a/src/Core/Services/Implementations/StripePaymentService.cs b/src/Core/Services/Implementations/StripePaymentService.cs index 32fc6017a..545e3dda7 100644 --- a/src/Core/Services/Implementations/StripePaymentService.cs +++ b/src/Core/Services/Implementations/StripePaymentService.cs @@ -710,6 +710,8 @@ namespace Bit.Core.Services private async Task FinalizeSubscriptionChangeAsync(IStorableSubscriber storableSubscriber, SubscriptionUpdate subscriptionUpdate, DateTime? prorationDate) { + // remember, when in doubt, throw + var sub = await _stripeAdapter.SubscriptionGetAsync(storableSubscriber.GatewaySubscriptionId); if (sub == null) { @@ -759,65 +761,71 @@ namespace Bit.Core.Services } } - var subResponse = await _stripeAdapter.SubscriptionUpdateAsync(sub.Id, subUpdateOptions); - - var invoice = await _stripeAdapter.InvoiceGetAsync(subResponse?.LatestInvoiceId, new Stripe.InvoiceGetOptions()); - if (invoice == null) - { - throw new BadRequestException("Unable to locate draft invoice for subscription update."); - } - string paymentIntentClientSecret = null; - if (invoice.AmountDue > 0 && updatedItemOptions.Any(i => i.Quantity > 0)) + try { - try + var subResponse = await _stripeAdapter.SubscriptionUpdateAsync(sub.Id, subUpdateOptions); + + var invoice = await _stripeAdapter.InvoiceGetAsync(subResponse?.LatestInvoiceId, new Stripe.InvoiceGetOptions()); + if (invoice == null) { - if (chargeNow) + throw new BadRequestException("Unable to locate draft invoice for subscription update."); + } + + if (invoice.AmountDue > 0 && updatedItemOptions.Any(i => i.Quantity > 0)) + { + try { - paymentIntentClientSecret = await PayInvoiceAfterSubscriptionChangeAsync( - storableSubscriber, invoice); - } - else - { - invoice = await _stripeAdapter.InvoiceFinalizeInvoiceAsync(subResponse.LatestInvoiceId, new Stripe.InvoiceFinalizeOptions + if (chargeNow) { - AutoAdvance = false, + paymentIntentClientSecret = await PayInvoiceAfterSubscriptionChangeAsync( + storableSubscriber, invoice); + } + else + { + invoice = await _stripeAdapter.InvoiceFinalizeInvoiceAsync(subResponse.LatestInvoiceId, new Stripe.InvoiceFinalizeOptions + { + AutoAdvance = false, + }); + await _stripeAdapter.InvoiceSendInvoiceAsync(invoice.Id, new Stripe.InvoiceSendOptions()); + paymentIntentClientSecret = null; + } + } + catch + { + // Need to revert the subscription + await _stripeAdapter.SubscriptionUpdateAsync(sub.Id, new Stripe.SubscriptionUpdateOptions + { + Items = subscriptionUpdate.RevertItemsOptions(sub), + // This proration behavior prevents a false "credit" from + // being applied forward to the next month's invoice + ProrationBehavior = "none", + CollectionMethod = collectionMethod, + DaysUntilDue = daysUntilDue, }); - await _stripeAdapter.InvoiceSendInvoiceAsync(invoice.Id, new Stripe.InvoiceSendOptions()); - paymentIntentClientSecret = null; + throw; } } - catch + else if (!invoice.Paid) + { + // Pay invoice with no charge to customer this completes the invoice immediately without waiting the scheduled 1h + invoice = await _stripeAdapter.InvoicePayAsync(subResponse.LatestInvoiceId); + paymentIntentClientSecret = null; + } + + } + finally + { + // Change back the subscription collection method and/or days until due + if (collectionMethod != "send_invoice" || daysUntilDue == null) { - // Need to revert the subscription await _stripeAdapter.SubscriptionUpdateAsync(sub.Id, new Stripe.SubscriptionUpdateOptions { - Items = subscriptionUpdate.RevertItemsOptions(sub), - // This proration behavior prevents a false "credit" from - // being applied forward to the next month's invoice - ProrationBehavior = "none", CollectionMethod = collectionMethod, DaysUntilDue = daysUntilDue, }); - throw; } } - else if (!invoice.Paid) - { - // Pay invoice with no charge to customer this completes the invoice immediately without waiting the scheduled 1h - invoice = await _stripeAdapter.InvoicePayAsync(subResponse.LatestInvoiceId); - paymentIntentClientSecret = null; - } - - // Change back the subscription collection method and/or days until due - if (collectionMethod != "send_invoice" || daysUntilDue == null) - { - await _stripeAdapter.SubscriptionUpdateAsync(sub.Id, new Stripe.SubscriptionUpdateOptions - { - CollectionMethod = collectionMethod, - DaysUntilDue = daysUntilDue, - }); - } return paymentIntentClientSecret; }