From 01d67dce4811b19bd62fc5585b93b8771e3461be Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Thu, 20 Jun 2024 12:45:28 -0500 Subject: [PATCH] [SM-654] Individual secret permissions (#4160) * Add new data and request models * Update authz handlers * Update secret commands to handle access policy updates * Update secret repository to handle access policy updates * Update secrets controller to handle access policy updates * Add tests * Add integration tests for secret create --- ...cessPoliciesUpdatesAuthorizationHandler.cs | 162 +++++ .../Secrets/SecretAuthorizationHandler.cs | 20 +- .../Commands/Secrets/CreateSecretCommand.cs | 8 +- .../Commands/Secrets/UpdateSecretCommand.cs | 20 +- .../SecretAccessPoliciesUpdatesQuery.cs | 24 + .../SecretsManagerCollectionExtensions.cs | 2 + .../Repositories/SecretRepository.cs | 194 +++++- ...oliciesUpdatesAuthorizationHandlerTests.cs | 656 ++++++++++++++++++ .../SecretAuthorizationHandlerTests.cs | 39 +- .../Secrets/CreateSecretCommandTests.cs | 4 +- .../Secrets/UpdateSecretCommandTests.cs | 109 +-- .../SecretAccessPoliciesUpdatesQueryTests.cs | 184 +++++ .../Controllers/SecretsController.cs | 43 +- .../Models/Request/AccessPolicyRequest.cs | 30 + .../SecretAccessPoliciesRequestsModel.cs | 42 ++ .../Request/SecretCreateRequestModel.cs | 2 + .../Request/SecretUpdateRequestModel.cs | 29 +- .../Utilities/AccessPolicyHelpers.cs | 10 +- ...ecretAccessPoliciesOperationRequirement.cs | 13 + .../Interfaces/ICreateSecretCommand.cs | 6 +- .../Interfaces/IUpdateSecretCommand.cs | 6 +- .../AccessPolicyUpdates/AccessPolicyUpdate.cs | 23 + .../SecretAccessPoliciesUpdates.cs | 36 + .../Models/Data/SecretAccessPolicies.cs | 111 +++ .../ISecretAccessPoliciesUpdatesQuery.cs | 10 + .../Repositories/ISecretRepository.cs | 5 +- .../Repositories/Noop/NoopSecretRepository.cs | 5 +- .../Controllers/SecretsControllerTests.cs | 324 ++++++--- .../Controllers/SecretsControllerTests.cs | 247 +++++-- .../Models/SecretAccessPoliciesTests.cs | 119 ++++ 30 files changed, 2141 insertions(+), 342 deletions(-) create mode 100644 bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandler.cs create mode 100644 bitwarden_license/src/Commercial.Core/SecretsManager/Queries/AccessPolicies/SecretAccessPoliciesUpdatesQuery.cs create mode 100644 bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandlerTests.cs create mode 100644 bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/AccessPolicies/SecretAccessPoliciesUpdatesQueryTests.cs create mode 100644 src/Api/SecretsManager/Models/Request/SecretAccessPoliciesRequestsModel.cs create mode 100644 src/Core/SecretsManager/AuthorizationRequirements/SecretAccessPoliciesOperationRequirement.cs create mode 100644 src/Core/SecretsManager/Models/Data/AccessPolicyUpdates/AccessPolicyUpdate.cs create mode 100644 src/Core/SecretsManager/Models/Data/AccessPolicyUpdates/SecretAccessPoliciesUpdates.cs create mode 100644 src/Core/SecretsManager/Queries/AccessPolicies/Interfaces/ISecretAccessPoliciesUpdatesQuery.cs create mode 100644 test/Core.Test/SecretsManager/Models/SecretAccessPoliciesTests.cs diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandler.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandler.cs new file mode 100644 index 000000000..a92a6a4c6 --- /dev/null +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandler.cs @@ -0,0 +1,162 @@ +#nullable enable +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.SecretsManager.AuthorizationRequirements; +using Bit.Core.SecretsManager.Enums.AccessPolicies; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; +using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces; +using Bit.Core.SecretsManager.Queries.Interfaces; +using Bit.Core.SecretsManager.Repositories; +using Microsoft.AspNetCore.Authorization; + +namespace Bit.Commercial.Core.SecretsManager.AuthorizationHandlers.AccessPolicies; + +public class SecretAccessPoliciesUpdatesAuthorizationHandler : AuthorizationHandler< + SecretAccessPoliciesOperationRequirement, + SecretAccessPoliciesUpdates> +{ + private readonly IAccessClientQuery _accessClientQuery; + private readonly ICurrentContext _currentContext; + private readonly ISameOrganizationQuery _sameOrganizationQuery; + private readonly ISecretRepository _secretRepository; + private readonly IServiceAccountRepository _serviceAccountRepository; + + public SecretAccessPoliciesUpdatesAuthorizationHandler(ICurrentContext currentContext, + IAccessClientQuery accessClientQuery, + ISecretRepository secretRepository, + ISameOrganizationQuery sameOrganizationQuery, + IServiceAccountRepository serviceAccountRepository) + { + _currentContext = currentContext; + _accessClientQuery = accessClientQuery; + _sameOrganizationQuery = sameOrganizationQuery; + _serviceAccountRepository = serviceAccountRepository; + _secretRepository = secretRepository; + } + + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, + SecretAccessPoliciesOperationRequirement requirement, + SecretAccessPoliciesUpdates resource) + { + if (!_currentContext.AccessSecretsManager(resource.OrganizationId)) + { + return; + } + + // Only users and admins should be able to manipulate access policies + var (accessClient, userId) = + await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId); + if (accessClient != AccessClientType.User && accessClient != AccessClientType.NoAccessCheck) + { + return; + } + + switch (requirement) + { + case not null when requirement == SecretAccessPoliciesOperations.Updates: + await CanUpdateAsync(context, requirement, resource, accessClient, + userId); + break; + case not null when requirement == SecretAccessPoliciesOperations.Create: + await CanCreateAsync(context, requirement, resource, accessClient, + userId); + break; + default: + throw new ArgumentException("Unsupported operation requirement type provided.", + nameof(requirement)); + } + } + + private async Task CanUpdateAsync(AuthorizationHandlerContext context, + SecretAccessPoliciesOperationRequirement requirement, + SecretAccessPoliciesUpdates resource, + AccessClientType accessClient, Guid userId) + { + var access = await _secretRepository + .AccessToSecretAsync(resource.SecretId, userId, accessClient); + if (!access.Write) + { + return; + } + + if (!await GranteesInTheSameOrganizationAsync(resource)) + { + return; + } + + // Users can only create access policies for service accounts they have access to. + // User can delete and update any service account access policy if they have write access to the secret. + if (await HasAccessToTargetServiceAccountsAsync(resource, accessClient, userId)) + { + context.Succeed(requirement); + } + } + + private async Task CanCreateAsync(AuthorizationHandlerContext context, + SecretAccessPoliciesOperationRequirement requirement, + SecretAccessPoliciesUpdates resource, + AccessClientType accessClient, Guid userId) + { + if (resource.UserAccessPolicyUpdates.Any(x => x.Operation != AccessPolicyOperation.Create) || + resource.GroupAccessPolicyUpdates.Any(x => x.Operation != AccessPolicyOperation.Create) || + resource.ServiceAccountAccessPolicyUpdates.Any(x => x.Operation != AccessPolicyOperation.Create)) + { + return; + } + + if (!await GranteesInTheSameOrganizationAsync(resource)) + { + return; + } + + // Users can only create access policies for service accounts they have access to. + if (await HasAccessToTargetServiceAccountsAsync(resource, accessClient, userId)) + { + context.Succeed(requirement); + } + } + + private async Task GranteesInTheSameOrganizationAsync(SecretAccessPoliciesUpdates resource) + { + var organizationUserIds = resource.UserAccessPolicyUpdates.Select(update => + update.AccessPolicy.OrganizationUserId!.Value).ToList(); + var groupIds = resource.GroupAccessPolicyUpdates.Select(update => + update.AccessPolicy.GroupId!.Value).ToList(); + var serviceAccountIds = resource.ServiceAccountAccessPolicyUpdates.Select(update => + update.AccessPolicy.ServiceAccountId!.Value).ToList(); + + var usersInSameOrg = organizationUserIds.Count == 0 || + await _sameOrganizationQuery.OrgUsersInTheSameOrgAsync(organizationUserIds, + resource.OrganizationId); + + var groupsInSameOrg = groupIds.Count == 0 || + await _sameOrganizationQuery.GroupsInTheSameOrgAsync(groupIds, resource.OrganizationId); + + var serviceAccountsInSameOrg = serviceAccountIds.Count == 0 || + await _serviceAccountRepository.ServiceAccountsAreInOrganizationAsync( + serviceAccountIds, + resource.OrganizationId); + + return usersInSameOrg && groupsInSameOrg && serviceAccountsInSameOrg; + } + + private async Task HasAccessToTargetServiceAccountsAsync(SecretAccessPoliciesUpdates resource, + AccessClientType accessClient, Guid userId) + { + var serviceAccountIdsToCheck = resource.ServiceAccountAccessPolicyUpdates + .Where(update => update.Operation == AccessPolicyOperation.Create).Select(update => + update.AccessPolicy.ServiceAccountId!.Value).ToList(); + + if (serviceAccountIdsToCheck.Count == 0) + { + return true; + } + + var serviceAccountsAccess = + await _serviceAccountRepository.AccessToServiceAccountsAsync(serviceAccountIdsToCheck, userId, + accessClient); + + return serviceAccountsAccess.Count == serviceAccountIdsToCheck.Count && + serviceAccountsAccess.All(a => a.Value.Write); + } +} diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs index 9fd94c89b..91f40df7a 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs @@ -109,9 +109,9 @@ public class SecretAuthorizationHandler : AuthorizationHandler p.Id).ToList(), resource.OrganizationId)) { @@ -174,11 +174,23 @@ public class SecretAuthorizationHandler : AuthorizationHandler GetAccessToUpdateSecretAsync(Secret resource, Guid userId, AccessClientType accessClient) { - var newProject = resource.Projects?.FirstOrDefault(); + // Request was to remove all projects from the secret. This is not allowed for non admin users. + if (resource.Projects?.Count == 0) + { + return false; + } + var access = (await _secretRepository.AccessToSecretAsync(resource.Id, userId, accessClient)).Write; + + // No project mapping changes requested, return secret access. + if (resource.Projects == null) + { + return access; + } + + var newProject = resource.Projects?.FirstOrDefault(); var accessToNew = newProject != null && (await _projectRepository.AccessToProjectAsync(newProject.Id, userId, accessClient)) .Write; 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 d224247c1..3127d5a79 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/CreateSecretCommand.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/CreateSecretCommand.cs @@ -1,5 +1,7 @@ -using Bit.Core.SecretsManager.Commands.Secrets.Interfaces; +#nullable enable +using Bit.Core.SecretsManager.Commands.Secrets.Interfaces; using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; using Bit.Core.SecretsManager.Repositories; namespace Bit.Commercial.Core.SecretsManager.Commands.Secrets; @@ -13,8 +15,8 @@ public class CreateSecretCommand : ICreateSecretCommand _secretRepository = secretRepository; } - public async Task CreateAsync(Secret secret) + public async Task CreateAsync(Secret secret, SecretAccessPoliciesUpdates? accessPoliciesUpdates) { - return await _secretRepository.CreateAsync(secret); + return await _secretRepository.CreateAsync(secret, accessPoliciesUpdates); } } 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 c3c757bae..65a551f5c 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,7 @@ -using Bit.Core.Exceptions; +#nullable enable using Bit.Core.SecretsManager.Commands.Secrets.Interfaces; using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; using Bit.Core.SecretsManager.Repositories; namespace Bit.Commercial.Core.SecretsManager.Commands.Secrets; @@ -14,21 +15,8 @@ public class UpdateSecretCommand : IUpdateSecretCommand _secretRepository = secretRepository; } - public async Task UpdateAsync(Secret updatedSecret) + public async Task UpdateAsync(Secret secret, SecretAccessPoliciesUpdates? accessPolicyUpdates) { - var secret = await _secretRepository.GetByIdAsync(updatedSecret.Id); - if (secret == null) - { - throw new NotFoundException(); - } - - secret.Key = updatedSecret.Key; - secret.Value = updatedSecret.Value; - secret.Note = updatedSecret.Note; - secret.Projects = updatedSecret.Projects; - secret.RevisionDate = DateTime.UtcNow; - - await _secretRepository.UpdateAsync(secret); - return secret; + return await _secretRepository.UpdateAsync(secret, accessPolicyUpdates); } } diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/AccessPolicies/SecretAccessPoliciesUpdatesQuery.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/AccessPolicies/SecretAccessPoliciesUpdatesQuery.cs new file mode 100644 index 000000000..29d810eb8 --- /dev/null +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/AccessPolicies/SecretAccessPoliciesUpdatesQuery.cs @@ -0,0 +1,24 @@ +#nullable enable +using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; +using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces; +using Bit.Core.SecretsManager.Repositories; + +namespace Bit.Commercial.Core.SecretsManager.Queries.AccessPolicies; + +public class SecretAccessPoliciesUpdatesQuery : ISecretAccessPoliciesUpdatesQuery +{ + private readonly IAccessPolicyRepository _accessPolicyRepository; + + public SecretAccessPoliciesUpdatesQuery(IAccessPolicyRepository accessPolicyRepository) + { + _accessPolicyRepository = accessPolicyRepository; + } + + public async Task GetAsync(SecretAccessPolicies accessPolicies, Guid userId) + { + var currentPolicies = await _accessPolicyRepository.GetSecretAccessPoliciesAsync(accessPolicies.SecretId, userId); + + return currentPolicies == null ? new SecretAccessPoliciesUpdates(accessPolicies) : currentPolicies.GetPolicyUpdates(accessPolicies); + } +} diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs index bd3709717..970d874f8 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs @@ -42,11 +42,13 @@ public static class SecretsManagerCollectionExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs index 333719c42..ae9a5032c 100644 --- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs +++ b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs @@ -1,7 +1,9 @@ using System.Linq.Expressions; using AutoMapper; using Bit.Core.Enums; +using Bit.Core.SecretsManager.Enums.AccessPolicies; using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; using Bit.Core.SecretsManager.Repositories; using Bit.Infrastructure.EntityFramework; using Bit.Infrastructure.EntityFramework.Repositories; @@ -136,8 +138,8 @@ public class SecretRepository : Repository CreateAsync( - Core.SecretsManager.Entities.Secret secret) + public async Task CreateAsync( + Core.SecretsManager.Entities.Secret secret, SecretAccessPoliciesUpdates? accessPoliciesUpdates = null) { await using var scope = ServiceScopeFactory.CreateAsyncScope(); var dbContext = GetDatabaseContext(scope); @@ -158,13 +160,14 @@ public class SecretRepository : Repository UpdateAsync(Core.SecretsManager.Entities.Secret secret) + public async Task UpdateAsync(Core.SecretsManager.Entities.Secret secret, + SecretAccessPoliciesUpdates? accessPoliciesUpdates = null) { await using var scope = ServiceScopeFactory.CreateAsyncScope(); var dbContext = GetDatabaseContext(scope); @@ -173,36 +176,30 @@ public class SecretRepository : Repository s.Projects) + .Include(s => s.UserAccessPolicies) + .Include(s => s.GroupAccessPolicies) + .Include(s => s.ServiceAccountAccessPolicies) .FirstAsync(s => s.Id == secret.Id); - var projectsToRemove = entity.Projects.Where(p => mappedEntity.Projects.All(mp => mp.Id != p.Id)).ToList(); - var projectsToAdd = mappedEntity.Projects.Where(p => entity.Projects.All(ep => ep.Id != p.Id)).ToList(); + dbContext.Entry(entity).CurrentValues.SetValues(mappedEntity); - foreach (var p in projectsToRemove) + if (secret.Projects != null) { - entity.Projects.Remove(p); + entity = await UpdateProjectMappingAsync(dbContext, entity, mappedEntity); } - foreach (var project in projectsToAdd) + if (accessPoliciesUpdates != null) { - var p = dbContext.AttachToOrGet(x => x.Id == project.Id, () => project); - entity.Projects.Add(p); - } - - var projectIds = projectsToRemove.Select(p => p.Id).Concat(projectsToAdd.Select(p => p.Id)).ToList(); - if (projectIds.Count > 0) - { - await UpdateServiceAccountRevisionsByProjectIdsAsync(dbContext, projectIds); + await UpdateSecretAccessPoliciesAsync(dbContext, entity, accessPoliciesUpdates); } await UpdateServiceAccountRevisionsBySecretIdsAsync(dbContext, [entity.Id]); - dbContext.Entry(entity).CurrentValues.SetValues(mappedEntity); await dbContext.SaveChangesAsync(); await transaction.CommitAsync(); - - return secret; + return Mapper.Map(entity); } + public async Task SoftDeleteManyByIdAsync(IEnumerable ids) { await using var scope = ServiceScopeFactory.CreateAsyncScope(); @@ -455,5 +452,162 @@ public class SecretRepository : Repository secrets.Select(s => new SecretAccess(s.Id, false, false)) }; + private static async Task UpdateProjectMappingAsync(DatabaseContext dbContext, Secret currentEntity, Secret updatedEntity) + { + var projectsToRemove = currentEntity.Projects.Where(p => updatedEntity.Projects.All(mp => mp.Id != p.Id)).ToList(); + var projectsToAdd = updatedEntity.Projects.Where(p => currentEntity.Projects.All(ep => ep.Id != p.Id)).ToList(); + + foreach (var p in projectsToRemove) + { + currentEntity.Projects.Remove(p); + } + + foreach (var project in projectsToAdd) + { + var p = dbContext.AttachToOrGet(x => x.Id == project.Id, () => project); + currentEntity.Projects.Add(p); + } + + var projectIds = projectsToRemove.Select(p => p.Id).Concat(projectsToAdd.Select(p => p.Id)).ToList(); + if (projectIds.Count > 0) + { + await UpdateServiceAccountRevisionsByProjectIdsAsync(dbContext, projectIds); + } + + return currentEntity; + } + + private static async Task DeleteSecretAccessPoliciesAsync(DatabaseContext dbContext, Secret entity, + SecretAccessPoliciesUpdates accessPoliciesUpdates) + { + var userAccessPoliciesIdsToDelete = entity.UserAccessPolicies.Where(uap => accessPoliciesUpdates + .UserAccessPolicyUpdates + .Any(apu => apu.Operation == AccessPolicyOperation.Delete && + apu.AccessPolicy.OrganizationUserId == uap.OrganizationUserId)) + .Select(uap => uap.Id) + .ToList(); + + var groupAccessPoliciesIdsToDelete = entity.GroupAccessPolicies.Where(gap => accessPoliciesUpdates + .GroupAccessPolicyUpdates + .Any(apu => apu.Operation == AccessPolicyOperation.Delete && apu.AccessPolicy.GroupId == gap.GroupId)) + .Select(gap => gap.Id) + .ToList(); + + var serviceAccountAccessPoliciesIdsToDelete = entity.ServiceAccountAccessPolicies.Where(gap => + accessPoliciesUpdates.ServiceAccountAccessPolicyUpdates + .Any(apu => apu.Operation == AccessPolicyOperation.Delete && + apu.AccessPolicy.ServiceAccountId == gap.ServiceAccountId)) + .Select(sap => sap.Id) + .ToList(); + + var accessPoliciesIdsToDelete = userAccessPoliciesIdsToDelete + .Concat(groupAccessPoliciesIdsToDelete) + .Concat(serviceAccountAccessPoliciesIdsToDelete) + .ToList(); + + await dbContext.AccessPolicies + .Where(ap => accessPoliciesIdsToDelete.Contains(ap.Id)) + .ExecuteDeleteAsync(); + } + + private static async Task UpsertSecretAccessPolicyAsync(DatabaseContext dbContext, BaseAccessPolicy updatedEntity, + AccessPolicyOperation accessPolicyOperation, AccessPolicy? currentEntity, DateTime currentDate) + { + switch (accessPolicyOperation) + { + case AccessPolicyOperation.Create when currentEntity == null: + updatedEntity.SetNewId(); + await dbContext.AddAsync(updatedEntity); + break; + + case AccessPolicyOperation.Update when currentEntity != null: + dbContext.AccessPolicies.Attach(currentEntity); + currentEntity.Read = updatedEntity.Read; + currentEntity.Write = updatedEntity.Write; + currentEntity.RevisionDate = currentDate; + break; + default: + throw new InvalidOperationException("Policy updates failed due to unexpected state."); + } + } + + private async Task UpsertSecretAccessPoliciesAsync(DatabaseContext dbContext, + Secret entity, + SecretAccessPoliciesUpdates policyUpdates) + { + var currentDate = DateTime.UtcNow; + + foreach (var policyUpdate in policyUpdates.UserAccessPolicyUpdates.Where(apu => + apu.Operation != AccessPolicyOperation.Delete)) + { + var currentEntity = entity.UserAccessPolicies?.FirstOrDefault(e => + e.OrganizationUserId == policyUpdate.AccessPolicy.OrganizationUserId!.Value); + + await UpsertSecretAccessPolicyAsync(dbContext, MapToEntity(policyUpdate.AccessPolicy), + policyUpdate.Operation, + currentEntity, + currentDate); + } + + foreach (var policyUpdate in policyUpdates.GroupAccessPolicyUpdates.Where(apu => + apu.Operation != AccessPolicyOperation.Delete)) + { + var currentEntity = entity.GroupAccessPolicies?.FirstOrDefault(e => + e.GroupId == policyUpdate.AccessPolicy.GroupId!.Value); + + await UpsertSecretAccessPolicyAsync(dbContext, MapToEntity(policyUpdate.AccessPolicy), + policyUpdate.Operation, + currentEntity, + currentDate); + } + + foreach (var policyUpdate in policyUpdates.ServiceAccountAccessPolicyUpdates.Where(apu => + apu.Operation != AccessPolicyOperation.Delete)) + { + var currentEntity = entity.ServiceAccountAccessPolicies?.FirstOrDefault(e => + e.ServiceAccountId == policyUpdate.AccessPolicy.ServiceAccountId!.Value); + + await UpsertSecretAccessPolicyAsync(dbContext, MapToEntity(policyUpdate.AccessPolicy), + policyUpdate.Operation, + currentEntity, + currentDate); + } + } + + private async Task UpdateSecretAccessPoliciesAsync(DatabaseContext dbContext, + Secret entity, + SecretAccessPoliciesUpdates? accessPoliciesUpdates) + { + if (accessPoliciesUpdates == null || !accessPoliciesUpdates.HasUpdates()) + { + return; + } + + if ((entity.UserAccessPolicies != null && entity.UserAccessPolicies.Count != 0) || + (entity.GroupAccessPolicies != null && entity.GroupAccessPolicies.Count != 0) || + (entity.ServiceAccountAccessPolicies != null && entity.ServiceAccountAccessPolicies.Count != 0)) + { + await DeleteSecretAccessPoliciesAsync(dbContext, entity, accessPoliciesUpdates); + } + + await UpsertSecretAccessPoliciesAsync(dbContext, entity, accessPoliciesUpdates); + + await UpdateServiceAccountRevisionsAsync(dbContext, + accessPoliciesUpdates.ServiceAccountAccessPolicyUpdates + .Select(sap => sap.AccessPolicy.ServiceAccountId!.Value).ToList()); + } + + private BaseAccessPolicy MapToEntity(Core.SecretsManager.Entities.BaseAccessPolicy baseAccessPolicy) => + baseAccessPolicy switch + { + Core.SecretsManager.Entities.UserSecretAccessPolicy accessPolicy => Mapper.Map( + accessPolicy), + Core.SecretsManager.Entities.GroupSecretAccessPolicy accessPolicy => Mapper.Map( + accessPolicy), + Core.SecretsManager.Entities.ServiceAccountSecretAccessPolicy accessPolicy => Mapper + .Map(accessPolicy), + _ => throw new ArgumentException("Unsupported access policy type") + }; + private record SecretAccess(Guid Id, bool Read, bool Write); } diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandlerTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandlerTests.cs new file mode 100644 index 000000000..f05bd8fbe --- /dev/null +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandlerTests.cs @@ -0,0 +1,656 @@ +using System.Reflection; +using System.Security.Claims; +using Bit.Commercial.Core.SecretsManager.AuthorizationHandlers.AccessPolicies; +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.SecretsManager.AuthorizationRequirements; +using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Enums.AccessPolicies; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; +using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces; +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.AccessPolicies; + +[SutProviderCustomize] +[ProjectCustomize] +public class SecretAccessPoliciesUpdatesAuthorizationHandlerTests +{ + [Fact] + public void SecretAccessPoliciesOperations_OnlyPublicStatic() + { + var publicStaticFields = + typeof(SecretAccessPoliciesOperations).GetFields(BindingFlags.Public | BindingFlags.Static); + var allFields = typeof(SecretAccessPoliciesOperations).GetFields(); + Assert.Equal(publicStaticFields.Length, allFields.Length); + } + + [Theory] + [BitAutoData] + public async Task Handler_AccessSecretsManagerFalse_DoesNotSucceed( + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + sutProvider.GetDependency().AccessSecretsManager(resource.OrganizationId) + .Returns(false); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.ServiceAccount)] + [BitAutoData(AccessClientType.Organization)] + public async Task Handler_UnsupportedClientTypes_DoesNotSucceed( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + SetupUserSubstitutes(sutProvider, accessClientType, resource); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task Handler_UnsupportedServiceAccountGrantedPoliciesOperationRequirement_Throws( + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + ClaimsPrincipal claimsPrincipal) + { + var requirement = new SecretAccessPoliciesOperationRequirement(); + SetupUserSubstitutes(sutProvider, AccessClientType.NoAccessCheck, resource); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await Assert.ThrowsAsync(() => sutProvider.Sut.HandleAsync(authzContext)); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck, false, false)] + [BitAutoData(AccessClientType.NoAccessCheck, true, false)] + [BitAutoData(AccessClientType.User, false, false)] + [BitAutoData(AccessClientType.User, true, false)] + public async Task Handler_CanUpdateAsync_UserHasNoWriteAccessToSecret_DoesNotSucceed( + AccessClientType accessClientType, + bool readAccess, + bool writeAccess, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + SetupUserSubstitutes(sutProvider, accessClientType, resource, userId); + sutProvider.GetDependency() + .AccessToSecretAsync(resource.SecretId, userId, accessClientType) + .Returns((readAccess, writeAccess)); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(false, false, false)] + [BitAutoData(true, false, false)] + [BitAutoData(false, true, false)] + [BitAutoData(true, true, false)] + [BitAutoData(false, false, true)] + [BitAutoData(true, false, true)] + [BitAutoData(false, true, true)] + public async Task Handler_CanUpdateAsync_TargetGranteesNotInSameOrganization_DoesNotSucceed( + bool orgUsersInSameOrg, + bool groupsInSameOrg, + bool serviceAccountsInSameOrg, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + SetupSameOrganizationRequest(sutProvider, AccessClientType.NoAccessCheck, resource, userId, orgUsersInSameOrg, + groupsInSameOrg, serviceAccountsInSameOrg); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(false, false, false)] + [BitAutoData(true, false, false)] + [BitAutoData(false, true, false)] + [BitAutoData(true, true, false)] + [BitAutoData(false, false, true)] + [BitAutoData(true, false, true)] + [BitAutoData(false, true, true)] + public async Task Handler_CanUpdateAsync_TargetGranteesNotInSameOrganizationHasZeroRequests_DoesNotSucceed( + bool orgUsersCountZero, + bool groupsCountZero, + bool serviceAccountsCountZero, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + resource = ClearAccessPolicyUpdate(resource, orgUsersCountZero, groupsCountZero, serviceAccountsCountZero); + SetupSameOrganizationRequest(sutProvider, AccessClientType.NoAccessCheck, resource, userId, false, false, + false); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck)] + [BitAutoData(AccessClientType.User)] + public async Task Handler_CanUpdateAsync_NoServiceAccountCreatesRequested_Success( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + + resource = RemoveAllServiceAccountCreates(resource); + SetupSameOrganizationRequest(sutProvider, accessClientType, resource, userId); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck)] + [BitAutoData(AccessClientType.User)] + public async Task Handler_CanUpdateAsync_NoAccessToTargetServiceAccounts_DoesNotSucceed( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + + SetupSameOrganizationRequest(sutProvider, accessClientType, resource, userId); + SetupNoServiceAccountAccess(sutProvider, resource, userId, accessClientType); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck)] + [BitAutoData(AccessClientType.User)] + public async Task Handler_CanUpdateAsync_ServiceAccountAccessResultsPartial_DoesNotSucceed( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + resource = AddServiceAccountCreateUpdate(resource); + SetupSameOrganizationRequest(sutProvider, accessClientType, resource, userId); + SetupPartialServiceAccountAccess(sutProvider, resource, userId, accessClientType); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck)] + [BitAutoData(AccessClientType.User)] + public async Task Handler_CanUpdateAsync_UserHasAccessToSomeServiceAccounts_DoesNotSucceed( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + resource = AddServiceAccountCreateUpdate(resource); + SetupSameOrganizationRequest(sutProvider, accessClientType, resource, userId); + SetupSomeServiceAccountAccess(sutProvider, resource, userId, accessClientType); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck)] + [BitAutoData(AccessClientType.User)] + public async Task Handler_CanUpdateAsync_UserHasAccessToAllServiceAccounts_Success( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + resource = AddServiceAccountCreateUpdate(resource); + SetupSameOrganizationRequest(sutProvider, accessClientType, resource, userId); + SetupAllServiceAccountAccess(sutProvider, resource, userId, accessClientType); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck)] + [BitAutoData(AccessClientType.User)] + public async Task Handler_CanCreateAsync_NotCreationOperations_DoesNotSucceed( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Create; + SetupUserSubstitutes(sutProvider, accessClientType, resource, userId); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(false, false, false)] + [BitAutoData(true, false, false)] + [BitAutoData(false, true, false)] + [BitAutoData(true, true, false)] + [BitAutoData(false, false, true)] + [BitAutoData(true, false, true)] + [BitAutoData(false, true, true)] + public async Task Handler_CanCreateAsync_TargetGranteesNotInSameOrganization_DoesNotSucceed( + bool orgUsersInSameOrg, + bool groupsInSameOrg, + bool serviceAccountsInSameOrg, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Create; + resource = SetAllToCreates(resource); + SetupSameOrganizationRequest(sutProvider, AccessClientType.NoAccessCheck, resource, userId, orgUsersInSameOrg, + groupsInSameOrg, serviceAccountsInSameOrg); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(false, false, false)] + [BitAutoData(true, false, false)] + [BitAutoData(false, true, false)] + [BitAutoData(true, true, false)] + [BitAutoData(false, false, true)] + [BitAutoData(true, false, true)] + [BitAutoData(false, true, true)] + public async Task Handler_CanCreateAsync_TargetGranteesNotInSameOrganizationHasZeroRequests_DoesNotSucceed( + bool orgUsersCountZero, + bool groupsCountZero, + bool serviceAccountsCountZero, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Create; + resource = SetAllToCreates(resource); + resource = ClearAccessPolicyUpdate(resource, orgUsersCountZero, groupsCountZero, serviceAccountsCountZero); + SetupSameOrganizationRequest(sutProvider, AccessClientType.NoAccessCheck, resource, userId, false, false, + false); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck)] + [BitAutoData(AccessClientType.User)] + public async Task Handler_CanCreateAsync_NoServiceAccountCreatesRequested_Success( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Create; + resource = SetAllToCreates(resource); + resource = RemoveAllServiceAccountCreates(resource); + SetupSameOrganizationRequest(sutProvider, accessClientType, resource, userId); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck)] + [BitAutoData(AccessClientType.User)] + public async Task Handler_CanCreateAsync_NoAccessToTargetServiceAccounts_DoesNotSucceed( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Create; + resource = SetAllToCreates(resource); + SetupSameOrganizationRequest(sutProvider, accessClientType, resource, userId); + SetupNoServiceAccountAccess(sutProvider, resource, userId, accessClientType); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck)] + [BitAutoData(AccessClientType.User)] + public async Task Handler_CanCreateAsync_ServiceAccountAccessResultsPartial_DoesNotSucceed( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Create; + resource = SetAllToCreates(resource); + resource = AddServiceAccountCreateUpdate(resource); + SetupSameOrganizationRequest(sutProvider, accessClientType, resource, userId); + SetupPartialServiceAccountAccess(sutProvider, resource, userId, accessClientType); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck)] + [BitAutoData(AccessClientType.User)] + public async Task Handler_CanCreateAsync_UserHasAccessToSomeServiceAccounts_DoesNotSucceed( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Create; + resource = SetAllToCreates(resource); + resource = AddServiceAccountCreateUpdate(resource); + SetupSameOrganizationRequest(sutProvider, accessClientType, resource, userId); + SetupSomeServiceAccountAccess(sutProvider, resource, userId, accessClientType); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(AccessClientType.NoAccessCheck)] + [BitAutoData(AccessClientType.User)] + public async Task Handler_CanCreateAsync_UserHasAccessToAllServiceAccounts_Success( + AccessClientType accessClientType, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Create; + resource = SetAllToCreates(resource); + resource = AddServiceAccountCreateUpdate(resource); + SetupSameOrganizationRequest(sutProvider, accessClientType, resource, userId); + SetupAllServiceAccountAccess(sutProvider, resource, userId, accessClientType); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + private static void SetupNoServiceAccountAccess( + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + AccessClientType accessClientType) + { + var createServiceAccountIds = resource.ServiceAccountAccessPolicyUpdates + .Where(ap => ap.Operation == AccessPolicyOperation.Create) + .Select(uap => uap.AccessPolicy.ServiceAccountId!.Value) + .ToList(); + sutProvider.GetDependency() + .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) + .Returns(createServiceAccountIds.ToDictionary(id => id, _ => (false, false))); + } + + private static void SetupPartialServiceAccountAccess( + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + AccessClientType accessClientType) + { + var accessResult = resource.ServiceAccountAccessPolicyUpdates + .Where(x => x.Operation == AccessPolicyOperation.Create) + .Select(x => x.AccessPolicy.ServiceAccountId!.Value) + .ToDictionary(id => id, _ => (true, true)); + accessResult[accessResult.First().Key] = (true, true); + accessResult.Remove(accessResult.Last().Key); + sutProvider.GetDependency() + .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) + .Returns(accessResult); + } + + private static void SetupSomeServiceAccountAccess( + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + AccessClientType accessClientType) + { + var accessResult = resource.ServiceAccountAccessPolicyUpdates + .Where(x => x.Operation == AccessPolicyOperation.Create) + .Select(x => x.AccessPolicy.ServiceAccountId!.Value) + .ToDictionary(id => id, _ => (false, false)); + + accessResult[accessResult.First().Key] = (true, true); + sutProvider.GetDependency() + .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) + .Returns(accessResult); + } + + private static void SetupAllServiceAccountAccess( + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + AccessClientType accessClientType) + { + var accessResult = resource.ServiceAccountAccessPolicyUpdates + .Where(x => x.Operation == AccessPolicyOperation.Create) + .Select(x => x.AccessPolicy.ServiceAccountId!.Value) + .ToDictionary(id => id, _ => (true, true)); + sutProvider.GetDependency() + .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) + .Returns(accessResult); + } + + private static void SetupUserSubstitutes( + SutProvider sutProvider, + AccessClientType accessClientType, + SecretAccessPoliciesUpdates resource, + Guid userId = new()) + { + sutProvider.GetDependency().AccessSecretsManager(resource.OrganizationId) + .Returns(true); + sutProvider.GetDependency().GetAccessClientAsync(default, resource.OrganizationId) + .ReturnsForAnyArgs((accessClientType, userId)); + } + + private static void SetupSameOrganizationRequest( + SutProvider sutProvider, + AccessClientType accessClientType, + SecretAccessPoliciesUpdates resource, + Guid userId = new(), + bool orgUsersInSameOrg = true, + bool groupsInSameOrg = true, + bool serviceAccountsInSameOrg = true) + { + SetupUserSubstitutes(sutProvider, accessClientType, resource, userId); + + sutProvider.GetDependency() + .AccessToSecretAsync(resource.SecretId, userId, accessClientType) + .Returns((true, true)); + + sutProvider.GetDependency() + .OrgUsersInTheSameOrgAsync(Arg.Any>(), resource.OrganizationId) + .Returns(orgUsersInSameOrg); + sutProvider.GetDependency() + .GroupsInTheSameOrgAsync(Arg.Any>(), resource.OrganizationId) + .Returns(groupsInSameOrg); + sutProvider.GetDependency() + .ServiceAccountsAreInOrganizationAsync(Arg.Any>(), resource.OrganizationId) + .Returns(serviceAccountsInSameOrg); + } + + private static SecretAccessPoliciesUpdates RemoveAllServiceAccountCreates( + SecretAccessPoliciesUpdates resource) + { + resource.ServiceAccountAccessPolicyUpdates = + resource.ServiceAccountAccessPolicyUpdates.Where(x => x.Operation != AccessPolicyOperation.Create); + return resource; + } + + private static SecretAccessPoliciesUpdates SetAllToCreates( + SecretAccessPoliciesUpdates resource) + { + resource.UserAccessPolicyUpdates = resource.UserAccessPolicyUpdates.Select(x => + { + x.Operation = AccessPolicyOperation.Create; + return x; + }); + resource.GroupAccessPolicyUpdates = resource.GroupAccessPolicyUpdates.Select(x => + { + x.Operation = AccessPolicyOperation.Create; + return x; + }); + resource.ServiceAccountAccessPolicyUpdates = resource.ServiceAccountAccessPolicyUpdates.Select(x => + { + x.Operation = AccessPolicyOperation.Create; + return x; + }); + + return resource; + } + + private static SecretAccessPoliciesUpdates AddServiceAccountCreateUpdate( + SecretAccessPoliciesUpdates resource) + { + resource.ServiceAccountAccessPolicyUpdates = resource.ServiceAccountAccessPolicyUpdates.Append( + new ServiceAccountSecretAccessPolicyUpdate + { + AccessPolicy = new ServiceAccountSecretAccessPolicy + { + ServiceAccountId = Guid.NewGuid(), + GrantedSecretId = resource.SecretId, + Read = true, + Write = true + } + }); + return resource; + } + + private static SecretAccessPoliciesUpdates ClearAccessPolicyUpdate(SecretAccessPoliciesUpdates resource, + bool orgUsersCountZero, + bool groupsCountZero, + bool serviceAccountsCountZero) + { + if (orgUsersCountZero) + { + resource.UserAccessPolicyUpdates = []; + } + + if (groupsCountZero) + { + resource.GroupAccessPolicyUpdates = []; + } + + if (serviceAccountsCountZero) + { + resource.ServiceAccountAccessPolicyUpdates = []; + } + + return resource; + } +} 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 index 97d672132..feaaf2634 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandlerTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandlerTests.cs @@ -352,14 +352,16 @@ public class SecretAuthorizationHandlerTests [Theory] [BitAutoData] - public async Task CanUpdateSecret_WithoutProjectUser_DoesNotSucceed( + public async Task CanUpdateSecret_ClearProjectsUser_DoesNotSucceed( SutProvider sutProvider, Secret secret, Guid userId, ClaimsPrincipal claimsPrincipal) { - secret.Projects = null; + secret.Projects = []; var requirement = SecretOperations.Update; SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId); + sutProvider.GetDependency().AccessToSecretAsync(secret.Id, Arg.Any(), Arg.Any()).Returns( + (true, true)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); @@ -370,12 +372,12 @@ public class SecretAuthorizationHandlerTests [Theory] [BitAutoData] - public async Task CanUpdateSecret_WithoutProjectAdmin_Success(SutProvider sutProvider, + public async Task CanUpdateSecret_ClearProjectsAdmin_Success(SutProvider sutProvider, Secret secret, Guid userId, ClaimsPrincipal claimsPrincipal) { - secret.Projects = null; + secret.Projects = []; var requirement = SecretOperations.Update; SetupPermission(sutProvider, PermissionType.RunAsAdmin, secret.OrganizationId, userId); var authzContext = new AuthorizationHandlerContext(new List { requirement }, @@ -386,6 +388,35 @@ public class SecretAuthorizationHandlerTests Assert.True(authzContext.HasSucceeded); } + [Theory] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, true, true)] + [BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, false, false)] + [BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, true, true)] + [BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, false, false)] + [BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, true, true)] + public async Task CanUpdateSecret_NoProjectChanges_ReturnsExpected(PermissionType permissionType, bool read, + bool write, bool expected, + SutProvider sutProvider, Secret secret, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretOperations.Update; + secret.Projects = null; + SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); + sutProvider.GetDependency() + .AccessToSecretAsync(secret.Id, userId, Arg.Any()).Returns( + (read, write)); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.Equal(expected, authzContext.HasSucceeded); + } + [Theory] [BitAutoData(PermissionType.RunAsUserWithPermission, true, true, true, false)] [BitAutoData(PermissionType.RunAsUserWithPermission, true, true, false, false)] 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 280aae26f..4ce12af82 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 @@ -20,9 +20,9 @@ public class CreateSecretCommandTests { data.Projects = new List() { mockProject }; - await sutProvider.Sut.CreateAsync(data); + await sutProvider.Sut.CreateAsync(data, null); await sutProvider.GetDependency().Received(1) - .CreateAsync(data); + .CreateAsync(data, null); } } 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 252fbb34b..299a6c048 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,12 +1,11 @@ -using Bit.Commercial.Core.SecretsManager.Commands.Secrets; -using Bit.Core.Exceptions; +#nullable enable +using Bit.Commercial.Core.SecretsManager.Commands.Secrets; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Repositories; using Bit.Core.Test.SecretsManager.AutoFixture.ProjectsFixture; using Bit.Core.Test.SecretsManager.AutoFixture.SecretsFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; -using Bit.Test.Common.Helpers; using NSubstitute; using Xunit; @@ -19,109 +18,13 @@ public class UpdateSecretCommandTests { [Theory] [BitAutoData] - public async Task UpdateAsync_SecretDoesNotExist_ThrowsNotFound(Secret data, SutProvider sutProvider) + public async Task UpdateAsync_Success(SutProvider sutProvider, Secret data, Project project) { - await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateAsync(data)); + data.Projects = new List { project }; - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().UpdateAsync(default); - } - - [Theory] - [BitAutoData] - public async Task UpdateAsync_Success(Secret existingSecret, Secret data, SutProvider sutProvider, Project mockProject) - { - sutProvider.GetDependency().GetByIdAsync(existingSecret.Id).Returns(existingSecret); - data.Projects = new List() { mockProject }; - - sutProvider.GetDependency().GetByIdAsync(data.Id).Returns(data); - await sutProvider.Sut.UpdateAsync(data); + await sutProvider.Sut.UpdateAsync(data, null); await sutProvider.GetDependency().Received(1) - .UpdateAsync(data); - } - - [Theory] - [BitAutoData] - public async Task UpdateAsync_DoesNotModifyOrganizationId(Secret existingSecret, SutProvider sutProvider) - { - var updatedOrgId = Guid.NewGuid(); - sutProvider.GetDependency().GetByIdAsync(existingSecret.Id).Returns(existingSecret); - - var secretUpdate = new Secret() - { - OrganizationId = updatedOrgId, - Id = existingSecret.Id, - Key = existingSecret.Key, - }; - - var result = await sutProvider.Sut.UpdateAsync(secretUpdate); - - Assert.Equal(existingSecret.OrganizationId, result.OrganizationId); - Assert.NotEqual(existingSecret.OrganizationId, updatedOrgId); - } - - [Theory] - [BitAutoData] - public async Task UpdateAsync_DoesNotModifyCreationDate(Secret existingSecret, SutProvider sutProvider) - { - sutProvider.GetDependency().GetByIdAsync(existingSecret.Id).Returns(existingSecret); - - var updatedCreationDate = DateTime.UtcNow; - var secretUpdate = new Secret() - { - CreationDate = updatedCreationDate, - Id = existingSecret.Id, - Key = existingSecret.Key, - OrganizationId = existingSecret.OrganizationId - }; - - var result = await sutProvider.Sut.UpdateAsync(secretUpdate); - - Assert.Equal(existingSecret.CreationDate, result.CreationDate); - Assert.NotEqual(existingSecret.CreationDate, updatedCreationDate); - } - - [Theory] - [BitAutoData] - public async Task UpdateAsync_DoesNotModifyDeletionDate(Secret existingSecret, SutProvider sutProvider) - { - sutProvider.GetDependency().GetByIdAsync(existingSecret.Id).Returns(existingSecret); - - var updatedDeletionDate = DateTime.UtcNow; - var secretUpdate = new Secret() - { - DeletedDate = updatedDeletionDate, - Id = existingSecret.Id, - Key = existingSecret.Key, - OrganizationId = existingSecret.OrganizationId - }; - - var result = await sutProvider.Sut.UpdateAsync(secretUpdate); - - Assert.Equal(existingSecret.DeletedDate, result.DeletedDate); - Assert.NotEqual(existingSecret.DeletedDate, updatedDeletionDate); - } - - - [Theory] - [BitAutoData] - public async Task UpdateAsync_RevisionDateIsUpdatedToUtcNow(Secret existingSecret, SutProvider sutProvider) - { - sutProvider.GetDependency().GetByIdAsync(existingSecret.Id).Returns(existingSecret); - - var updatedRevisionDate = DateTime.UtcNow.AddDays(10); - var secretUpdate = new Secret() - { - RevisionDate = updatedRevisionDate, - Id = existingSecret.Id, - Key = existingSecret.Key, - OrganizationId = existingSecret.OrganizationId - }; - - var result = await sutProvider.Sut.UpdateAsync(secretUpdate); - - Assert.NotEqual(secretUpdate.RevisionDate, result.RevisionDate); - AssertHelper.AssertRecent(result.RevisionDate); + .UpdateAsync(data, null); } } - diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/AccessPolicies/SecretAccessPoliciesUpdatesQueryTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/AccessPolicies/SecretAccessPoliciesUpdatesQueryTests.cs new file mode 100644 index 000000000..e6d641934 --- /dev/null +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/AccessPolicies/SecretAccessPoliciesUpdatesQueryTests.cs @@ -0,0 +1,184 @@ +#nullable enable +using Bit.Commercial.Core.SecretsManager.Queries.AccessPolicies; +using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Enums.AccessPolicies; +using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Repositories; +using Bit.Core.Test.SecretsManager.AutoFixture.ProjectsFixture; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using NSubstitute.ReturnsExtensions; +using Xunit; + +namespace Bit.Commercial.Core.Test.SecretsManager.Queries.AccessPolicies; + +[SutProviderCustomize] +[ProjectCustomize] +public class SecretAccessPoliciesUpdatesQueryTests +{ + [Theory] + [BitAutoData] + public async Task GetAsync_NoCurrentAccessPolicies_ReturnsAllCreates( + SutProvider sutProvider, + SecretAccessPolicies data, + Guid userId) + { + sutProvider.GetDependency() + .GetSecretAccessPoliciesAsync(data.SecretId, userId) + .ReturnsNullForAnyArgs(); + + var result = await sutProvider.Sut.GetAsync(data, userId); + + Assert.Equal(data.SecretId, result.SecretId); + Assert.Equal(data.OrganizationId, result.OrganizationId); + + Assert.Equal(data.UserAccessPolicies.Count(), result.UserAccessPolicyUpdates.Count()); + Assert.All(result.UserAccessPolicyUpdates, p => + { + Assert.Equal(AccessPolicyOperation.Create, p.Operation); + Assert.Contains(data.UserAccessPolicies, x => x == p.AccessPolicy); + }); + + Assert.Equal(data.GroupAccessPolicies.Count(), result.GroupAccessPolicyUpdates.Count()); + Assert.All(result.GroupAccessPolicyUpdates, p => + { + Assert.Equal(AccessPolicyOperation.Create, p.Operation); + Assert.Contains(data.GroupAccessPolicies, x => x == p.AccessPolicy); + }); + + Assert.Equal(data.ServiceAccountAccessPolicies.Count(), result.ServiceAccountAccessPolicyUpdates.Count()); + Assert.All(result.ServiceAccountAccessPolicyUpdates, p => + { + Assert.Equal(AccessPolicyOperation.Create, p.Operation); + Assert.Contains(data.ServiceAccountAccessPolicies, x => x == p.AccessPolicy); + }); + } + + [Theory] + [BitAutoData] + public async Task GetAsync_CurrentAccessPolicies_ReturnsChanges( + SutProvider sutProvider, + SecretAccessPolicies data, + Guid userId, + UserSecretAccessPolicy userPolicyToDelete, + GroupSecretAccessPolicy groupPolicyToDelete, + ServiceAccountSecretAccessPolicy serviceAccountPolicyToDelete) + { + data = SetupSecretAccessPolicies(data); + var userPolicyChanges = SetupUserAccessPolicies(data, userPolicyToDelete); + var groupPolicyChanges = SetupGroupAccessPolicies(data, groupPolicyToDelete); + var serviceAccountPolicyChanges = SetupServiceAccountAccessPolicies(data, serviceAccountPolicyToDelete); + + var currentPolicies = new SecretAccessPolicies + { + SecretId = data.SecretId, + OrganizationId = data.OrganizationId, + UserAccessPolicies = [userPolicyChanges.Update, userPolicyChanges.Delete], + GroupAccessPolicies = [groupPolicyChanges.Update, groupPolicyChanges.Delete], + ServiceAccountAccessPolicies = [serviceAccountPolicyChanges.Update, serviceAccountPolicyChanges.Delete] + }; + + sutProvider.GetDependency() + .GetSecretAccessPoliciesAsync(data.SecretId, userId) + .ReturnsForAnyArgs(currentPolicies); + + var result = await sutProvider.Sut.GetAsync(data, userId); + + Assert.Equal(data.SecretId, result.SecretId); + Assert.Equal(data.OrganizationId, result.OrganizationId); + + Assert.Single(result.UserAccessPolicyUpdates.Where(x => + x.Operation == AccessPolicyOperation.Delete && x.AccessPolicy == userPolicyChanges.Delete)); + Assert.Single(result.UserAccessPolicyUpdates.Where(x => + x.Operation == AccessPolicyOperation.Update && + x.AccessPolicy.OrganizationUserId == userPolicyChanges.Update.OrganizationUserId)); + Assert.Equal(result.UserAccessPolicyUpdates.Count() - 2, + result.UserAccessPolicyUpdates.Count(x => x.Operation == AccessPolicyOperation.Create)); + + Assert.Single(result.GroupAccessPolicyUpdates.Where(x => + x.Operation == AccessPolicyOperation.Delete && x.AccessPolicy == groupPolicyChanges.Delete)); + Assert.Single(result.GroupAccessPolicyUpdates.Where(x => + x.Operation == AccessPolicyOperation.Update && + x.AccessPolicy.GroupId == groupPolicyChanges.Update.GroupId)); + Assert.Equal(result.GroupAccessPolicyUpdates.Count() - 2, + result.GroupAccessPolicyUpdates.Count(x => x.Operation == AccessPolicyOperation.Create)); + + Assert.Single(result.ServiceAccountAccessPolicyUpdates.Where(x => + x.Operation == AccessPolicyOperation.Delete && x.AccessPolicy == serviceAccountPolicyChanges.Delete)); + Assert.Single(result.ServiceAccountAccessPolicyUpdates.Where(x => + x.Operation == AccessPolicyOperation.Update && + x.AccessPolicy.ServiceAccountId == serviceAccountPolicyChanges.Update.ServiceAccountId)); + Assert.Equal(result.ServiceAccountAccessPolicyUpdates.Count() - 2, + result.ServiceAccountAccessPolicyUpdates.Count(x => x.Operation == AccessPolicyOperation.Create)); + } + + private static (UserSecretAccessPolicy Update, UserSecretAccessPolicy Delete) SetupUserAccessPolicies( + SecretAccessPolicies data, UserSecretAccessPolicy currentPolicyToDelete) + { + currentPolicyToDelete.GrantedSecretId = data.SecretId; + + var updatePolicy = new UserSecretAccessPolicy + { + OrganizationUserId = data.UserAccessPolicies.First().OrganizationUserId, + GrantedSecretId = data.SecretId, + Read = !data.ServiceAccountAccessPolicies.First().Read, + Write = !data.ServiceAccountAccessPolicies.First().Write + }; + + return (updatePolicy, currentPolicyToDelete); + } + + private static (GroupSecretAccessPolicy Update, GroupSecretAccessPolicy Delete) SetupGroupAccessPolicies( + SecretAccessPolicies data, GroupSecretAccessPolicy currentPolicyToDelete) + { + currentPolicyToDelete.GrantedSecretId = data.SecretId; + + var updatePolicy = new GroupSecretAccessPolicy + { + GroupId = data.GroupAccessPolicies.First().GroupId, + GrantedSecretId = data.SecretId, + Read = !data.ServiceAccountAccessPolicies.First().Read, + Write = !data.ServiceAccountAccessPolicies.First().Write + }; + + return (updatePolicy, currentPolicyToDelete); + } + + private static (ServiceAccountSecretAccessPolicy Update, ServiceAccountSecretAccessPolicy Delete) + SetupServiceAccountAccessPolicies(SecretAccessPolicies data, + ServiceAccountSecretAccessPolicy currentPolicyToDelete) + { + currentPolicyToDelete.GrantedSecretId = data.SecretId; + + var updatePolicy = new ServiceAccountSecretAccessPolicy + { + ServiceAccountId = data.ServiceAccountAccessPolicies.First().ServiceAccountId, + GrantedSecretId = data.SecretId, + Read = !data.ServiceAccountAccessPolicies.First().Read, + Write = !data.ServiceAccountAccessPolicies.First().Write + }; + + return (updatePolicy, currentPolicyToDelete); + } + + private static SecretAccessPolicies SetupSecretAccessPolicies(SecretAccessPolicies data) + { + foreach (var policy in data.UserAccessPolicies) + { + policy.GrantedSecretId = data.SecretId; + } + + foreach (var policy in data.GroupAccessPolicies) + { + policy.GrantedSecretId = data.SecretId; + } + + foreach (var policy in data.ServiceAccountAccessPolicies) + { + policy.GrantedSecretId = data.SecretId; + } + + return data; + } +} diff --git a/src/Api/SecretsManager/Controllers/SecretsController.cs b/src/Api/SecretsManager/Controllers/SecretsController.cs index c45333cb7..34c6a9723 100644 --- a/src/Api/SecretsManager/Controllers/SecretsController.cs +++ b/src/Api/SecretsManager/Controllers/SecretsController.cs @@ -10,6 +10,8 @@ using Bit.Core.SecretsManager.AuthorizationRequirements; using Bit.Core.SecretsManager.Commands.Secrets.Interfaces; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; +using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces; using Bit.Core.SecretsManager.Queries.Interfaces; using Bit.Core.SecretsManager.Queries.Secrets.Interfaces; using Bit.Core.SecretsManager.Repositories; @@ -34,6 +36,7 @@ public class SecretsController : Controller private readonly IDeleteSecretCommand _deleteSecretCommand; private readonly IAccessClientQuery _accessClientQuery; private readonly ISecretsSyncQuery _secretsSyncQuery; + private readonly ISecretAccessPoliciesUpdatesQuery _secretAccessPoliciesUpdatesQuery; private readonly IUserService _userService; private readonly IEventService _eventService; private readonly IReferenceEventService _referenceEventService; @@ -49,6 +52,7 @@ public class SecretsController : Controller IDeleteSecretCommand deleteSecretCommand, IAccessClientQuery accessClientQuery, ISecretsSyncQuery secretsSyncQuery, + ISecretAccessPoliciesUpdatesQuery secretAccessPoliciesUpdatesQuery, IUserService userService, IEventService eventService, IReferenceEventService referenceEventService, @@ -63,6 +67,7 @@ public class SecretsController : Controller _deleteSecretCommand = deleteSecretCommand; _accessClientQuery = accessClientQuery; _secretsSyncQuery = secretsSyncQuery; + _secretAccessPoliciesUpdatesQuery = secretAccessPoliciesUpdatesQuery; _userService = userService; _eventService = eventService; _referenceEventService = referenceEventService; @@ -88,7 +93,8 @@ public class SecretsController : Controller } [HttpPost("organizations/{organizationId}/secrets")] - public async Task CreateAsync([FromRoute] Guid organizationId, [FromBody] SecretCreateRequestModel createRequest) + public async Task CreateAsync([FromRoute] Guid organizationId, + [FromBody] SecretCreateRequestModel createRequest) { var secret = createRequest.ToSecret(organizationId); var authorizationResult = await _authorizationService.AuthorizeAsync(User, secret, SecretOperations.Create); @@ -97,7 +103,22 @@ public class SecretsController : Controller throw new NotFoundException(); } - var result = await _createSecretCommand.CreateAsync(secret); + SecretAccessPoliciesUpdates accessPoliciesUpdates = null; + if (createRequest.AccessPoliciesRequests != null) + { + secret.SetNewId(); + accessPoliciesUpdates = + new SecretAccessPoliciesUpdates( + createRequest.AccessPoliciesRequests.ToSecretAccessPolicies(secret.Id, organizationId)); + var accessPolicyAuthorizationResult = await _authorizationService.AuthorizeAsync(User, + accessPoliciesUpdates, SecretAccessPoliciesOperations.Create); + if (!accessPolicyAuthorizationResult.Succeeded) + { + throw new NotFoundException(); + } + } + + var result = await _createSecretCommand.CreateAsync(secret, accessPoliciesUpdates); // Creating a secret means you have read & write permission. return new SecretResponseModel(result, true, true); @@ -162,14 +183,28 @@ public class SecretsController : Controller throw new NotFoundException(); } - var updatedSecret = updateRequest.ToSecret(id, secret.OrganizationId); + var updatedSecret = updateRequest.ToSecret(secret); var authorizationResult = await _authorizationService.AuthorizeAsync(User, updatedSecret, SecretOperations.Update); if (!authorizationResult.Succeeded) { throw new NotFoundException(); } - var result = await _updateSecretCommand.UpdateAsync(updatedSecret); + SecretAccessPoliciesUpdates accessPoliciesUpdates = null; + if (updateRequest.AccessPoliciesRequests != null) + { + var userId = _userService.GetProperUserId(User)!.Value; + accessPoliciesUpdates = await _secretAccessPoliciesUpdatesQuery.GetAsync(updateRequest.AccessPoliciesRequests.ToSecretAccessPolicies(id, secret.OrganizationId), userId); + + var accessPolicyAuthorizationResult = await _authorizationService.AuthorizeAsync(User, accessPoliciesUpdates, SecretAccessPoliciesOperations.Updates); + if (!accessPolicyAuthorizationResult.Succeeded) + { + throw new NotFoundException(); + } + + } + + var result = await _updateSecretCommand.UpdateAsync(updatedSecret, accessPoliciesUpdates); // Updating a secret means you have read & write permission. return new SecretResponseModel(result, true, true); diff --git a/src/Api/SecretsManager/Models/Request/AccessPolicyRequest.cs b/src/Api/SecretsManager/Models/Request/AccessPolicyRequest.cs index eef6181c9..75e944e3a 100644 --- a/src/Api/SecretsManager/Models/Request/AccessPolicyRequest.cs +++ b/src/Api/SecretsManager/Models/Request/AccessPolicyRequest.cs @@ -25,6 +25,16 @@ public class AccessPolicyRequest Write = Write }; + public UserSecretAccessPolicy ToUserSecretAccessPolicy(Guid secretId, Guid organizationId) => + new() + { + OrganizationUserId = GranteeId, + GrantedSecretId = secretId, + GrantedSecret = new Secret { OrganizationId = organizationId, Id = secretId }, + Read = Read, + Write = Write + }; + public GroupProjectAccessPolicy ToGroupProjectAccessPolicy(Guid projectId, Guid organizationId) => new() { @@ -35,6 +45,16 @@ public class AccessPolicyRequest Write = Write }; + public GroupSecretAccessPolicy ToGroupSecretAccessPolicy(Guid secretId, Guid organizationId) => + new() + { + GroupId = GranteeId, + GrantedSecretId = secretId, + GrantedSecret = new Secret { OrganizationId = organizationId, Id = secretId }, + Read = Read, + Write = Write + }; + public ServiceAccountProjectAccessPolicy ToServiceAccountProjectAccessPolicy(Guid projectId, Guid organizationId) => new() { @@ -45,6 +65,16 @@ public class AccessPolicyRequest Write = Write }; + public ServiceAccountSecretAccessPolicy ToServiceAccountSecretAccessPolicy(Guid secretId, Guid organizationId) => + new() + { + ServiceAccountId = GranteeId, + GrantedSecretId = secretId, + GrantedSecret = new Secret { OrganizationId = organizationId, Id = secretId }, + Read = Read, + Write = Write + }; + public UserServiceAccountAccessPolicy ToUserServiceAccountAccessPolicy(Guid id, Guid organizationId) => new() { diff --git a/src/Api/SecretsManager/Models/Request/SecretAccessPoliciesRequestsModel.cs b/src/Api/SecretsManager/Models/Request/SecretAccessPoliciesRequestsModel.cs new file mode 100644 index 000000000..245eb275f --- /dev/null +++ b/src/Api/SecretsManager/Models/Request/SecretAccessPoliciesRequestsModel.cs @@ -0,0 +1,42 @@ +#nullable enable +using Bit.Api.SecretsManager.Utilities; +using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Models.Data; + +namespace Bit.Api.SecretsManager.Models.Request; + +public class SecretAccessPoliciesRequestsModel +{ + public required IEnumerable UserAccessPolicyRequests { get; set; } + + public required IEnumerable GroupAccessPolicyRequests { get; set; } + + public required IEnumerable ServiceAccountAccessPolicyRequests { get; set; } + + public SecretAccessPolicies ToSecretAccessPolicies(Guid secretId, Guid organizationId) + { + var userAccessPolicies = UserAccessPolicyRequests + .Select(x => x.ToUserSecretAccessPolicy(secretId, organizationId)).ToList(); + var groupAccessPolicies = GroupAccessPolicyRequests + .Select(x => x.ToGroupSecretAccessPolicy(secretId, organizationId)).ToList(); + var serviceAccountAccessPolicies = ServiceAccountAccessPolicyRequests + .Select(x => x.ToServiceAccountSecretAccessPolicy(secretId, organizationId)).ToList(); + + var policies = new List(); + policies.AddRange(userAccessPolicies); + policies.AddRange(groupAccessPolicies); + policies.AddRange(serviceAccountAccessPolicies); + + AccessPolicyHelpers.CheckForDistinctAccessPolicies(policies); + AccessPolicyHelpers.CheckAccessPoliciesHaveReadPermission(policies); + + return new SecretAccessPolicies + { + SecretId = secretId, + OrganizationId = organizationId, + UserAccessPolicies = userAccessPolicies, + GroupAccessPolicies = groupAccessPolicies, + ServiceAccountAccessPolicies = serviceAccountAccessPolicies + }; + } +} diff --git a/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs b/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs index fd895594a..6c0d41c2d 100644 --- a/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs +++ b/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs @@ -23,6 +23,8 @@ public class SecretCreateRequestModel : IValidatableObject public Guid[] ProjectIds { get; set; } + public SecretAccessPoliciesRequestsModel AccessPoliciesRequests { get; set; } + public Secret ToSecret(Guid organizationId) { return new Secret() diff --git a/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs b/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs index a08ed90c3..7d298bfa0 100644 --- a/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs +++ b/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs @@ -23,18 +23,27 @@ public class SecretUpdateRequestModel : IValidatableObject public Guid[] ProjectIds { get; set; } - public Secret ToSecret(Guid id, Guid organizationId) + public SecretAccessPoliciesRequestsModel AccessPoliciesRequests { get; set; } + + public Secret ToSecret(Secret secret) { - return new Secret() + secret.Key = Key; + secret.Value = Value; + secret.Note = Note; + secret.RevisionDate = DateTime.UtcNow; + + if (secret.Projects?.FirstOrDefault()?.Id == ProjectIds?.FirstOrDefault()) { - Id = id, - OrganizationId = organizationId, - Key = Key, - Value = Value, - Note = Note, - DeletedDate = null, - Projects = ProjectIds != null && ProjectIds.Any() ? ProjectIds.Select(x => new Project() { Id = x }).ToList() : null, - }; + secret.Projects = null; + } + else + { + secret.Projects = ProjectIds != null && ProjectIds.Length != 0 + ? ProjectIds.Select(x => new Project() { Id = x }).ToList() + : []; + } + + return secret; } public IEnumerable Validate(ValidationContext validationContext) diff --git a/src/Api/SecretsManager/Utilities/AccessPolicyHelpers.cs b/src/Api/SecretsManager/Utilities/AccessPolicyHelpers.cs index 553b68145..9eec6b688 100644 --- a/src/Api/SecretsManager/Utilities/AccessPolicyHelpers.cs +++ b/src/Api/SecretsManager/Utilities/AccessPolicyHelpers.cs @@ -13,12 +13,16 @@ public static class AccessPolicyHelpers return baseAccessPolicy switch { UserProjectAccessPolicy ap => new Tuple(ap.OrganizationUserId, ap.GrantedProjectId), - GroupProjectAccessPolicy ap => new Tuple(ap.GroupId, ap.GrantedProjectId), - ServiceAccountProjectAccessPolicy ap => new Tuple(ap.ServiceAccountId, - ap.GrantedProjectId), + UserSecretAccessPolicy ap => new Tuple(ap.OrganizationUserId, ap.GrantedSecretId), UserServiceAccountAccessPolicy ap => new Tuple(ap.OrganizationUserId, ap.GrantedServiceAccountId), + GroupProjectAccessPolicy ap => new Tuple(ap.GroupId, ap.GrantedProjectId), + GroupSecretAccessPolicy ap => new Tuple(ap.GroupId, ap.GrantedSecretId), GroupServiceAccountAccessPolicy ap => new Tuple(ap.GroupId, ap.GrantedServiceAccountId), + ServiceAccountProjectAccessPolicy ap => new Tuple(ap.ServiceAccountId, + ap.GrantedProjectId), + ServiceAccountSecretAccessPolicy ap => new Tuple(ap.ServiceAccountId, + ap.GrantedSecretId), _ => throw new ArgumentException("Unsupported access policy type provided.", nameof(baseAccessPolicy)), }; }).ToList(); diff --git a/src/Core/SecretsManager/AuthorizationRequirements/SecretAccessPoliciesOperationRequirement.cs b/src/Core/SecretsManager/AuthorizationRequirements/SecretAccessPoliciesOperationRequirement.cs new file mode 100644 index 000000000..367e67519 --- /dev/null +++ b/src/Core/SecretsManager/AuthorizationRequirements/SecretAccessPoliciesOperationRequirement.cs @@ -0,0 +1,13 @@ +using Microsoft.AspNetCore.Authorization.Infrastructure; + +namespace Bit.Core.SecretsManager.AuthorizationRequirements; + +public class SecretAccessPoliciesOperationRequirement : OperationAuthorizationRequirement +{ +} + +public static class SecretAccessPoliciesOperations +{ + public static readonly SecretAccessPoliciesOperationRequirement Updates = new() { Name = nameof(Updates) }; + public static readonly SecretAccessPoliciesOperationRequirement Create = new() { Name = nameof(Create) }; +} diff --git a/src/Core/SecretsManager/Commands/Secrets/Interfaces/ICreateSecretCommand.cs b/src/Core/SecretsManager/Commands/Secrets/Interfaces/ICreateSecretCommand.cs index 975734617..66f56ab7f 100644 --- a/src/Core/SecretsManager/Commands/Secrets/Interfaces/ICreateSecretCommand.cs +++ b/src/Core/SecretsManager/Commands/Secrets/Interfaces/ICreateSecretCommand.cs @@ -1,8 +1,10 @@ -using Bit.Core.SecretsManager.Entities; +#nullable enable +using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; namespace Bit.Core.SecretsManager.Commands.Secrets.Interfaces; public interface ICreateSecretCommand { - Task CreateAsync(Secret secret); + Task CreateAsync(Secret secret, SecretAccessPoliciesUpdates? accessPoliciesUpdates); } diff --git a/src/Core/SecretsManager/Commands/Secrets/Interfaces/IUpdateSecretCommand.cs b/src/Core/SecretsManager/Commands/Secrets/Interfaces/IUpdateSecretCommand.cs index 8c2f61abc..9335ba148 100644 --- a/src/Core/SecretsManager/Commands/Secrets/Interfaces/IUpdateSecretCommand.cs +++ b/src/Core/SecretsManager/Commands/Secrets/Interfaces/IUpdateSecretCommand.cs @@ -1,8 +1,10 @@ -using Bit.Core.SecretsManager.Entities; +#nullable enable +using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; namespace Bit.Core.SecretsManager.Commands.Secrets.Interfaces; public interface IUpdateSecretCommand { - Task UpdateAsync(Secret secret); + Task UpdateAsync(Secret secret, SecretAccessPoliciesUpdates? accessPolicyUpdates); } diff --git a/src/Core/SecretsManager/Models/Data/AccessPolicyUpdates/AccessPolicyUpdate.cs b/src/Core/SecretsManager/Models/Data/AccessPolicyUpdates/AccessPolicyUpdate.cs new file mode 100644 index 000000000..b5d107402 --- /dev/null +++ b/src/Core/SecretsManager/Models/Data/AccessPolicyUpdates/AccessPolicyUpdate.cs @@ -0,0 +1,23 @@ +#nullable enable +using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Enums.AccessPolicies; + +namespace Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; + +public class UserSecretAccessPolicyUpdate +{ + public AccessPolicyOperation Operation { get; set; } + public required UserSecretAccessPolicy AccessPolicy { get; set; } +} + +public class GroupSecretAccessPolicyUpdate +{ + public AccessPolicyOperation Operation { get; set; } + public required GroupSecretAccessPolicy AccessPolicy { get; set; } +} + +public class ServiceAccountSecretAccessPolicyUpdate +{ + public AccessPolicyOperation Operation { get; set; } + public required ServiceAccountSecretAccessPolicy AccessPolicy { get; set; } +} diff --git a/src/Core/SecretsManager/Models/Data/AccessPolicyUpdates/SecretAccessPoliciesUpdates.cs b/src/Core/SecretsManager/Models/Data/AccessPolicyUpdates/SecretAccessPoliciesUpdates.cs new file mode 100644 index 000000000..d5c765a71 --- /dev/null +++ b/src/Core/SecretsManager/Models/Data/AccessPolicyUpdates/SecretAccessPoliciesUpdates.cs @@ -0,0 +1,36 @@ +#nullable enable +using Bit.Core.SecretsManager.Enums.AccessPolicies; + +namespace Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; + +public class SecretAccessPoliciesUpdates +{ + public SecretAccessPoliciesUpdates(SecretAccessPolicies accessPolicies) + { + SecretId = accessPolicies.SecretId; + OrganizationId = accessPolicies.OrganizationId; + UserAccessPolicyUpdates = + accessPolicies.UserAccessPolicies.Select(x => + new UserSecretAccessPolicyUpdate { Operation = AccessPolicyOperation.Create, AccessPolicy = x }); + + GroupAccessPolicyUpdates = + accessPolicies.GroupAccessPolicies.Select(x => + new GroupSecretAccessPolicyUpdate { Operation = AccessPolicyOperation.Create, AccessPolicy = x }); + + ServiceAccountAccessPolicyUpdates = accessPolicies.ServiceAccountAccessPolicies.Select(x => + new ServiceAccountSecretAccessPolicyUpdate { Operation = AccessPolicyOperation.Create, AccessPolicy = x }); + } + + public SecretAccessPoliciesUpdates() { } + + public Guid SecretId { get; set; } + public Guid OrganizationId { get; set; } + public IEnumerable UserAccessPolicyUpdates { get; set; } = []; + public IEnumerable GroupAccessPolicyUpdates { get; set; } = []; + public IEnumerable ServiceAccountAccessPolicyUpdates { get; set; } = []; + + public bool HasUpdates() => + UserAccessPolicyUpdates.Any() || + GroupAccessPolicyUpdates.Any() || + ServiceAccountAccessPolicyUpdates.Any(); +} diff --git a/src/Core/SecretsManager/Models/Data/SecretAccessPolicies.cs b/src/Core/SecretsManager/Models/Data/SecretAccessPolicies.cs index 9b7fccb63..d3fb0cba2 100644 --- a/src/Core/SecretsManager/Models/Data/SecretAccessPolicies.cs +++ b/src/Core/SecretsManager/Models/Data/SecretAccessPolicies.cs @@ -1,5 +1,7 @@ #nullable enable using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Enums.AccessPolicies; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; namespace Bit.Core.SecretsManager.Models.Data; @@ -32,4 +34,113 @@ public class SecretAccessPolicies public IEnumerable UserAccessPolicies { get; set; } = []; public IEnumerable GroupAccessPolicies { get; set; } = []; public IEnumerable ServiceAccountAccessPolicies { get; set; } = []; + + public SecretAccessPoliciesUpdates GetPolicyUpdates(SecretAccessPolicies requested) => + new() + { + SecretId = SecretId, + OrganizationId = OrganizationId, + UserAccessPolicyUpdates = GetUserPolicyUpdates(requested.UserAccessPolicies.ToList()), + GroupAccessPolicyUpdates = GetGroupPolicyUpdates(requested.GroupAccessPolicies.ToList()), + ServiceAccountAccessPolicyUpdates = + GetServiceAccountPolicyUpdates(requested.ServiceAccountAccessPolicies.ToList()) + }; + + private static List GetPolicyUpdates( + List currentPolicies, + List requestedPolicies, + Func, List> getIds, + Func, List> getIdsToBeUpdated, + Func, List, AccessPolicyOperation, List> createPolicyUpdates) + where TPolicy : class + where TPolicyUpdate : class + { + var currentIds = getIds(currentPolicies); + var requestedIds = getIds(requestedPolicies); + + var idsToBeDeleted = currentIds.Except(requestedIds).ToList(); + var idsToBeCreated = requestedIds.Except(currentIds).ToList(); + var idsToBeUpdated = getIdsToBeUpdated(requestedPolicies); + + var policiesToBeDeleted = createPolicyUpdates(currentPolicies, idsToBeDeleted, AccessPolicyOperation.Delete); + var policiesToBeCreated = createPolicyUpdates(requestedPolicies, idsToBeCreated, AccessPolicyOperation.Create); + var policiesToBeUpdated = createPolicyUpdates(requestedPolicies, idsToBeUpdated, AccessPolicyOperation.Update); + + return policiesToBeDeleted.Concat(policiesToBeCreated).Concat(policiesToBeUpdated).ToList(); + } + + private static List GetOrganizationUserIds(IEnumerable policies) => + policies.Select(ap => ap.OrganizationUserId!.Value).ToList(); + + private static List GetGroupIds(IEnumerable policies) => + policies.Select(ap => ap.GroupId!.Value).ToList(); + + private static List GetServiceAccountIds(IEnumerable policies) => + policies.Select(ap => ap.ServiceAccountId!.Value).ToList(); + + private static List CreateUserPolicyUpdates( + IEnumerable policies, List userIds, + AccessPolicyOperation operation) => + policies + .Where(ap => userIds.Contains(ap.OrganizationUserId!.Value)) + .Select(ap => new UserSecretAccessPolicyUpdate { Operation = operation, AccessPolicy = ap }) + .ToList(); + + private static List CreateGroupPolicyUpdates( + IEnumerable policies, List groupIds, + AccessPolicyOperation operation) => + policies + .Where(ap => groupIds.Contains(ap.GroupId!.Value)) + .Select(ap => new GroupSecretAccessPolicyUpdate { Operation = operation, AccessPolicy = ap }) + .ToList(); + + private static List CreateServiceAccountPolicyUpdates( + IEnumerable policies, List serviceAccountIds, + AccessPolicyOperation operation) => + policies + .Where(ap => serviceAccountIds.Contains(ap.ServiceAccountId!.Value)) + .Select(ap => new ServiceAccountSecretAccessPolicyUpdate { Operation = operation, AccessPolicy = ap }) + .ToList(); + + + private List GetUserPolicyUpdates(List requestedPolicies) => + GetPolicyUpdates(UserAccessPolicies.ToList(), requestedPolicies, GetOrganizationUserIds, GetUserIdsToBeUpdated, + CreateUserPolicyUpdates); + + private List + GetGroupPolicyUpdates(List requestedPolicies) => + GetPolicyUpdates(GroupAccessPolicies.ToList(), requestedPolicies, GetGroupIds, GetGroupIdsToBeUpdated, + CreateGroupPolicyUpdates); + + private List GetServiceAccountPolicyUpdates( + List requestedPolicies) => + GetPolicyUpdates(ServiceAccountAccessPolicies.ToList(), requestedPolicies, GetServiceAccountIds, + GetServiceAccountIdsToBeUpdated, CreateServiceAccountPolicyUpdates); + + private List GetUserIdsToBeUpdated(IEnumerable requested) => + UserAccessPolicies + .Where(currentAp => requested.Any(requestedAp => + requestedAp.GrantedSecretId == currentAp.GrantedSecretId && + requestedAp.OrganizationUserId == currentAp.OrganizationUserId && + (requestedAp.Write != currentAp.Write || requestedAp.Read != currentAp.Read))) + .Select(ap => ap.OrganizationUserId!.Value) + .ToList(); + + private List GetGroupIdsToBeUpdated(IEnumerable requested) => + GroupAccessPolicies + .Where(currentAp => requested.Any(requestedAp => + requestedAp.GrantedSecretId == currentAp.GrantedSecretId && + requestedAp.GroupId == currentAp.GroupId && + (requestedAp.Write != currentAp.Write || requestedAp.Read != currentAp.Read))) + .Select(ap => ap.GroupId!.Value) + .ToList(); + + private List GetServiceAccountIdsToBeUpdated(IEnumerable requested) => + ServiceAccountAccessPolicies + .Where(currentAp => requested.Any(requestedAp => + requestedAp.GrantedSecretId == currentAp.GrantedSecretId && + requestedAp.ServiceAccountId == currentAp.ServiceAccountId && + (requestedAp.Write != currentAp.Write || requestedAp.Read != currentAp.Read))) + .Select(ap => ap.ServiceAccountId!.Value) + .ToList(); } diff --git a/src/Core/SecretsManager/Queries/AccessPolicies/Interfaces/ISecretAccessPoliciesUpdatesQuery.cs b/src/Core/SecretsManager/Queries/AccessPolicies/Interfaces/ISecretAccessPoliciesUpdatesQuery.cs new file mode 100644 index 000000000..322f08abd --- /dev/null +++ b/src/Core/SecretsManager/Queries/AccessPolicies/Interfaces/ISecretAccessPoliciesUpdatesQuery.cs @@ -0,0 +1,10 @@ +#nullable enable +using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; + +namespace Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces; + +public interface ISecretAccessPoliciesUpdatesQuery +{ + Task GetAsync(SecretAccessPolicies accessPolicies, Guid userId); +} diff --git a/src/Core/SecretsManager/Repositories/ISecretRepository.cs b/src/Core/SecretsManager/Repositories/ISecretRepository.cs index b16ff8dfc..8492bac50 100644 --- a/src/Core/SecretsManager/Repositories/ISecretRepository.cs +++ b/src/Core/SecretsManager/Repositories/ISecretRepository.cs @@ -1,6 +1,7 @@ using Bit.Core.Enums; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; namespace Bit.Core.SecretsManager.Repositories; @@ -13,8 +14,8 @@ public interface ISecretRepository Task> GetManyByOrganizationIdInTrashByIdsAsync(Guid organizationId, IEnumerable ids); Task> GetManyByIds(IEnumerable ids); Task GetByIdAsync(Guid id); - Task CreateAsync(Secret secret); - Task UpdateAsync(Secret secret); + Task CreateAsync(Secret secret, SecretAccessPoliciesUpdates accessPoliciesUpdates = null); + Task UpdateAsync(Secret secret, SecretAccessPoliciesUpdates accessPoliciesUpdates = null); Task SoftDeleteManyByIdAsync(IEnumerable ids); Task HardDeleteManyByIdAsync(IEnumerable ids); Task RestoreManyByIdAsync(IEnumerable ids); diff --git a/src/Core/SecretsManager/Repositories/Noop/NoopSecretRepository.cs b/src/Core/SecretsManager/Repositories/Noop/NoopSecretRepository.cs index ddec1efb2..0448bbaf2 100644 --- a/src/Core/SecretsManager/Repositories/Noop/NoopSecretRepository.cs +++ b/src/Core/SecretsManager/Repositories/Noop/NoopSecretRepository.cs @@ -1,6 +1,7 @@ using Bit.Core.Enums; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; namespace Bit.Core.SecretsManager.Repositories.Noop; @@ -45,12 +46,12 @@ public class NoopSecretRepository : ISecretRepository return Task.FromResult(null as Secret); } - public Task CreateAsync(Secret secret) + public Task CreateAsync(Secret secret, SecretAccessPoliciesUpdates accessPoliciesUpdates) { return Task.FromResult(null as Secret); } - public Task UpdateAsync(Secret secret) + public Task UpdateAsync(Secret secret, SecretAccessPoliciesUpdates accessPoliciesUpdates) { return Task.FromResult(null as Secret); } diff --git a/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs b/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs index afe6ddeac..d6cbfe9de 100644 --- a/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs +++ b/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs @@ -5,6 +5,7 @@ using Bit.Api.IntegrationTest.SecretsManager.Helpers; using Bit.Api.Models.Response; using Bit.Api.SecretsManager.Models.Request; using Bit.Api.SecretsManager.Models.Response; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Repositories; @@ -148,20 +149,14 @@ public class SecretsControllerTests : IClassFixture, IAsy Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); } - [Fact] - public async Task CreateWithoutProject_RunAsAdmin_Success() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Create_WithoutProject_RunAsAdmin_Success(bool withAccessPolicies) { - var (org, _) = await _organizationHelper.Initialize(true, true, true); - await _loginHelper.LoginAsync(_email); + var (organizationUser, request) = await SetupSecretCreateRequestAsync(withAccessPolicies); - var request = new SecretCreateRequestModel - { - Key = _mockEncryptedString, - Value = _mockEncryptedString, - Note = _mockEncryptedString, - }; - - var response = await _client.PostAsJsonAsync($"/organizations/{org.Id}/secrets", request); + var response = await _client.PostAsJsonAsync($"/organizations/{organizationUser.OrganizationId}/secrets", request); response.EnsureSuccessStatusCode(); var result = await response.Content.ReadFromJsonAsync(); @@ -180,6 +175,17 @@ public class SecretsControllerTests : IClassFixture, IAsy AssertHelper.AssertRecent(createdSecret.RevisionDate); AssertHelper.AssertRecent(createdSecret.CreationDate); Assert.Null(createdSecret.DeletedDate); + + if (withAccessPolicies) + { + var secretAccessPolicies = await _accessPolicyRepository.GetSecretAccessPoliciesAsync(result.Id, organizationUser.UserId!.Value); + Assert.NotNull(secretAccessPolicies); + Assert.NotEmpty(secretAccessPolicies.UserAccessPolicies); + Assert.Equal(organizationUser.Id, secretAccessPolicies.UserAccessPolicies.First().OrganizationUserId); + Assert.Equal(result.Id, secretAccessPolicies.UserAccessPolicies.First().GrantedSecretId); + Assert.True(secretAccessPolicies.UserAccessPolicies.First().Read); + Assert.True(secretAccessPolicies.UserAccessPolicies.First().Write); + } } [Fact] @@ -243,65 +249,52 @@ public class SecretsControllerTests : IClassFixture, IAsy Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); } - [Theory] - [InlineData(PermissionType.RunAsAdmin)] - [InlineData(PermissionType.RunAsUserWithPermission)] - public async Task CreateWithProject_Success(PermissionType permissionType) + [Fact] + public async Task Create_RunAsServiceAccount_WithAccessPolicies_NotFound() { - var (org, orgAdminUser) = await _organizationHelper.Initialize(true, true, true); - await _loginHelper.LoginAsync(_email); + var (organizationUser, secretRequest) = + await SetupSecretWithProjectCreateRequestAsync(PermissionType.RunAsServiceAccountWithPermission, true); - var accessType = AccessClientType.NoAccessCheck; + var response = + await _client.PostAsJsonAsync($"/organizations/{organizationUser.OrganizationId}/secrets", secretRequest); - var project = await _projectRepository.CreateAsync(new Project() - { - Id = new Guid(), - OrganizationId = org.Id, - Name = _mockEncryptedString - }); + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } - var orgUserId = (Guid)orgAdminUser.UserId!; + [Theory] + [InlineData(PermissionType.RunAsAdmin, false)] + [InlineData(PermissionType.RunAsAdmin, true)] + [InlineData(PermissionType.RunAsUserWithPermission, false)] + [InlineData(PermissionType.RunAsUserWithPermission, true)] + [InlineData(PermissionType.RunAsServiceAccountWithPermission, false)] + public async Task Create_WithProject_Success(PermissionType permissionType, bool withAccessPolicies) + { + var (organizationUser, secretRequest) = await SetupSecretWithProjectCreateRequestAsync(permissionType, withAccessPolicies); - if (permissionType == PermissionType.RunAsUserWithPermission) - { - var (email, orgUser) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); - await _loginHelper.LoginAsync(email); - await _loginHelper.LoginAsync(email); - accessType = AccessClientType.User; - - var accessPolicies = new List - { - new UserProjectAccessPolicy - { - GrantedProjectId = project.Id, OrganizationUserId = orgUser.Id , Read = true, Write = true, - }, - }; - orgUserId = (Guid)orgUser.UserId!; - await _accessPolicyRepository.CreateManyAsync(accessPolicies); - } - - var secretRequest = new SecretCreateRequestModel() - { - Key = _mockEncryptedString, - Value = _mockEncryptedString, - Note = _mockEncryptedString, - ProjectIds = new[] { project.Id }, - }; - var secretResponse = await _client.PostAsJsonAsync($"/organizations/{org.Id}/secrets", secretRequest); + var secretResponse = await _client.PostAsJsonAsync($"/organizations/{organizationUser.OrganizationId}/secrets", secretRequest); secretResponse.EnsureSuccessStatusCode(); - var secretResult = await secretResponse.Content.ReadFromJsonAsync(); + var result = await secretResponse.Content.ReadFromJsonAsync(); - var result = (await _secretRepository.GetManyDetailsByProjectIdAsync(project.Id, orgUserId, accessType)).First(); - var secret = result.Secret; + Assert.NotNull(result); + var secret = await _secretRepository.GetByIdAsync(result.Id); + Assert.Equal(secret.Id, result.Id); + Assert.Equal(secret.OrganizationId, result.OrganizationId); + Assert.Equal(secret.Key, result.Key); + Assert.Equal(secret.Value, result.Value); + Assert.Equal(secret.Note, result.Note); + Assert.Equal(secret.CreationDate, result.CreationDate); + Assert.Equal(secret.RevisionDate, result.RevisionDate); - Assert.NotNull(secretResult); - Assert.Equal(secret.Id, secretResult.Id); - Assert.Equal(secret.OrganizationId, secretResult.OrganizationId); - Assert.Equal(secret.Key, secretResult.Key); - Assert.Equal(secret.Value, secretResult.Value); - Assert.Equal(secret.Note, secretResult.Note); - Assert.Equal(secret.CreationDate, secretResult.CreationDate); - Assert.Equal(secret.RevisionDate, secretResult.RevisionDate); + if (withAccessPolicies) + { + var secretAccessPolicies = await _accessPolicyRepository.GetSecretAccessPoliciesAsync(secret.Id, organizationUser.UserId!.Value); + Assert.NotNull(secretAccessPolicies); + Assert.NotEmpty(secretAccessPolicies.UserAccessPolicies); + Assert.Equal(organizationUser.Id, secretAccessPolicies.UserAccessPolicies.First().OrganizationUserId); + Assert.Equal(secret.Id, secretAccessPolicies.UserAccessPolicies.First().GrantedSecretId); + Assert.True(secretAccessPolicies.UserAccessPolicies.First().Read); + Assert.True(secretAccessPolicies.UserAccessPolicies.First().Write); + } } [Theory] @@ -523,37 +516,24 @@ public class SecretsControllerTests : IClassFixture, IAsy } [Theory] - [InlineData(PermissionType.RunAsAdmin)] - [InlineData(PermissionType.RunAsUserWithPermission)] - [InlineData(PermissionType.RunAsServiceAccountWithPermission)] - public async Task Update_Success(PermissionType permissionType) + [InlineData(PermissionType.RunAsServiceAccountWithPermission, true)] + public async Task Update_RunAsServiceAccountWithAccessPolicyUpdate_NotFound(PermissionType permissionType, bool withAccessPolices) { - var (org, _) = await _organizationHelper.Initialize(true, true, true); - var project = await _projectRepository.CreateAsync(new Project() - { - Id = Guid.NewGuid(), - OrganizationId = org.Id, - Name = _mockEncryptedString - }); + var (secret, request) = await SetupSecretUpdateRequestAsync(permissionType, withAccessPolices); - await SetupProjectPermissionAndLoginAsync(permissionType, project); + var response = await _client.PutAsJsonAsync($"/secrets/{secret.Id}", request); + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } - var secret = await _secretRepository.CreateAsync(new Secret - { - OrganizationId = org.Id, - Key = _mockEncryptedString, - Value = _mockEncryptedString, - Note = _mockEncryptedString, - Projects = permissionType != PermissionType.RunAsAdmin ? new List() { project } : null - }); - - var request = new SecretUpdateRequestModel() - { - Key = _mockEncryptedString, - Value = "2.3Uk+WNBIoU5xzmVFNcoWzz==|1MsPIYuRfdOHfu/0uY6H2Q==|/98xy4wb6pHP1VTZ9JcNCYgQjEUMFPlqJgCwRk1YXKg=", - Note = _mockEncryptedString, - ProjectIds = permissionType != PermissionType.RunAsAdmin ? new Guid[] { project.Id } : null - }; + [Theory] + [InlineData(PermissionType.RunAsAdmin, false)] + [InlineData(PermissionType.RunAsAdmin, true)] + [InlineData(PermissionType.RunAsUserWithPermission, false)] + [InlineData(PermissionType.RunAsUserWithPermission, true)] + [InlineData(PermissionType.RunAsServiceAccountWithPermission, false)] + public async Task Update_Success(PermissionType permissionType, bool withAccessPolices) + { + var (secret, request) = await SetupSecretUpdateRequestAsync(permissionType, withAccessPolices); var response = await _client.PutAsJsonAsync($"/secrets/{secret.Id}", request); response.EnsureSuccessStatusCode(); @@ -575,6 +555,19 @@ public class SecretsControllerTests : IClassFixture, IAsy Assert.Null(updatedSecret.DeletedDate); Assert.NotEqual(secret.Value, updatedSecret.Value); Assert.NotEqual(secret.RevisionDate, updatedSecret.RevisionDate); + + if (withAccessPolices) + { + var secretAccessPolicies = await _accessPolicyRepository.GetSecretAccessPoliciesAsync(secret.Id, + request.AccessPoliciesRequests.UserAccessPolicyRequests.First().GranteeId); + Assert.NotNull(secretAccessPolicies); + Assert.NotEmpty(secretAccessPolicies.UserAccessPolicies); + Assert.Equal(request.AccessPoliciesRequests.UserAccessPolicyRequests.First().GranteeId, + secretAccessPolicies.UserAccessPolicies.First().OrganizationUserId); + Assert.Equal(secret.Id, secretAccessPolicies.UserAccessPolicies.First().GrantedSecretId); + Assert.True(secretAccessPolicies.UserAccessPolicies.First().Read); + Assert.True(secretAccessPolicies.UserAccessPolicies.First().Write); + } } [Fact] @@ -978,4 +971,153 @@ public class SecretsControllerTests : IClassFixture, IAsy sa.RevisionDate = revisionDate; await _serviceAccountRepository.ReplaceAsync(sa); } + + private async Task<(OrganizationUser, SecretCreateRequestModel)> SetupSecretCreateRequestAsync( + bool withAccessPolicies) + { + var (_, organizationUser) = await _organizationHelper.Initialize(true, true, true); + await _loginHelper.LoginAsync(_email); + + var request = new SecretCreateRequestModel + { + Key = _mockEncryptedString, + Value = _mockEncryptedString, + Note = _mockEncryptedString + }; + + if (withAccessPolicies) + { + request.AccessPoliciesRequests = new SecretAccessPoliciesRequestsModel + { + UserAccessPolicyRequests = + [ + new AccessPolicyRequest { GranteeId = organizationUser.Id, Read = true, Write = true } + ], + GroupAccessPolicyRequests = [], + ServiceAccountAccessPolicyRequests = [] + }; + } + + return (organizationUser, request); + } + + private async Task<(OrganizationUser, SecretCreateRequestModel)> SetupSecretWithProjectCreateRequestAsync( + PermissionType permissionType, bool withAccessPolicies) + { + var (org, orgAdminUser) = await _organizationHelper.Initialize(true, true, true); + await _loginHelper.LoginAsync(_email); + + var project = await _projectRepository.CreateAsync(new Project + { + Id = new Guid(), + OrganizationId = org.Id, + Name = _mockEncryptedString + }); + + var currentOrganizationUser = orgAdminUser; + + if (permissionType == PermissionType.RunAsUserWithPermission) + { + var (email, orgUser) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); + await _loginHelper.LoginAsync(email); + + var accessPolicies = new List + { + new UserProjectAccessPolicy + { + GrantedProjectId = project.Id, OrganizationUserId = orgUser.Id, Read = true, Write = true + } + }; + currentOrganizationUser = orgUser; + await _accessPolicyRepository.CreateManyAsync(accessPolicies); + } + + if (permissionType == PermissionType.RunAsServiceAccountWithPermission) + { + var apiKeyDetails = await _organizationHelper.CreateNewServiceAccountApiKeyAsync(); + await _loginHelper.LoginWithApiKeyAsync(apiKeyDetails); + + var accessPolicies = new List + { + new ServiceAccountProjectAccessPolicy + { + GrantedProjectId = project.Id, + ServiceAccountId = apiKeyDetails.ApiKey.ServiceAccountId, + Read = true, + Write = true + } + }; + await _accessPolicyRepository.CreateManyAsync(accessPolicies); + } + + var secretRequest = new SecretCreateRequestModel + { + Key = _mockEncryptedString, + Value = _mockEncryptedString, + Note = _mockEncryptedString, + ProjectIds = [project.Id] + }; + + if (withAccessPolicies) + { + secretRequest.AccessPoliciesRequests = new SecretAccessPoliciesRequestsModel + { + UserAccessPolicyRequests = + [ + new AccessPolicyRequest { GranteeId = currentOrganizationUser.Id, Read = true, Write = true } + ], + GroupAccessPolicyRequests = [], + ServiceAccountAccessPolicyRequests = [] + }; + } + + return (currentOrganizationUser, secretRequest); + } + + private async Task<(Secret, SecretUpdateRequestModel)> SetupSecretUpdateRequestAsync(PermissionType permissionType, + bool withAccessPolicies) + { + var (org, adminOrgUser) = await _organizationHelper.Initialize(true, true, true); + var project = await _projectRepository.CreateAsync(new Project + { + Id = Guid.NewGuid(), + OrganizationId = org.Id, + Name = _mockEncryptedString + }); + + await SetupProjectPermissionAndLoginAsync(permissionType, project); + + var secret = await _secretRepository.CreateAsync(new Secret + { + OrganizationId = org.Id, + Key = _mockEncryptedString, + Value = _mockEncryptedString, + Note = _mockEncryptedString, + Projects = permissionType != PermissionType.RunAsAdmin ? new List { project } : null + }); + + var request = new SecretUpdateRequestModel + { + Key = _mockEncryptedString, + Value = + "2.3Uk+WNBIoU5xzmVFNcoWzz==|1MsPIYuRfdOHfu/0uY6H2Q==|/98xy4wb6pHP1VTZ9JcNCYgQjEUMFPlqJgCwRk1YXKg=", + Note = _mockEncryptedString, + ProjectIds = permissionType != PermissionType.RunAsAdmin ? [project.Id] : null + }; + + if (!withAccessPolicies) + { + return (secret, request); + } + + request.AccessPoliciesRequests = new SecretAccessPoliciesRequestsModel + { + UserAccessPolicyRequests = + [new AccessPolicyRequest { GranteeId = adminOrgUser.Id, Read = true, Write = true }], + GroupAccessPolicyRequests = [], + ServiceAccountAccessPolicyRequests = [] + }; + + return (secret, request); + } } diff --git a/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs index 097ee27d4..3eea25b39 100644 --- a/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs @@ -8,6 +8,8 @@ using Bit.Core.Exceptions; using Bit.Core.SecretsManager.Commands.Secrets.Interfaces; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; +using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces; using Bit.Core.SecretsManager.Queries.Interfaces; using Bit.Core.SecretsManager.Queries.Secrets.Interfaces; using Bit.Core.SecretsManager.Repositories; @@ -130,119 +132,158 @@ public class SecretsControllerTests [Theory] [BitAutoData] - public async Task CreateSecret_NoAccess_Throws(SutProvider sutProvider, SecretCreateRequestModel data, Guid organizationId, Guid userId) + public async Task CreateSecret_NoAccess_Throws(SutProvider sutProvider, + SecretCreateRequestModel data, 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) }; - } + data = SetupSecretCreateRequest(sutProvider, data, organizationId); sutProvider.GetDependency() .AuthorizeAsync(Arg.Any(), data.ToSecret(organizationId), Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Failed()); - var resultSecret = data.ToSecret(organizationId); - sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); - - sutProvider.GetDependency().CreateAsync(default).ReturnsForAnyArgs(resultSecret); - await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(organizationId, data)); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .CreateAsync(Arg.Any()); + .CreateAsync(Arg.Any(), Arg.Any()); } [Theory] [BitAutoData] - public async Task CreateSecret_Success(SutProvider sutProvider, SecretCreateRequestModel data, Guid organizationId, Guid userId) + public async Task CreateSecret_NoAccessPolicyUpdates_Success(SutProvider sutProvider, + SecretCreateRequestModel data, 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(organizationId), - Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); - - var resultSecret = data.ToSecret(organizationId); - sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); - - sutProvider.GetDependency().CreateAsync(default).ReturnsForAnyArgs(resultSecret); + data = SetupSecretCreateRequest(sutProvider, data, organizationId); await sutProvider.Sut.CreateAsync(organizationId, data); await sutProvider.GetDependency().Received(1) - .CreateAsync(Arg.Any()); + .CreateAsync(Arg.Any(), null); } [Theory] [BitAutoData] - public async Task UpdateSecret_NoAccess_Throws(SutProvider sutProvider, SecretUpdateRequestModel data, Guid secretId, Guid organizationId, Secret mockSecret) + public async Task CreateSecret_AccessPolicyUpdates_NoAccess_Throws(SutProvider sutProvider, + SecretCreateRequestModel data, 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) }; - } + data = SetupSecretCreateRequest(sutProvider, data, organizationId, true); sutProvider.GetDependency() - .AuthorizeAsync(Arg.Any(), data.ToSecret(secretId, organizationId), + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Failed()); + + await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(organizationId, data)); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .CreateAsync(Arg.Any(), Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task CreateSecret_AccessPolicyUpdate_Success(SutProvider sutProvider, + SecretCreateRequestModel data, Guid organizationId) + { + data = SetupSecretCreateRequest(sutProvider, data, organizationId, true); + + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + + await sutProvider.Sut.CreateAsync(organizationId, data); + + await sutProvider.GetDependency().Received(1) + .CreateAsync(Arg.Any(), Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task UpdateSecret_NoAccess_Throws(SutProvider sutProvider, + SecretUpdateRequestModel data, Secret currentSecret) + { + data = SetupSecretUpdateRequest(data); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Failed()); - sutProvider.GetDependency().GetByIdAsync(secretId).ReturnsForAnyArgs(mockSecret); + sutProvider.GetDependency().GetByIdAsync(currentSecret.Id).ReturnsForAnyArgs(currentSecret); - var resultSecret = data.ToSecret(secretId, organizationId); - sutProvider.GetDependency().UpdateAsync(default).ReturnsForAnyArgs(resultSecret); + sutProvider.GetDependency() + .UpdateAsync(Arg.Any(), Arg.Any()) + .ReturnsForAnyArgs(data.ToSecret(currentSecret)); - await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSecretAsync(secretId, data)); + await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSecretAsync(currentSecret.Id, data)); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .UpdateAsync(Arg.Any()); + .UpdateAsync(Arg.Any(), Arg.Any()); } [Theory] [BitAutoData] - public async Task UpdateSecret_SecretDoesNotExist_Throws(SutProvider sutProvider, SecretUpdateRequestModel data, Guid secretId, Guid organizationId) + public async Task UpdateSecret_SecretDoesNotExist_Throws(SutProvider sutProvider, + SecretUpdateRequestModel data, Secret currentSecret) { - // 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) }; - } + data = SetupSecretUpdateRequest(data); 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 Task 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), + .AuthorizeAsync(Arg.Any(), Arg.Any(), Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); - sutProvider.GetDependency().GetByIdAsync(secretId).ReturnsForAnyArgs(mockSecret); - var resultSecret = data.ToSecret(secretId, organizationId); - sutProvider.GetDependency().UpdateAsync(default).ReturnsForAnyArgs(resultSecret); + sutProvider.GetDependency() + .UpdateAsync(Arg.Any(), Arg.Any()) + .ReturnsForAnyArgs(data.ToSecret(currentSecret)); - await sutProvider.Sut.UpdateSecretAsync(secretId, data); + await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSecretAsync(currentSecret.Id, data)); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .UpdateAsync(Arg.Any(), Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task UpdateSecret_NoAccessPolicyUpdates_Success(SutProvider sutProvider, + SecretUpdateRequestModel data, Secret currentSecret) + { + data = SetupSecretUpdateRequest(data); + + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + sutProvider.GetDependency().GetByIdAsync(currentSecret.Id).ReturnsForAnyArgs(currentSecret); + + sutProvider.GetDependency() + .UpdateAsync(Arg.Any(), Arg.Any()) + .ReturnsForAnyArgs(data.ToSecret(currentSecret)); + + await sutProvider.Sut.UpdateSecretAsync(currentSecret.Id, data); await sutProvider.GetDependency().Received(1) - .UpdateAsync(Arg.Any()); + .UpdateAsync(Arg.Any(), null); + } + + [Theory] + [BitAutoData] + public async Task UpdateSecret_AccessPolicyUpdate_NoAccess_Throws(SutProvider sutProvider, + SecretUpdateRequestModel data, Secret currentSecret, SecretAccessPoliciesUpdates accessPoliciesUpdates) + { + data = SetupSecretUpdateAccessPoliciesRequest(sutProvider, data, currentSecret, accessPoliciesUpdates); + + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Failed()); + + await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSecretAsync(currentSecret.Id, data)); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .UpdateAsync(Arg.Any(), Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task UpdateSecret_AccessPolicyUpdate_Access_Success(SutProvider sutProvider, + SecretUpdateRequestModel data, Secret currentSecret, SecretAccessPoliciesUpdates accessPoliciesUpdates) + { + data = SetupSecretUpdateAccessPoliciesRequest(sutProvider, data, currentSecret, accessPoliciesUpdates); + + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + await sutProvider.Sut.UpdateSecretAsync(currentSecret.Id, data); + await sutProvider.GetDependency().Received(1) + .UpdateAsync(Arg.Any(), Arg.Any()); } [Theory] @@ -539,4 +580,62 @@ public class SecretsControllerTests { return nullLastSyncedDate ? null : DateTime.UtcNow.AddDays(-1); } + + private static SecretCreateRequestModel SetupSecretCreateRequest(SutProvider sutProvider, SecretCreateRequestModel data, Guid organizationId, bool accessPolicyRequest = false) + { + // We currently only allow a secret to be in one project at a time + if (data.ProjectIds != null && data.ProjectIds.Length > 1) + { + data.ProjectIds = [data.ProjectIds.ElementAt(0)]; + } + + if (!accessPolicyRequest) + { + data.AccessPoliciesRequests = null; + } + + sutProvider.GetDependency() + .CreateAsync(Arg.Any(), Arg.Any()) + .ReturnsForAnyArgs(data.ToSecret(organizationId)); + + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + return data; + } + + private static SecretUpdateRequestModel SetupSecretUpdateRequest(SecretUpdateRequestModel data, bool accessPolicyRequest = false) + { + // We currently only allow a secret to be in one project at a time + if (data.ProjectIds != null && data.ProjectIds.Length > 1) + { + data.ProjectIds = [data.ProjectIds.ElementAt(0)]; + } + + if (!accessPolicyRequest) + { + data.AccessPoliciesRequests = null; + } + + return data; + } + + private static SecretUpdateRequestModel SetupSecretUpdateAccessPoliciesRequest(SutProvider sutProvider, SecretUpdateRequestModel data, Secret currentSecret, SecretAccessPoliciesUpdates accessPoliciesUpdates) + { + data = SetupSecretUpdateRequest(data, true); + + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + sutProvider.GetDependency().GetByIdAsync(currentSecret.Id).ReturnsForAnyArgs(currentSecret); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).ReturnsForAnyArgs(Guid.NewGuid()); + sutProvider.GetDependency() + .GetAsync(Arg.Any(), Arg.Any()) + .ReturnsForAnyArgs(accessPoliciesUpdates); + sutProvider.GetDependency() + .UpdateAsync(Arg.Any(), Arg.Any()) + .ReturnsForAnyArgs(data.ToSecret(currentSecret)); + return data; + } } diff --git a/test/Core.Test/SecretsManager/Models/SecretAccessPoliciesTests.cs b/test/Core.Test/SecretsManager/Models/SecretAccessPoliciesTests.cs new file mode 100644 index 000000000..fa8deff50 --- /dev/null +++ b/test/Core.Test/SecretsManager/Models/SecretAccessPoliciesTests.cs @@ -0,0 +1,119 @@ +#nullable enable +using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Enums.AccessPolicies; +using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.Test.SecretsManager.AutoFixture.ProjectsFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Xunit; + +namespace Bit.Core.Test.SecretsManager.Models; + +[SutProviderCustomize] +[ProjectCustomize] +public class SecretAccessPoliciesTests +{ + [Theory] + [BitAutoData] + public void GetPolicyUpdates_NoChanges_ReturnsEmptyList(SecretAccessPolicies data) + { + var result = data.GetPolicyUpdates(data); + + Assert.Empty(result.UserAccessPolicyUpdates); + Assert.Empty(result.GroupAccessPolicyUpdates); + Assert.Empty(result.ServiceAccountAccessPolicyUpdates); + } + + [Fact] + public void GetPolicyUpdates_ReturnsCorrectPolicyChanges() + { + var secretId = Guid.NewGuid(); + var updatedId = Guid.NewGuid(); + var createId = Guid.NewGuid(); + var unChangedId = Guid.NewGuid(); + var deleteId = Guid.NewGuid(); + + var existing = new SecretAccessPolicies + { + UserAccessPolicies = new List + { + new() { OrganizationUserId = updatedId, GrantedSecretId = secretId, Read = true, Write = true }, + new() { OrganizationUserId = unChangedId, GrantedSecretId = secretId, Read = true, Write = true }, + new() { OrganizationUserId = deleteId, GrantedSecretId = secretId, Read = true, Write = true } + }, + GroupAccessPolicies = new List + { + new() { GroupId = updatedId, GrantedSecretId = secretId, Read = true, Write = true }, + new() { GroupId = unChangedId, GrantedSecretId = secretId, Read = true, Write = true }, + new() { GroupId = deleteId, GrantedSecretId = secretId, Read = true, Write = true } + }, + ServiceAccountAccessPolicies = new List + { + new() { ServiceAccountId = updatedId, GrantedSecretId = secretId, Read = true, Write = true }, + new() { ServiceAccountId = unChangedId, GrantedSecretId = secretId, Read = true, Write = true }, + new() { ServiceAccountId = deleteId, GrantedSecretId = secretId, Read = true, Write = true } + } + }; + + var requested = new SecretAccessPolicies + { + UserAccessPolicies = new List + { + new() { OrganizationUserId = updatedId, GrantedSecretId = secretId, Read = true, Write = false }, + new() { OrganizationUserId = createId, GrantedSecretId = secretId, Read = false, Write = true }, + new() { OrganizationUserId = unChangedId, GrantedSecretId = secretId, Read = true, Write = true } + }, + GroupAccessPolicies = new List + { + new() { GroupId = updatedId, GrantedSecretId = secretId, Read = true, Write = false }, + new() { GroupId = createId, GrantedSecretId = secretId, Read = false, Write = true }, + new() { GroupId = unChangedId, GrantedSecretId = secretId, Read = true, Write = true } + }, + ServiceAccountAccessPolicies = new List + { + new() { ServiceAccountId = updatedId, GrantedSecretId = secretId, Read = true, Write = false }, + new() { ServiceAccountId = createId, GrantedSecretId = secretId, Read = false, Write = true }, + new() { ServiceAccountId = unChangedId, GrantedSecretId = secretId, Read = true, Write = true } + } + }; + + + var result = existing.GetPolicyUpdates(requested); + + Assert.Contains(createId, result.UserAccessPolicyUpdates + .Where(pu => pu.Operation == AccessPolicyOperation.Create) + .Select(pu => pu.AccessPolicy.OrganizationUserId!.Value)); + Assert.Contains(createId, result.GroupAccessPolicyUpdates + .Where(pu => pu.Operation == AccessPolicyOperation.Create) + .Select(pu => pu.AccessPolicy.GroupId!.Value)); + Assert.Contains(createId, result.ServiceAccountAccessPolicyUpdates + .Where(pu => pu.Operation == AccessPolicyOperation.Create) + .Select(pu => pu.AccessPolicy.ServiceAccountId!.Value)); + + Assert.Contains(deleteId, result.UserAccessPolicyUpdates + .Where(pu => pu.Operation == AccessPolicyOperation.Delete) + .Select(pu => pu.AccessPolicy.OrganizationUserId!.Value)); + Assert.Contains(deleteId, result.GroupAccessPolicyUpdates + .Where(pu => pu.Operation == AccessPolicyOperation.Delete) + .Select(pu => pu.AccessPolicy.GroupId!.Value)); + Assert.Contains(deleteId, result.ServiceAccountAccessPolicyUpdates + .Where(pu => pu.Operation == AccessPolicyOperation.Delete) + .Select(pu => pu.AccessPolicy.ServiceAccountId!.Value)); + + Assert.Contains(updatedId, result.UserAccessPolicyUpdates + .Where(pu => pu.Operation == AccessPolicyOperation.Update) + .Select(pu => pu.AccessPolicy.OrganizationUserId!.Value)); + Assert.Contains(updatedId, result.GroupAccessPolicyUpdates + .Where(pu => pu.Operation == AccessPolicyOperation.Update) + .Select(pu => pu.AccessPolicy.GroupId!.Value)); + Assert.Contains(updatedId, result.ServiceAccountAccessPolicyUpdates + .Where(pu => pu.Operation == AccessPolicyOperation.Update) + .Select(pu => pu.AccessPolicy.ServiceAccountId!.Value)); + + Assert.DoesNotContain(unChangedId, result.UserAccessPolicyUpdates + .Select(pu => pu.AccessPolicy.OrganizationUserId!.Value)); + Assert.DoesNotContain(unChangedId, result.GroupAccessPolicyUpdates + .Select(pu => pu.AccessPolicy.GroupId!.Value)); + Assert.DoesNotContain(unChangedId, result.ServiceAccountAccessPolicyUpdates + .Select(pu => pu.AccessPolicy.ServiceAccountId!.Value)); + } +}