From 05f11a8ee1ab2a993a234a6d375e026e79830e64 Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Thu, 8 Jun 2023 16:40:35 -0500 Subject: [PATCH] [SM-706] Extract Authorization From Create/Update Secret Commands (#2896) * Extract authorization from commands * Swap to request model validation. * Swap to pattern detection --- .../Secrets/SecretAuthorizationHandler.cs | 123 ++++++ .../Commands/Secrets/CreateSecretCommand.cs | 39 +- .../Commands/Secrets/UpdateSecretCommand.cs | 44 +- .../SecretsManagerCollectionExtensions.cs | 2 + .../SecretAuthorizationHandlerTests.cs | 377 ++++++++++++++++++ .../Secrets/CreateSecretCommandTests.cs | 58 +-- .../Secrets/UpdateSecretCommandTests.cs | 55 +-- .../Controllers/SecretsController.cs | 34 +- .../Request/SecretCreateRequestModel.cs | 12 +- .../Request/SecretUpdateRequestModel.cs | 15 +- .../SecretOperationRequirement.cs | 13 + .../Interfaces/ICreateSecretCommand.cs | 2 +- .../Interfaces/IUpdateSecretCommand.cs | 2 +- .../Controllers/SecretsControllerTests.cs | 3 - .../Controllers/SecretsControllerTests.cs | 126 ++++-- 15 files changed, 674 insertions(+), 231 deletions(-) create mode 100644 bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs create mode 100644 bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandlerTests.cs create mode 100644 src/Core/SecretsManager/AuthorizationRequirements/SecretOperationRequirement.cs diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs new file mode 100644 index 0000000000..7ddc8b8867 --- /dev/null +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs @@ -0,0 +1,123 @@ +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.SecretsManager.AuthorizationRequirements; +using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Queries.Interfaces; +using Bit.Core.SecretsManager.Repositories; +using Microsoft.AspNetCore.Authorization; + +namespace Bit.Commercial.Core.SecretsManager.AuthorizationHandlers.Secrets; + +public class SecretAuthorizationHandler : AuthorizationHandler +{ + private readonly ICurrentContext _currentContext; + private readonly IAccessClientQuery _accessClientQuery; + private readonly IProjectRepository _projectRepository; + private readonly ISecretRepository _secretRepository; + + public SecretAuthorizationHandler(ICurrentContext currentContext, IAccessClientQuery accessClientQuery, + IProjectRepository projectRepository, ISecretRepository secretRepository) + { + _currentContext = currentContext; + _accessClientQuery = accessClientQuery; + _projectRepository = projectRepository; + _secretRepository = secretRepository; + } + + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, + SecretOperationRequirement requirement, + Secret resource) + { + if (!_currentContext.AccessSecretsManager(resource.OrganizationId)) + { + return; + } + + switch (requirement) + { + case not null when requirement == SecretOperations.Create: + await CanCreateSecretAsync(context, requirement, resource); + break; + case not null when requirement == SecretOperations.Update: + await CanUpdateSecretAsync(context, requirement, resource); + break; + default: + throw new ArgumentException("Unsupported operation requirement type provided.", nameof(requirement)); + } + } + + private async Task CanCreateSecretAsync(AuthorizationHandlerContext context, + SecretOperationRequirement requirement, Secret resource) + { + var (accessClient, userId) = await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId); + var project = resource.Projects?.FirstOrDefault(); + + if (project == null && accessClient != AccessClientType.NoAccessCheck) + { + return; + } + + // All projects should be apart of the same organization + if (resource.Projects != null + && resource.Projects.Any() + && !await _projectRepository.ProjectsAreInOrganization(resource.Projects.Select(p => p.Id).ToList(), + resource.OrganizationId)) + { + return; + } + + var hasAccess = accessClient switch + { + AccessClientType.NoAccessCheck => true, + AccessClientType.User => (await _projectRepository.AccessToProjectAsync(project!.Id, userId, accessClient)) + .Write, + AccessClientType.ServiceAccount => false, + _ => false, + }; + + if (hasAccess) + { + context.Succeed(requirement); + } + } + + private async Task CanUpdateSecretAsync(AuthorizationHandlerContext context, + SecretOperationRequirement requirement, Secret resource) + { + var (accessClient, userId) = await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId); + + // All projects should be apart of the same organization + if (resource.Projects != null + && resource.Projects.Any() + && !await _projectRepository.ProjectsAreInOrganization(resource.Projects.Select(p => p.Id).ToList(), + resource.OrganizationId)) + { + return; + } + + bool hasAccess; + + switch (accessClient) + { + case AccessClientType.NoAccessCheck: + hasAccess = true; + break; + case AccessClientType.User: + var newProject = resource.Projects?.FirstOrDefault(); + var access = (await _secretRepository.AccessToSecretAsync(resource.Id, userId, accessClient)).Write; + var accessToNew = newProject != null && + (await _projectRepository.AccessToProjectAsync(newProject.Id, userId, accessClient)) + .Write; + hasAccess = access && accessToNew; + break; + default: + hasAccess = false; + break; + } + + if (hasAccess) + { + context.Succeed(requirement); + } + } +} diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/CreateSecretCommand.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/CreateSecretCommand.cs index 8d5e090827..d224247c1d 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/CreateSecretCommand.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/CreateSecretCommand.cs @@ -1,7 +1,4 @@ -using Bit.Core.Context; -using Bit.Core.Enums; -using Bit.Core.Exceptions; -using Bit.Core.SecretsManager.Commands.Secrets.Interfaces; +using Bit.Core.SecretsManager.Commands.Secrets.Interfaces; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Repositories; @@ -10,44 +7,14 @@ namespace Bit.Commercial.Core.SecretsManager.Commands.Secrets; public class CreateSecretCommand : ICreateSecretCommand { private readonly ISecretRepository _secretRepository; - private readonly IProjectRepository _projectRepository; - private readonly ICurrentContext _currentContext; - public CreateSecretCommand(ISecretRepository secretRepository, IProjectRepository projectRepository, ICurrentContext currentContext) + public CreateSecretCommand(ISecretRepository secretRepository) { _secretRepository = secretRepository; - _projectRepository = projectRepository; - _currentContext = currentContext; } - public async Task CreateAsync(Secret secret, Guid userId) + public async Task CreateAsync(Secret secret) { - var orgAdmin = await _currentContext.OrganizationAdmin(secret.OrganizationId); - var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin); - var project = secret.Projects?.FirstOrDefault(); - - if (project == null && !orgAdmin) - { - throw new NotFoundException(); - } - - if (secret.Projects != null && secret.Projects.Any() && !(await _projectRepository.ProjectsAreInOrganization(secret.Projects.Select(p => p.Id).ToList(), secret.OrganizationId))) - { - throw new NotFoundException(); - } - - var hasAccess = accessClient switch - { - AccessClientType.NoAccessCheck => true, - AccessClientType.User => (await _projectRepository.AccessToProjectAsync(project.Id, userId, accessClient)).Write, - _ => false, - }; - - if (!hasAccess) - { - throw new NotFoundException(); - } - return await _secretRepository.CreateAsync(secret); } } diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/UpdateSecretCommand.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/UpdateSecretCommand.cs index 0b5509efd8..c3c757bae7 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/UpdateSecretCommand.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/UpdateSecretCommand.cs @@ -1,6 +1,4 @@ -using Bit.Core.Context; -using Bit.Core.Enums; -using Bit.Core.Exceptions; +using Bit.Core.Exceptions; using Bit.Core.SecretsManager.Commands.Secrets.Interfaces; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Repositories; @@ -10,33 +8,16 @@ namespace Bit.Commercial.Core.SecretsManager.Commands.Secrets; public class UpdateSecretCommand : IUpdateSecretCommand { private readonly ISecretRepository _secretRepository; - private readonly IProjectRepository _projectRepository; - private readonly ICurrentContext _currentContext; - public UpdateSecretCommand(ISecretRepository secretRepository, IProjectRepository projectRepository, ICurrentContext currentContext) + public UpdateSecretCommand(ISecretRepository secretRepository) { _secretRepository = secretRepository; - _projectRepository = projectRepository; - _currentContext = currentContext; } - public async Task UpdateAsync(Secret updatedSecret, Guid userId) + public async Task UpdateAsync(Secret updatedSecret) { var secret = await _secretRepository.GetByIdAsync(updatedSecret.Id); - if (secret == null || !_currentContext.AccessSecretsManager(secret.OrganizationId)) - { - throw new NotFoundException(); - } - - if (updatedSecret.Projects != null && updatedSecret.Projects.Any() && !(await _projectRepository.ProjectsAreInOrganization(updatedSecret.Projects.Select(p => p.Id).ToList(), secret.OrganizationId))) - { - throw new NotFoundException(); - } - - var orgAdmin = await _currentContext.OrganizationAdmin(secret.OrganizationId); - var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin); - - if (!await HasAccessToOriginalAndUpdatedProject(accessClient, secret, updatedSecret, userId)) + if (secret == null) { throw new NotFoundException(); } @@ -50,21 +31,4 @@ public class UpdateSecretCommand : IUpdateSecretCommand await _secretRepository.UpdateAsync(secret); return secret; } - - private async Task HasAccessToOriginalAndUpdatedProject(AccessClientType accessClient, Secret secret, Secret updatedSecret, Guid userId) - { - switch (accessClient) - { - case AccessClientType.NoAccessCheck: - return true; - case AccessClientType.User: - var oldProject = secret.Projects?.FirstOrDefault(); - var newProject = updatedSecret.Projects?.FirstOrDefault(); - var accessToOld = oldProject != null && (await _projectRepository.AccessToProjectAsync(oldProject.Id, userId, accessClient)).Write; - var accessToNew = newProject != null && (await _projectRepository.AccessToProjectAsync(newProject.Id, userId, accessClient)).Write; - return accessToOld && accessToNew; - default: - return false; - } - } } diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs index 4f01f74604..b5266e2868 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs @@ -1,4 +1,5 @@ using Bit.Commercial.Core.SecretsManager.AuthorizationHandlers.Projects; +using Bit.Commercial.Core.SecretsManager.AuthorizationHandlers.Secrets; using Bit.Commercial.Core.SecretsManager.AuthorizationHandlers.ServiceAccounts; using Bit.Commercial.Core.SecretsManager.Commands.AccessPolicies; using Bit.Commercial.Core.SecretsManager.Commands.AccessTokens; @@ -26,6 +27,7 @@ public static class SecretsManagerCollectionExtensions public static void AddSecretsManagerServices(this IServiceCollection services) { services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandlerTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandlerTests.cs new file mode 100644 index 0000000000..24b9d481e5 --- /dev/null +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandlerTests.cs @@ -0,0 +1,377 @@ +using System.Reflection; +using System.Security.Claims; +using Bit.Commercial.Core.SecretsManager.AuthorizationHandlers.Secrets; +using Bit.Commercial.Core.Test.SecretsManager.Enums; +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.SecretsManager.AuthorizationRequirements; +using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Queries.Interfaces; +using Bit.Core.SecretsManager.Repositories; +using Bit.Core.Test.SecretsManager.AutoFixture.ProjectsFixture; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Authorization; +using NSubstitute; +using Xunit; + +namespace Bit.Commercial.Core.Test.SecretsManager.AuthorizationHandlers.Secrets; + +[SutProviderCustomize] +[ProjectCustomize] +public class SecretAuthorizationHandlerTests +{ + private static void SetupPermission(SutProvider sutProvider, + PermissionType permissionType, Guid organizationId, Guid userId = new(), + AccessClientType clientType = AccessClientType.User) + { + sutProvider.GetDependency().AccessSecretsManager(organizationId) + .Returns(true); + + sutProvider.GetDependency().ProjectsAreInOrganization(default, default) + .ReturnsForAnyArgs(true); + + switch (permissionType) + { + case PermissionType.RunAsAdmin: + sutProvider.GetDependency().GetAccessClientAsync(default, organizationId).ReturnsForAnyArgs( + (AccessClientType.NoAccessCheck, userId)); + break; + case PermissionType.RunAsUserWithPermission: + sutProvider.GetDependency().GetAccessClientAsync(default, organizationId).ReturnsForAnyArgs( + (clientType, userId)); + break; + default: + throw new ArgumentOutOfRangeException(nameof(permissionType), permissionType, null); + } + } + + [Fact] + public void SecretOperations_OnlyPublicStatic() + { + var publicStaticFields = typeof(SecretOperations).GetFields(BindingFlags.Public | BindingFlags.Static); + var allFields = typeof(SecretOperations).GetFields(); + Assert.Equal(publicStaticFields.Length, allFields.Length); + } + + [Theory] + [BitAutoData] + public async Task Handler_UnsupportedSecretOperationRequirement_Throws( + SutProvider sutProvider, Secret secret, ClaimsPrincipal claimsPrincipal) + { + sutProvider.GetDependency().AccessSecretsManager(secret.OrganizationId) + .Returns(true); + var requirement = new SecretOperationRequirement(); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await Assert.ThrowsAsync(() => sutProvider.Sut.HandleAsync(authzContext)); + } + + [Theory] + [BitAutoData] + public async Task Handler_SupportedSecretOperationRequirement_Throws( + SutProvider sutProvider, Secret secret, ClaimsPrincipal claimsPrincipal) + { + sutProvider.GetDependency().AccessSecretsManager(secret.OrganizationId) + .Returns(true); + var requirements = typeof(SecretOperations).GetFields(BindingFlags.Public | BindingFlags.Static) + .Select(i => (SecretOperationRequirement)i.GetValue(null)); + + foreach (var req in requirements) + { + var authzContext = new AuthorizationHandlerContext(new List { req }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + } + } + + [Theory] + [BitAutoData] + public async Task CanCreateSecret_AccessToSecretsManagerFalse_DoesNotSucceed( + SutProvider sutProvider, Secret secret, + ClaimsPrincipal claimsPrincipal) + { + sutProvider.GetDependency().AccessSecretsManager(secret.OrganizationId) + .Returns(false); + var requirement = SecretOperations.Create; + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.ServiceAccount)] + [BitAutoData(AccessClientType.Organization)] + public async Task CanCreateSecret_NotSupportedClientTypes_DoesNotSucceed(AccessClientType clientType, + SutProvider sutProvider, Secret secret, Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretOperations.Create; + SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId, clientType); + sutProvider.GetDependency() + .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns( + (true, true)); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanCreateSecret_ProjectsNotInOrg_DoesNotSucceed( + SutProvider sutProvider, Secret secret, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretOperations.Create; + SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId); + sutProvider.GetDependency().ProjectsAreInOrganization(default, default) + .ReturnsForAnyArgs(false); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanCreateSecret_WithoutProjectUser_DoesNotSucceed( + SutProvider sutProvider, Secret secret, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + secret.Projects = null; + var requirement = SecretOperations.Create; + SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanCreateSecret_WithoutProjectAdmin_Success(SutProvider sutProvider, + Secret secret, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + secret.Projects = null; + var requirement = SecretOperations.Create; + SetupPermission(sutProvider, PermissionType.RunAsAdmin, secret.OrganizationId, userId); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, false)] + public async Task CanCreateSecret_DoesNotSucceed(PermissionType permissionType, bool read, bool write, + SutProvider sutProvider, Secret secret, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretOperations.Create; + SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); + sutProvider.GetDependency() + .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns( + (read, write)); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(PermissionType.RunAsAdmin, true, true)] + [BitAutoData(PermissionType.RunAsAdmin, false, true)] + [BitAutoData(PermissionType.RunAsAdmin, true, false)] + [BitAutoData(PermissionType.RunAsAdmin, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, true)] + public async Task CanCreateSecret_Success(PermissionType permissionType, bool read, bool write, + SutProvider sutProvider, Secret secret, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretOperations.Create; + SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); + sutProvider.GetDependency() + .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns( + (read, write)); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanUpdateSecret_AccessToSecretsManagerFalse_DoesNotSucceed( + SutProvider sutProvider, Secret secret, + ClaimsPrincipal claimsPrincipal) + { + sutProvider.GetDependency().AccessSecretsManager(secret.OrganizationId) + .Returns(false); + var requirement = SecretOperations.Update; + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.ServiceAccount)] + [BitAutoData(AccessClientType.Organization)] + public async Task CanUpdateSecret_NotSupportedClientTypes_DoesNotSucceed(AccessClientType clientType, + SutProvider sutProvider, Secret secret, Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretOperations.Update; + SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId, clientType); + sutProvider.GetDependency() + .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns( + (true, true)); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanUpdateSecret_ProjectsNotInOrg_DoesNotSucceed( + SutProvider sutProvider, Secret secret, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretOperations.Update; + SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId); + sutProvider.GetDependency().ProjectsAreInOrganization(default, default) + .ReturnsForAnyArgs(false); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanUpdateSecret_WithoutProjectUser_DoesNotSucceed( + SutProvider sutProvider, Secret secret, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + secret.Projects = null; + var requirement = SecretOperations.Update; + SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanUpdateSecret_WithoutProjectAdmin_Success(SutProvider sutProvider, + Secret secret, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + secret.Projects = null; + var requirement = SecretOperations.Update; + SetupPermission(sutProvider, PermissionType.RunAsAdmin, secret.OrganizationId, userId); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, true, true, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, true, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, true, true, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, true, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, false, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, false, false, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, false, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, false, false, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, false, false, false)] + public async Task CanUpdateSecret_DoesNotSucceed(PermissionType permissionType, bool read, bool write, + bool projectRead, bool projectWrite, + SutProvider sutProvider, Secret secret, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretOperations.Update; + SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); + sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, default).Returns( + (read, write)); + sutProvider.GetDependency() + .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns( + (projectRead, projectWrite)); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(PermissionType.RunAsAdmin, true, true)] + [BitAutoData(PermissionType.RunAsAdmin, false, true)] + [BitAutoData(PermissionType.RunAsAdmin, true, false)] + [BitAutoData(PermissionType.RunAsAdmin, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, true)] + public async Task CanUpdateSecret_Success(PermissionType permissionType, bool read, bool write, + SutProvider sutProvider, Secret secret, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretOperations.Update; + SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); + sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, default).Returns( + (read, write)); + sutProvider.GetDependency() + .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, default).Returns( + (read, write)); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } +} diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/Secrets/CreateSecretCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/Secrets/CreateSecretCommandTests.cs index bf2605ba20..280aae26fd 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/Secrets/CreateSecretCommandTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/Secrets/CreateSecretCommandTests.cs @@ -1,8 +1,4 @@ using Bit.Commercial.Core.SecretsManager.Commands.Secrets; -using Bit.Commercial.Core.Test.SecretsManager.Enums; -using Bit.Core.Context; -using Bit.Core.Enums; -using Bit.Core.Exceptions; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Repositories; using Bit.Core.Test.SecretsManager.AutoFixture.SecretsFixture; @@ -18,61 +14,15 @@ namespace Bit.Commercial.Core.Test.SecretsManager.Commands.Secrets; public class CreateSecretCommandTests { [Theory] - [BitAutoData(PermissionType.RunAsAdmin)] - [BitAutoData(PermissionType.RunAsUserWithPermission)] - public async Task CreateAsync_Success(PermissionType permissionType, Secret data, - SutProvider sutProvider, Guid userId, Project mockProject) + [BitAutoData] + public async Task CreateAsync_Success(Secret data, + SutProvider sutProvider, Project mockProject) { - sutProvider.GetDependency().ProjectsAreInOrganization(default, default).ReturnsForAnyArgs(true); - - mockProject.OrganizationId = data.OrganizationId; data.Projects = new List() { mockProject }; - if (permissionType == PermissionType.RunAsAdmin) - { - sutProvider.GetDependency().OrganizationAdmin(data.OrganizationId).Returns(true); - sutProvider.GetDependency().AccessToProjectAsync((Guid)data.Projects?.First().Id, userId, AccessClientType.NoAccessCheck) - .Returns((true, true)); - } - else - { - sutProvider.GetDependency().OrganizationAdmin(data.OrganizationId).Returns(false); - sutProvider.GetDependency().AccessToProjectAsync((Guid)data.Projects?.First().Id, userId, AccessClientType.User) - .Returns((true, true)); - } - - await sutProvider.Sut.CreateAsync(data, userId); + await sutProvider.Sut.CreateAsync(data); await sutProvider.GetDependency().Received(1) .CreateAsync(data); } - - - [Theory] - [BitAutoData] - public async Task CreateAsync_UserWithoutPermission_ThrowsNotFound(Secret data, - SutProvider sutProvider, Guid userId, Project mockProject) - { - data.Projects = new List() { mockProject }; - - sutProvider.GetDependency().OrganizationAdmin(data.OrganizationId).Returns(false); - sutProvider.GetDependency().AccessToProjectAsync((Guid)data.Projects?.First().Id, userId, AccessClientType.User) - .Returns((false, false)); - - await Assert.ThrowsAsync(() => - sutProvider.Sut.CreateAsync(data, userId)); - } - - [Theory] - [BitAutoData] - public async Task CreateAsync_NoProjects_User_ThrowsNotFound(Secret data, - SutProvider sutProvider, Guid userId) - { - data.Projects = null; - sutProvider.GetDependency().OrganizationAdmin(data.OrganizationId).Returns(false); - - await Assert.ThrowsAsync(() => - sutProvider.Sut.CreateAsync(data, userId)); - } } - diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/Secrets/UpdateSecretCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/Secrets/UpdateSecretCommandTests.cs index fff5d1a936..252fbb34bd 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/Secrets/UpdateSecretCommandTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/Secrets/UpdateSecretCommandTests.cs @@ -1,7 +1,4 @@ using Bit.Commercial.Core.SecretsManager.Commands.Secrets; -using Bit.Commercial.Core.Test.SecretsManager.Enums; -using Bit.Core.Context; -using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Repositories; @@ -24,36 +21,20 @@ public class UpdateSecretCommandTests [BitAutoData] public async Task UpdateAsync_SecretDoesNotExist_ThrowsNotFound(Secret data, SutProvider sutProvider) { - var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateAsync(data, default)); + await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateAsync(data)); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().UpdateAsync(default); } [Theory] - [BitAutoData(PermissionType.RunAsAdmin)] - [BitAutoData(PermissionType.RunAsUserWithPermission)] - public async Task UpdateAsync_Success(PermissionType permissionType, Secret data, SutProvider sutProvider, Guid userId, Project mockProject) + [BitAutoData] + public async Task UpdateAsync_Success(Secret existingSecret, Secret data, SutProvider sutProvider, Project mockProject) { - sutProvider.GetDependency().AccessSecretsManager(data.OrganizationId).Returns(true); - sutProvider.GetDependency().ProjectsAreInOrganization(default, default).ReturnsForAnyArgs(true); - - mockProject.OrganizationId = data.OrganizationId; + sutProvider.GetDependency().GetByIdAsync(existingSecret.Id).Returns(existingSecret); data.Projects = new List() { mockProject }; - if (permissionType == PermissionType.RunAsAdmin) - { - sutProvider.GetDependency().OrganizationAdmin(data.OrganizationId).Returns(true); - } - else - { - sutProvider.GetDependency().OrganizationAdmin(data.OrganizationId).Returns(false); - sutProvider.GetDependency() - .AccessToProjectAsync((Guid)data.Projects?.First().Id, userId, AccessClientType.User) - .Returns((true, true)); - } - sutProvider.GetDependency().GetByIdAsync(data.Id).Returns(data); - await sutProvider.Sut.UpdateAsync(data, userId); + await sutProvider.Sut.UpdateAsync(data); await sutProvider.GetDependency().Received(1) .UpdateAsync(data); @@ -61,13 +42,10 @@ public class UpdateSecretCommandTests [Theory] [BitAutoData] - public async Task UpdateAsync_DoesNotModifyOrganizationId(Secret existingSecret, SutProvider sutProvider, Guid userId) + public async Task UpdateAsync_DoesNotModifyOrganizationId(Secret existingSecret, SutProvider sutProvider) { var updatedOrgId = Guid.NewGuid(); - sutProvider.GetDependency().OrganizationAdmin(existingSecret.OrganizationId).Returns(true); - sutProvider.GetDependency().AccessSecretsManager(existingSecret.OrganizationId).Returns(true); sutProvider.GetDependency().GetByIdAsync(existingSecret.Id).Returns(existingSecret); - sutProvider.GetDependency().OrganizationAdmin(updatedOrgId).Returns(true); var secretUpdate = new Secret() { @@ -76,7 +54,7 @@ public class UpdateSecretCommandTests Key = existingSecret.Key, }; - var result = await sutProvider.Sut.UpdateAsync(secretUpdate, userId); + var result = await sutProvider.Sut.UpdateAsync(secretUpdate); Assert.Equal(existingSecret.OrganizationId, result.OrganizationId); Assert.NotEqual(existingSecret.OrganizationId, updatedOrgId); @@ -84,11 +62,9 @@ public class UpdateSecretCommandTests [Theory] [BitAutoData] - public async Task UpdateAsync_DoesNotModifyCreationDate(Secret existingSecret, SutProvider sutProvider, Guid userId) + public async Task UpdateAsync_DoesNotModifyCreationDate(Secret existingSecret, SutProvider sutProvider) { - sutProvider.GetDependency().AccessSecretsManager(existingSecret.OrganizationId).Returns(true); sutProvider.GetDependency().GetByIdAsync(existingSecret.Id).Returns(existingSecret); - sutProvider.GetDependency().OrganizationAdmin(existingSecret.OrganizationId).Returns(true); var updatedCreationDate = DateTime.UtcNow; var secretUpdate = new Secret() @@ -99,7 +75,7 @@ public class UpdateSecretCommandTests OrganizationId = existingSecret.OrganizationId }; - var result = await sutProvider.Sut.UpdateAsync(secretUpdate, userId); + var result = await sutProvider.Sut.UpdateAsync(secretUpdate); Assert.Equal(existingSecret.CreationDate, result.CreationDate); Assert.NotEqual(existingSecret.CreationDate, updatedCreationDate); @@ -107,11 +83,9 @@ public class UpdateSecretCommandTests [Theory] [BitAutoData] - public async Task UpdateAsync_DoesNotModifyDeletionDate(Secret existingSecret, SutProvider sutProvider, Guid userId) + public async Task UpdateAsync_DoesNotModifyDeletionDate(Secret existingSecret, SutProvider sutProvider) { - sutProvider.GetDependency().AccessSecretsManager(existingSecret.OrganizationId).Returns(true); sutProvider.GetDependency().GetByIdAsync(existingSecret.Id).Returns(existingSecret); - sutProvider.GetDependency().OrganizationAdmin(existingSecret.OrganizationId).Returns(true); var updatedDeletionDate = DateTime.UtcNow; var secretUpdate = new Secret() @@ -122,7 +96,7 @@ public class UpdateSecretCommandTests OrganizationId = existingSecret.OrganizationId }; - var result = await sutProvider.Sut.UpdateAsync(secretUpdate, userId); + var result = await sutProvider.Sut.UpdateAsync(secretUpdate); Assert.Equal(existingSecret.DeletedDate, result.DeletedDate); Assert.NotEqual(existingSecret.DeletedDate, updatedDeletionDate); @@ -131,12 +105,9 @@ public class UpdateSecretCommandTests [Theory] [BitAutoData] - public async Task UpdateAsync_RevisionDateIsUpdatedToUtcNow(Secret existingSecret, SutProvider sutProvider, Guid userId) + public async Task UpdateAsync_RevisionDateIsUpdatedToUtcNow(Secret existingSecret, SutProvider sutProvider) { - sutProvider.GetDependency().OrganizationAdmin(existingSecret.OrganizationId).Returns(true); - sutProvider.GetDependency().AccessSecretsManager(existingSecret.OrganizationId).Returns(true); sutProvider.GetDependency().GetByIdAsync(existingSecret.Id).Returns(existingSecret); - sutProvider.GetDependency().OrganizationAdmin(existingSecret.OrganizationId).Returns(true); var updatedRevisionDate = DateTime.UtcNow.AddDays(10); var secretUpdate = new Secret() @@ -147,7 +118,7 @@ public class UpdateSecretCommandTests OrganizationId = existingSecret.OrganizationId }; - var result = await sutProvider.Sut.UpdateAsync(secretUpdate, userId); + var result = await sutProvider.Sut.UpdateAsync(secretUpdate); Assert.NotEqual(secretUpdate.RevisionDate, result.RevisionDate); AssertHelper.AssertRecent(result.RevisionDate); diff --git a/src/Api/SecretsManager/Controllers/SecretsController.cs b/src/Api/SecretsManager/Controllers/SecretsController.cs index 53a3a8b802..dcf62f322b 100644 --- a/src/Api/SecretsManager/Controllers/SecretsController.cs +++ b/src/Api/SecretsManager/Controllers/SecretsController.cs @@ -6,6 +6,7 @@ using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Identity; using Bit.Core.Repositories; +using Bit.Core.SecretsManager.AuthorizationRequirements; using Bit.Core.SecretsManager.Commands.Secrets.Interfaces; using Bit.Core.SecretsManager.Repositories; using Bit.Core.Services; @@ -32,6 +33,7 @@ public class SecretsController : Controller private readonly IUserService _userService; private readonly IEventService _eventService; private readonly IReferenceEventService _referenceEventService; + private readonly IAuthorizationService _authorizationService; public SecretsController( ICurrentContext currentContext, @@ -43,7 +45,8 @@ public class SecretsController : Controller IDeleteSecretCommand deleteSecretCommand, IUserService userService, IEventService eventService, - IReferenceEventService referenceEventService) + IReferenceEventService referenceEventService, + IAuthorizationService authorizationService) { _currentContext = currentContext; _projectRepository = projectRepository; @@ -55,6 +58,7 @@ public class SecretsController : Controller _userService = userService; _eventService = eventService; _referenceEventService = referenceEventService; + _authorizationService = authorizationService; } @@ -78,18 +82,14 @@ public class SecretsController : Controller [HttpPost("organizations/{organizationId}/secrets")] public async Task CreateAsync([FromRoute] Guid organizationId, [FromBody] SecretCreateRequestModel createRequest) { - if (!_currentContext.AccessSecretsManager(organizationId)) + var secret = createRequest.ToSecret(organizationId); + var authorizationResult = await _authorizationService.AuthorizeAsync(User, secret, SecretOperations.Create); + if (!authorizationResult.Succeeded) { throw new NotFoundException(); } - if (createRequest.ProjectIds != null && createRequest.ProjectIds.Length > 1) - { - throw new BadRequestException(); - } - - var userId = _userService.GetProperUserId(User).Value; - var result = await _createSecretCommand.CreateAsync(createRequest.ToSecret(organizationId), userId); + var result = await _createSecretCommand.CreateAsync(secret); // Creating a secret means you have read & write permission. return new SecretResponseModel(result, true, true); @@ -148,14 +148,20 @@ public class SecretsController : Controller [HttpPut("secrets/{id}")] public async Task UpdateSecretAsync([FromRoute] Guid id, [FromBody] SecretUpdateRequestModel updateRequest) { - if (updateRequest.ProjectIds != null && updateRequest.ProjectIds.Length > 1) + var secret = await _secretRepository.GetByIdAsync(id); + if (secret == null) { - throw new BadRequestException(); + throw new NotFoundException(); } - var userId = _userService.GetProperUserId(User).Value; - var secret = updateRequest.ToSecret(id); - var result = await _updateSecretCommand.UpdateAsync(secret, userId); + var updatedSecret = updateRequest.ToSecret(id, secret.OrganizationId); + var authorizationResult = await _authorizationService.AuthorizeAsync(User, updatedSecret, SecretOperations.Update); + if (!authorizationResult.Succeeded) + { + throw new NotFoundException(); + } + + var result = await _updateSecretCommand.UpdateAsync(updatedSecret); // Updating a secret means you have read & write permission. return new SecretResponseModel(result, true, true); diff --git a/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs b/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs index 1edc9f06ea..c243d6e465 100644 --- a/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs +++ b/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs @@ -4,7 +4,7 @@ using Bit.Core.Utilities; namespace Bit.Api.SecretsManager.Models.Request; -public class SecretCreateRequestModel +public class SecretCreateRequestModel : IValidatableObject { [Required] [EncryptedString] @@ -32,4 +32,14 @@ public class SecretCreateRequestModel Projects = ProjectIds != null && ProjectIds.Any() ? ProjectIds.Select(x => new Project() { Id = x }).ToList() : null, }; } + + public IEnumerable Validate(ValidationContext validationContext) + { + if (ProjectIds is { Length: > 1 }) + { + yield return new ValidationResult( + $"Only one project assignment is supported.", + new[] { nameof(ProjectIds) }); + } + } } diff --git a/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs b/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs index 0a3e651a83..6416849d24 100644 --- a/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs +++ b/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs @@ -4,7 +4,7 @@ using Bit.Core.Utilities; namespace Bit.Api.SecretsManager.Models.Request; -public class SecretUpdateRequestModel +public class SecretUpdateRequestModel : IValidatableObject { [Required] [EncryptedString] @@ -20,11 +20,12 @@ public class SecretUpdateRequestModel public Guid[] ProjectIds { get; set; } - public Secret ToSecret(Guid id) + public Secret ToSecret(Guid id, Guid organizationId) { return new Secret() { Id = id, + OrganizationId = organizationId, Key = Key, Value = Value, Note = Note, @@ -32,4 +33,14 @@ public class SecretUpdateRequestModel Projects = ProjectIds != null && ProjectIds.Any() ? ProjectIds.Select(x => new Project() { Id = x }).ToList() : null, }; } + + public IEnumerable Validate(ValidationContext validationContext) + { + if (ProjectIds is { Length: > 1 }) + { + yield return new ValidationResult( + $"Only one project assignment is supported.", + new[] { nameof(ProjectIds) }); + } + } } diff --git a/src/Core/SecretsManager/AuthorizationRequirements/SecretOperationRequirement.cs b/src/Core/SecretsManager/AuthorizationRequirements/SecretOperationRequirement.cs new file mode 100644 index 0000000000..8b1e8a6134 --- /dev/null +++ b/src/Core/SecretsManager/AuthorizationRequirements/SecretOperationRequirement.cs @@ -0,0 +1,13 @@ +using Microsoft.AspNetCore.Authorization.Infrastructure; + +namespace Bit.Core.SecretsManager.AuthorizationRequirements; + +public class SecretOperationRequirement : OperationAuthorizationRequirement +{ +} + +public static class SecretOperations +{ + public static readonly SecretOperationRequirement Create = new() { Name = nameof(Create) }; + public static readonly SecretOperationRequirement Update = new() { Name = nameof(Update) }; +} diff --git a/src/Core/SecretsManager/Commands/Secrets/Interfaces/ICreateSecretCommand.cs b/src/Core/SecretsManager/Commands/Secrets/Interfaces/ICreateSecretCommand.cs index 2fad757847..9757346175 100644 --- a/src/Core/SecretsManager/Commands/Secrets/Interfaces/ICreateSecretCommand.cs +++ b/src/Core/SecretsManager/Commands/Secrets/Interfaces/ICreateSecretCommand.cs @@ -4,5 +4,5 @@ namespace Bit.Core.SecretsManager.Commands.Secrets.Interfaces; public interface ICreateSecretCommand { - Task CreateAsync(Secret secret, Guid userId); + Task CreateAsync(Secret secret); } diff --git a/src/Core/SecretsManager/Commands/Secrets/Interfaces/IUpdateSecretCommand.cs b/src/Core/SecretsManager/Commands/Secrets/Interfaces/IUpdateSecretCommand.cs index 23e6910c8b..8c2f61abc0 100644 --- a/src/Core/SecretsManager/Commands/Secrets/Interfaces/IUpdateSecretCommand.cs +++ b/src/Core/SecretsManager/Commands/Secrets/Interfaces/IUpdateSecretCommand.cs @@ -4,5 +4,5 @@ namespace Bit.Core.SecretsManager.Commands.Secrets.Interfaces; public interface IUpdateSecretCommand { - Task UpdateAsync(Secret secret, Guid userId); + Task UpdateAsync(Secret secret); } diff --git a/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs b/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs index 9b19ff0f13..88b1a33508 100644 --- a/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs +++ b/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs @@ -148,11 +148,8 @@ public class SecretsControllerTests : IClassFixture, IAsy var (org, _) = await _organizationHelper.Initialize(true, true); await LoginAsync(_email); - var project = await _projectRepository.CreateAsync(new Project { OrganizationId = org.Id, Name = "123" }); - var request = new SecretCreateRequestModel { - ProjectIds = new Guid[] { project.Id }, Key = _mockEncryptedString, Value = _mockEncryptedString, Note = _mockEncryptedString, diff --git a/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs index 8922b392d7..dc677a19f4 100644 --- a/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs @@ -1,4 +1,5 @@ -using Bit.Api.SecretsManager.Controllers; +using System.Security.Claims; +using Bit.Api.SecretsManager.Controllers; using Bit.Api.SecretsManager.Models.Request; using Bit.Api.Test.SecretsManager.Enums; using Bit.Core.Context; @@ -13,6 +14,7 @@ using Bit.Core.Test.SecretsManager.AutoFixture.SecretsFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Bit.Test.Common.Helpers; +using Microsoft.AspNetCore.Authorization; using NSubstitute; using Xunit; @@ -125,9 +127,8 @@ public class SecretsControllerTests } [Theory] - [BitAutoData(PermissionType.RunAsAdmin)] - [BitAutoData(PermissionType.RunAsUserWithPermission)] - public async void CreateSecret_Success(PermissionType permissionType, SutProvider sutProvider, SecretCreateRequestModel data, Guid organizationId, Project mockProject, Guid userId) + [BitAutoData] + public async void CreateSecret_NoAccess_Throws(SutProvider sutProvider, SecretCreateRequestModel data, Guid organizationId, Guid userId) { // We currently only allow a secret to be in one project at a time if (data.ProjectIds != null && data.ProjectIds.Length > 1) @@ -135,33 +136,23 @@ public class SecretsControllerTests data.ProjectIds = new Guid[] { data.ProjectIds.ElementAt(0) }; } + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), data.ToSecret(organizationId), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Failed()); + var resultSecret = data.ToSecret(organizationId); sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); - if (permissionType == PermissionType.RunAsAdmin) - { - sutProvider.GetDependency().OrganizationAdmin(organizationId).Returns(true); - } - else - { - resultSecret.Projects = new List() { mockProject }; - sutProvider.GetDependency().OrganizationAdmin(organizationId).Returns(false); - sutProvider.GetDependency().AccessToProjectAsync(Arg.Any(), Arg.Any(), AccessClientType.User) - .Returns((true, true)); - } + sutProvider.GetDependency().CreateAsync(default).ReturnsForAnyArgs(resultSecret); - sutProvider.GetDependency().AccessSecretsManager(organizationId).Returns(true); - sutProvider.GetDependency().CreateAsync(default, userId).ReturnsForAnyArgs(resultSecret); - - var result = await sutProvider.Sut.CreateAsync(organizationId, data); - await sutProvider.GetDependency().Received(1) - .CreateAsync(Arg.Any(), userId); + await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(organizationId, data)); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .CreateAsync(Arg.Any()); } [Theory] - [BitAutoData(PermissionType.RunAsAdmin)] - [BitAutoData(PermissionType.RunAsUserWithPermission)] - public async void UpdateSecret_Success(PermissionType permissionType, SutProvider sutProvider, SecretUpdateRequestModel data, Guid secretId, Guid organizationId, Guid userId, Project mockProject) + [BitAutoData] + public async void CreateSecret_Success(SutProvider sutProvider, SecretCreateRequestModel data, Guid organizationId, Guid userId) { // We currently only allow a secret to be in one project at a time if (data.ProjectIds != null && data.ProjectIds.Length > 1) @@ -169,26 +160,87 @@ public class SecretsControllerTests data.ProjectIds = new Guid[] { data.ProjectIds.ElementAt(0) }; } + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), data.ToSecret(organizationId), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + + var resultSecret = data.ToSecret(organizationId); sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); - if (permissionType == PermissionType.RunAsAdmin) + sutProvider.GetDependency().CreateAsync(default).ReturnsForAnyArgs(resultSecret); + + await sutProvider.Sut.CreateAsync(organizationId, data); + + await sutProvider.GetDependency().Received(1) + .CreateAsync(Arg.Any()); + } + + [Theory] + [BitAutoData] + public async void UpdateSecret_NoAccess_Throws(SutProvider sutProvider, SecretUpdateRequestModel data, Guid secretId, Guid organizationId, Secret mockSecret) + { + // We currently only allow a secret to be in one project at a time + if (data.ProjectIds != null && data.ProjectIds.Length > 1) { - sutProvider.GetDependency().OrganizationAdmin(organizationId).Returns(true); - } - else - { - data.ProjectIds = new Guid[] { mockProject.Id }; - sutProvider.GetDependency().OrganizationAdmin(organizationId).Returns(false); - sutProvider.GetDependency().AccessToProjectAsync(default, default, default) - .Returns((true, true)); + data.ProjectIds = new Guid[] { data.ProjectIds.ElementAt(0) }; } - var resultSecret = data.ToSecret(secretId); - sutProvider.GetDependency().UpdateAsync(default, userId).ReturnsForAnyArgs(resultSecret); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), data.ToSecret(secretId, organizationId), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Failed()); + sutProvider.GetDependency().GetByIdAsync(secretId).ReturnsForAnyArgs(mockSecret); - var result = await sutProvider.Sut.UpdateSecretAsync(secretId, data); + var resultSecret = data.ToSecret(secretId, organizationId); + sutProvider.GetDependency().UpdateAsync(default).ReturnsForAnyArgs(resultSecret); + + await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSecretAsync(secretId, data)); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .UpdateAsync(Arg.Any()); + } + + [Theory] + [BitAutoData] + public async void UpdateSecret_SecretDoesNotExist_Throws(SutProvider sutProvider, SecretUpdateRequestModel data, Guid secretId, Guid organizationId) + { + // We currently only allow a secret to be in one project at a time + if (data.ProjectIds != null && data.ProjectIds.Length > 1) + { + data.ProjectIds = new Guid[] { data.ProjectIds.ElementAt(0) }; + } + + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), data.ToSecret(secretId, organizationId), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Failed()); + + var resultSecret = data.ToSecret(secretId, organizationId); + sutProvider.GetDependency().UpdateAsync(default).ReturnsForAnyArgs(resultSecret); + + await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSecretAsync(secretId, data)); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .UpdateAsync(Arg.Any()); + } + + [Theory] + [BitAutoData] + public async void UpdateSecret_Success(SutProvider sutProvider, SecretUpdateRequestModel data, Guid secretId, Guid organizationId, Secret mockSecret) + { + // We currently only allow a secret to be in one project at a time + if (data.ProjectIds != null && data.ProjectIds.Length > 1) + { + data.ProjectIds = new Guid[] { data.ProjectIds.ElementAt(0) }; + } + + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), data.ToSecret(secretId, organizationId), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + sutProvider.GetDependency().GetByIdAsync(secretId).ReturnsForAnyArgs(mockSecret); + + var resultSecret = data.ToSecret(secretId, organizationId); + sutProvider.GetDependency().UpdateAsync(default).ReturnsForAnyArgs(resultSecret); + + await sutProvider.Sut.UpdateSecretAsync(secretId, data); await sutProvider.GetDependency().Received(1) - .UpdateAsync(Arg.Any(), userId); + .UpdateAsync(Arg.Any()); } [Theory]