mirror of
https://github.com/bitwarden/server.git
synced 2026-01-31 14:13:18 +08:00
Add the stop the duplicate
This commit is contained in:
Cy Okeke
@@ -115,12 +115,7 @@ public class SubscriptionUpdatedHandler : ISubscriptionUpdatedHandler
|
||||
await VoidOpenInvoices(subscription.Id);
|
||||
}
|
||||
|
||||
// Only disable premium if the user doesn't have other active premium subscriptions
|
||||
// This prevents incorrectly disabling premium when webhook events arrive out of order
|
||||
if (!await HasOtherActivePremiumSubscriptionsAsync(userId.Value, subscription.Id))
|
||||
{
|
||||
await _userService.DisablePremiumAsync(userId.Value, currentPeriodEnd);
|
||||
}
|
||||
await _userService.DisablePremiumAsync(userId.Value, currentPeriodEnd);
|
||||
|
||||
break;
|
||||
}
|
||||
@@ -134,12 +129,7 @@ public class SubscriptionUpdatedHandler : ISubscriptionUpdatedHandler
|
||||
await CancelSubscription(subscription.Id);
|
||||
await VoidOpenInvoices(subscription.Id);
|
||||
|
||||
// Only disable premium if the user doesn't have other active premium subscriptions
|
||||
// This prevents incorrectly disabling premium when webhook events arrive out of order
|
||||
if (!await HasOtherActivePremiumSubscriptionsAsync(userId.Value, subscription.Id))
|
||||
{
|
||||
await _userService.DisablePremiumAsync(userId.Value, currentPeriodEnd);
|
||||
}
|
||||
await _userService.DisablePremiumAsync(userId.Value, currentPeriodEnd);
|
||||
}
|
||||
|
||||
break;
|
||||
@@ -173,11 +163,20 @@ public class SubscriptionUpdatedHandler : ISubscriptionUpdatedHandler
|
||||
}
|
||||
case StripeSubscriptionStatus.Active:
|
||||
{
|
||||
if (userId.HasValue)
|
||||
if (userId.HasValue)
|
||||
{
|
||||
// When a Premium user subscription becomes active, proactively clean up any
|
||||
// other active premium subscriptions left over from previous failed attempts.
|
||||
// This prevents users from being double-charged and avoids duplicate
|
||||
// subscriptions lingering on the account.
|
||||
if (await IsPremiumSubscriptionAsync(subscription))
|
||||
{
|
||||
await _userService.EnablePremiumAsync(userId.Value, currentPeriodEnd);
|
||||
await CancelOtherPremiumSubscriptionsAsync(userId.Value, subscription.Id);
|
||||
}
|
||||
break;
|
||||
|
||||
await _userService.EnablePremiumAsync(userId.Value, currentPeriodEnd);
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -219,64 +218,65 @@ public class SubscriptionUpdatedHandler : ISubscriptionUpdatedHandler
|
||||
var premiumPlans = await _pricingClient.ListPremiumPlans();
|
||||
var premiumPriceIds = premiumPlans.SelectMany(p => new[] { p.Seat.StripePriceId, p.Storage.StripePriceId }).ToHashSet();
|
||||
|
||||
// Add hardcoded fallbacks for safety to ensure all premium plan variants are covered
|
||||
premiumPriceIds.Add(IStripeEventUtilityService.PremiumPlanId);
|
||||
premiumPriceIds.Add(IStripeEventUtilityService.PremiumPlanIdAppStore);
|
||||
|
||||
return subscription.Items.Any(i => premiumPriceIds.Contains(i.Price.Id));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Checks if a user has other active premium subscriptions besides the current one being processed.
|
||||
/// This prevents incorrectly disabling premium when a user has multiple subscriptions due to payment retry scenarios.
|
||||
/// Cancels any other premium subscriptions for the given user, leaving the current
|
||||
/// subscription as the single source of truth. This is used when a new subscription
|
||||
/// successfully becomes active after previous failed attempts, to prevent duplicate
|
||||
/// subscriptions and overcharging.
|
||||
/// </summary>
|
||||
/// <param name="userId">The user ID to check</param>
|
||||
/// <param name="currentSubscriptionId">The current subscription being processed (to exclude from the check)</param>
|
||||
/// <returns>True if the user has other active premium subscriptions, false otherwise</returns>
|
||||
private async Task<bool> HasOtherActivePremiumSubscriptionsAsync(Guid userId, string currentSubscriptionId)
|
||||
/// <param name="userId">The user ID whose subscriptions should be cleaned up.</param>
|
||||
/// <param name="currentSubscriptionId">The active subscription that should be kept.</param>
|
||||
private async Task CancelOtherPremiumSubscriptionsAsync(Guid userId, string currentSubscriptionId)
|
||||
{
|
||||
var user = await _userService.GetUserByIdAsync(userId);
|
||||
if (user == null || string.IsNullOrEmpty(user.Email))
|
||||
{
|
||||
return false;
|
||||
return;
|
||||
}
|
||||
|
||||
// Find the customer by email
|
||||
var customerOptions = new CustomerListOptions
|
||||
{
|
||||
Email = user.Email,
|
||||
Limit = 1
|
||||
Limit = 100
|
||||
};
|
||||
|
||||
var customers = await _stripeFacade.ListCustomers(customerOptions);
|
||||
var customer = customers.FirstOrDefault();
|
||||
|
||||
if (customer == null)
|
||||
foreach (var customer in customers)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
// List all active subscriptions for this customer
|
||||
var subscriptionOptions = new SubscriptionListOptions
|
||||
{
|
||||
Customer = customer.Id,
|
||||
Status = "active"
|
||||
};
|
||||
|
||||
var activeSubscriptions = await _stripeFacade.ListSubscriptions(subscriptionOptions);
|
||||
|
||||
// Check if any other subscription (not the current one) is a premium subscription
|
||||
foreach (var sub in activeSubscriptions.Where(s => s.Id != currentSubscriptionId))
|
||||
{
|
||||
if (await IsPremiumSubscriptionAsync(sub))
|
||||
var subscriptionOptions = new SubscriptionListOptions
|
||||
{
|
||||
_logger.LogInformation(
|
||||
"User {UserId} has another active premium subscription {SubscriptionId} while processing {CurrentSubscriptionId}",
|
||||
userId, sub.Id, currentSubscriptionId);
|
||||
return true;
|
||||
Customer = customer.Id
|
||||
};
|
||||
|
||||
var subscriptions = await _stripeFacade.ListSubscriptions(subscriptionOptions);
|
||||
|
||||
foreach (var sub in subscriptions.Where(s => s.Id != currentSubscriptionId))
|
||||
{
|
||||
// Only cancel subscriptions that are premium and in a status where cancellation
|
||||
// makes sense (they are or were granting access / being billed).
|
||||
if (sub.Status is StripeSubscriptionStatus.Active
|
||||
or StripeSubscriptionStatus.Trialing
|
||||
or StripeSubscriptionStatus.PastDue
|
||||
or StripeSubscriptionStatus.Incomplete
|
||||
or StripeSubscriptionStatus.Unpaid
|
||||
or StripeSubscriptionStatus.IncompleteExpired)
|
||||
{
|
||||
if (await IsPremiumSubscriptionAsync(sub))
|
||||
{
|
||||
_logger.LogInformation(
|
||||
"Cancelling duplicate premium subscription {SubscriptionId} for user {UserId} while keeping {CurrentSubscriptionId}",
|
||||
sub.Id, userId, currentSubscriptionId);
|
||||
|
||||
await CancelSubscription(sub.Id);
|
||||
await VoidOpenInvoices(sub.Id);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -548,6 +548,7 @@ public class SubscriptionUpdatedHandlerTests
|
||||
{
|
||||
// Arrange
|
||||
var userId = Guid.NewGuid();
|
||||
var userEmail = "active-user@example.com";
|
||||
var currentPeriodEnd = DateTime.UtcNow.AddDays(30);
|
||||
var subscription = new Subscription
|
||||
{
|
||||
@@ -564,6 +565,16 @@ public class SubscriptionUpdatedHandlerTests
|
||||
|
||||
var parsedEvent = new Event { Data = new EventData() };
|
||||
|
||||
var user = new Core.Entities.User { Id = userId, Email = userEmail };
|
||||
_userService.GetUserByIdAsync(userId).Returns(user);
|
||||
|
||||
var customer = new Customer { Id = "cus_123", Email = userEmail };
|
||||
_stripeFacade.ListCustomers(Arg.Is<CustomerListOptions>(o => o.Email == userEmail))
|
||||
.Returns(new StripeList<Customer> { Data = new List<Customer> { customer } });
|
||||
|
||||
_stripeFacade.ListSubscriptions(Arg.Is<SubscriptionListOptions>(o => o.Customer == customer.Id))
|
||||
.Returns(new StripeList<Subscription> { Data = new List<Subscription> { subscription } });
|
||||
|
||||
_stripeEventService.GetSubscription(Arg.Any<Event>(), Arg.Any<bool>(), Arg.Any<List<string>>())
|
||||
.Returns(subscription);
|
||||
|
||||
@@ -578,6 +589,8 @@ public class SubscriptionUpdatedHandlerTests
|
||||
.EnablePremiumAsync(userId, currentPeriodEnd);
|
||||
await _userService.Received(1)
|
||||
.UpdatePremiumExpirationAsync(userId, currentPeriodEnd);
|
||||
await _stripeFacade.DidNotReceive()
|
||||
.CancelSubscription(subscription.Id, Arg.Any<SubscriptionCancelOptions>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -1309,6 +1322,98 @@ public class SubscriptionUpdatedHandlerTests
|
||||
.CancelSubscription(incompleteSubscriptionId, Arg.Any<SubscriptionCancelOptions>());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task HandleAsync_ActiveUserSubscription_CancelsOtherActivePremiumSubscriptions()
|
||||
{
|
||||
// Arrange
|
||||
var userId = Guid.NewGuid();
|
||||
var userEmail = "duplicate-premium@example.com";
|
||||
var currentPeriodEnd = DateTime.UtcNow.AddDays(30);
|
||||
var currentSubscriptionId = "sub_current";
|
||||
var otherSubscriptionId = "sub_other";
|
||||
var customerId = "cus_123";
|
||||
|
||||
var currentSubscription = new Subscription
|
||||
{
|
||||
Id = currentSubscriptionId,
|
||||
Status = StripeSubscriptionStatus.Active,
|
||||
Metadata = new Dictionary<string, string> { { "userId", userId.ToString() } },
|
||||
Items = new StripeList<SubscriptionItem>
|
||||
{
|
||||
Data =
|
||||
[
|
||||
new SubscriptionItem
|
||||
{
|
||||
CurrentPeriodEnd = currentPeriodEnd,
|
||||
Price = new Price { Id = IStripeEventUtilityService.PremiumPlanId }
|
||||
}
|
||||
]
|
||||
}
|
||||
};
|
||||
|
||||
var otherActiveSubscription = new Subscription
|
||||
{
|
||||
Id = otherSubscriptionId,
|
||||
Status = StripeSubscriptionStatus.Active,
|
||||
Items = new StripeList<SubscriptionItem>
|
||||
{
|
||||
Data =
|
||||
[
|
||||
new SubscriptionItem
|
||||
{
|
||||
CurrentPeriodEnd = currentPeriodEnd,
|
||||
Price = new Price { Id = IStripeEventUtilityService.PremiumPlanId }
|
||||
}
|
||||
]
|
||||
}
|
||||
};
|
||||
|
||||
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(currentSubscription);
|
||||
|
||||
_stripeEventUtilityService.GetIdsFromMetadata(Arg.Any<Dictionary<string, string>>())
|
||||
.Returns(Tuple.Create<Guid?, Guid?, Guid?>(null, userId, null));
|
||||
|
||||
var user = new Core.Entities.User { Id = userId, Email = userEmail };
|
||||
_userService.GetUserByIdAsync(userId).Returns(user);
|
||||
|
||||
var customer = new Customer { Id = customerId, Email = userEmail };
|
||||
_stripeFacade.ListCustomers(Arg.Is<CustomerListOptions>(o => o.Email == userEmail))
|
||||
.Returns(new StripeList<Customer> { Data = new List<Customer> { customer } });
|
||||
|
||||
_stripeFacade.ListSubscriptions(Arg.Is<SubscriptionListOptions>(o => o.Customer == customerId))
|
||||
.Returns(new StripeList<Subscription>
|
||||
{
|
||||
Data = new List<Subscription> { currentSubscription, otherActiveSubscription }
|
||||
});
|
||||
|
||||
_stripeFacade.ListInvoices(Arg.Any<InvoiceListOptions>())
|
||||
.Returns(new StripeList<Invoice> { Data = new List<Invoice>() });
|
||||
|
||||
// Act
|
||||
await _sut.HandleAsync(parsedEvent);
|
||||
|
||||
// Assert - the duplicate active subscription should be cancelled, but the current one kept
|
||||
await _stripeFacade.Received(1)
|
||||
.CancelSubscription(otherSubscriptionId, Arg.Any<SubscriptionCancelOptions>());
|
||||
await _stripeFacade.DidNotReceive()
|
||||
.CancelSubscription(currentSubscriptionId, Arg.Any<SubscriptionCancelOptions>());
|
||||
await _userService.Received(1)
|
||||
.EnablePremiumAsync(userId, currentPeriodEnd);
|
||||
}
|
||||
|
||||
public static IEnumerable<object[]> GetNonActiveSubscriptions()
|
||||
{
|
||||
return new List<object[]>
|
||||
|
||||
Reference in New Issue
Block a user