From cab23cb1099e81dc866ef0237ae0f6d2a715daf5 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Tue, 11 Jul 2023 19:07:57 +1000 Subject: [PATCH] [AC-1503] Fix Stripe integration on organization upgrade (#3084) * Fix SM parameters not being passed to Stripe * Fix flaky test * Fix error message --- .../Business/SubscriptionCreateOptions.cs | 16 ++++---- ...UpdateSecretsManagerSubscriptionCommand.cs | 4 +- .../UpgradeOrganizationPlanCommand.cs | 4 +- src/Core/Services/IPaymentService.cs | 4 +- .../Implementations/StripePaymentService.cs | 6 +-- ...eSecretsManagerSubscriptionCommandTests.cs | 4 +- .../Services/OrganizationServiceTests.cs | 14 +++---- .../Services/StripePaymentServiceTests.cs | 39 ++++++++++++++++--- 8 files changed, 56 insertions(+), 35 deletions(-) diff --git a/src/Core/Models/Business/SubscriptionCreateOptions.cs b/src/Core/Models/Business/SubscriptionCreateOptions.cs index bd51f431f0..86524a55b6 100644 --- a/src/Core/Models/Business/SubscriptionCreateOptions.cs +++ b/src/Core/Models/Business/SubscriptionCreateOptions.cs @@ -7,7 +7,7 @@ namespace Bit.Core.Models.Business; public class OrganizationSubscriptionOptionsBase : Stripe.SubscriptionCreateOptions { public OrganizationSubscriptionOptionsBase(Organization org, List plans, TaxInfo taxInfo, int additionalSeats, - int additionalStorageGb, bool premiumAccessAddon, int additionalSmSeats = 0, int additionalServiceAccounts = 0) + int additionalStorageGb, bool premiumAccessAddon, int additionalSmSeats, int additionalServiceAccounts) { Items = new List(); Metadata = new Dictionary @@ -95,9 +95,9 @@ public class OrganizationPurchaseSubscriptionOptions : OrganizationSubscriptionO { public OrganizationPurchaseSubscriptionOptions( Organization org, List plans, - TaxInfo taxInfo, int additionalSeats = 0, - int additionalStorageGb = 0, bool premiumAccessAddon = false, - int additionalSmSeats = 0, int additionalServiceAccounts = 0) : + TaxInfo taxInfo, int additionalSeats, + int additionalStorageGb, bool premiumAccessAddon, + int additionalSmSeats, int additionalServiceAccounts) : base(org, plans, taxInfo, additionalSeats, additionalStorageGb, premiumAccessAddon, additionalSmSeats, additionalServiceAccounts) { OffSession = true; @@ -109,10 +109,10 @@ public class OrganizationUpgradeSubscriptionOptions : OrganizationSubscriptionOp { public OrganizationUpgradeSubscriptionOptions( string customerId, Organization org, - List plans, TaxInfo taxInfo, - int additionalSeats = 0, int additionalStorageGb = 0, - bool premiumAccessAddon = false, int additionalSmSeats = 0, int additionalServiceAccounts = 0) : - base(org, plans, taxInfo, additionalSeats, additionalStorageGb, premiumAccessAddon, additionalSmSeats, additionalServiceAccounts) + List plans, OrganizationUpgrade upgrade) : + base(org, plans, upgrade.TaxInfo, upgrade.AdditionalSeats, upgrade.AdditionalStorageGb, + upgrade.PremiumAccessAddon, upgrade.AdditionalSmSeats.GetValueOrDefault(), + upgrade.AdditionalServiceAccounts.GetValueOrDefault()) { Customer = customerId; } diff --git a/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs b/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs index 9237253f79..a57c874da7 100644 --- a/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs @@ -240,7 +240,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs if (currentSeats > update.SmSeats) { throw new BadRequestException($"Your organization currently has {currentSeats} Secrets Manager seats. " + - $"Your plan only allows ({update.SmSeats}) Secrets Manager seats. Remove some Secrets Manager users."); + $"Your plan only allows {update.SmSeats} Secrets Manager seats. Remove some Secrets Manager users."); } } } @@ -294,7 +294,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs if (currentServiceAccounts > update.SmServiceAccounts) { throw new BadRequestException($"Your organization currently has {currentServiceAccounts} Service Accounts. " + - $"Your plan only allows ({update.SmServiceAccounts}) Service Accounts. Remove some Service Accounts."); + $"Your plan only allows {update.SmServiceAccounts} Service Accounts. Remove some Service Accounts."); } } } diff --git a/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpgradeOrganizationPlanCommand.cs b/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpgradeOrganizationPlanCommand.cs index 62ee67442f..24a459ea32 100644 --- a/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpgradeOrganizationPlanCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpgradeOrganizationPlanCommand.cs @@ -225,9 +225,7 @@ public class UpgradeOrganizationPlanCommand : IUpgradeOrganizationPlanCommand : StaticStore.Plans.Where(p => p.Type == upgrade.Plan && p.BitwardenProduct == BitwardenProductType.PasswordManager).ToList(); paymentIntentClientSecret = await _paymentService.UpgradeFreeOrganizationAsync(organization, - organizationUpgradePlan, - upgrade.AdditionalStorageGb, upgrade.AdditionalSeats, upgrade.PremiumAccessAddon, upgrade.TaxInfo - , upgrade.AdditionalSmSeats.GetValueOrDefault(), upgrade.AdditionalServiceAccounts.GetValueOrDefault()); + organizationUpgradePlan, upgrade); success = string.IsNullOrWhiteSpace(paymentIntentClientSecret); } else diff --git a/src/Core/Services/IPaymentService.cs b/src/Core/Services/IPaymentService.cs index 21140a58c8..d636fa2265 100644 --- a/src/Core/Services/IPaymentService.cs +++ b/src/Core/Services/IPaymentService.cs @@ -14,9 +14,7 @@ public interface IPaymentService int additionalServiceAccount = 0); Task SponsorOrganizationAsync(Organization org, OrganizationSponsorship sponsorship); Task RemoveOrganizationSponsorshipAsync(Organization org, OrganizationSponsorship sponsorship); - Task UpgradeFreeOrganizationAsync(Organization org, List plans, - short additionalStorageGb, int additionalSeats, bool premiumAccessAddon, TaxInfo taxInfo, - int additionalSmSeats = 0, int additionalServiceAccounts = 0); + Task UpgradeFreeOrganizationAsync(Organization org, List plans, OrganizationUpgrade upgrade); Task PurchasePremiumAsync(User user, PaymentMethodType paymentMethodType, string paymentToken, short additionalStorageGb, TaxInfo taxInfo); Task AdjustSeatsAsync(Organization organization, Plan plan, int additionalSeats, DateTime? prorationDate = null); diff --git a/src/Core/Services/Implementations/StripePaymentService.cs b/src/Core/Services/Implementations/StripePaymentService.cs index 9bd45ff0cf..1d482ccc1b 100644 --- a/src/Core/Services/Implementations/StripePaymentService.cs +++ b/src/Core/Services/Implementations/StripePaymentService.cs @@ -224,8 +224,7 @@ public class StripePaymentService : IPaymentService ChangeOrganizationSponsorship(org, sponsorship, false); public async Task UpgradeFreeOrganizationAsync(Organization org, List plans, - short additionalStorageGb, int additionalSeats, bool premiumAccessAddon, TaxInfo taxInfo, - int additionalSmSeats = 0, int additionalServiceAccounts = 0) + OrganizationUpgrade upgrade) { if (!string.IsNullOrWhiteSpace(org.GatewaySubscriptionId)) { @@ -241,6 +240,7 @@ public class StripePaymentService : IPaymentService throw new GatewayException("Could not find customer payment profile."); } + var taxInfo = upgrade.TaxInfo; if (taxInfo != null && !string.IsNullOrWhiteSpace(taxInfo.BillingAddressCountry) && !string.IsNullOrWhiteSpace(taxInfo.BillingAddressPostalCode)) { var taxRateSearch = new TaxRate @@ -258,7 +258,7 @@ public class StripePaymentService : IPaymentService } } - var subCreateOptions = new OrganizationUpgradeSubscriptionOptions(customer.Id, org, plans, taxInfo, additionalSeats, additionalStorageGb, premiumAccessAddon); + var subCreateOptions = new OrganizationUpgradeSubscriptionOptions(customer.Id, org, plans, upgrade); var (stripePaymentMethod, paymentMethodType) = IdentifyPaymentMethod(customer, subCreateOptions); var subscription = await ChargeForNewSubscriptionAsync(org, customer, false, diff --git a/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpdateSecretsManagerSubscriptionCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpdateSecretsManagerSubscriptionCommandTests.cs index 9210140bc6..5bd08a136c 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpdateSecretsManagerSubscriptionCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpdateSecretsManagerSubscriptionCommandTests.cs @@ -524,7 +524,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSecretsManagerSubscription(update)); - Assert.Contains("Your organization currently has 8 Secrets Manager seats. Your plan only allows (7) Secrets Manager seats. Remove some Secrets Manager users", exception.Message); + Assert.Contains("Your organization currently has 8 Secrets Manager seats. Your plan only allows 7 Secrets Manager seats. Remove some Secrets Manager users", exception.Message); await VerifyDependencyNotCalledAsync(sutProvider); } @@ -724,7 +724,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests .Returns(currentServiceAccounts); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSecretsManagerSubscription(organizationUpdate)); - Assert.Contains("Your organization currently has 301 Service Accounts. Your plan only allows (300) Service Accounts. Remove some Service Accounts", exception.Message); + Assert.Contains("Your organization currently has 301 Service Accounts. Your plan only allows 300 Service Accounts. Remove some Service Accounts", exception.Message); await sutProvider.GetDependency().Received(1).GetServiceAccountCountByOrganizationIdAsync(organization.Id); await VerifyDependencyNotCalledAsync(sutProvider); } diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index f3c88663e1..7729d748cd 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -194,20 +194,16 @@ public class OrganizationServiceTests [Theory] [BitAutoData] - public async Task SignUpAsync_SecretManagerValidation_ShouldThrowException(OrganizationSignup signup, SutProvider sutProvider) + public async Task SignUpAsync_SecretManager_AdditionalServiceAccounts_NotAllowedByPlan_ShouldThrowException(OrganizationSignup signup, SutProvider sutProvider) { - signup.AdditionalSmSeats = 10; - signup.AdditionalSeats = 10; - signup.Plan = PlanType.EnterpriseAnnually; + signup.AdditionalSmSeats = 0; + signup.AdditionalSeats = 0; + signup.Plan = PlanType.Free; signup.UseSecretsManager = true; signup.PaymentMethodType = PaymentMethodType.Card; signup.PremiumAccessAddon = false; signup.AdditionalServiceAccounts = 10; - - var purchaseOrganizationPlan = StaticStore.Plans.Where(x => x.Type == signup.Plan && x.BitwardenProduct == BitwardenProductType.PasswordManager).ToList(); - var secretsManagerPlan = StaticStore.GetSecretsManagerPlan(PlanType.EnterpriseAnnually); - secretsManagerPlan.HasAdditionalServiceAccountOption = false; - purchaseOrganizationPlan.Add(secretsManagerPlan); + signup.AdditionalStorageGb = 0; var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.SignUpAsync(signup)); diff --git a/test/Core.Test/Services/StripePaymentServiceTests.cs b/test/Core.Test/Services/StripePaymentServiceTests.cs index 96f9a6a6f3..91e383cb58 100644 --- a/test/Core.Test/Services/StripePaymentServiceTests.cs +++ b/test/Core.Test/Services/StripePaymentServiceTests.cs @@ -443,6 +443,8 @@ public class StripePaymentServiceTests public async void PurchaseOrganizationAsync_SM_Paypal(SutProvider sutProvider, Organization organization, string paymentToken, TaxInfo taxInfo) { var plans = StaticStore.Plans.Where(p => p.Type == PlanType.EnterpriseAnnually).ToList(); + var passwordManagerPlan = plans.Single(p => p.BitwardenProduct == BitwardenProductType.PasswordManager); + var secretsManagerPlan = plans.Single(p => p.BitwardenProduct == BitwardenProductType.SecretsManager); var stripeAdapter = sutProvider.GetDependency(); stripeAdapter.CustomerCreateAsync(default).ReturnsForAnyArgs(new Stripe.Customer @@ -465,8 +467,12 @@ public class StripePaymentServiceTests var braintreeGateway = sutProvider.GetDependency(); braintreeGateway.Customer.CreateAsync(default).ReturnsForAnyArgs(customerResult); + var additionalStorage = (short)2; + var additionalSeats = 10; + var additionalSmSeats = 5; + var additionalServiceAccounts = 20; var result = await sutProvider.Sut.PurchaseOrganizationAsync(organization, PaymentMethodType.PayPal, paymentToken, plans, - 2, 10, false, taxInfo, false, 10, 10); + additionalStorage, additionalSeats, false, taxInfo, false, additionalSmSeats, additionalServiceAccounts); Assert.Null(result); Assert.Equal(GatewayType.Stripe, organization.Gateway); @@ -495,7 +501,11 @@ public class StripePaymentServiceTests s.Customer == "C-1" && s.Expand[0] == "latest_invoice.payment_intent" && s.Metadata[organization.GatewayIdField()] == organization.Id.ToString() && - s.Items.Count > 0 + s.Items.Count == 4 && + s.Items.Count(i => i.Plan == passwordManagerPlan.StripeSeatPlanId && i.Quantity == additionalSeats) == 1 && + s.Items.Count(i => i.Plan == passwordManagerPlan.StripeStoragePlanId && i.Quantity == additionalStorage) == 1 && + s.Items.Count(i => i.Plan == secretsManagerPlan.StripeSeatPlanId && i.Quantity == additionalSmSeats) == 1 && + s.Items.Count(i => i.Plan == secretsManagerPlan.StripeServiceAccountPlanId && i.Quantity == additionalServiceAccounts) == 1 )); } @@ -600,7 +610,17 @@ public class StripePaymentServiceTests stripeAdapter.SubscriptionCreateAsync(default).ReturnsForAnyArgs(new Stripe.Subscription { }); var plans = StaticStore.Plans.Where(p => p.Type == PlanType.EnterpriseAnnually).ToList(); - var result = await sutProvider.Sut.UpgradeFreeOrganizationAsync(organization, plans, 0, 0, false, taxInfo); + + var upgrade = new OrganizationUpgrade() + { + AdditionalStorageGb = 0, + AdditionalSeats = 0, + PremiumAccessAddon = false, + TaxInfo = taxInfo, + AdditionalSmSeats = 0, + AdditionalServiceAccounts = 0 + }; + var result = await sutProvider.Sut.UpgradeFreeOrganizationAsync(organization, plans, upgrade); Assert.Null(result); } @@ -626,9 +646,18 @@ public class StripePaymentServiceTests }); stripeAdapter.SubscriptionCreateAsync(default).ReturnsForAnyArgs(new Stripe.Subscription { }); + var upgrade = new OrganizationUpgrade() + { + AdditionalStorageGb = 1, + AdditionalSeats = 10, + PremiumAccessAddon = false, + TaxInfo = taxInfo, + AdditionalSmSeats = 5, + AdditionalServiceAccounts = 50 + }; + var plans = StaticStore.Plans.Where(p => p.Type == PlanType.EnterpriseAnnually).ToList(); - var result = await sutProvider.Sut.UpgradeFreeOrganizationAsync(organization, plans, 0, 0, - false, taxInfo); + var result = await sutProvider.Sut.UpgradeFreeOrganizationAsync(organization, plans, upgrade); Assert.Null(result); }