nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-07-01 08:02:49 -05:00
Code Activity

[AC-2888] Improve consolidated billing error handling (#4548)

Browse Source 
* Fix error handling in provider setup process

This update ensures that when 'enable-consolidated-billing' is on, any exception thrown during the Stripe customer or subscription setup process for the provider will block the remainder of the setup process so the provider does not enter an invalid state

* Refactor the way BillingException is thrown

Made it simpler to just use the exception constructor and also ensured it was added to the exception handling middleware so it could provide a simple response to the client

* Handle all Stripe exceptions in exception handling middleware

* Fixed error response output for billing's provider controllers

* Cleaned up billing owned provider controllers

Changes were made based on feature updates by product and stuff that's no longer needed. No need to expose sensitive endpoints when they're not being used.

* Reafctored get invoices

Removed unnecssarily bloated method from SubscriberService

* Updated error handling for generating the client invoice report

* Moved get provider subscription to controller

This is only used once and the service layer doesn't seem like the correct choice anymore when thinking about error handling with retrieval

* Handled bad request for update tax information

* Split out Stripe configuration from unauthorization

* Run dotnet format

* Addison's feedback
This commit is contained in:
Alex Morask
2024-07-31 09:26:44 -04:00
committed by GitHub
parent 85ddd080cb
commit 398741cec4
33 changed files with 777 additions and 1260 deletions
Download Patch File Download Diff File Expand all files Collapse all files

196
test/Core.Test/Billing/Services/SubscriberServiceTests.cs
View File

@ -5,7 +5,6 @@ using Bit.Core.Billing.Constants;
using Bit.Core.Billing.Models;
using Bit.Core.Billing.Services.Implementations;
using Bit.Core.Enums;
using Bit.Core.Models.BitStripe;
using Bit.Core.Services;
using Bit.Core.Settings;
using Bit.Test.Common.AutoFixture;
@ -29,8 +28,9 @@ namespace Bit.Core.Test.Billing.Services;
public class SubscriberServiceTests
{
#region CancelSubscription
[Theory, BitAutoData]
public async Task CancelSubscription_SubscriptionInactive_ContactSupport(
public async Task CancelSubscription_SubscriptionInactive_ThrowsBillingException(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
@ -45,7 +45,7 @@ public class SubscriberServiceTests
.SubscriptionGetAsync(organization.GatewaySubscriptionId)
.Returns(subscription);
await ThrowsContactSupportAsync(() =>
await ThrowsBillingExceptionAsync(() =>
sutProvider.Sut.CancelSubscription(organization, new OffboardingSurveyResponse(), false));
await stripeAdapter
@ -192,9 +192,11 @@ public class SubscriberServiceTests
.DidNotReceiveWithAnyArgs()
.SubscriptionCancelAsync(Arg.Any<string>(), Arg.Any<SubscriptionCancelOptions>()); ;
}
#endregion
#region GetCustomer
[Theory, BitAutoData]
public async Task GetCustomer_NullSubscriber_ThrowsArgumentNullException(
SutProvider<SubscriberService> sutProvider)
@ -256,9 +258,11 @@ public class SubscriberServiceTests
Assert.Equivalent(customer, gotCustomer);
}
#endregion
#region GetCustomerOrThrow
[Theory, BitAutoData]
public async Task GetCustomerOrThrow_NullSubscriber_ThrowsArgumentNullException(
SutProvider<SubscriberService> sutProvider)
@ -266,17 +270,17 @@ public class SubscriberServiceTests
async () => await sutProvider.Sut.GetCustomerOrThrow(null));
[Theory, BitAutoData]
public async Task GetCustomerOrThrow_NoGatewayCustomerId_ContactSupport(
public async Task GetCustomerOrThrow_NoGatewayCustomerId_ThrowsBillingException(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
organization.GatewayCustomerId = null;
await ThrowsContactSupportAsync(async () => await sutProvider.Sut.GetCustomerOrThrow(organization));
await ThrowsBillingExceptionAsync(async () => await sutProvider.Sut.GetCustomerOrThrow(organization));
}
[Theory, BitAutoData]
public async Task GetCustomerOrThrow_NoCustomer_ContactSupport(
public async Task GetCustomerOrThrow_NoCustomer_ThrowsBillingException(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
@ -284,11 +288,11 @@ public class SubscriberServiceTests
.CustomerGetAsync(organization.GatewayCustomerId)
.ReturnsNull();
await ThrowsContactSupportAsync(async () => await sutProvider.Sut.GetCustomerOrThrow(organization));
await ThrowsBillingExceptionAsync(async () => await sutProvider.Sut.GetCustomerOrThrow(organization));
}
[Theory, BitAutoData]
public async Task GetCustomerOrThrow_StripeException_ContactSupport(
public async Task GetCustomerOrThrow_StripeException_ThrowsBillingException(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
@ -298,10 +302,10 @@ public class SubscriberServiceTests
.CustomerGetAsync(organization.GatewayCustomerId)
.ThrowsAsync(stripeException);
await ThrowsContactSupportAsync(
await ThrowsBillingExceptionAsync(
async () => await sutProvider.Sut.GetCustomerOrThrow(organization),
"An error occurred while trying to retrieve a Stripe Customer",
stripeException);
message: "An error occurred while trying to retrieve a Stripe customer",
innerException: stripeException);
}
[Theory, BitAutoData]
@ -319,108 +323,6 @@ public class SubscriberServiceTests
Assert.Equivalent(customer, gotCustomer);
}
#endregion
#region GetInvoices
[Theory, BitAutoData]
public async Task GetInvoices_NullSubscriber_ThrowsArgumentNullException(
SutProvider<SubscriberService> sutProvider)
=> await Assert.ThrowsAsync<ArgumentNullException>(
async () => await sutProvider.Sut.GetInvoices(null));
[Theory, BitAutoData]
public async Task GetCustomer_NoGatewayCustomerId_ReturnsEmptyList(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
organization.GatewayCustomerId = null;
var invoices = await sutProvider.Sut.GetInvoices(organization);
Assert.Empty(invoices);
}
[Theory, BitAutoData]
public async Task GetInvoices_StripeException_ReturnsEmptyList(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
sutProvider.GetDependency<IStripeAdapter>()
.InvoiceListAsync(Arg.Any<StripeInvoiceListOptions>())
.ThrowsAsync<StripeException>();
var invoices = await sutProvider.Sut.GetInvoices(organization);
Assert.Empty(invoices);
}
[Theory, BitAutoData]
public async Task GetInvoices_NullOptions_Succeeds(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
var invoices = new List<Invoice>
{
new ()
{
Created = new DateTime(2024, 6, 1),
Number = "2",
Status = "open",
Total = 100000,
HostedInvoiceUrl = "https://example.com/invoice/2",
InvoicePdf = "https://example.com/invoice/2/pdf"
},
new ()
{
Created = new DateTime(2024, 5, 1),
Number = "1",
Status = "paid",
Total = 100000,
HostedInvoiceUrl = "https://example.com/invoice/1",
InvoicePdf = "https://example.com/invoice/1/pdf"
}
};
sutProvider.GetDependency<IStripeAdapter>()
.InvoiceListAsync(Arg.Is<StripeInvoiceListOptions>(options => options.Customer == organization.GatewayCustomerId))
.Returns(invoices);
var gotInvoices = await sutProvider.Sut.GetInvoices(organization);
Assert.Equivalent(invoices, gotInvoices);
}
[Theory, BitAutoData]
public async Task GetInvoices_ProvidedOptions_Succeeds(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
var invoices = new List<Invoice>
{
new ()
{
Created = new DateTime(2024, 5, 1),
Number = "1",
Status = "paid",
Total = 100000,
}
};
sutProvider.GetDependency<IStripeAdapter>()
.InvoiceListAsync(Arg.Is<StripeInvoiceListOptions>(
options =>
options.Customer == organization.GatewayCustomerId &&
options.Status == "paid"))
.Returns(invoices);
var gotInvoices = await sutProvider.Sut.GetInvoices(organization, new StripeInvoiceListOptions
{
Status = "paid"
});
Assert.Equivalent(invoices, gotInvoices);
}
#endregion
@ -795,17 +697,17 @@ public class SubscriberServiceTests
async () => await sutProvider.Sut.GetSubscriptionOrThrow(null));
[Theory, BitAutoData]
public async Task GetSubscriptionOrThrow_NoGatewaySubscriptionId_ContactSupport(
public async Task GetSubscriptionOrThrow_NoGatewaySubscriptionId_ThrowsBillingException(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
organization.GatewaySubscriptionId = null;
await ThrowsContactSupportAsync(async () => await sutProvider.Sut.GetSubscriptionOrThrow(organization));
await ThrowsBillingExceptionAsync(async () => await sutProvider.Sut.GetSubscriptionOrThrow(organization));
}
[Theory, BitAutoData]
public async Task GetSubscriptionOrThrow_NoSubscription_ContactSupport(
public async Task GetSubscriptionOrThrow_NoSubscription_ThrowsBillingException(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
@ -813,11 +715,11 @@ public class SubscriberServiceTests
.SubscriptionGetAsync(organization.GatewaySubscriptionId)
.ReturnsNull();
await ThrowsContactSupportAsync(async () => await sutProvider.Sut.GetSubscriptionOrThrow(organization));
await ThrowsBillingExceptionAsync(async () => await sutProvider.Sut.GetSubscriptionOrThrow(organization));
}
[Theory, BitAutoData]
public async Task GetSubscriptionOrThrow_StripeException_ContactSupport(
public async Task GetSubscriptionOrThrow_StripeException_ThrowsBillingException(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
@ -827,10 +729,10 @@ public class SubscriberServiceTests
.SubscriptionGetAsync(organization.GatewaySubscriptionId)
.ThrowsAsync(stripeException);
await ThrowsContactSupportAsync(
await ThrowsBillingExceptionAsync(
async () => await sutProvider.Sut.GetSubscriptionOrThrow(organization),
"An error occurred while trying to retrieve a Stripe Subscription",
stripeException);
message: "An error occurred while trying to retrieve a Stripe subscription",
innerException: stripeException);
}
[Theory, BitAutoData]
@ -911,12 +813,12 @@ public class SubscriberServiceTests
#region RemovePaymentMethod
[Theory, BitAutoData]
public async Task RemovePaymentMethod_NullSubscriber_ArgumentNullException(
public async Task RemovePaymentMethod_NullSubscriber_ThrowsArgumentNullException(
SutProvider<SubscriberService> sutProvider) =>
await Assert.ThrowsAsync<ArgumentNullException>(() => sutProvider.Sut.RemovePaymentMethod(null));
[Theory, BitAutoData]
public async Task RemovePaymentMethod_Braintree_NoCustomer_ContactSupport(
public async Task RemovePaymentMethod_Braintree_NoCustomer_ThrowsBillingException(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
@ -940,7 +842,7 @@ public class SubscriberServiceTests
braintreeGateway.Customer.Returns(customerGateway);
await ThrowsContactSupportAsync(() => sutProvider.Sut.RemovePaymentMethod(organization));
await ThrowsBillingExceptionAsync(() => sutProvider.Sut.RemovePaymentMethod(organization));
await customerGateway.Received(1).FindAsync(braintreeCustomerId);
@ -987,7 +889,7 @@ public class SubscriberServiceTests
}
[Theory, BitAutoData]
public async Task RemovePaymentMethod_Braintree_CustomerUpdateFails_ContactSupport(
public async Task RemovePaymentMethod_Braintree_CustomerUpdateFails_ThrowsBillingException(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
@ -1028,7 +930,7 @@ public class SubscriberServiceTests
Arg.Is<CustomerRequest>(request => request.DefaultPaymentMethodToken == null))
.Returns(updateBraintreeCustomerResult);
await ThrowsContactSupportAsync(() => sutProvider.Sut.RemovePaymentMethod(organization));
await ThrowsBillingExceptionAsync(() => sutProvider.Sut.RemovePaymentMethod(organization));
await customerGateway.Received(1).FindAsync(braintreeCustomerId);
@ -1042,7 +944,7 @@ public class SubscriberServiceTests
}
[Theory, BitAutoData]
public async Task RemovePaymentMethod_Braintree_PaymentMethodDeleteFails_RollBack_ContactSupport(
public async Task RemovePaymentMethod_Braintree_PaymentMethodDeleteFails_RollBack_ThrowsBillingException(
Organization organization,
SutProvider<SubscriberService> sutProvider)
{
@ -1086,7 +988,7 @@ public class SubscriberServiceTests
paymentMethodGateway.DeleteAsync(paymentMethod.Token).Returns(deleteBraintreePaymentMethodResult);
await ThrowsContactSupportAsync(() => sutProvider.Sut.RemovePaymentMethod(organization));
await ThrowsBillingExceptionAsync(() => sutProvider.Sut.RemovePaymentMethod(organization));
await customerGateway.Received(1).FindAsync(braintreeCustomerId);
@ -1206,42 +1108,42 @@ public class SubscriberServiceTests
#region UpdatePaymentMethod
[Theory, BitAutoData]
public async Task UpdatePaymentMethod_NullSubscriber_ArgumentNullException(
public async Task UpdatePaymentMethod_NullSubscriber_ThrowsArgumentNullException(
SutProvider<SubscriberService> sutProvider)
=> await Assert.ThrowsAsync<ArgumentNullException>(() => sutProvider.Sut.UpdatePaymentMethod(null, null));
[Theory, BitAutoData]
public async Task UpdatePaymentMethod_NullTokenizedPaymentMethod_ArgumentNullException(
public async Task UpdatePaymentMethod_NullTokenizedPaymentMethod_ThrowsArgumentNullException(
Provider provider,
SutProvider<SubscriberService> sutProvider)
=> await Assert.ThrowsAsync<ArgumentNullException>(() => sutProvider.Sut.UpdatePaymentMethod(provider, null));
[Theory, BitAutoData]
public async Task UpdatePaymentMethod_NoToken_ContactSupport(
public async Task UpdatePaymentMethod_NoToken_ThrowsBillingException(
Provider provider,
SutProvider<SubscriberService> sutProvider)
{
sutProvider.GetDependency<IStripeAdapter>().CustomerGetAsync(provider.GatewayCustomerId)
.Returns(new Customer());
await ThrowsContactSupportAsync(() =>
await ThrowsBillingExceptionAsync(() =>
sutProvider.Sut.UpdatePaymentMethod(provider, new TokenizedPaymentMethodDTO(PaymentMethodType.Card, null)));
}
[Theory, BitAutoData]
public async Task UpdatePaymentMethod_UnsupportedPaymentMethod_ContactSupport(
public async Task UpdatePaymentMethod_UnsupportedPaymentMethod_ThrowsBillingException(
Provider provider,
SutProvider<SubscriberService> sutProvider)
{
sutProvider.GetDependency<IStripeAdapter>().CustomerGetAsync(provider.GatewayCustomerId)
.Returns(new Customer());
await ThrowsContactSupportAsync(() =>
await ThrowsBillingExceptionAsync(() =>
sutProvider.Sut.UpdatePaymentMethod(provider, new TokenizedPaymentMethodDTO(PaymentMethodType.BitPay, "TOKEN")));
}
[Theory, BitAutoData]
public async Task UpdatePaymentMethod_BankAccount_IncorrectNumberOfSetupIntentsForToken_ContactSupport(
public async Task UpdatePaymentMethod_BankAccount_IncorrectNumberOfSetupIntentsForToken_ThrowsBillingException(
Provider provider,
SutProvider<SubscriberService> sutProvider)
{
@ -1253,7 +1155,7 @@ public class SubscriberServiceTests
stripeAdapter.SetupIntentList(Arg.Is<SetupIntentListOptions>(options => options.PaymentMethod == "TOKEN"))
.Returns([new SetupIntent(), new SetupIntent()]);
await ThrowsContactSupportAsync(() =>
await ThrowsBillingExceptionAsync(() =>
sutProvider.Sut.UpdatePaymentMethod(provider, new TokenizedPaymentMethodDTO(PaymentMethodType.BankAccount, "TOKEN")));
}
@ -1348,7 +1250,7 @@ public class SubscriberServiceTests
}
[Theory, BitAutoData]
public async Task UpdatePaymentMethod_Braintree_NullCustomer_ContactSupport(
public async Task UpdatePaymentMethod_Braintree_NullCustomer_ThrowsBillingException(
Provider provider,
SutProvider<SubscriberService> sutProvider)
{
@ -1368,13 +1270,13 @@ public class SubscriberServiceTests
customerGateway.FindAsync(braintreeCustomerId).ReturnsNull();
await ThrowsContactSupportAsync(() => sutProvider.Sut.UpdatePaymentMethod(provider, new TokenizedPaymentMethodDTO(PaymentMethodType.PayPal, "TOKEN")));
await ThrowsBillingExceptionAsync(() => sutProvider.Sut.UpdatePaymentMethod(provider, new TokenizedPaymentMethodDTO(PaymentMethodType.PayPal, "TOKEN")));
await paymentMethodGateway.DidNotReceiveWithAnyArgs().CreateAsync(Arg.Any<PaymentMethodRequest>());
}
[Theory, BitAutoData]
public async Task UpdatePaymentMethod_Braintree_ReplacePaymentMethod_CreatePaymentMethodFails_ContactSupport(
public async Task UpdatePaymentMethod_Braintree_ReplacePaymentMethod_CreatePaymentMethodFails_ThrowsBillingException(
Provider provider,
SutProvider<SubscriberService> sutProvider)
{
@ -1406,13 +1308,13 @@ public class SubscriberServiceTests
options => options.CustomerId == braintreeCustomerId && options.PaymentMethodNonce == "TOKEN"))
.Returns(createPaymentMethodResult);
await ThrowsContactSupportAsync(() => sutProvider.Sut.UpdatePaymentMethod(provider, new TokenizedPaymentMethodDTO(PaymentMethodType.PayPal, "TOKEN")));
await ThrowsBillingExceptionAsync(() => sutProvider.Sut.UpdatePaymentMethod(provider, new TokenizedPaymentMethodDTO(PaymentMethodType.PayPal, "TOKEN")));
await customerGateway.DidNotReceiveWithAnyArgs().UpdateAsync(Arg.Any<string>(), Arg.Any<CustomerRequest>());
}
[Theory, BitAutoData]
public async Task UpdatePaymentMethod_Braintree_ReplacePaymentMethod_UpdateCustomerFails_DeletePaymentMethod_ContactSupport(
public async Task UpdatePaymentMethod_Braintree_ReplacePaymentMethod_UpdateCustomerFails_DeletePaymentMethod_ThrowsBillingException(
Provider provider,
SutProvider<SubscriberService> sutProvider)
{
@ -1458,7 +1360,7 @@ public class SubscriberServiceTests
options.DefaultPaymentMethodToken == createPaymentMethodResult.Target.Token))
.Returns(updateCustomerResult);
await ThrowsContactSupportAsync(() => sutProvider.Sut.UpdatePaymentMethod(provider, new TokenizedPaymentMethodDTO(PaymentMethodType.PayPal, "TOKEN")));
await ThrowsBillingExceptionAsync(() => sutProvider.Sut.UpdatePaymentMethod(provider, new TokenizedPaymentMethodDTO(PaymentMethodType.PayPal, "TOKEN")));
await paymentMethodGateway.Received(1).DeleteAsync(createPaymentMethodResult.Target.Token);
}
@ -1531,7 +1433,7 @@ public class SubscriberServiceTests
}
[Theory, BitAutoData]
public async Task UpdatePaymentMethod_Braintree_CreateCustomer_CustomerUpdateFails_ContactSupport(
public async Task UpdatePaymentMethod_Braintree_CreateCustomer_CustomerUpdateFails_ThrowsBillingException(
Provider provider,
SutProvider<SubscriberService> sutProvider)
{
@ -1564,7 +1466,7 @@ public class SubscriberServiceTests
options.PaymentMethodNonce == "TOKEN"))
.Returns(createCustomerResult);
await ThrowsContactSupportAsync(() =>
await ThrowsBillingExceptionAsync(() =>
sutProvider.Sut.UpdatePaymentMethod(provider,
new TokenizedPaymentMethodDTO(PaymentMethodType.PayPal, "TOKEN")));
@ -1648,7 +1550,7 @@ public class SubscriberServiceTests
stripeAdapter.CustomerGetAsync(provider.GatewayCustomerId, Arg.Is<CustomerGetOptions>(
options => options.Expand.Contains("tax_ids"))).Returns(customer);
var taxInformation = new TaxInformationDTO(
var taxInformation = new TaxInformation(
"US",
"12345",
"123456789",
@ -1685,9 +1587,9 @@ public class SubscriberServiceTests
() => sutProvider.Sut.VerifyBankAccount(null, (0, 0)));
[Theory, BitAutoData]
public async Task VerifyBankAccount_NoSetupIntentId_ContactSupport(
public async Task VerifyBankAccount_NoSetupIntentId_ThrowsBillingException(
Provider provider,
SutProvider<SubscriberService> sutProvider) => await ThrowsContactSupportAsync(() => sutProvider.Sut.VerifyBankAccount(provider, (1, 1)));
SutProvider<SubscriberService> sutProvider) => await ThrowsBillingExceptionAsync(() => sutProvider.Sut.VerifyBankAccount(provider, (1, 1)));
[Theory, BitAutoData]
public async Task VerifyBankAccount_MakesCorrectInvocations(

17
test/Core.Test/Billing/Utilities.cs
View File

@ -1,23 +1,22 @@
using Bit.Core.Billing;
using Xunit;
using static Bit.Core.Billing.Utilities;
namespace Bit.Core.Test.Billing;
public static class Utilities
{
public static async Task ThrowsContactSupportAsync(
public static async Task ThrowsBillingExceptionAsync(
Func<Task> function,
string internalMessage = null,
string response = null,
string message = null,
Exception innerException = null)
{
var contactSupport = ContactSupport(internalMessage, innerException);
var expected = new BillingException(response, message, innerException);
var exception = await Assert.ThrowsAsync<BillingException>(function);
var actual = await Assert.ThrowsAsync<BillingException>(function);
Assert.Equal(contactSupport.ClientFriendlyMessage, exception.ClientFriendlyMessage);
Assert.Equal(contactSupport.Message, exception.Message);
Assert.Equal(contactSupport.InnerException, exception.InnerException);
Assert.Equal(expected.Response, actual.Response);
Assert.Equal(expected.Message, actual.Message);
Assert.Equal(expected.InnerException, actual.InnerException);
}
}