diff --git a/src/Admin/AdminConsole/Controllers/ProvidersController.cs b/src/Admin/AdminConsole/Controllers/ProvidersController.cs index f331e973e50b..fb45c78659f4 100644 --- a/src/Admin/AdminConsole/Controllers/ProvidersController.cs +++ b/src/Admin/AdminConsole/Controllers/ProvidersController.cs @@ -449,7 +449,14 @@ private async Task GetEditModel(Guid id) } var providerPlans = await _providerPlanRepository.GetByProviderId(id); - var payByInvoice = ((await _subscriberService.GetCustomer(provider))?.ApprovedToPayByInvoice() ?? false); + var customer = await _subscriberService.GetCustomer(provider); + if (customer is { Deleted: true }) + { + TempData["Warning"] = + "Billing information could not be fully loaded. The Stripe customer may have been deleted. " + + "You can still edit the provider and set a valid Gateway Customer ID."; + } + var payByInvoice = customer?.ApprovedToPayByInvoice() ?? false; return new ProviderEditModel( provider, users, providerOrganizations, diff --git a/src/Admin/Controllers/UsersController.cs b/src/Admin/Controllers/UsersController.cs index 5d63b0aeca6d..edcbb06770af 100644 --- a/src/Admin/Controllers/UsersController.cs +++ b/src/Admin/Controllers/UsersController.cs @@ -5,6 +5,8 @@ using Bit.Admin.Services; using Bit.Admin.Utilities; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; +using Bit.Core.Billing.Constants; +using Bit.Core.Billing.Models; using Bit.Core.Billing.Services; using Bit.Core.Repositories; using Bit.Core.Services; @@ -13,6 +15,7 @@ using Bit.Core.Vault.Repositories; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; +using Stripe; namespace Bit.Admin.Controllers; @@ -109,8 +112,35 @@ public async Task Edit(Guid id) } var ciphers = await _cipherRepository.GetManyByUserIdAsync(id, withOrganizations: false); - var billingInfo = await _paymentService.GetBillingAsync(user); - var billingHistoryInfo = await _paymentService.GetBillingHistoryAsync(user); + BillingInfo? billingInfo = null; + BillingHistoryInfo? billingHistoryInfo = null; + try + { + billingInfo = await _paymentService.GetBillingAsync(user); + billingHistoryInfo = await _paymentService.GetBillingHistoryAsync(user); + } + catch (StripeException ex) when (ex.StripeError?.Code == StripeConstants.ErrorCodes.ResourceMissing) + { + billingInfo = null; + billingHistoryInfo = null; + _logger.LogError(ex, + "Billing information for user {UserId} could not be loaded because the Stripe customer was not found. It may have been deleted.", + user.Id); + TempData["Warning"] = + "Billing information could not be loaded. The Stripe customer may have been deleted. " + + "You can still edit the user and set a valid Gateway Customer ID."; + } + catch (Exception ex) + { + billingInfo = null; + billingHistoryInfo = null; + _logger.LogError(ex, + "Failed to load billing information for user {UserId}.", + user.Id); + TempData["Error"] = + "Billing information could not be loaded. You can still edit the user or try reloading the page. " + + "Contact support if the problem persists."; + } var isTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(user); var verifiedDomain = await _userService.IsClaimedByAnyOrganizationAsync(user.Id); var deviceVerificationRequired = await _userService.ActiveNewDeviceVerificationException(user.Id); diff --git a/src/Admin/Views/Users/Edit.cshtml b/src/Admin/Views/Users/Edit.cshtml index dd80889110f0..a9a07a369c1c 100644 --- a/src/Admin/Views/Users/Edit.cshtml +++ b/src/Admin/Views/Users/Edit.cshtml @@ -115,7 +115,7 @@ } -@if (canViewBillingInformation) +@if (canViewBillingInformation && Model.BillingInfo != null && Model.BillingHistoryInfo != null) {

Billing Information

@await Html.PartialAsync("_BillingInformation", diff --git a/src/Core/Billing/Extensions/CustomerExtensions.cs b/src/Core/Billing/Extensions/CustomerExtensions.cs index aa22331f7c10..e00be55aa04e 100644 --- a/src/Core/Billing/Extensions/CustomerExtensions.cs +++ b/src/Core/Billing/Extensions/CustomerExtensions.cs @@ -24,6 +24,7 @@ public static decimal GetBillingBalance(this Customer customer) } public static bool ApprovedToPayByInvoice(this Customer customer) - => customer.Metadata.TryGetValue(StripeConstants.MetadataKeys.InvoiceApproved, out var value) && + => customer.Metadata != null && + customer.Metadata.TryGetValue(StripeConstants.MetadataKeys.InvoiceApproved, out var value) && int.TryParse(value, out var invoiceApproved) && invoiceApproved == 1; } diff --git a/test/Admin.Test/AdminConsole/Controllers/ProvidersControllerTests.cs b/test/Admin.Test/AdminConsole/Controllers/ProvidersControllerTests.cs index 85a32b1531c5..2dd57c0bd733 100644 --- a/test/Admin.Test/AdminConsole/Controllers/ProvidersControllerTests.cs +++ b/test/Admin.Test/AdminConsole/Controllers/ProvidersControllerTests.cs @@ -3,12 +3,17 @@ using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.Providers.Interfaces; +using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Enums; +using Bit.Core.Billing.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ViewFeatures; using NSubstitute; using NSubstitute.ReceivedExtensions; +using Stripe; namespace Admin.Test.AdminConsole.Controllers; @@ -186,4 +191,39 @@ public async Task CreateResellerAsync_RedirectsToExpectedPage_AfterCreatingProvi Assert.Equal(expectedProviderId, actualResult.RouteValues["Id"]); } #endregion + + #region Edit (GET) + [BitAutoData] + [SutProviderCustomize] + [Theory] + public async Task Edit_Get_DeletedStripeCustomer_StillRendersPageWithWarning( + Provider provider, + SutProvider sutProvider) + { + // PM-40292: a deleted Stripe customer is returned as a stub (Deleted = true) with null + // Metadata. The page must still render, PayByInvoice must default to false rather than + // NRE'ing, and the admin must be warned so they can fix the Gateway Customer ID. + provider.Type = ProviderType.Msp; + provider.Status = ProviderStatusType.Billable; + + sutProvider.GetDependency().GetByIdAsync(provider.Id).Returns(provider); + sutProvider.GetDependency() + .GetCustomer(provider) + .Returns(new Customer { Deleted = true, Metadata = null }); + + sutProvider.Sut.TempData = + new TempDataDictionary(new DefaultHttpContext(), Substitute.For()); + + var result = await sutProvider.Sut.Edit(provider.Id); + + var view = Assert.IsType(result); + var model = Assert.IsType(view.Model); + Assert.False(model.PayByInvoice); + Assert.True(sutProvider.Sut.TempData.ContainsKey("Warning")); + Assert.Equal( + "Billing information could not be fully loaded. The Stripe customer may have been deleted. " + + "You can still edit the provider and set a valid Gateway Customer ID.", + (string)sutProvider.Sut.TempData["Warning"]); + } + #endregion } diff --git a/test/Admin.Test/Controllers/UsersControllerTests.cs b/test/Admin.Test/Controllers/UsersControllerTests.cs new file mode 100644 index 000000000000..cf83b2ffe759 --- /dev/null +++ b/test/Admin.Test/Controllers/UsersControllerTests.cs @@ -0,0 +1,131 @@ +using Bit.Admin.Controllers; +using Bit.Admin.Models; +using Bit.Core.Billing.Constants; +using Bit.Core.Billing.Models; +using Bit.Core.Billing.Services; +using Bit.Core.Entities; +using Bit.Core.Repositories; +using Bit.Core.Vault.Repositories; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ViewFeatures; +using NSubstitute; +using NSubstitute.ExceptionExtensions; +using Stripe; + +namespace Admin.Test.Controllers; + +[ControllerCustomize(typeof(UsersController))] +[SutProviderCustomize] +public class UsersControllerTests +{ + private static void StubEditGetDependencies(SutProvider sutProvider, User user) + { + sutProvider.GetDependency().GetByIdAsync(user.Id).Returns(user); + sutProvider.GetDependency() + .GetManyByUserIdAsync(user.Id, false) + .Returns(new List()); + + sutProvider.Sut.TempData = + new TempDataDictionary(new DefaultHttpContext(), Substitute.For()); + } + + [BitAutoData] + [SutProviderCustomize] + [Theory] + public async Task Edit_Get_BillingLoadThrows_StillRendersPageWithWarning( + User user, + SutProvider sutProvider) + { + // PM-40292: a deleted Stripe customer makes GetBillingAsync throw. The page must still + // render so an admin can correct the Gateway Customer ID rather than being locked out. + StubEditGetDependencies(sutProvider, user); + + sutProvider.GetDependency() + .GetBillingAsync(user) + .ThrowsAsync(new StripeException + { + StripeError = new StripeError { Code = StripeConstants.ErrorCodes.ResourceMissing } + }); + + var result = await sutProvider.Sut.Edit(user.Id); + + var view = Assert.IsType(result); + var model = Assert.IsType(view.Model); + Assert.Null(model.BillingInfo); + Assert.Null(model.BillingHistoryInfo); + Assert.True(sutProvider.Sut.TempData.ContainsKey("Warning")); + Assert.Equal( + "Billing information could not be loaded. The Stripe customer may have been deleted. " + + "You can still edit the user and set a valid Gateway Customer ID.", + (string)sutProvider.Sut.TempData["Warning"]); + } + + [BitAutoData] + [SutProviderCustomize] + [Theory] + public async Task Edit_Get_BillingHistoryLoadThrows_StillRendersPageWithWarning( + User user, + BillingInfo billingInfo, + SutProvider sutProvider) + { + // PM-40292: GetBillingAsync can succeed while GetBillingHistoryAsync throws. The catch must + // reset both values so the billing section is hidden and the page renders, rather than + // falling through with a non-null BillingInfo and a null BillingHistoryInfo. + StubEditGetDependencies(sutProvider, user); + + sutProvider.GetDependency() + .GetBillingAsync(user) + .Returns(billingInfo); + sutProvider.GetDependency() + .GetBillingHistoryAsync(user) + .ThrowsAsync(new StripeException + { + StripeError = new StripeError { Code = StripeConstants.ErrorCodes.ResourceMissing } + }); + + var result = await sutProvider.Sut.Edit(user.Id); + + var view = Assert.IsType(result); + var model = Assert.IsType(view.Model); + Assert.Null(model.BillingInfo); + Assert.Null(model.BillingHistoryInfo); + Assert.True(sutProvider.Sut.TempData.ContainsKey("Warning")); + Assert.Equal( + "Billing information could not be loaded. The Stripe customer may have been deleted. " + + "You can still edit the user and set a valid Gateway Customer ID.", + (string)sutProvider.Sut.TempData["Warning"]); + } + + [BitAutoData] + [SutProviderCustomize] + [Theory] + public async Task Edit_Get_BillingLoadThrowsUnexpectedError_StillRendersPageWithErrorToast( + User user, + SutProvider sutProvider) + { + // PM-40292: a billing-load failure that is NOT a missing Stripe customer (resource_missing) + // must fall through to the generic catch, which surfaces a neutral error toast rather than + // asserting the customer was deleted. + StubEditGetDependencies(sutProvider, user); + + sutProvider.GetDependency() + .GetBillingAsync(user) + .ThrowsAsync(new StripeException { StripeError = new StripeError { Code = "api_error" } }); + + var result = await sutProvider.Sut.Edit(user.Id); + + var view = Assert.IsType(result); + var model = Assert.IsType(view.Model); + Assert.Null(model.BillingInfo); + Assert.Null(model.BillingHistoryInfo); + Assert.False(sutProvider.Sut.TempData.ContainsKey("Warning")); + Assert.True(sutProvider.Sut.TempData.ContainsKey("Error")); + Assert.Equal( + "Billing information could not be loaded. You can still edit the user or try reloading the page. " + + "Contact support if the problem persists.", + (string)sutProvider.Sut.TempData["Error"]); + } +} diff --git a/test/Core.Test/Billing/Extensions/CustomerExtensionsTests.cs b/test/Core.Test/Billing/Extensions/CustomerExtensionsTests.cs new file mode 100644 index 000000000000..b483ae0ab114 --- /dev/null +++ b/test/Core.Test/Billing/Extensions/CustomerExtensionsTests.cs @@ -0,0 +1,49 @@ +using Bit.Core.Billing.Constants; +using Bit.Core.Billing.Extensions; +using Stripe; +using Xunit; + +namespace Bit.Core.Test.Billing.Extensions; + +public class CustomerExtensionsTests +{ + [Fact] + public void ApprovedToPayByInvoice_NullMetadata_ReturnsFalse() + { + // PM-40292: a deleted Stripe customer retrieve returns a stub with null Metadata. + // The unguarded TryGetValue previously NRE'd here, crashing the Provider admin page. + var customer = new Customer { Metadata = null }; + + Assert.False(customer.ApprovedToPayByInvoice()); + } + + [Fact] + public void ApprovedToPayByInvoice_Approved_ReturnsTrue() + { + var customer = new Customer + { + Metadata = new Dictionary { [StripeConstants.MetadataKeys.InvoiceApproved] = "1" } + }; + + Assert.True(customer.ApprovedToPayByInvoice()); + } + + [Fact] + public void ApprovedToPayByInvoice_NotApproved_ReturnsFalse() + { + var customer = new Customer + { + Metadata = new Dictionary { [StripeConstants.MetadataKeys.InvoiceApproved] = "0" } + }; + + Assert.False(customer.ApprovedToPayByInvoice()); + } + + [Fact] + public void ApprovedToPayByInvoice_KeyMissing_ReturnsFalse() + { + var customer = new Customer { Metadata = new Dictionary() }; + + Assert.False(customer.ApprovedToPayByInvoice()); + } +}