From a76a9cb8001f8cd54d2d89cb9f45ad26bb71d76a Mon Sep 17 00:00:00 2001 From: Jimmy Vo Date: Thu, 12 Dec 2024 10:18:11 -0500 Subject: [PATCH] [PM-14826] Add UsePolicies check to GET endpoints (#5046) GetByToken and GetMasterPasswordPolicy endpoints provide policy information, so if the organization is not using policies, then we avoid the rest of the logic. --- .../Controllers/PoliciesController.cs | 32 +- .../Controllers/PoliciesControllerTests.cs | 278 +++++++++++++++++- 2 files changed, 290 insertions(+), 20 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/PoliciesController.cs b/src/Api/AdminConsole/Controllers/PoliciesController.cs index 1167d7a86ccc..7de6f6e73066 100644 --- a/src/Api/AdminConsole/Controllers/PoliciesController.cs +++ b/src/Api/AdminConsole/Controllers/PoliciesController.cs @@ -27,19 +27,20 @@ namespace Bit.Api.AdminConsole.Controllers; [Authorize("Application")] public class PoliciesController : Controller { - private readonly IPolicyRepository _policyRepository; - private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly IUserService _userService; private readonly ICurrentContext _currentContext; + private readonly IFeatureService _featureService; private readonly GlobalSettings _globalSettings; + private readonly IOrganizationHasVerifiedDomainsQuery _organizationHasVerifiedDomainsQuery; + private readonly IOrganizationRepository _organizationRepository; private readonly IDataProtector _organizationServiceDataProtector; + private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; - private readonly IFeatureService _featureService; - private readonly IOrganizationHasVerifiedDomainsQuery _organizationHasVerifiedDomainsQuery; + private readonly IPolicyRepository _policyRepository; + private readonly IUserService _userService; + private readonly ISavePolicyCommand _savePolicyCommand; - public PoliciesController( - IPolicyRepository policyRepository, + public PoliciesController(IPolicyRepository policyRepository, IOrganizationUserRepository organizationUserRepository, IUserService userService, ICurrentContext currentContext, @@ -48,6 +49,7 @@ public PoliciesController( IDataProtectorTokenFactory orgUserInviteTokenDataFactory, IFeatureService featureService, IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery, + IOrganizationRepository organizationRepository, ISavePolicyCommand savePolicyCommand) { _policyRepository = policyRepository; @@ -57,7 +59,7 @@ public PoliciesController( _globalSettings = globalSettings; _organizationServiceDataProtector = dataProtectionProvider.CreateProtector( "OrganizationServiceDataProtector"); - + _organizationRepository = organizationRepository; _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; _featureService = featureService; _organizationHasVerifiedDomainsQuery = organizationHasVerifiedDomainsQuery; @@ -104,6 +106,13 @@ public async Task> Get(string orgId) public async Task> GetByToken(Guid orgId, [FromQuery] string email, [FromQuery] string token, [FromQuery] Guid organizationUserId) { + var organization = await _organizationRepository.GetByIdAsync(orgId); + + if (organization is not { UsePolicies: true }) + { + throw new NotFoundException(); + } + // TODO: PM-4142 - remove old token validation logic once 3 releases of backwards compatibility are complete var newTokenValid = OrgUserInviteTokenable.ValidateOrgUserInviteStringToken( _orgUserInviteTokenDataFactory, token, organizationUserId, email); @@ -158,6 +167,13 @@ public async Task> GetByInvitedUser(Guid [HttpGet("master-password")] public async Task GetMasterPasswordPolicy(Guid orgId) { + var organization = await _organizationRepository.GetByIdAsync(orgId); + + if (organization is not { UsePolicies: true }) + { + throw new NotFoundException(); + } + var userId = _userService.GetProperUserId(User).Value; var orgUser = await _organizationUserRepository.GetByOrganizationAsync(orgId, userId); diff --git a/test/Api.Test/Controllers/PoliciesControllerTests.cs b/test/Api.Test/Controllers/PoliciesControllerTests.cs index 1b96ace5d0cb..1f652c80f5bc 100644 --- a/test/Api.Test/Controllers/PoliciesControllerTests.cs +++ b/test/Api.Test/Controllers/PoliciesControllerTests.cs @@ -6,11 +6,13 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Tokens; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; @@ -28,10 +30,19 @@ public class PoliciesControllerTests [Theory] [BitAutoData] public async Task GetMasterPasswordPolicy_WhenCalled_ReturnsMasterPasswordPolicy( - SutProvider sutProvider, Guid orgId, Guid userId, OrganizationUser orgUser, - Policy policy, MasterPasswordPolicyData mpPolicyData) + SutProvider sutProvider, + Guid orgId, Guid userId, + OrganizationUser orgUser, + Policy policy, + MasterPasswordPolicyData mpPolicyData, + Organization organization) { // Arrange + organization.UsePolicies = true; + + var organizationRepository = sutProvider.GetDependency(); + organizationRepository.GetByIdAsync(orgId).Returns(organization); + sutProvider.GetDependency() .GetProperUserId(Arg.Any()) .Returns((Guid?)userId); @@ -135,6 +146,39 @@ public async Task GetMasterPasswordPolicy_PolicyNotEnabled_ThrowsNotFoundExcepti await Assert.ThrowsAsync(() => sutProvider.Sut.GetMasterPasswordPolicy(orgId)); } + [Theory] + [BitAutoData] + public async Task GetMasterPasswordPolicy_WhenUsePoliciesIsFalse_ThrowsNotFoundException( + SutProvider sutProvider, + Guid orgId) + { + // Arrange + var organizationRepository = sutProvider.GetDependency(); + organizationRepository.GetByIdAsync(orgId).Returns((Organization)null); + + + // Act & Assert + await Assert.ThrowsAsync(() => sutProvider.Sut.GetMasterPasswordPolicy(orgId)); + } + + [Theory] + [BitAutoData] + public async Task GetMasterPasswordPolicy_WhenOrgIsNull_ThrowsNotFoundException( + SutProvider sutProvider, + Guid orgId, + Organization organization) + { + // Arrange + organization.UsePolicies = false; + + var organizationRepository = sutProvider.GetDependency(); + organizationRepository.GetByIdAsync(orgId).Returns(organization); + + + // Act & Assert + await Assert.ThrowsAsync(() => sutProvider.Sut.GetMasterPasswordPolicy(orgId)); + } + [Theory] [BitAutoData] public async Task Get_WhenUserCanManagePolicies_WithExistingType_ReturnsExistingPolicy( @@ -142,16 +186,16 @@ public async Task Get_WhenUserCanManagePolicies_WithExistingType_ReturnsExisting { // Arrange sutProvider.GetDependency() - .ManagePolicies(orgId) - .Returns(true); + .ManagePolicies(orgId) + .Returns(true); policy.Type = (PolicyType)type; policy.Enabled = true; policy.Data = null; sutProvider.GetDependency() - .GetByOrganizationIdTypeAsync(orgId, (PolicyType)type) - .Returns(policy); + .GetByOrganizationIdTypeAsync(orgId, (PolicyType)type) + .Returns(policy); // Act var result = await sutProvider.Sut.Get(orgId, type); @@ -171,12 +215,12 @@ public async Task Get_WhenUserCanManagePolicies_WithNonExistingType_ReturnsDefau { // Arrange sutProvider.GetDependency() - .ManagePolicies(orgId) - .Returns(true); + .ManagePolicies(orgId) + .Returns(true); sutProvider.GetDependency() - .GetByOrganizationIdTypeAsync(orgId, (PolicyType)type) - .Returns((Policy)null); + .GetByOrganizationIdTypeAsync(orgId, (PolicyType)type) + .Returns((Policy)null); // Act var result = await sutProvider.Sut.Get(orgId, type); @@ -194,11 +238,221 @@ public async Task Get_WhenUserCannotManagePolicies_ThrowsNotFoundException( { // Arrange sutProvider.GetDependency() - .ManagePolicies(orgId) - .Returns(false); + .ManagePolicies(orgId) + .Returns(false); // Act & Assert await Assert.ThrowsAsync(() => sutProvider.Sut.Get(orgId, type)); } + [Theory] + [BitAutoData] + public async Task GetByToken_WhenOrganizationUseUsePoliciesIsFalse_ThrowsNotFoundException( + SutProvider sutProvider, Guid orgId, Guid organizationUserId, string token, string email, + Organization organization) + { + // Arrange + organization.UsePolicies = false; + + var organizationRepository = sutProvider.GetDependency(); + organizationRepository.GetByIdAsync(orgId).Returns(organization); + + + // Act & Assert + await Assert.ThrowsAsync(() => + sutProvider.Sut.GetByToken(orgId, email, token, organizationUserId)); + } + + [Theory] + [BitAutoData] + public async Task GetByToken_WhenOrganizationIsNull_ThrowsNotFoundException( + SutProvider sutProvider, Guid orgId, Guid organizationUserId, string token, string email) + { + // Arrange + var organizationRepository = sutProvider.GetDependency(); + organizationRepository.GetByIdAsync(orgId).Returns((Organization)null); + + // Act & Assert + await Assert.ThrowsAsync(() => + sutProvider.Sut.GetByToken(orgId, email, token, organizationUserId)); + } + + [Theory] + [BitAutoData] + public async Task GetByToken_WhenTokenIsInvalid_ThrowsNotFoundException( + SutProvider sutProvider, + Guid orgId, + Guid organizationUserId, + string token, + string email, + Organization organization + ) + { + // Arrange + organization.UsePolicies = true; + + var organizationRepository = sutProvider.GetDependency(); + organizationRepository.GetByIdAsync(orgId).Returns(organization); + + var decryptedToken = Substitute.For(); + decryptedToken.Valid.Returns(false); + + var orgUserInviteTokenDataFactory = sutProvider.GetDependency>(); + + orgUserInviteTokenDataFactory.TryUnprotect(token, out Arg.Any()) + .Returns(x => + { + x[1] = decryptedToken; + return true; + }); + + // Act & Assert + await Assert.ThrowsAsync(() => + sutProvider.Sut.GetByToken(orgId, email, token, organizationUserId)); + } + + [Theory] + [BitAutoData] + public async Task GetByToken_WhenUserIsNull_ThrowsNotFoundException( + SutProvider sutProvider, + Guid orgId, + Guid organizationUserId, + string token, + string email, + Organization organization + ) + { + // Arrange + organization.UsePolicies = true; + + var organizationRepository = sutProvider.GetDependency(); + organizationRepository.GetByIdAsync(orgId).Returns(organization); + + var decryptedToken = Substitute.For(); + decryptedToken.Valid.Returns(true); + decryptedToken.OrgUserId = organizationUserId; + decryptedToken.OrgUserEmail = email; + + var orgUserInviteTokenDataFactory = sutProvider.GetDependency>(); + + orgUserInviteTokenDataFactory.TryUnprotect(token, out Arg.Any()) + .Returns(x => + { + x[1] = decryptedToken; + return true; + }); + + sutProvider.GetDependency() + .GetByIdAsync(organizationUserId) + .Returns((OrganizationUser)null); + + // Act & Assert + await Assert.ThrowsAsync(() => + sutProvider.Sut.GetByToken(orgId, email, token, organizationUserId)); + } + + [Theory] + [BitAutoData] + public async Task GetByToken_WhenUserOrgIdDoesNotMatchOrgId_ThrowsNotFoundException( + SutProvider sutProvider, + Guid orgId, + Guid organizationUserId, + string token, + string email, + OrganizationUser orgUser, + Organization organization + ) + { + // Arrange + organization.UsePolicies = true; + + var organizationRepository = sutProvider.GetDependency(); + organizationRepository.GetByIdAsync(orgId).Returns(organization); + + var decryptedToken = Substitute.For(); + decryptedToken.Valid.Returns(true); + decryptedToken.OrgUserId = organizationUserId; + decryptedToken.OrgUserEmail = email; + + var orgUserInviteTokenDataFactory = sutProvider.GetDependency>(); + + orgUserInviteTokenDataFactory.TryUnprotect(token, out Arg.Any()) + .Returns(x => + { + x[1] = decryptedToken; + return true; + }); + + orgUser.OrganizationId = Guid.Empty; + + sutProvider.GetDependency() + .GetByIdAsync(organizationUserId) + .Returns(orgUser); + + // Act & Assert + await Assert.ThrowsAsync(() => + sutProvider.Sut.GetByToken(orgId, email, token, organizationUserId)); + } + + [Theory] + [BitAutoData] + public async Task GetByToken_ShouldReturnEnabledPolicies( + SutProvider sutProvider, + Guid orgId, + Guid organizationUserId, + string token, + string email, + OrganizationUser orgUser, + Organization organization + ) + { + // Arrange + organization.UsePolicies = true; + + var organizationRepository = sutProvider.GetDependency(); + organizationRepository.GetByIdAsync(orgId).Returns(organization); + + var decryptedToken = Substitute.For(); + decryptedToken.Valid.Returns(true); + decryptedToken.OrgUserId = organizationUserId; + decryptedToken.OrgUserEmail = email; + + var orgUserInviteTokenDataFactory = sutProvider.GetDependency>(); + + orgUserInviteTokenDataFactory.TryUnprotect(token, out Arg.Any()) + .Returns(x => + { + x[1] = decryptedToken; + return true; + }); + + orgUser.OrganizationId = orgId; + sutProvider.GetDependency() + .GetByIdAsync(organizationUserId) + .Returns(orgUser); + + var enabledPolicy = Substitute.For(); + enabledPolicy.Enabled = true; + var disabledPolicy = Substitute.For(); + disabledPolicy.Enabled = false; + + var policies = new[] { enabledPolicy, disabledPolicy }; + + + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(orgId) + .Returns(policies); + + // Act + var result = await sutProvider.Sut.GetByToken(orgId, email, token, organizationUserId); + + // Assert + var expectedPolicy = result.Data.Single(); + + Assert.NotNull(result); + + Assert.Equal(enabledPolicy.Id, expectedPolicy.Id); + Assert.Equal(enabledPolicy.Type, expectedPolicy.Type); + Assert.Equal(enabledPolicy.Enabled, expectedPolicy.Enabled); + } }