diff --git a/src/Core/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommand.cs b/src/Core/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommand.cs index fa131083f8..e785bb22d3 100644 --- a/src/Core/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommand.cs +++ b/src/Core/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommand.cs @@ -72,6 +72,10 @@ public class CreatePremiumCloudHostedSubscriptionCommand( BillingAddress billingAddress, short additionalStorageGb) => HandleAsync(async () => { + // A "terminal" subscription is one that has ended and cannot be renewed/reactivated. + // 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)) { @@ -82,9 +86,12 @@ public class CreatePremiumCloudHostedSubscriptionCommand( SubscriptionStatus.Canceled or SubscriptionStatus.IncompleteExpired; } - catch + catch (Exception ex) { - // Subscription doesn't exist or can't be fetched, proceed as normal + // 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); } } diff --git a/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs b/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs index 051500a314..a9b9cd8a27 100644 --- a/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs +++ b/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs @@ -1093,67 +1093,6 @@ public class SubscriptionUpdatedHandlerTests return (providerId, newSubscription, provider, parsedEvent); } - [Fact] - public async Task HandleAsync_IncompleteUserSubscriptionWithoutOpenInvoice_DoesNotCancelSubscription() - { - // Arrange - var userId = Guid.NewGuid(); - var subscriptionId = "sub_123"; - var currentPeriodEnd = DateTime.UtcNow.AddDays(30); - var paidInvoice = new Invoice - { - Id = "inv_123", - Status = StripeInvoiceStatus.Paid - }; - var subscription = new Subscription - { - Id = subscriptionId, - Status = StripeSubscriptionStatus.Incomplete, - Metadata = new Dictionary { { "userId", userId.ToString() } }, - LatestInvoice = paidInvoice, - Items = new StripeList - { - 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 }); - - _stripeEventService.GetSubscription(Arg.Any(), Arg.Any(), Arg.Any>()) - .Returns(subscription); - - _stripeEventUtilityService.GetIdsFromMetadata(Arg.Any>()) - .Returns(Tuple.Create(null, userId, null)); - - // Act - await _sut.HandleAsync(parsedEvent); - - // Assert - await _userService.DidNotReceive() - .DisablePremiumAsync(Arg.Any(), Arg.Any()); - await _stripeFacade.DidNotReceive() - .CancelSubscription(Arg.Any(), Arg.Any()); - await _stripeFacade.DidNotReceive() - .ListInvoices(Arg.Any()); - } - public static IEnumerable GetNonActiveSubscriptions() { return new List diff --git a/test/Core.Test/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommandTests.cs b/test/Core.Test/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommandTests.cs index da287dc02b..0e38850111 100644 --- a/test/Core.Test/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommandTests.cs +++ b/test/Core.Test/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommandTests.cs @@ -812,4 +812,255 @@ public class CreatePremiumCloudHostedSubscriptionCommandTests await _userService.Received(1).SaveUserAsync(user); } + [Theory, BitAutoData] + public async Task Run_UserWithCanceledSubscription_AllowsResubscribe( + User user, + TokenizedPaymentMethod paymentMethod, + BillingAddress billingAddress) + { + // Arrange + user.Premium = true; // User still has Premium flag set + user.GatewayCustomerId = "existing_customer_123"; + user.GatewaySubscriptionId = "sub_canceled_123"; + paymentMethod.Type = TokenizablePaymentMethodType.Card; + paymentMethod.Token = "card_token_123"; + billingAddress.Country = "US"; + billingAddress.PostalCode = "12345"; + + var existingCanceledSubscription = Substitute.For(); + existingCanceledSubscription.Id = "sub_canceled_123"; + existingCanceledSubscription.Status = "canceled"; // Terminal status + + var mockCustomer = Substitute.For(); + mockCustomer.Id = "existing_customer_123"; + mockCustomer.Address = new Address { Country = "US", PostalCode = "12345" }; + mockCustomer.Metadata = new Dictionary(); + + var newSubscription = Substitute.For(); + newSubscription.Id = "sub_new_123"; + newSubscription.Status = "active"; + newSubscription.Items = new StripeList + { + Data = + [ + new SubscriptionItem + { + CurrentPeriodEnd = DateTime.UtcNow.AddDays(30) + } + ] + }; + + _stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId).Returns(existingCanceledSubscription); + _hasPaymentMethodQuery.Run(Arg.Any()).Returns(true); + _subscriberService.GetCustomerOrThrow(Arg.Any(), Arg.Any()).Returns(mockCustomer); + _stripeAdapter.CreateSubscriptionAsync(Arg.Any()).Returns(newSubscription); + + // Act + var result = await _command.Run(user, paymentMethod, billingAddress, 0); + + // Assert + Assert.True(result.IsT0); // Should succeed, not return "Already a premium user" + Assert.True(user.Premium); + Assert.Equal(newSubscription.Id, user.GatewaySubscriptionId); + await _stripeAdapter.Received(1).CreateSubscriptionAsync(Arg.Any()); + await _userService.Received(1).SaveUserAsync(user); + } + + [Theory, BitAutoData] + public async Task Run_UserWithIncompleteExpiredSubscription_AllowsResubscribe( + User user, + TokenizedPaymentMethod paymentMethod, + BillingAddress billingAddress) + { + // Arrange + user.Premium = true; // User still has Premium flag set + user.GatewayCustomerId = "existing_customer_123"; + user.GatewaySubscriptionId = "sub_incomplete_expired_123"; + paymentMethod.Type = TokenizablePaymentMethodType.Card; + paymentMethod.Token = "card_token_123"; + billingAddress.Country = "US"; + billingAddress.PostalCode = "12345"; + + var existingExpiredSubscription = Substitute.For(); + existingExpiredSubscription.Id = "sub_incomplete_expired_123"; + existingExpiredSubscription.Status = "incomplete_expired"; // Terminal status + + var mockCustomer = Substitute.For(); + mockCustomer.Id = "existing_customer_123"; + mockCustomer.Address = new Address { Country = "US", PostalCode = "12345" }; + mockCustomer.Metadata = new Dictionary(); + + var newSubscription = Substitute.For(); + newSubscription.Id = "sub_new_123"; + newSubscription.Status = "active"; + newSubscription.Items = new StripeList + { + Data = + [ + new SubscriptionItem + { + CurrentPeriodEnd = DateTime.UtcNow.AddDays(30) + } + ] + }; + + _stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId).Returns(existingExpiredSubscription); + _hasPaymentMethodQuery.Run(Arg.Any()).Returns(true); + _subscriberService.GetCustomerOrThrow(Arg.Any(), Arg.Any()).Returns(mockCustomer); + _stripeAdapter.CreateSubscriptionAsync(Arg.Any()).Returns(newSubscription); + + // Act + var result = await _command.Run(user, paymentMethod, billingAddress, 0); + + // Assert + Assert.True(result.IsT0); // Should succeed, not return "Already a premium user" + Assert.True(user.Premium); + Assert.Equal(newSubscription.Id, user.GatewaySubscriptionId); + await _stripeAdapter.Received(1).CreateSubscriptionAsync(Arg.Any()); + await _userService.Received(1).SaveUserAsync(user); + } + + [Theory, BitAutoData] + public async Task Run_UserWithActiveSubscription_PremiumTrue_ReturnsBadRequest( + User user, + TokenizedPaymentMethod paymentMethod, + BillingAddress billingAddress) + { + // Arrange + user.Premium = true; + user.GatewaySubscriptionId = "sub_active_123"; + paymentMethod.Type = TokenizablePaymentMethodType.Card; + + var existingActiveSubscription = Substitute.For(); + existingActiveSubscription.Id = "sub_active_123"; + existingActiveSubscription.Status = "active"; // NOT a terminal status + + _stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId).Returns(existingActiveSubscription); + + // Act + var result = await _command.Run(user, paymentMethod, billingAddress, 0); + + // Assert + Assert.True(result.IsT1); + var badRequest = result.AsT1; + Assert.Equal("Already a premium user.", badRequest.Response); + // Verify no subscription creation was attempted + await _stripeAdapter.DidNotReceive().CreateSubscriptionAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task Run_SubscriptionFetchThrows_ProceedsWithCreation( + User user, + TokenizedPaymentMethod paymentMethod, + BillingAddress billingAddress) + { + // Arrange + user.Premium = false; + user.GatewayCustomerId = "existing_customer_123"; + user.GatewaySubscriptionId = "sub_nonexistent_123"; + paymentMethod.Type = TokenizablePaymentMethodType.Card; + paymentMethod.Token = "card_token_123"; + billingAddress.Country = "US"; + billingAddress.PostalCode = "12345"; + + // Simulate Stripe exception when fetching subscription (e.g., subscription doesn't exist) + _stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId) + .Returns(_ => throw new Stripe.StripeException("Subscription not found")); + + var mockCustomer = Substitute.For(); + mockCustomer.Id = "existing_customer_123"; + mockCustomer.Address = new Address { Country = "US", PostalCode = "12345" }; + mockCustomer.Metadata = new Dictionary(); + + var newSubscription = Substitute.For(); + newSubscription.Id = "sub_new_123"; + newSubscription.Status = "active"; + newSubscription.Items = new StripeList + { + Data = + [ + new SubscriptionItem + { + CurrentPeriodEnd = DateTime.UtcNow.AddDays(30) + } + ] + }; + + _hasPaymentMethodQuery.Run(Arg.Any()).Returns(true); + _subscriberService.GetCustomerOrThrow(Arg.Any(), Arg.Any()).Returns(mockCustomer); + _stripeAdapter.CreateSubscriptionAsync(Arg.Any()).Returns(newSubscription); + + // Act + var result = await _command.Run(user, paymentMethod, billingAddress, 0); + + // Assert - Should proceed successfully despite the exception + Assert.True(result.IsT0); + Assert.True(user.Premium); + await _stripeAdapter.Received(1).CreateSubscriptionAsync(Arg.Any()); + await _userService.Received(1).SaveUserAsync(user); + } + + [Theory, BitAutoData] + public async Task Run_ResubscribeWithTerminalSubscription_UpdatesPaymentMethod( + User user, + TokenizedPaymentMethod paymentMethod, + BillingAddress billingAddress) + { + // Arrange + user.Premium = true; + user.GatewayCustomerId = "existing_customer_123"; + user.GatewaySubscriptionId = "sub_canceled_123"; + paymentMethod.Type = TokenizablePaymentMethodType.Card; + paymentMethod.Token = "new_card_token_456"; + billingAddress.Country = "US"; + billingAddress.PostalCode = "12345"; + + var existingCanceledSubscription = Substitute.For(); + existingCanceledSubscription.Id = "sub_canceled_123"; + existingCanceledSubscription.Status = "canceled"; // Terminal status + + var mockCustomer = Substitute.For(); + mockCustomer.Id = "existing_customer_123"; + mockCustomer.Address = new Address { Country = "US", PostalCode = "12345" }; + mockCustomer.Metadata = new Dictionary(); + + var newSubscription = Substitute.For(); + newSubscription.Id = "sub_new_123"; + newSubscription.Status = "active"; + newSubscription.Items = new StripeList + { + Data = + [ + new SubscriptionItem + { + CurrentPeriodEnd = DateTime.UtcNow.AddDays(30) + } + ] + }; + + MaskedPaymentMethod mockMaskedPaymentMethod = new MaskedCard + { + Brand = "visa", + Last4 = "4567", + Expiration = "12/2026" + }; + + _stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId).Returns(existingCanceledSubscription); + _hasPaymentMethodQuery.Run(Arg.Any()).Returns(true); // Has old payment method + _updatePaymentMethodCommand.Run(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(mockMaskedPaymentMethod); + _subscriberService.GetCustomerOrThrow(Arg.Any(), Arg.Any()).Returns(mockCustomer); + _stripeAdapter.CreateSubscriptionAsync(Arg.Any()).Returns(newSubscription); + + // Act + var result = await _command.Run(user, paymentMethod, billingAddress, 0); + + // Assert + Assert.True(result.IsT0); + // Verify payment method was updated because of terminal subscription + await _updatePaymentMethodCommand.Received(1).Run(user, paymentMethod, billingAddress); + await _stripeAdapter.Received(1).CreateSubscriptionAsync(Arg.Any()); + await _userService.Received(1).SaveUserAsync(user); + } + } diff --git a/test/Core.Test/Billing/Subscriptions/Queries/GetBitwardenSubscriptionQueryTests.cs b/test/Core.Test/Billing/Subscriptions/Queries/GetBitwardenSubscriptionQueryTests.cs index a12a0e4cb0..a8b901aa52 100644 --- a/test/Core.Test/Billing/Subscriptions/Queries/GetBitwardenSubscriptionQueryTests.cs +++ b/test/Core.Test/Billing/Subscriptions/Queries/GetBitwardenSubscriptionQueryTests.cs @@ -31,6 +31,30 @@ public class GetBitwardenSubscriptionQueryTests _stripeAdapter); } + [Fact] + public async Task Run_UserWithoutGatewaySubscriptionId_ReturnsNull() + { + var user = CreateUser(); + user.GatewaySubscriptionId = null; + + var result = await _query.Run(user); + + Assert.Null(result); + await _stripeAdapter.DidNotReceive().GetSubscriptionAsync(Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task Run_UserWithEmptyGatewaySubscriptionId_ReturnsNull() + { + var user = CreateUser(); + user.GatewaySubscriptionId = string.Empty; + + var result = await _query.Run(user); + + Assert.Null(result); + await _stripeAdapter.DidNotReceive().GetSubscriptionAsync(Arg.Any(), Arg.Any()); + } + [Fact] public async Task Run_IncompleteStatus_ReturnsBitwardenSubscriptionWithSuspension() {