From 45ec57f81b7f25bbd2f14243e254448accfa4c23 Mon Sep 17 00:00:00 2001 From: Conner Turnbull <133619638+cturnbull-bitwarden@users.noreply.github.com> Date: Wed, 17 Jul 2024 16:15:28 -0400 Subject: [PATCH] [AC-2887] Added Billing Authorization Where Missing (#4525) * Added missing authorization validation to OrganizationBillingController endpoints * Moved authorization validation to top of each method * Resolved broken unit tests and added some new ones --- .../OrganizationBillingController.cs | 10 +++ .../Controllers/OrganizationsController.cs | 12 ++-- .../OrganizationBillingControllerTests.cs | 69 +++++++++++++++++++ 3 files changed, 85 insertions(+), 6 deletions(-) diff --git a/src/Api/Billing/Controllers/OrganizationBillingController.cs b/src/Api/Billing/Controllers/OrganizationBillingController.cs index 2f5b493567..840f012ba1 100644 --- a/src/Api/Billing/Controllers/OrganizationBillingController.cs +++ b/src/Api/Billing/Controllers/OrganizationBillingController.cs @@ -20,6 +20,11 @@ public class OrganizationBillingController( [HttpGet("metadata")] public async Task GetMetadataAsync([FromRoute] Guid organizationId) { + if (!await currentContext.ViewBillingHistory(organizationId)) + { + return TypedResults.Unauthorized(); + } + var metadata = await organizationBillingService.GetMetadata(organizationId); if (metadata == null) @@ -35,6 +40,11 @@ public class OrganizationBillingController( [HttpGet("history")] public async Task GetHistoryAsync([FromRoute] Guid organizationId) { + if (!await currentContext.ViewBillingHistory(organizationId)) + { + return TypedResults.Unauthorized(); + } + var organization = await organizationRepository.GetByIdAsync(organizationId); if (organization == null) diff --git a/src/Api/Billing/Controllers/OrganizationsController.cs b/src/Api/Billing/Controllers/OrganizationsController.cs index f3323ae806..d09246b186 100644 --- a/src/Api/Billing/Controllers/OrganizationsController.cs +++ b/src/Api/Billing/Controllers/OrganizationsController.cs @@ -162,13 +162,13 @@ public class OrganizationsController( [SelfHosted(NotSelfHostedOnly = true)] public async Task PostSmSubscription(Guid id, [FromBody] SecretsManagerSubscriptionUpdateRequestModel model) { - var organization = await organizationRepository.GetByIdAsync(id); - if (organization == null) + if (!await currentContext.EditSubscription(id)) { throw new NotFoundException(); } - if (!await currentContext.EditSubscription(id)) + var organization = await organizationRepository.GetByIdAsync(id); + if (organization == null) { throw new NotFoundException(); } @@ -195,13 +195,13 @@ public class OrganizationsController( [SelfHosted(NotSelfHostedOnly = true)] public async Task PostSubscribeSecretsManagerAsync(Guid id, [FromBody] SecretsManagerSubscribeRequestModel model) { - var organization = await organizationRepository.GetByIdAsync(id); - if (organization == null) + if (!await currentContext.EditSubscription(id)) { throw new NotFoundException(); } - if (!await currentContext.EditSubscription(id)) + var organization = await organizationRepository.GetByIdAsync(id); + if (organization == null) { throw new NotFoundException(); } diff --git a/test/Api.Test/Billing/Controllers/OrganizationBillingControllerTests.cs b/test/Api.Test/Billing/Controllers/OrganizationBillingControllerTests.cs index 021705bed5..fd5c8cdd31 100644 --- a/test/Api.Test/Billing/Controllers/OrganizationBillingControllerTests.cs +++ b/test/Api.Test/Billing/Controllers/OrganizationBillingControllerTests.cs @@ -1,7 +1,11 @@ using Bit.Api.Billing.Controllers; using Bit.Api.Billing.Models.Responses; +using Bit.Core.AdminConsole.Entities; using Bit.Core.Billing.Models; using Bit.Core.Billing.Services; +using Bit.Core.Context; +using Bit.Core.Repositories; +using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Microsoft.AspNetCore.Http.HttpResults; @@ -14,11 +18,26 @@ namespace Bit.Api.Test.Billing.Controllers; [SutProviderCustomize] public class OrganizationBillingControllerTests { + [Theory, BitAutoData] + public async Task GetMetadataAsync_Unauthorized_ReturnsUnauthorized( + Guid organizationId, + SutProvider sutProvider) + { + sutProvider.GetDependency().ViewBillingHistory(organizationId).Returns(false); + + var result = await sutProvider.Sut.GetMetadataAsync(organizationId); + + Assert.IsType(result); + } + [Theory, BitAutoData] public async Task GetMetadataAsync_MetadataNull_NotFound( Guid organizationId, SutProvider sutProvider) { + sutProvider.GetDependency().ViewBillingHistory(organizationId).Returns(true); + sutProvider.GetDependency().GetMetadata(organizationId).Returns((OrganizationMetadataDTO)null); + var result = await sutProvider.Sut.GetMetadataAsync(organizationId); Assert.IsType(result); @@ -29,6 +48,7 @@ public class OrganizationBillingControllerTests Guid organizationId, SutProvider sutProvider) { + sutProvider.GetDependency().ViewBillingHistory(organizationId).Returns(true); sutProvider.GetDependency().GetMetadata(organizationId) .Returns(new OrganizationMetadataDTO(true)); @@ -40,4 +60,53 @@ public class OrganizationBillingControllerTests Assert.True(organizationMetadataResponse.IsOnSecretsManagerStandalone); } + + [Theory, BitAutoData] + public async Task GetHistoryAsync_Unauthorized_ReturnsUnauthorized( + Guid organizationId, + SutProvider sutProvider) + { + sutProvider.GetDependency().ViewBillingHistory(organizationId).Returns(false); + + var result = await sutProvider.Sut.GetHistoryAsync(organizationId); + + Assert.IsType(result); + } + + [Theory, BitAutoData] + public async Task GetHistoryAsync_OrganizationNotFound_ReturnsNotFound( + Guid organizationId, + SutProvider sutProvider) + { + sutProvider.GetDependency().ViewBillingHistory(organizationId).Returns(true); + sutProvider.GetDependency().GetByIdAsync(organizationId).Returns((Organization)null); + + var result = await sutProvider.Sut.GetHistoryAsync(organizationId); + + Assert.IsType(result); + } + + [Theory] + [BitAutoData] + public async Task GetHistoryAsync_OK( + Guid organizationId, + Organization organization, + SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().ViewBillingHistory(organizationId).Returns(true); + sutProvider.GetDependency().GetByIdAsync(organizationId).Returns(organization); + + // Manually create a BillingHistoryInfo object to avoid requiring AutoFixture to create HttpResponseHeaders + var billingInfo = new BillingHistoryInfo(); + + sutProvider.GetDependency().GetBillingHistoryAsync(organization).Returns(billingInfo); + + // Act + var result = await sutProvider.Sut.GetHistoryAsync(organizationId); + + // Assert + var okResult = Assert.IsType>(result); + Assert.Equal(billingInfo, okResult.Value); + } }