1
0
mirror of https://github.com/bitwarden/server synced 2026-02-10 13:40:10 +00:00

Resolve the pr comment on missed requirements

This commit is contained in:
Cy Okeke
2026-01-26 10:12:28 +01:00
parent 0992d541ae
commit 8b3f29a86f
5 changed files with 59 additions and 63 deletions

View File

@@ -109,12 +109,6 @@ public class SubscriptionUpdatedHandler : ISubscriptionUpdatedHandler
break;
}
if (await IsPremiumSubscriptionAsync(subscription))
{
await CancelSubscription(subscription.Id);
await VoidOpenInvoices(subscription.Id);
}
await _userService.DisablePremiumAsync(userId.Value, currentPeriodEnd);
break;

View File

@@ -3,7 +3,9 @@ using Bit.Core.Billing.Commands;
using Bit.Core.Billing.Constants;
using Bit.Core.Billing.Payment.Models;
using Bit.Core.Billing.Services;
using Bit.Core.Billing.Subscriptions.Models;
using Bit.Core.Entities;
using Bit.Core.Services;
using Bit.Core.Settings;
using Bit.Core.Utilities;
using Braintree;
@@ -23,6 +25,7 @@ public interface IUpdatePaymentMethodCommand
public class UpdatePaymentMethodCommand(
IBraintreeGateway braintreeGateway,
IBraintreeService braintreeService,
IGlobalSettings globalSettings,
ILogger<UpdatePaymentMethodCommand> logger,
ISetupIntentCache setupIntentCache,
@@ -141,6 +144,24 @@ public class UpdatePaymentMethodCommand(
await stripeAdapter.UpdateCustomerAsync(customer.Id, new CustomerUpdateOptions { Metadata = metadata });
}
// If the subscriber has an incomplete subscription, pay the invoice with the new PayPal payment method
if (!string.IsNullOrEmpty(subscriber.GatewaySubscriptionId))
{
var subscription = await stripeAdapter.GetSubscriptionAsync(subscriber.GatewaySubscriptionId);
if (subscription.Status == StripeConstants.SubscriptionStatus.Incomplete)
{
var invoice = await stripeAdapter.UpdateInvoiceAsync(subscription.LatestInvoiceId,
new InvoiceUpdateOptions
{
AutoAdvance = false,
Expand = ["customer"]
});
await braintreeService.PayInvoice(new UserId(subscriber.Id), invoice);
}
}
var payPalAccount = braintreeCustomer.DefaultPaymentMethod as PayPalAccount;
return MaskedPaymentMethod.From(payPalAccount!);

View File

@@ -76,24 +76,7 @@ public class CreatePremiumCloudHostedSubscriptionCommand(
// These are: 'canceled' (user canceled) and 'incomplete_expired' (payment failed and time expired).
// We allow users with terminal subscriptions to create a new subscription even if user.Premium is still true,
// enabling the resubscribe workflow without requiring Premium status to be cleared first.
var hasTerminalSubscription = false;
if (!string.IsNullOrEmpty(user.GatewaySubscriptionId))
{
try
{
var existingSubscription = await stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId);
hasTerminalSubscription = existingSubscription.Status is
SubscriptionStatus.Canceled or
SubscriptionStatus.IncompleteExpired;
}
catch (Exception ex)
{
// Subscription doesn't exist in Stripe or can't be fetched (e.g., network issues, invalid ID)
// Log the issue but proceed with subscription creation to avoid blocking legitimate resubscribe attempts
_logger.LogWarning(ex, "Unable to fetch existing subscription {SubscriptionId} for user {UserId}. Proceeding with subscription creation",
user.GatewaySubscriptionId, user.Id);
}
}
var hasTerminalSubscription = await HasTerminalSubscriptionAsync(user);
if (user.Premium && !hasTerminalSubscription)
{
@@ -158,7 +141,7 @@ public class CreatePremiumCloudHostedSubscriptionCommand(
},
_ =>
{
if (subscription.Status is not (SubscriptionStatus.Active or SubscriptionStatus.Incomplete))
if (subscription.Status != SubscriptionStatus.Active)
{
return;
}
@@ -395,4 +378,28 @@ public class CreatePremiumCloudHostedSubscriptionCommand(
return subscription;
}
private async Task<bool> HasTerminalSubscriptionAsync(User user)
{
if (string.IsNullOrEmpty(user.GatewaySubscriptionId))
{
return false;
}
try
{
var existingSubscription = await stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId);
return existingSubscription.Status is
SubscriptionStatus.Canceled or
SubscriptionStatus.IncompleteExpired;
}
catch (Exception ex)
{
// Subscription doesn't exist in Stripe or can't be fetched (e.g., network issues, invalid ID)
// Log the issue but proceed with subscription creation to avoid blocking legitimate resubscribe attempts
_logger.LogWarning(ex, "Unable to fetch existing subscription {SubscriptionId} for user {UserId}. Proceeding with subscription creation",
user.GatewaySubscriptionId, user.Id);
return false;
}
}
}

View File

@@ -377,7 +377,7 @@ public class SubscriptionUpdatedHandlerTests
}
[Fact]
public async Task HandleAsync_UnpaidUserSubscription_DisablesPremiumAndCancelsSubscription()
public async Task HandleAsync_UnpaidUserSubscription_DisablesPremium()
{
// Arrange
var userId = Guid.NewGuid();
@@ -403,40 +403,26 @@ public class SubscriptionUpdatedHandlerTests
var parsedEvent = new Event { Data = new EventData() };
var premiumPlan = new PremiumPlan
{
Name = "Premium",
Available = true,
LegacyYear = null,
Seat = new PremiumPurchasable { Price = 10M, StripePriceId = IStripeEventUtilityService.PremiumPlanId },
Storage = new PremiumPurchasable { Price = 4M, StripePriceId = "storage-plan-personal" }
};
_pricingClient.ListPremiumPlans().Returns(new List<PremiumPlan> { premiumPlan });
_stripeEventService.GetSubscription(Arg.Any<Event>(), Arg.Any<bool>(), Arg.Any<List<string>>())
.Returns(subscription);
_stripeEventUtilityService.GetIdsFromMetadata(Arg.Any<Dictionary<string, string>>())
.Returns(Tuple.Create<Guid?, Guid?, Guid?>(null, userId, null));
_stripeFacade.ListInvoices(Arg.Any<InvoiceListOptions>())
.Returns(new StripeList<Invoice> { Data = new List<Invoice>() });
// Act
await _sut.HandleAsync(parsedEvent);
// Assert
await _userService.Received(1)
.DisablePremiumAsync(userId, currentPeriodEnd);
await _stripeFacade.Received(1)
.CancelSubscription(subscriptionId, Arg.Any<SubscriptionCancelOptions>());
await _stripeFacade.Received(1)
.ListInvoices(Arg.Is<InvoiceListOptions>(o =>
o.Status == StripeInvoiceStatus.Open && o.Subscription == subscriptionId));
await _stripeFacade.DidNotReceive()
.CancelSubscription(Arg.Any<string>(), Arg.Any<SubscriptionCancelOptions>());
await _stripeFacade.DidNotReceive()
.ListInvoices(Arg.Any<InvoiceListOptions>());
}
[Fact]
public async Task HandleAsync_IncompleteExpiredUserSubscription_DisablesPremiumAndCancelsSubscription()
public async Task HandleAsync_IncompleteExpiredUserSubscription_DisablesPremium()
{
// Arrange
var userId = Guid.NewGuid();
@@ -462,36 +448,22 @@ public class SubscriptionUpdatedHandlerTests
var parsedEvent = new Event { Data = new EventData() };
var premiumPlan = new PremiumPlan
{
Name = "Premium",
Available = true,
LegacyYear = null,
Seat = new PremiumPurchasable { Price = 10M, StripePriceId = IStripeEventUtilityService.PremiumPlanId },
Storage = new PremiumPurchasable { Price = 4M, StripePriceId = "storage-plan-personal" }
};
_pricingClient.ListPremiumPlans().Returns(new List<PremiumPlan> { premiumPlan });
_stripeEventService.GetSubscription(Arg.Any<Event>(), Arg.Any<bool>(), Arg.Any<List<string>>())
.Returns(subscription);
_stripeEventUtilityService.GetIdsFromMetadata(Arg.Any<Dictionary<string, string>>())
.Returns(Tuple.Create<Guid?, Guid?, Guid?>(null, userId, null));
_stripeFacade.ListInvoices(Arg.Any<InvoiceListOptions>())
.Returns(new StripeList<Invoice> { Data = new List<Invoice>() });
// Act
await _sut.HandleAsync(parsedEvent);
// Assert
await _userService.Received(1)
.DisablePremiumAsync(userId, currentPeriodEnd);
await _stripeFacade.Received(1)
.CancelSubscription(subscriptionId, Arg.Any<SubscriptionCancelOptions>());
await _stripeFacade.Received(1)
.ListInvoices(Arg.Is<InvoiceListOptions>(o =>
o.Status == StripeInvoiceStatus.Open && o.Subscription == subscriptionId));
await _stripeFacade.DidNotReceive()
.CancelSubscription(Arg.Any<string>(), Arg.Any<SubscriptionCancelOptions>());
await _stripeFacade.DidNotReceive()
.ListInvoices(Arg.Any<InvoiceListOptions>());
}
[Fact]

View File

@@ -4,6 +4,7 @@ using Bit.Core.Billing.Constants;
using Bit.Core.Billing.Payment.Commands;
using Bit.Core.Billing.Payment.Models;
using Bit.Core.Billing.Services;
using Bit.Core.Services;
using Bit.Core.Settings;
using Bit.Core.Test.Billing.Extensions;
using Braintree;
@@ -32,6 +33,7 @@ public class UpdatePaymentMethodCommandTests
{
_command = new UpdatePaymentMethodCommand(
_braintreeGateway,
Substitute.For<IBraintreeService>(),
_globalSettings,
Substitute.For<ILogger<UpdatePaymentMethodCommand>>(),
_setupIntentCache,