diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectPeopleAccessPoliciesAuthorizationHandler.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectPeopleAccessPoliciesAuthorizationHandler.cs index 64f7effd4e15..ba836f56d50b 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectPeopleAccessPoliciesAuthorizationHandler.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectPeopleAccessPoliciesAuthorizationHandler.cs @@ -1,5 +1,6 @@ using Bit.Core.Context; using Bit.Core.Enums; +using Bit.Core.Exceptions; using Bit.Core.SecretsManager.AuthorizationRequirements; using Bit.Core.SecretsManager.Models.Data; using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces; @@ -15,6 +16,7 @@ public class ProjectPeopleAccessPolicies> { private readonly IAccessClientQuery _accessClientQuery; + private readonly IAccessPolicyRepository _accessPolicyRepository; private readonly ICurrentContext _currentContext; private readonly IProjectRepository _projectRepository; private readonly ISameOrganizationQuery _sameOrganizationQuery; @@ -22,12 +24,14 @@ public class public ProjectPeopleAccessPoliciesAuthorizationHandler(ICurrentContext currentContext, IAccessClientQuery accessClientQuery, ISameOrganizationQuery sameOrganizationQuery, - IProjectRepository projectRepository) + IProjectRepository projectRepository, + IAccessPolicyRepository accessPolicyRepository) { _currentContext = currentContext; _accessClientQuery = accessClientQuery; _sameOrganizationQuery = sameOrganizationQuery; _projectRepository = projectRepository; + _accessPolicyRepository = accessPolicyRepository; } protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, @@ -39,13 +43,8 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext 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) { @@ -63,27 +62,54 @@ private async Task CanReplaceProjectPeopleAsync(AuthorizationHandlerContext cont AccessClientType accessClient, Guid userId) { var access = await _projectRepository.AccessToProjectAsync(resource.Id, userId, accessClient); - if (access.Write) + if (!access.Manage) { - if (resource.UserAccessPolicies != null && resource.UserAccessPolicies.Any()) + return; + } + + var newUserManageCount = resource.UserAccessPolicies?.Count(ap => ap.Manage) ?? 0; + var newGroupManageCount = resource.GroupAccessPolicies?.Count(ap => ap.Manage) ?? 0; + if (newUserManageCount + newGroupManageCount == 0) + { + var currentPolicies = await _accessPolicyRepository.GetPeoplePoliciesByGrantedProjectIdAsync(resource.Id, userId); + if (currentPolicies.Any(ap => ap.Manage)) { - var orgUserIds = resource.UserAccessPolicies.Select(ap => ap.OrganizationUserId!.Value).ToList(); - if (!await _sameOrganizationQuery.OrgUsersInTheSameOrgAsync(orgUserIds, resource.OrganizationId)) - { - return; - } + throw new BadRequestException("At least one user or group must retain Manage permission on this project."); } + } - if (resource.GroupAccessPolicies != null && resource.GroupAccessPolicies.Any()) + if (accessClient == AccessClientType.ServiceAccount) + { + var hasManageGrant = (resource.UserAccessPolicies?.Any(ap => ap.Manage) ?? false) || + (resource.GroupAccessPolicies?.Any(ap => ap.Manage) ?? false); + if (hasManageGrant) { - var groupIds = resource.GroupAccessPolicies.Select(ap => ap.GroupId!.Value).ToList(); - if (!await _sameOrganizationQuery.GroupsInTheSameOrgAsync(groupIds, resource.OrganizationId)) + var creatorId = await _projectRepository.GetProjectCreatorServiceAccountIdAsync(resource.Id); + if (creatorId != userId) { return; } } + } - context.Succeed(requirement); + if (resource.UserAccessPolicies != null && resource.UserAccessPolicies.Any()) + { + var orgUserIds = resource.UserAccessPolicies.Select(ap => ap.OrganizationUserId!.Value).ToList(); + if (!await _sameOrganizationQuery.OrgUsersInTheSameOrgAsync(orgUserIds, resource.OrganizationId)) + { + return; + } } + + if (resource.GroupAccessPolicies != null && resource.GroupAccessPolicies.Any()) + { + var groupIds = resource.GroupAccessPolicies.Select(ap => ap.GroupId!.Value).ToList(); + if (!await _sameOrganizationQuery.GroupsInTheSameOrgAsync(groupIds, resource.OrganizationId)) + { + return; + } + } + + context.Succeed(requirement); } } diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectServiceAccountsAccessPoliciesAuthorizationHandler.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectServiceAccountsAccessPoliciesAuthorizationHandler.cs index ace4a2116c20..f6ce6aff6c06 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectServiceAccountsAccessPoliciesAuthorizationHandler.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectServiceAccountsAccessPoliciesAuthorizationHandler.cs @@ -39,13 +39,8 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext 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) { @@ -67,11 +62,34 @@ private async Task CanUpdateAsync(AuthorizationHandlerContext context, var access = await _projectRepository.AccessToProjectAsync(resource.ProjectId, userId, accessClient); - if (!access.Write) + if (!access.Manage) { return; } + if (accessClient == AccessClientType.ServiceAccount) + { + var hasManageGrant = resource.ServiceAccountAccessPolicyUpdates + .Any(u => (u.Operation == AccessPolicyOperation.Create || u.Operation == AccessPolicyOperation.Update) + && u.AccessPolicy.Manage); + if (hasManageGrant) + { + var creatorId = await _projectRepository.GetProjectCreatorServiceAccountIdAsync(resource.ProjectId); + if (creatorId != userId) + { + return; + } + + if (resource.ServiceAccountAccessPolicyUpdates.Any(u => + (u.Operation == AccessPolicyOperation.Create || u.Operation == AccessPolicyOperation.Update) && + u.AccessPolicy.Manage && + u.AccessPolicy.ServiceAccountId != userId)) + { + return; + } + } + } + var serviceAccountIds = resource.ServiceAccountAccessPolicyUpdates.Select(update => update.AccessPolicy.ServiceAccountId!.Value).ToList(); @@ -84,7 +102,7 @@ await _serviceAccountRepository.ServiceAccountsAreInOrganizationAsync(serviceAcc } // 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 project. + // Users can delete and update any service account access policy if they have Manage access to the project. var serviceAccountIdsToCheck = resource.ServiceAccountAccessPolicyUpdates .Where(update => update.Operation == AccessPolicyOperation.Create).Select(update => update.AccessPolicy.ServiceAccountId!.Value).ToList(); @@ -99,7 +117,7 @@ await _serviceAccountRepository.ServiceAccountsAreInOrganizationAsync(serviceAcc await _serviceAccountRepository.AccessToServiceAccountsAsync(serviceAccountIdsToCheck, userId, accessClient); if (serviceAccountsAccess.Count == serviceAccountIdsToCheck.Count && - serviceAccountsAccess.All(a => a.Value.Write)) + serviceAccountsAccess.All(a => a.Value.Manage)) { context.Succeed(requirement); } diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandler.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandler.cs index a92a6a4c6da8..74876f813a78 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandler.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandler.cs @@ -17,6 +17,7 @@ public class SecretAccessPoliciesUpdatesAuthorizationHandler : AuthorizationHand { private readonly IAccessClientQuery _accessClientQuery; private readonly ICurrentContext _currentContext; + private readonly IProjectRepository _projectRepository; private readonly ISameOrganizationQuery _sameOrganizationQuery; private readonly ISecretRepository _secretRepository; private readonly IServiceAccountRepository _serviceAccountRepository; @@ -25,13 +26,15 @@ public SecretAccessPoliciesUpdatesAuthorizationHandler(ICurrentContext currentCo IAccessClientQuery accessClientQuery, ISecretRepository secretRepository, ISameOrganizationQuery sameOrganizationQuery, - IServiceAccountRepository serviceAccountRepository) + IServiceAccountRepository serviceAccountRepository, + IProjectRepository projectRepository) { _currentContext = currentContext; _accessClientQuery = accessClientQuery; _sameOrganizationQuery = sameOrganizationQuery; _serviceAccountRepository = serviceAccountRepository; _secretRepository = secretRepository; + _projectRepository = projectRepository; } protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, @@ -43,13 +46,8 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext 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) { @@ -74,18 +72,36 @@ private async Task CanUpdateAsync(AuthorizationHandlerContext context, { var access = await _secretRepository .AccessToSecretAsync(resource.SecretId, userId, accessClient); - if (!access.Write) + if (!access.Manage) { return; } + if (accessClient == AccessClientType.ServiceAccount) + { + var hasManageGrant = + resource.UserAccessPolicyUpdates.Any(u => + (u.Operation == AccessPolicyOperation.Create || u.Operation == AccessPolicyOperation.Update) && u.AccessPolicy.Manage) || + resource.GroupAccessPolicyUpdates.Any(u => + (u.Operation == AccessPolicyOperation.Create || u.Operation == AccessPolicyOperation.Update) && u.AccessPolicy.Manage) || + resource.ServiceAccountAccessPolicyUpdates.Any(u => + (u.Operation == AccessPolicyOperation.Create || u.Operation == AccessPolicyOperation.Update) && u.AccessPolicy.Manage); + if (hasManageGrant) + { + if (!await _projectRepository.IsServiceAccountCreatorOfAnyProjectForSecretAsync(resource.SecretId, userId)) + { + 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. + // User can delete and update any service account access policy if they have Manage access to the secret. if (await HasAccessToTargetServiceAccountsAsync(resource, accessClient, userId)) { context.Succeed(requirement); @@ -97,6 +113,12 @@ private async Task CanCreateAsync(AuthorizationHandlerContext context, SecretAccessPoliciesUpdates resource, AccessClientType accessClient, Guid userId) { + var access = await _secretRepository.AccessToSecretAsync(resource.SecretId, userId, accessClient); + if (!access.Manage) + { + return; + } + 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)) @@ -104,6 +126,21 @@ private async Task CanCreateAsync(AuthorizationHandlerContext context, return; } + if (accessClient == AccessClientType.ServiceAccount) + { + var hasManageGrant = + resource.UserAccessPolicyUpdates.Any(u => u.AccessPolicy.Manage) || + resource.GroupAccessPolicyUpdates.Any(u => u.AccessPolicy.Manage) || + resource.ServiceAccountAccessPolicyUpdates.Any(u => u.AccessPolicy.Manage); + if (hasManageGrant) + { + if (!await _projectRepository.IsServiceAccountCreatorOfAnyProjectForSecretAsync(resource.SecretId, userId)) + { + return; + } + } + } + if (!await GranteesInTheSameOrganizationAsync(resource)) { return; @@ -157,6 +194,6 @@ await _serviceAccountRepository.AccessToServiceAccountsAsync(serviceAccountIdsTo accessClient); return serviceAccountsAccess.Count == serviceAccountIdsToCheck.Count && - serviceAccountsAccess.All(a => a.Value.Write); + serviceAccountsAccess.All(a => a.Value.Manage); } } diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountGrantedPoliciesAuthorizationHandler.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountGrantedPoliciesAuthorizationHandler.cs index d5b40541698f..ce5aa409c436 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountGrantedPoliciesAuthorizationHandler.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountGrantedPoliciesAuthorizationHandler.cs @@ -38,13 +38,8 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext 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) { @@ -65,7 +60,7 @@ private async Task CanUpdateAsync(AuthorizationHandlerContext context, var access = await _serviceAccountRepository.AccessToServiceAccountAsync(resource.ServiceAccountId, userId, accessClient); - if (access.Write) + if (access.Manage) { var projectIdsToCheck = resource.ProjectGrantedPolicyUpdates.Select(update => update.AccessPolicy.GrantedProjectId!.Value).ToList(); @@ -79,7 +74,7 @@ await _serviceAccountRepository.AccessToServiceAccountAsync(resource.ServiceAcco var projectsAccess = await _projectRepository.AccessToProjectsAsync(projectIdsToCheck, userId, accessClient); - if (projectsAccess.Count == projectIdsToCheck.Count && projectsAccess.All(a => a.Value.Write)) + if (projectsAccess.Count == projectIdsToCheck.Count && projectsAccess.All(a => a.Value.Manage)) { context.Succeed(requirement); } diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountPeopleAccessPoliciesAuthorizationHandler.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountPeopleAccessPoliciesAuthorizationHandler.cs index 6e1c7cbc18e9..2d0ac5495769 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountPeopleAccessPoliciesAuthorizationHandler.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountPeopleAccessPoliciesAuthorizationHandler.cs @@ -39,9 +39,16 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext return; } - // Only users and admins should be able to manipulate access policies var (accessClient, userId) = await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId); + + // Policy decision: service accounts cannot manage human grantees on service accounts. + // This is intentional and must not be removed without security review. + if (accessClient == AccessClientType.ServiceAccount) + { + return; + } + if (accessClient != AccessClientType.User && accessClient != AccessClientType.NoAccessCheck) { return; @@ -63,7 +70,7 @@ private async Task CanReplaceServiceAccountPeopleAsync(AuthorizationHandlerConte AccessClientType accessClient, Guid userId) { var access = await _serviceAccountRepository.AccessToServiceAccountAsync(resource.Id, userId, accessClient); - if (access.Write) + if (access.Manage) { if (resource.UserAccessPolicies != null && resource.UserAccessPolicies.Any()) { 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 91f40df7ab58..a159682b9a3b 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs @@ -160,15 +160,9 @@ private async Task CanReadAccessPoliciesAsync(AuthorizationHandlerContext contex { var (accessClient, userId) = await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId); - // Only users and admins can read access policies - if (accessClient != AccessClientType.User && accessClient != AccessClientType.NoAccessCheck) - { - return; - } - var access = await _secretRepository.AccessToSecretAsync(resource.Id, userId, accessClient); - if (access.Write) + if (access.Manage) { context.Succeed(requirement); } diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/AccessPolicies/UpdateSecretAccessPoliciesCommand.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/AccessPolicies/UpdateSecretAccessPoliciesCommand.cs new file mode 100644 index 000000000000..c2c8d71a5a4a --- /dev/null +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/AccessPolicies/UpdateSecretAccessPoliciesCommand.cs @@ -0,0 +1,26 @@ +#nullable enable +using Bit.Core.SecretsManager.Commands.AccessPolicies.Interfaces; +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; +using Bit.Core.SecretsManager.Repositories; + +namespace Bit.Commercial.Core.SecretsManager.Commands.AccessPolicies; + +public class UpdateSecretAccessPoliciesCommand : IUpdateSecretAccessPoliciesCommand +{ + private readonly IAccessPolicyRepository _accessPolicyRepository; + + public UpdateSecretAccessPoliciesCommand(IAccessPolicyRepository accessPolicyRepository) + { + _accessPolicyRepository = accessPolicyRepository; + } + + public async Task UpdateAsync(SecretAccessPoliciesUpdates accessPoliciesUpdates) + { + if (!accessPoliciesUpdates.HasUpdates()) + { + return; + } + + await _accessPolicyRepository.UpdateSecretAccessPoliciesAsync(accessPoliciesUpdates); + } +} diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Projects/CreateProjectCommand.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Projects/CreateProjectCommand.cs index 9f37c35f78e3..dc5e8e6aff80 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Projects/CreateProjectCommand.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Projects/CreateProjectCommand.cs @@ -38,18 +38,29 @@ public async Task CreateAsync(Project project, Guid id, IdentityClientT throw new NotFoundException(); } + if (identityClientType == IdentityClientType.ServiceAccount) + { + project.CreatedByServiceAccountId = id; + } + var createdProject = await _projectRepository.CreateAsync(project); if (identityClientType == IdentityClientType.User) { var orgUser = await _organizationUserRepository.GetByOrganizationAsync(createdProject.OrganizationId, id); + if (orgUser == null) + { + throw new NotFoundException(); + } + var accessPolicy = new UserProjectAccessPolicy() { OrganizationUserId = orgUser.Id, GrantedProjectId = createdProject.Id, Read = true, Write = true, + Manage = true, }; await _accessPolicyRepository.CreateManyAsync(new List { accessPolicy }); @@ -63,6 +74,7 @@ public async Task CreateAsync(Project project, Guid id, IdentityClientT GrantedProjectId = createdProject.Id, Read = true, Write = true, + Manage = true, }; await _accessPolicyRepository.CreateManyAsync(new List { serviceAccountProjectAccessPolicy }); diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs index 8d20100281b6..b653f4968de8 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs @@ -72,5 +72,6 @@ public static void AddSecretsManagerServices(this IServiceCollection services) services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); } } diff --git a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/AccessPolicyRepository.cs b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/AccessPolicyRepository.cs index 93e4cf7c9724..d837d93e47ed 100644 --- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/AccessPolicyRepository.cs +++ b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/AccessPolicyRepository.cs @@ -1,5 +1,6 @@ using AutoMapper; using Bit.Core.Enums; +using Bit.Core.Exceptions; using Bit.Core.SecretsManager.Enums.AccessPolicies; using Bit.Core.SecretsManager.Models.Data; using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; @@ -165,6 +166,8 @@ public async Task> { using var scope = ServiceScopeFactory.CreateScope(); var dbContext = GetDatabaseContext(scope); + await using var transaction = await dbContext.Database.BeginTransactionAsync( + System.Data.IsolationLevel.Serializable); var peoplePolicyEntities = await dbContext.AccessPolicies.Where(ap => ap.Discriminator != AccessPolicyDiscriminator.ServiceAccountProject && (((UserProjectAccessPolicy)ap).GrantedProjectId == peopleAccessPolicies.Id || @@ -211,6 +214,7 @@ await UpsertPeoplePoliciesAsync(dbContext, groupPolicyEntities); await dbContext.SaveChangesAsync(); + await transaction.CommitAsync(); return await GetPeoplePoliciesByGrantedProjectIdAsync(peopleAccessPolicies.Id, userId); } @@ -242,6 +246,8 @@ public async Task> { using var scope = ServiceScopeFactory.CreateScope(); var dbContext = GetDatabaseContext(scope); + await using var transaction = await dbContext.Database.BeginTransactionAsync( + System.Data.IsolationLevel.Serializable); var peoplePolicyEntities = await dbContext.AccessPolicies.Where(ap => ((UserServiceAccountAccessPolicy)ap).GrantedServiceAccountId == peopleAccessPolicies.Id || ((GroupServiceAccountAccessPolicy)ap).GrantedServiceAccountId == peopleAccessPolicies.Id).ToListAsync(); @@ -286,6 +292,7 @@ await UpsertPeoplePoliciesAsync(dbContext, peopleAccessPolicies.ToBaseAccessPolicies().Select(MapToEntity).ToList(), userPolicyEntities, groupPolicyEntities); await dbContext.SaveChangesAsync(); + await transaction.CommitAsync(); return await GetPeoplePoliciesByGrantedServiceAccountIdAsync(peopleAccessPolicies.Id, userId); } @@ -452,6 +459,129 @@ await UpsertServiceAccountProjectPoliciesAsync(dbContext, currentAccessPolicies, entities.Select(e => MapToCore(e.ap, e.CurrentUserInGroup)).ToList()); } + public async Task UpdateSecretAccessPoliciesAsync(SecretAccessPoliciesUpdates accessPoliciesUpdates) + { + await using var scope = ServiceScopeFactory.CreateAsyncScope(); + var dbContext = GetDatabaseContext(scope); + await using var transaction = await dbContext.Database.BeginTransactionAsync(System.Data.IsolationLevel.Serializable); + + var currentDate = DateTime.UtcNow; + var secretId = accessPoliciesUpdates.SecretId; + + var currentUserPolicies = await dbContext.UserSecretAccessPolicy + .Where(ap => ap.GrantedSecretId == secretId) + .ToListAsync(); + + var currentGroupPolicies = await dbContext.GroupSecretAccessPolicy + .Where(ap => ap.GrantedSecretId == secretId) + .ToListAsync(); + + var currentSAPolicies = await dbContext.ServiceAccountSecretAccessPolicy + .Where(ap => ap.GrantedSecretId == secretId) + .ToListAsync(); + + // Delete removed policies + var userIdsToDelete = accessPoliciesUpdates.UserAccessPolicyUpdates + .Where(u => u.Operation == AccessPolicyOperation.Delete) + .Select(u => u.AccessPolicy.OrganizationUserId!.Value).ToList(); + var groupIdsToDelete = accessPoliciesUpdates.GroupAccessPolicyUpdates + .Where(u => u.Operation == AccessPolicyOperation.Delete) + .Select(u => u.AccessPolicy.GroupId!.Value).ToList(); + var saIdsToDelete = accessPoliciesUpdates.ServiceAccountAccessPolicyUpdates + .Where(u => u.Operation == AccessPolicyOperation.Delete) + .Select(u => u.AccessPolicy.ServiceAccountId!.Value).ToList(); + + if (userIdsToDelete.Count > 0) + { + await dbContext.UserSecretAccessPolicy + .Where(ap => ap.GrantedSecretId == accessPoliciesUpdates.SecretId && + userIdsToDelete.Contains(ap.OrganizationUserId!.Value)) + .ExecuteDeleteAsync(); + } + + if (groupIdsToDelete.Count > 0) + { + await dbContext.GroupSecretAccessPolicy + .Where(ap => ap.GrantedSecretId == accessPoliciesUpdates.SecretId && + groupIdsToDelete.Contains(ap.GroupId!.Value)) + .ExecuteDeleteAsync(); + } + + if (saIdsToDelete.Count > 0) + { + await dbContext.ServiceAccountSecretAccessPolicy + .Where(ap => ap.GrantedSecretId == accessPoliciesUpdates.SecretId && + saIdsToDelete.Contains(ap.ServiceAccountId!.Value)) + .ExecuteDeleteAsync(); + } + + // Upsert user policies + foreach (var update in accessPoliciesUpdates.UserAccessPolicyUpdates + .Where(u => u.Operation != AccessPolicyOperation.Delete)) + { + var entity = MapToEntity(update.AccessPolicy); + var existing = currentUserPolicies.FirstOrDefault(ap => + ap.OrganizationUserId == update.AccessPolicy.OrganizationUserId); + if (existing != null) + { + existing.Read = entity.Read; + existing.Write = entity.Write; + existing.Manage = entity.Manage; + existing.RevisionDate = currentDate; + } + else + { + entity.SetNewId(); + await dbContext.AddAsync(entity); + } + } + + // Upsert group policies + foreach (var update in accessPoliciesUpdates.GroupAccessPolicyUpdates + .Where(u => u.Operation != AccessPolicyOperation.Delete)) + { + var entity = MapToEntity(update.AccessPolicy); + var existing = currentGroupPolicies.FirstOrDefault(ap => + ap.GroupId == update.AccessPolicy.GroupId); + if (existing != null) + { + existing.Read = entity.Read; + existing.Write = entity.Write; + existing.Manage = entity.Manage; + existing.RevisionDate = currentDate; + } + else + { + entity.SetNewId(); + await dbContext.AddAsync(entity); + } + } + + // Upsert service account policies + foreach (var update in accessPoliciesUpdates.ServiceAccountAccessPolicyUpdates + .Where(u => u.Operation != AccessPolicyOperation.Delete)) + { + var entity = MapToEntity(update.AccessPolicy); + var existing = currentSAPolicies.FirstOrDefault(ap => + ap.ServiceAccountId == update.AccessPolicy.ServiceAccountId); + if (existing != null) + { + existing.Read = entity.Read; + existing.Write = entity.Write; + existing.Manage = entity.Manage; + existing.RevisionDate = currentDate; + } + else + { + entity.SetNewId(); + await dbContext.AddAsync(entity); + } + } + + await dbContext.SaveChangesAsync(); + await transaction.CommitAsync(); + } + private static async Task UpsertPeoplePoliciesAsync(DatabaseContext dbContext, List policies, IReadOnlyCollection userPolicyEntities, IReadOnlyCollection groupPolicyEntities) @@ -477,6 +607,7 @@ private static async Task UpsertPeoplePoliciesAsync(DatabaseContext dbContext, dbContext.AccessPolicies.Attach(currentEntity); currentEntity.Read = updatedEntity.Read; currentEntity.Write = updatedEntity.Write; + currentEntity.Manage = updatedEntity.Manage; currentEntity.RevisionDate = currentDate; } else @@ -510,6 +641,7 @@ private async Task UpsertServiceAccountProjectPoliciesAsync(DatabaseContext dbCo dbContext.AccessPolicies.Attach(currentEntity); currentEntity.Read = updatedEntity.Read; currentEntity.Write = updatedEntity.Write; + currentEntity.Manage = updatedEntity.Manage; currentEntity.RevisionDate = currentDate; break; default: @@ -606,12 +738,20 @@ private IQueryable ToPermiss AccessPolicy = Mapper.Map(ap), HasPermission = - (ap.GrantedProject.UserAccessPolicies.Any(p => p.OrganizationUser.UserId == userId && p.Write) || + (ap.GrantedProject.UserAccessPolicies.Any(p => p.OrganizationUser.UserId == userId && p.Manage) || ap.GrantedProject.GroupAccessPolicies.Any(p => - p.Group.GroupUsers.Any(gu => gu.OrganizationUser.UserId == userId && p.Write))) && - (ap.ServiceAccount.UserAccessPolicies.Any(p => p.OrganizationUser.UserId == userId && p.Write) || + p.Group.GroupUsers.Any(gu => gu.OrganizationUser.UserId == userId && p.Manage))) && + (ap.ServiceAccount.UserAccessPolicies.Any(p => p.OrganizationUser.UserId == userId && p.Manage) || ap.ServiceAccount.GroupAccessPolicies.Any(p => - p.Group.GroupUsers.Any(gu => gu.OrganizationUser.UserId == userId && p.Write))) + p.Group.GroupUsers.Any(gu => gu.OrganizationUser.UserId == userId && p.Manage))) + }), + AccessClientType.ServiceAccount => query.Select(ap => new ServiceAccountProjectAccessPolicyPermissionDetails + { + AccessPolicy = + Mapper.Map(ap), + HasPermission = + ap.ServiceAccount.ProjectAccessPolicies.Any(p => + p.GrantedProjectId == ap.GrantedProjectId && p.Manage) }), _ => throw new ArgumentOutOfRangeException(nameof(accessClientType), accessClientType, null) }; @@ -620,11 +760,10 @@ private IQueryable ToPermiss private static async Task UpdateServiceAccountRevisionAsync(DatabaseContext dbContext, Guid serviceAccountId) { - var entity = await dbContext.ServiceAccount.FindAsync(serviceAccountId); - if (entity != null) - { - entity.RevisionDate = DateTime.UtcNow; - } + var utcNow = DateTime.UtcNow; + await dbContext.ServiceAccount + .Where(sa => sa.Id == serviceAccountId) + .ExecuteUpdateAsync(setters => setters.SetProperty(sa => sa.RevisionDate, utcNow)); } private static async Task UpdateServiceAccountsRevisionAsync(DatabaseContext dbContext, List serviceAccountIds) diff --git a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ProjectRepository.cs b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ProjectRepository.cs index 78d90f9525b2..adf856f4f61a 100644 --- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ProjectRepository.cs +++ b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ProjectRepository.cs @@ -64,6 +64,8 @@ public async Task GetProjectCountByOrganizationIdAsync(Guid organizationId) { AccessClientType.NoAccessCheck => query, AccessClientType.User => query.Where(UserHasWriteAccessToProject(userId)), + AccessClientType.ServiceAccount => query.Where(p => + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Write)), _ => throw new ArgumentOutOfRangeException(nameof(accessType), accessType, null), }; @@ -135,7 +137,7 @@ await dbContext.Secret return projects; } - public async Task<(bool Read, bool Write)> AccessToProjectAsync(Guid id, Guid userId, AccessClientType accessType) + public async Task<(bool Read, bool Write, bool Manage)> AccessToProjectAsync(Guid id, Guid userId, AccessClientType accessType) { using var scope = ServiceScopeFactory.CreateScope(); var dbContext = GetDatabaseContext(scope); @@ -146,7 +148,7 @@ await dbContext.Secret var accessQuery = BuildProjectAccessQuery(projectQuery, userId, accessType); var policy = await accessQuery.FirstOrDefaultAsync(); - return policy == null ? (false, false) : (policy.Read, policy.Write); + return policy == null ? (false, false, false) : (policy.Read, policy.Write, policy.Manage); } public async Task ProjectsAreInOrganization(List projectIds, Guid organizationId) @@ -158,7 +160,7 @@ public async Task ProjectsAreInOrganization(List projectIds, Guid or return projectIds.Count == results.Count; } - public async Task> AccessToProjectsAsync( + public async Task> AccessToProjectsAsync( IEnumerable projectIds, Guid userId, AccessClientType accessType) @@ -169,7 +171,7 @@ public async Task ProjectsAreInOrganization(List projectIds, Guid or var projectsQuery = dbContext.Project.Where(p => projectIds.Contains(p.Id)); var accessQuery = BuildProjectAccessQuery(projectsQuery, userId, accessType); - return await accessQuery.ToDictionaryAsync(pa => pa.Id, pa => (pa.Read, pa.Write)); + return await accessQuery.ToDictionaryAsync(pa => pa.Id, pa => (pa.Read, pa.Write, pa.Manage)); } public async Task GetProjectCountByOrganizationIdAsync(Guid organizationId, Guid userId, @@ -224,13 +226,13 @@ public async Task GetProjectCountsByIdAsync(Guid projectId, Guid return projectCounts; } - private record ProjectAccess(Guid Id, bool Read, bool Write); + private record ProjectAccess(Guid Id, bool Read, bool Write, bool Manage); private static IQueryable BuildProjectAccessQuery(IQueryable projectQuery, Guid userId, AccessClientType accessType) => accessType switch { - AccessClientType.NoAccessCheck => projectQuery.Select(p => new ProjectAccess(p.Id, true, true)), + AccessClientType.NoAccessCheck => projectQuery.Select(p => new ProjectAccess(p.Id, true, true, true)), AccessClientType.User => projectQuery.Select(p => new ProjectAccess ( p.Id, @@ -239,15 +241,19 @@ private static IQueryable BuildProjectAccessQuery(IQueryable gu.OrganizationUser.User.Id == userId && ap.Read)), p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Write) || p.GroupAccessPolicies.Any(ap => - ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Write)) + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Write)), + p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Manage) || + p.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Manage)) )), AccessClientType.ServiceAccount => projectQuery.Select(p => new ProjectAccess ( p.Id, p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Read), - p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Write) + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Write), + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Manage) )), - _ => projectQuery.Select(p => new ProjectAccess(p.Id, false, false)) + _ => projectQuery.Select(p => new ProjectAccess(p.Id, false, false, false)) }; private IQueryable ProjectToPermissionDetails(IQueryable query, Guid userId, AccessClientType accessType) @@ -259,6 +265,7 @@ private IQueryable ProjectToPermissionDetails(IQueryab Project = Mapper.Map(p), Read = true, Write = true, + Manage = true, }), AccessClientType.User => query.Where(UserHasReadAccessToProject(userId)).Select(ProjectToPermissionsUser(userId, true)), AccessClientType.ServiceAccount => query.Where(ServiceAccountHasReadAccessToProject(userId)).Select(ProjectToPermissionsServiceAccount(userId, true)), @@ -275,6 +282,9 @@ private Expression> ProjectToPermissions Write = p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Write) || p.GroupAccessPolicies.Any(ap => ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Write)), + Manage = p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Manage) || + p.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Manage)), }; private Expression> ProjectToPermissionsServiceAccount(Guid userId, bool read) => @@ -283,6 +293,7 @@ private Expression> ProjectToPermissions Project = Mapper.Map(p), Read = read, Write = p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccount.Id == userId && ap.Write), + Manage = p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccount.Id == userId && ap.Manage), }; private static Expression> UserHasReadAccessToProject(Guid userId) => p => @@ -295,4 +306,28 @@ private static Expression> UserHasWriteAccessToProject(Guid private static Expression> ServiceAccountHasReadAccessToProject(Guid serviceAccountId) => p => p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccount.Id == serviceAccountId && ap.Read); + + public async Task GetProjectCreatorServiceAccountIdAsync(Guid id) + { + await using var scope = ServiceScopeFactory.CreateAsyncScope(); + var dbContext = GetDatabaseContext(scope); + return await dbContext.Project + .Where(p => p.Id == id && p.DeletedDate == null) + .Select(p => p.CreatedByServiceAccountId) + .FirstOrDefaultAsync(); + } + + public async Task IsServiceAccountCreatorOfAnyProjectForSecretAsync(Guid secretId, Guid serviceAccountId) + { + await using var scope = ServiceScopeFactory.CreateAsyncScope(); + var dbContext = GetDatabaseContext(scope); + var projectCreatorIds = await dbContext.Secret + .Where(s => s.Id == secretId && s.DeletedDate == null) + .SelectMany(s => s.Projects!) + .Where(p => p.DeletedDate == null) + .Select(p => p.CreatedByServiceAccountId) + .ToListAsync(); + + return projectCreatorIds.Count > 0 && projectCreatorIds.All(id => id == serviceAccountId); + } } 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 e783e4511864..c6f2ca65651e 100644 --- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs +++ b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs @@ -133,12 +133,12 @@ public async Task> GetManyDetailsByOrganiza .OrderBy(c => c.RevisionDate) .ToListAsync(); - // This should be changed if/when we allow non admins to access trashed items return Mapper.Map>(secrets).Select(s => new SecretPermissionDetails { Secret = s, Read = true, Write = true, + Manage = true, }); } } @@ -301,7 +301,7 @@ await dbContext.Secret.Where(c => secretIds.Contains(c.Id)) return secrets; } - public async Task<(bool Read, bool Write)> AccessToSecretAsync(Guid id, Guid userId, AccessClientType accessType) + public async Task<(bool Read, bool Write, bool Manage)> AccessToSecretAsync(Guid id, Guid userId, AccessClientType accessType) { await using var scope = ServiceScopeFactory.CreateAsyncScope(); var dbContext = GetDatabaseContext(scope); @@ -313,10 +313,10 @@ await dbContext.Secret.Where(c => secretIds.Contains(c.Id)) var policy = await query.FirstOrDefaultAsync(); - return policy == null ? (false, false) : (policy.Read, policy.Write); + return policy == null ? (false, false, false) : (policy.Read, policy.Write, policy.Manage); } - public async Task> AccessToSecretsAsync( + public async Task> AccessToSecretsAsync( IEnumerable ids, Guid userId, AccessClientType accessType) @@ -329,7 +329,7 @@ await dbContext.Secret.Where(c => secretIds.Contains(c.Id)) var accessQuery = BuildSecretAccessQuery(secrets, userId, accessType); - return await accessQuery.ToDictionaryAsync(sa => sa.Id, sa => (sa.Read, sa.Write)); + return await accessQuery.ToDictionaryAsync(sa => sa.Id, sa => (sa.Read, sa.Write, sa.Manage)); } public async Task EmptyTrash(DateTime currentDate, uint deleteAfterThisNumberOfDays) @@ -368,6 +368,7 @@ private IQueryable SecretToPermissionDetails(IQueryable Secret = Mapper.Map(s), Read = true, Write = true, + Manage = true, }), AccessClientType.User => query.Where(UserHasReadAccessToSecret(userId)).Select(SecretToPermissionsUser(userId, true)), AccessClientType.ServiceAccount => query.Where(ServiceAccountHasReadAccessToSecret(userId)).Select(s => @@ -377,6 +378,8 @@ private IQueryable SecretToPermissionDetails(IQueryable Read = true, Write = s.Projects.Any(p => p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Write)), + Manage = s.Projects.Any(p => + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Manage)), }), _ => throw new ArgumentOutOfRangeException(nameof(accessType), accessType, null), }; @@ -395,7 +398,15 @@ private Expression> SecretToPermissionsUse s.Projects.Any(p => p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Write) || p.GroupAccessPolicies.Any(ap => - ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Write))) + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Write))), + Manage = + s.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Manage) || + s.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Manage)) || + s.Projects.Any(p => + p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Manage) || + p.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Manage))) }; private static Expression> ServiceAccountHasReadAccessToSecret(Guid serviceAccountId) => s => @@ -472,7 +483,7 @@ private static IQueryable BuildSecretAccessQuery(IQueryable accessType switch { - AccessClientType.NoAccessCheck => secrets.Select(s => new SecretAccess(s.Id, true, true)), + AccessClientType.NoAccessCheck => secrets.Select(s => new SecretAccess(s.Id, true, true, true)), AccessClientType.User => secrets.Select(s => new SecretAccess( s.Id, s.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == accessClientId && ap.Read) || @@ -488,7 +499,14 @@ private static IQueryable BuildSecretAccessQuery(IQueryable p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == accessClientId && ap.Write) || p.GroupAccessPolicies.Any(ap => - ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == accessClientId && ap.Write))) + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == accessClientId && ap.Write))), + s.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == accessClientId && ap.Manage) || + s.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == accessClientId && ap.Manage)) || + s.Projects.Any(p => + p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == accessClientId && ap.Manage) || + p.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == accessClientId && ap.Manage))) )), AccessClientType.ServiceAccount => secrets.Select(s => new SecretAccess( s.Id, @@ -497,9 +515,12 @@ private static IQueryable BuildSecretAccessQuery(IQueryable ap.ServiceAccountId == accessClientId && ap.Read)), s.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == accessClientId && ap.Write) || s.Projects.Any(p => - p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == accessClientId && ap.Write)) + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == accessClientId && ap.Write)), + s.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == accessClientId && ap.Manage) || + s.Projects.Any(p => + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == accessClientId && ap.Manage)) )), - _ => secrets.Select(s => new SecretAccess(s.Id, false, false)) + _ => secrets.Select(s => new SecretAccess(s.Id, false, false, false)) }; private static async Task UpdateProjectMappingAsync(DatabaseContext dbContext, Secret currentEntity, Secret updatedEntity) @@ -574,6 +595,7 @@ private static async Task UpsertSecretAccessPolicyAsync(DatabaseContext dbContex dbContext.AccessPolicies.Attach(currentEntity); currentEntity.Read = updatedEntity.Read; currentEntity.Write = updatedEntity.Write; + currentEntity.Manage = updatedEntity.Manage; currentEntity.RevisionDate = currentDate; break; default: @@ -659,5 +681,5 @@ private BaseAccessPolicy MapToEntity(Core.SecretsManager.Entities.BaseAccessPoli _ => throw new ArgumentException("Unsupported access policy type") }; - private record SecretAccess(Guid Id, bool Read, bool Write); + private record SecretAccess(Guid Id, bool Read, bool Write, bool Manage); } diff --git a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ServiceAccountRepository.cs b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ServiceAccountRepository.cs index 10822d87d3cd..457e11f4024c 100644 --- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ServiceAccountRepository.cs +++ b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ServiceAccountRepository.cs @@ -53,6 +53,7 @@ public ServiceAccountRepository(IServiceScopeFactory serviceScopeFactory, IMappe { AccessClientType.NoAccessCheck => query, AccessClientType.User => query.Where(UserHasWriteAccessToServiceAccount(userId)), + AccessClientType.ServiceAccount => query.Where(_ => false), _ => throw new ArgumentOutOfRangeException(nameof(accessType), accessType, null), }; @@ -87,7 +88,7 @@ await dbContext.ServiceAccount await transaction.CommitAsync(); } - public async Task<(bool Read, bool Write)> AccessToServiceAccountAsync(Guid id, Guid userId, + public async Task<(bool Read, bool Write, bool Manage)> AccessToServiceAccountAsync(Guid id, Guid userId, AccessClientType accessType) { await using var scope = ServiceScopeFactory.CreateAsyncScope(); @@ -98,10 +99,10 @@ await dbContext.ServiceAccount var accessQuery = BuildServiceAccountAccessQuery(serviceAccountQuery, userId, accessType); var access = await accessQuery.FirstOrDefaultAsync(); - return access == null ? (false, false) : (access.Read, access.Write); + return access == null ? (false, false, false) : (access.Read, access.Write, access.Manage); } - public async Task> AccessToServiceAccountsAsync( + public async Task> AccessToServiceAccountsAsync( IEnumerable ids, Guid userId, AccessClientType accessType) @@ -112,7 +113,7 @@ await dbContext.ServiceAccount var serviceAccountsQuery = dbContext.ServiceAccount.Where(p => ids.Contains(p.Id)); var accessQuery = BuildServiceAccountAccessQuery(serviceAccountsQuery, userId, accessType); - return await accessQuery.ToDictionaryAsync(access => access.Id, access => (access.Read, access.Write)); + return await accessQuery.ToDictionaryAsync(access => access.Id, access => (access.Read, access.Write, access.Manage)); } public async Task GetServiceAccountCountByOrganizationIdAsync(Guid organizationId) @@ -213,13 +214,13 @@ public async Task> GetManyByOrganizati return results; } - private record ServiceAccountAccess(Guid Id, bool Read, bool Write); + private record ServiceAccountAccess(Guid Id, bool Read, bool Write, bool Manage); private static IQueryable BuildServiceAccountAccessQuery(IQueryable serviceAccountQuery, Guid userId, AccessClientType accessType) => accessType switch { - AccessClientType.NoAccessCheck => serviceAccountQuery.Select(sa => new ServiceAccountAccess(sa.Id, true, true)), + AccessClientType.NoAccessCheck => serviceAccountQuery.Select(sa => new ServiceAccountAccess(sa.Id, true, true, true)), AccessClientType.User => serviceAccountQuery.Select(sa => new ServiceAccountAccess ( sa.Id, @@ -228,10 +229,13 @@ private static IQueryable BuildServiceAccountAccessQuery(I ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Read)), sa.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Write) || sa.GroupAccessPolicies.Any(ap => - ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Write)) + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Write)), + sa.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Manage) || + sa.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Manage)) )), - AccessClientType.ServiceAccount => serviceAccountQuery.Select(sa => new ServiceAccountAccess(sa.Id, false, false)), - _ => serviceAccountQuery.Select(sa => new ServiceAccountAccess(sa.Id, false, false)) + AccessClientType.ServiceAccount => serviceAccountQuery.Select(sa => new ServiceAccountAccess(sa.Id, false, false, false)), + _ => serviceAccountQuery.Select(sa => new ServiceAccountAccess(sa.Id, false, false, false)) }; private static Expression> UserHasReadAccessToServiceAccount(Guid userId) => sa => diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectPeopleAccessPoliciesAuthorizationHandlerTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectPeopleAccessPoliciesAuthorizationHandlerTests.cs index f12ede6175d9..5d44c72db5e0 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectPeopleAccessPoliciesAuthorizationHandlerTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectPeopleAccessPoliciesAuthorizationHandlerTests.cs @@ -4,6 +4,7 @@ using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.SecretsManager.AuthorizationRequirements; +using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Models.Data; using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces; using Bit.Core.SecretsManager.Queries.Interfaces; @@ -23,7 +24,7 @@ public class ProjectPeopleAccessPoliciesAuthorizationHandlerTests { private static void SetupUserPermission(SutProvider sutProvider, AccessClientType accessClientType, ProjectPeopleAccessPolicies resource, Guid userId = new(), bool read = true, - bool write = true) + bool write = true, bool manage = true) { sutProvider.GetDependency().AccessSecretsManager(resource.OrganizationId) .Returns(true); @@ -31,7 +32,7 @@ private static void SetupUserPermission(SutProvider().AccessToProjectAsync(resource.Id, userId, accessClientType) - .Returns((read, write)); + .Returns((read, write, manage)); } private static void SetupOrganizationUsers(SutProvider sutProvider, @@ -46,6 +47,28 @@ private static void SetupGroups(SutProvider>(), resource.OrganizationId) .Returns(true); + [Theory] + [BitAutoData] + public async Task Handler_ServiceAccountCaller_WithoutManage_DoesNotSucceed( + SutProvider sutProvider, + ProjectPeopleAccessPolicies resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = ProjectPeopleAccessPoliciesOperations.Replace; + SetupUserPermission(sutProvider, AccessClientType.ServiceAccount, resource, userId, manage: false); + SetupOrganizationUsers(sutProvider, resource); + SetupGroups(sutProvider, resource); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Fact] public void PeopleAccessPoliciesOperations_OnlyPublicStatic() { @@ -91,14 +114,13 @@ public async Task Handler_AccessSecretsManagerFalse_DoesNotSucceed( } [Theory] - [BitAutoData(AccessClientType.ServiceAccount)] [BitAutoData(AccessClientType.Organization)] public async Task Handler_UnsupportedClientTypes_DoesNotSucceed(AccessClientType clientType, SutProvider sutProvider, ProjectPeopleAccessPolicies resource, ClaimsPrincipal claimsPrincipal) { - var requirement = new ProjectPeopleAccessPoliciesOperationRequirement(); - SetupUserPermission(sutProvider, clientType, resource); + var requirement = ProjectPeopleAccessPoliciesOperations.Replace; + SetupUserPermission(sutProvider, clientType, resource, manage: false); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, resource); @@ -151,21 +173,149 @@ public async Task ReplaceProjectPeople_GroupNotInOrg_DoesNotSucceed(AccessClient } [Theory] - [BitAutoData(AccessClientType.User, false, false, false)] - [BitAutoData(AccessClientType.User, false, true, true)] - [BitAutoData(AccessClientType.User, true, false, false)] - [BitAutoData(AccessClientType.User, true, true, true)] - [BitAutoData(AccessClientType.NoAccessCheck, false, false, false)] - [BitAutoData(AccessClientType.NoAccessCheck, false, true, true)] - [BitAutoData(AccessClientType.NoAccessCheck, true, false, false)] - [BitAutoData(AccessClientType.NoAccessCheck, true, true, true)] + [BitAutoData(AccessClientType.User)] + [BitAutoData(AccessClientType.NoAccessCheck)] + public async Task ReplaceProjectPeople_ZeroNewManage_Succeeds( + AccessClientType accessClient, + SutProvider sutProvider, + ProjectPeopleAccessPolicies resource, + ClaimsPrincipal claimsPrincipal, + Guid userId) + { + var requirement = ProjectPeopleAccessPoliciesOperations.Replace; + SetupUserPermission(sutProvider, accessClient, resource, userId); + SetupOrganizationUsers(sutProvider, resource); + SetupGroups(sutProvider, resource); + + resource.UserAccessPolicies = resource.UserAccessPolicies? + .Select(ap => { ap.Manage = false; return ap; }) + .ToList(); + resource.GroupAccessPolicies = resource.GroupAccessPolicies? + .Select(ap => { ap.Manage = false; return ap; }) + .ToList(); + + var authzContext = new AuthorizationHandlerContext( + new List { requirement }, claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task ReplaceProjectPeople_ServiceAccount_Manage_ProjectCreatedBySA_Succeeds( + SutProvider sutProvider, + ProjectPeopleAccessPolicies resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = ProjectPeopleAccessPoliciesOperations.Replace; + SetupUserPermission(sutProvider, AccessClientType.ServiceAccount, resource, userId); + + resource.UserAccessPolicies = new List + { + new() { OrganizationUserId = Guid.NewGuid(), Manage = true } + }; + resource.GroupAccessPolicies = new List(); + + sutProvider.GetDependency() + .GetProjectCreatorServiceAccountIdAsync(resource.Id) + .Returns(userId); + + SetupOrganizationUsers(sutProvider, resource); + SetupGroups(sutProvider, resource); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task ReplaceProjectPeople_ServiceAccount_Manage_ProjectNotCreatedBySA_DoesNotSucceed( + SutProvider sutProvider, + ProjectPeopleAccessPolicies resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = ProjectPeopleAccessPoliciesOperations.Replace; + SetupUserPermission(sutProvider, AccessClientType.ServiceAccount, resource, userId); + + resource.UserAccessPolicies = new List + { + new() { OrganizationUserId = Guid.NewGuid(), Manage = true } + }; + resource.GroupAccessPolicies = new List(); + + sutProvider.GetDependency() + .GetProjectCreatorServiceAccountIdAsync(resource.Id) + .Returns(Guid.NewGuid()); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task ReplaceProjectPeople_ServiceAccount_IsCreator_UserFromForeignOrg_DoesNotSucceed( + SutProvider sutProvider, + ProjectPeopleAccessPolicies resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + // Even when the SA is the project creator it must not be able to + // grant access to users from a different organization. + var requirement = ProjectPeopleAccessPoliciesOperations.Replace; + SetupUserPermission(sutProvider, AccessClientType.ServiceAccount, resource, userId); + + resource.UserAccessPolicies = new List + { + new() { OrganizationUserId = Guid.NewGuid(), Manage = true } + }; + resource.GroupAccessPolicies = new List(); + + // SA IS the creator + sutProvider.GetDependency() + .GetProjectCreatorServiceAccountIdAsync(resource.Id) + .Returns(userId); + + // User is from a FOREIGN org + sutProvider.GetDependency() + .OrgUsersInTheSameOrgAsync(Arg.Any>(), 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.User, false, false, false, false)] + [BitAutoData(AccessClientType.User, false, false, true, true)] + [BitAutoData(AccessClientType.User, true, true, false, false)] + [BitAutoData(AccessClientType.User, true, true, true, true)] + [BitAutoData(AccessClientType.NoAccessCheck, false, false, false, false)] + [BitAutoData(AccessClientType.NoAccessCheck, false, false, true, true)] + [BitAutoData(AccessClientType.NoAccessCheck, true, true, false, false)] + [BitAutoData(AccessClientType.NoAccessCheck, true, true, true, true)] public async Task ReplaceProjectPeople_AccessCheck(AccessClientType accessClient, bool read, bool write, - bool expected, + bool manage, bool expected, SutProvider sutProvider, ProjectPeopleAccessPolicies resource, ClaimsPrincipal claimsPrincipal, Guid userId) { var requirement = ProjectPeopleAccessPoliciesOperations.Replace; - SetupUserPermission(sutProvider, accessClient, resource, userId, read, write); + SetupUserPermission(sutProvider, accessClient, resource, userId, read, write, manage); SetupOrganizationUsers(sutProvider, resource); SetupGroups(sutProvider, resource); diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectServiceAccountsAccessPoliciesAuthorizationHandlerTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectServiceAccountsAccessPoliciesAuthorizationHandlerTests.cs index 17c92443cc12..9067625c6063 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectServiceAccountsAccessPoliciesAuthorizationHandlerTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectServiceAccountsAccessPoliciesAuthorizationHandlerTests.cs @@ -89,7 +89,7 @@ public async Task Handler_UnsupportedProjectServiceAccountsPoliciesOperationRequ [BitAutoData(AccessClientType.NoAccessCheck, true, false)] [BitAutoData(AccessClientType.User, false, false)] [BitAutoData(AccessClientType.User, true, false)] - public async Task Handler_UserHasNoWriteAccessToProject_DoesNotSucceed( + public async Task Handler_UserHasNoManageAccessToProject_DoesNotSucceed( AccessClientType accessClientType, bool projectReadAccess, bool projectWriteAccess, @@ -102,7 +102,7 @@ public async Task Handler_UserHasNoWriteAccessToProject_DoesNotSucceed( SetupUserSubstitutes(sutProvider, accessClientType, resource, userId); sutProvider.GetDependency() .AccessToProjectAsync(resource.ProjectId, userId, accessClientType) - .Returns((projectReadAccess, projectWriteAccess)); + .Returns((projectReadAccess, projectWriteAccess, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, resource); @@ -123,7 +123,7 @@ public async Task Handler_ServiceAccountsInDifferentOrganization_DoesNotSucceed( SetupUserSubstitutes(sutProvider, AccessClientType.NoAccessCheck, resource, userId); sutProvider.GetDependency() .AccessToProjectAsync(resource.ProjectId, userId, AccessClientType.NoAccessCheck) - .Returns((true, true)); + .Returns((true, true, true)); sutProvider.GetDependency() .ServiceAccountsAreInOrganizationAsync(Arg.Any>(), resource.OrganizationId) .Returns(false); @@ -173,7 +173,7 @@ public async Task Handler_UserHasNoAccessToCreateServiceAccounts_DoesNotSucceed( var accessResult = resource.ServiceAccountAccessPolicyUpdates .Where(x => x.Operation == AccessPolicyOperation.Create) .Select(x => x.AccessPolicy.ServiceAccountId!.Value) - .ToDictionary(id => id, _ => (false, false)); + .ToDictionary(id => id, _ => (false, false, false)); sutProvider.GetDependency() .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) @@ -205,9 +205,9 @@ public async Task Handler_AccessResultsPartial_DoesNotSucceed( var accessResult = resource.ServiceAccountAccessPolicyUpdates .Where(x => x.Operation == AccessPolicyOperation.Create) .Select(x => x.AccessPolicy.ServiceAccountId!.Value) - .ToDictionary(id => id, _ => (false, false)); + .ToDictionary(id => id, _ => (false, false, false)); - accessResult[accessResult.First().Key] = (true, true); + accessResult[accessResult.First().Key] = (true, true, false); accessResult.Remove(accessResult.Last().Key); sutProvider.GetDependency() .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) @@ -239,9 +239,9 @@ public async Task Handler_UserHasAccessToSomeCreateServiceAccounts_DoesNotSuccee var accessResult = resource.ServiceAccountAccessPolicyUpdates .Where(x => x.Operation == AccessPolicyOperation.Create) .Select(x => x.AccessPolicy.ServiceAccountId!.Value) - .ToDictionary(id => id, _ => (false, false)); + .ToDictionary(id => id, _ => (false, false, false)); - accessResult[accessResult.First().Key] = (true, true); + accessResult[accessResult.First().Key] = (true, true, false); sutProvider.GetDependency() .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) .Returns(accessResult); @@ -272,7 +272,7 @@ public async Task Handler_UserHasAccessToAllCreateServiceAccounts_Success( var accessResult = resource.ServiceAccountAccessPolicyUpdates .Where(x => x.Operation == AccessPolicyOperation.Create) .Select(x => x.AccessPolicy.ServiceAccountId!.Value) - .ToDictionary(id => id, _ => (true, true)); + .ToDictionary(id => id, _ => (true, true, true)); sutProvider.GetDependency() .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) @@ -309,7 +309,7 @@ private static void SetupServiceAccountsAccessTest( sutProvider.GetDependency() .AccessToProjectAsync(resource.ProjectId, userId, accessClientType) - .Returns((true, true)); + .Returns((true, true, true)); sutProvider.GetDependency() .ServiceAccountsAreInOrganizationAsync(Arg.Any>(), resource.OrganizationId) .Returns(true); @@ -339,4 +339,229 @@ private static ProjectServiceAccountsAccessPoliciesUpdates RemoveAllCreates( resource.ServiceAccountAccessPolicyUpdates.Where(x => x.Operation != AccessPolicyOperation.Create); return resource; } + + [Theory] + [BitAutoData] + public async Task Handler_SA_GrantManage_ProjectNotCreatedBySA_DoesNotSucceed( + SutProvider sutProvider, + ProjectServiceAccountsAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = ProjectServiceAccountsAccessPoliciesOperations.Updates; + SetupUserSubstitutes(sutProvider, AccessClientType.ServiceAccount, resource, userId); + + sutProvider.GetDependency() + .AccessToProjectAsync(resource.ProjectId, userId, AccessClientType.ServiceAccount) + .Returns((true, true, true)); + + var otherSaId = Guid.NewGuid(); + resource.ServiceAccountAccessPolicyUpdates = resource.ServiceAccountAccessPolicyUpdates.Append( + new ServiceAccountProjectAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Create, + AccessPolicy = new ServiceAccountProjectAccessPolicy + { + ServiceAccountId = otherSaId, + GrantedProjectId = resource.ProjectId, + Read = true, + Write = true, + Manage = true + } + }); + + sutProvider.GetDependency() + .GetProjectCreatorServiceAccountIdAsync(resource.ProjectId) + .Returns(Guid.NewGuid()); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task Handler_SA_GrantManage_ToItself_OnProjectItCreated_Succeeds( + SutProvider sutProvider, + ProjectServiceAccountsAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = ProjectServiceAccountsAccessPoliciesOperations.Updates; + SetupUserSubstitutes(sutProvider, AccessClientType.ServiceAccount, resource, userId); + + sutProvider.GetDependency() + .AccessToProjectAsync(resource.ProjectId, userId, AccessClientType.ServiceAccount) + .Returns((true, true, true)); + + resource.ServiceAccountAccessPolicyUpdates = new List + { + new() + { + Operation = AccessPolicyOperation.Create, + AccessPolicy = new ServiceAccountProjectAccessPolicy + { + ServiceAccountId = userId, + GrantedProjectId = resource.ProjectId, + Read = true, + Write = true, + Manage = true + } + } + }; + + sutProvider.GetDependency() + .GetProjectCreatorServiceAccountIdAsync(resource.ProjectId) + .Returns(userId); + + sutProvider.GetDependency() + .ServiceAccountsAreInOrganizationAsync(Arg.Any>(), resource.OrganizationId) + .Returns(true); + + sutProvider.GetDependency() + .AccessToServiceAccountsAsync(Arg.Any>(), userId, AccessClientType.ServiceAccount) + .Returns(new Dictionary { { userId, (true, true, true) } }); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task Handler_SA_GrantManage_ToDifferentSA_EvenAsCreator_DoesNotSucceed( + SutProvider sutProvider, + ProjectServiceAccountsAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = ProjectServiceAccountsAccessPoliciesOperations.Updates; + SetupUserSubstitutes(sutProvider, AccessClientType.ServiceAccount, resource, userId); + + sutProvider.GetDependency() + .AccessToProjectAsync(resource.ProjectId, userId, AccessClientType.ServiceAccount) + .Returns((true, true, true)); + + var otherSaId = Guid.NewGuid(); + resource.ServiceAccountAccessPolicyUpdates = new List + { + new() + { + Operation = AccessPolicyOperation.Create, + AccessPolicy = new ServiceAccountProjectAccessPolicy + { + ServiceAccountId = otherSaId, + GrantedProjectId = resource.ProjectId, + Read = true, + Write = true, + Manage = true + } + } + }; + + sutProvider.GetDependency() + .GetProjectCreatorServiceAccountIdAsync(resource.ProjectId) + .Returns(userId); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task Handler_SA_GrantManage_ViaUpdateOperation_NotCreator_DoesNotSucceed( + SutProvider sutProvider, + ProjectServiceAccountsAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + // Update op with Manage=true must trigger the delegation guard, + // not bypass it by using a non-Create operation. + var requirement = ProjectServiceAccountsAccessPoliciesOperations.Updates; + SetupUserSubstitutes(sutProvider, AccessClientType.ServiceAccount, resource, userId); + + sutProvider.GetDependency() + .AccessToProjectAsync(resource.ProjectId, userId, AccessClientType.ServiceAccount) + .Returns((true, true, true)); + + resource.ServiceAccountAccessPolicyUpdates = new List + { + new() + { + Operation = AccessPolicyOperation.Update, + AccessPolicy = new ServiceAccountProjectAccessPolicy + { + ServiceAccountId = userId, + GrantedProjectId = resource.ProjectId, + Read = true, + Write = true, + Manage = true + } + } + }; + + sutProvider.GetDependency() + .GetProjectCreatorServiceAccountIdAsync(resource.ProjectId) + .Returns(Guid.NewGuid()); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task Handler_SA_GrantManage_NullCreatedByServiceAccountId_DoesNotSucceed( + SutProvider sutProvider, + ProjectServiceAccountsAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = ProjectServiceAccountsAccessPoliciesOperations.Updates; + SetupUserSubstitutes(sutProvider, AccessClientType.ServiceAccount, resource, userId); + + sutProvider.GetDependency() + .AccessToProjectAsync(resource.ProjectId, userId, AccessClientType.ServiceAccount) + .Returns((true, true, true)); + + resource.ServiceAccountAccessPolicyUpdates = new List + { + new() + { + Operation = AccessPolicyOperation.Create, + AccessPolicy = new ServiceAccountProjectAccessPolicy + { + ServiceAccountId = userId, + GrantedProjectId = resource.ProjectId, + Read = true, + Write = true, + Manage = true + } + } + }; + + sutProvider.GetDependency() + .GetProjectCreatorServiceAccountIdAsync(resource.ProjectId) + .Returns((Guid?)null); + + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } } 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 index f05bd8fbe9e7..32ec065b9264 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandlerTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandlerTests.cs @@ -102,7 +102,7 @@ public async Task Handler_CanUpdateAsync_UserHasNoWriteAccessToSecret_DoesNotSuc SetupUserSubstitutes(sutProvider, accessClientType, resource, userId); sutProvider.GetDependency() .AccessToSecretAsync(resource.SecretId, userId, accessClientType) - .Returns((readAccess, writeAccess)); + .Returns((readAccess, writeAccess, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, resource); @@ -304,6 +304,34 @@ public async Task Handler_CanCreateAsync_NotCreationOperations_DoesNotSucceed( Assert.False(authzContext.HasSucceeded); } + [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_CanCreateAsync_UserHasNoManageAccessToSecret_DoesNotSucceed( + AccessClientType accessClientType, + bool readAccess, + bool writeAccess, + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Create; + resource = SetAllToCreates(resource); + SetupUserSubstitutes(sutProvider, accessClientType, resource, userId); + sutProvider.GetDependency() + .AccessToSecretAsync(resource.SecretId, userId, accessClientType) + .Returns((readAccess, writeAccess, false)); + 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)] @@ -482,6 +510,74 @@ public async Task Handler_CanCreateAsync_UserHasAccessToAllServiceAccounts_Succe Assert.True(authzContext.HasSucceeded); } + [Theory] + [BitAutoData] + public async Task Handler_CanUpdateAsync_SA_ManageGrant_IsCreator_Success( + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + resource = AddManageServiceAccountUpdate(resource, userId); + SetupSameOrganizationRequest(sutProvider, AccessClientType.ServiceAccount, resource, userId); + SetupAllServiceAccountAccess(sutProvider, resource, userId, AccessClientType.ServiceAccount); + sutProvider.GetDependency() + .IsServiceAccountCreatorOfAnyProjectForSecretAsync(resource.SecretId, userId) + .Returns(true); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task Handler_CanUpdateAsync_SA_ManageGrant_NotCreator_DoesNotSucceed( + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + resource = AddManageServiceAccountUpdate(resource, userId); + SetupSameOrganizationRequest(sutProvider, AccessClientType.ServiceAccount, resource, userId); + sutProvider.GetDependency() + .IsServiceAccountCreatorOfAnyProjectForSecretAsync(resource.SecretId, userId) + .Returns(false); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task Handler_CanUpdateAsync_SA_NoManageGrant_DoesNotCheckCreator_Success( + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + resource = RemoveAllServiceAccountCreates(resource); + resource = ClearAllManageFlags(resource); + SetupSameOrganizationRequest(sutProvider, AccessClientType.ServiceAccount, resource, userId); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + await sutProvider.GetDependency() + .DidNotReceive() + .IsServiceAccountCreatorOfAnyProjectForSecretAsync(Arg.Any(), Arg.Any()); + } + private static void SetupNoServiceAccountAccess( SutProvider sutProvider, SecretAccessPoliciesUpdates resource, @@ -494,7 +590,7 @@ private static void SetupNoServiceAccountAccess( .ToList(); sutProvider.GetDependency() .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) - .Returns(createServiceAccountIds.ToDictionary(id => id, _ => (false, false))); + .Returns(createServiceAccountIds.ToDictionary(id => id, _ => (false, false, false))); } private static void SetupPartialServiceAccountAccess( @@ -506,8 +602,8 @@ private static void SetupPartialServiceAccountAccess( 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); + .ToDictionary(id => id, _ => (true, true, false)); + accessResult[accessResult.First().Key] = (true, true, false); accessResult.Remove(accessResult.Last().Key); sutProvider.GetDependency() .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) @@ -523,9 +619,9 @@ private static void SetupSomeServiceAccountAccess( var accessResult = resource.ServiceAccountAccessPolicyUpdates .Where(x => x.Operation == AccessPolicyOperation.Create) .Select(x => x.AccessPolicy.ServiceAccountId!.Value) - .ToDictionary(id => id, _ => (false, false)); + .ToDictionary(id => id, _ => (false, false, false)); - accessResult[accessResult.First().Key] = (true, true); + accessResult[accessResult.First().Key] = (true, true, false); sutProvider.GetDependency() .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) .Returns(accessResult); @@ -540,7 +636,7 @@ private static void SetupAllServiceAccountAccess( var accessResult = resource.ServiceAccountAccessPolicyUpdates .Where(x => x.Operation == AccessPolicyOperation.Create) .Select(x => x.AccessPolicy.ServiceAccountId!.Value) - .ToDictionary(id => id, _ => (true, true)); + .ToDictionary(id => id, _ => (true, true, true)); sutProvider.GetDependency() .AccessToServiceAccountsAsync(Arg.Any>(), userId, accessClientType) .Returns(accessResult); @@ -571,7 +667,7 @@ private static void SetupSameOrganizationRequest( sutProvider.GetDependency() .AccessToSecretAsync(resource.SecretId, userId, accessClientType) - .Returns((true, true)); + .Returns((true, true, true)); sutProvider.GetDependency() .OrgUsersInTheSameOrgAsync(Arg.Any>(), resource.OrganizationId) @@ -653,4 +749,97 @@ private static SecretAccessPoliciesUpdates ClearAccessPolicyUpdate(SecretAccessP return resource; } + + private static SecretAccessPoliciesUpdates AddManageServiceAccountUpdate( + SecretAccessPoliciesUpdates resource, Guid serviceAccountId) + { + resource.ServiceAccountAccessPolicyUpdates = resource.ServiceAccountAccessPolicyUpdates.Append( + new ServiceAccountSecretAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Create, + AccessPolicy = new ServiceAccountSecretAccessPolicy + { + ServiceAccountId = serviceAccountId, + GrantedSecretId = resource.SecretId, + Read = true, + Write = true, + Manage = true + } + }); + return resource; + } + + private static SecretAccessPoliciesUpdates ClearAllManageFlags(SecretAccessPoliciesUpdates resource) + { + foreach (var u in resource.UserAccessPolicyUpdates) u.AccessPolicy.Manage = false; + foreach (var u in resource.GroupAccessPolicyUpdates) u.AccessPolicy.Manage = false; + foreach (var u in resource.ServiceAccountAccessPolicyUpdates) u.AccessPolicy.Manage = false; + return resource; + } + + // H-1: SA Update with Manage — only the creator SA may escalate to Manage via Update + private static SecretAccessPoliciesUpdates AddManageServiceAccountUpdateOperation( + SecretAccessPoliciesUpdates resource, Guid serviceAccountId) + { + resource.ServiceAccountAccessPolicyUpdates = resource.ServiceAccountAccessPolicyUpdates.Append( + new ServiceAccountSecretAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Update, + AccessPolicy = new ServiceAccountSecretAccessPolicy + { + ServiceAccountId = serviceAccountId, + GrantedSecretId = resource.SecretId, + Read = true, + Write = true, + Manage = true + } + }); + return resource; + } + + [Theory] + [BitAutoData] + public async Task Handler_CanUpdateAsync_SA_ManageGrant_ViaUpdate_IsCreator_Succeeds( + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretAccessPoliciesOperations.Updates; + resource = AddManageServiceAccountUpdateOperation(resource, userId); + SetupSameOrganizationRequest(sutProvider, AccessClientType.ServiceAccount, resource, userId); + SetupAllServiceAccountAccess(sutProvider, resource, userId, AccessClientType.ServiceAccount); + sutProvider.GetDependency() + .IsServiceAccountCreatorOfAnyProjectForSecretAsync(resource.SecretId, userId) + .Returns(true); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.True(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task Handler_CanUpdateAsync_SA_ManageGrant_ViaUpdate_NotCreator_DoesNotSucceed( + SutProvider sutProvider, + SecretAccessPoliciesUpdates resource, + Guid userId, + ClaimsPrincipal claimsPrincipal) + { + // H-1 regression: a SA that is NOT the creator must not escalate to Manage via Update. + var requirement = SecretAccessPoliciesOperations.Updates; + resource = AddManageServiceAccountUpdateOperation(resource, userId); + SetupSameOrganizationRequest(sutProvider, AccessClientType.ServiceAccount, resource, userId); + sutProvider.GetDependency() + .IsServiceAccountCreatorOfAnyProjectForSecretAsync(resource.SecretId, userId) + .Returns(false); // different SA is the creator + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, resource); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } } diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountGrantedPoliciesAuthorizationHandlerTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountGrantedPoliciesAuthorizationHandlerTests.cs index 45fe8c588f98..c5864d330522 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountGrantedPoliciesAuthorizationHandlerTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountGrantedPoliciesAuthorizationHandlerTests.cs @@ -100,7 +100,7 @@ public async Task Handler_UserHasNoWriteAccessToServiceAccount_DoesNotSucceed( SetupUserSubstitutes(sutProvider, accessClientType, resource, userId); sutProvider.GetDependency() .AccessToServiceAccountAsync(resource.ServiceAccountId, userId, accessClientType) - .Returns((saReadAccess, saWriteAccess)); + .Returns((saReadAccess, saWriteAccess, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, resource); @@ -121,7 +121,7 @@ public async Task Handler_GrantedProjectsInDifferentOrganization_DoesNotSucceed( SetupUserSubstitutes(sutProvider, AccessClientType.NoAccessCheck, resource, userId); sutProvider.GetDependency() .AccessToServiceAccountAsync(resource.ServiceAccountId, userId, AccessClientType.NoAccessCheck) - .Returns((true, true)); + .Returns((true, true, false)); sutProvider.GetDependency() .ProjectsAreInOrganization(Arg.Any>(), resource.OrganizationId) .Returns(false); @@ -148,7 +148,7 @@ public async Task Handler_UserHasNoAccessToGrantedProjects_DoesNotSucceed( sutProvider.GetDependency() .AccessToProjectsAsync(Arg.Any>(), userId, accessClientType) - .Returns(projectIds.ToDictionary(projectId => projectId, _ => (false, false))); + .Returns(projectIds.ToDictionary(projectId => projectId, _ => (false, false, false))); var authzContext = new AuthorizationHandlerContext(new List { requirement }, @@ -172,8 +172,8 @@ public async Task Handler_UserHasAccessToSomeGrantedProjects_DoesNotSucceed( var requirement = ServiceAccountGrantedPoliciesOperations.Updates; var projectIds = SetupProjectAccessTest(sutProvider, accessClientType, resource, userId); - var accessResult = projectIds.ToDictionary(projectId => projectId, _ => (false, false)); - accessResult[projectIds.First()] = (true, true); + var accessResult = projectIds.ToDictionary(projectId => projectId, _ => (false, false, false)); + accessResult[projectIds.First()] = (true, true, false); sutProvider.GetDependency() .AccessToProjectsAsync(Arg.Any>(), userId, accessClientType) .Returns(accessResult); @@ -200,7 +200,7 @@ public async Task Handler_AccessResultsPartial_DoesNotSucceed( var requirement = ServiceAccountGrantedPoliciesOperations.Updates; var projectIds = SetupProjectAccessTest(sutProvider, accessClientType, resource, userId); - var accessResult = projectIds.ToDictionary(projectId => projectId, _ => (false, false)); + var accessResult = projectIds.ToDictionary(projectId => projectId, _ => (false, false, false)); accessResult.Remove(projectIds.First()); sutProvider.GetDependency() .AccessToProjectsAsync(Arg.Any>(), userId, accessClientType) @@ -229,7 +229,7 @@ public async Task Handler_UserHasAccessToAllGrantedProjects_Success( sutProvider.GetDependency() .AccessToProjectsAsync(Arg.Any>(), userId, accessClientType) - .Returns(projectIds.ToDictionary(projectId => projectId, _ => (true, true))); + .Returns(projectIds.ToDictionary(projectId => projectId, _ => (true, true, true))); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, resource); @@ -261,7 +261,7 @@ private static List SetupProjectAccessTest( sutProvider.GetDependency() .AccessToServiceAccountAsync(resource.ServiceAccountId, userId, accessClientType) - .Returns((true, true)); + .Returns((true, true, true)); sutProvider.GetDependency() .ProjectsAreInOrganization(Arg.Any>(), resource.OrganizationId) .Returns(true); diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountPeopleAccessPoliciesAuthorizationHandlerTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountPeopleAccessPoliciesAuthorizationHandlerTests.cs index f2a18e2271a9..43b6e2462d05 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountPeopleAccessPoliciesAuthorizationHandlerTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountPeopleAccessPoliciesAuthorizationHandlerTests.cs @@ -23,7 +23,8 @@ private static void SetupUserPermission( SutProvider sutProvider, AccessClientType accessClientType, ServiceAccountPeopleAccessPolicies resource, Guid userId = new(), bool read = true, - bool write = true) + bool write = true, + bool manage = true) { sutProvider.GetDependency().AccessSecretsManager(resource.OrganizationId) .Returns(true); @@ -32,7 +33,7 @@ private static void SetupUserPermission( (accessClientType, userId)); sutProvider.GetDependency() .AccessToServiceAccountAsync(resource.Id, userId, accessClientType) - .Returns((read, write)); + .Returns((read, write, manage)); } private static void SetupOrganizationUsers( @@ -157,22 +158,26 @@ public async Task ReplaceServiceAccountPeople_GroupNotInOrg_DoesNotSucceed(Acces } [Theory] - [BitAutoData(AccessClientType.User, false, false, false)] - [BitAutoData(AccessClientType.User, false, true, true)] - [BitAutoData(AccessClientType.User, true, false, false)] - [BitAutoData(AccessClientType.User, true, true, true)] - [BitAutoData(AccessClientType.NoAccessCheck, false, false, false)] - [BitAutoData(AccessClientType.NoAccessCheck, false, true, true)] - [BitAutoData(AccessClientType.NoAccessCheck, true, false, false)] - [BitAutoData(AccessClientType.NoAccessCheck, true, true, true)] + [BitAutoData(AccessClientType.User, false, false, false, false)] + [BitAutoData(AccessClientType.User, false, false, true, true)] + [BitAutoData(AccessClientType.User, false, true, false, false)] + [BitAutoData(AccessClientType.User, true, false, true, true)] + [BitAutoData(AccessClientType.User, true, true, false, false)] + [BitAutoData(AccessClientType.User, true, true, true, true)] + [BitAutoData(AccessClientType.NoAccessCheck, false, false, false, false)] + [BitAutoData(AccessClientType.NoAccessCheck, false, false, true, true)] + [BitAutoData(AccessClientType.NoAccessCheck, false, true, false, false)] + [BitAutoData(AccessClientType.NoAccessCheck, true, false, true, true)] + [BitAutoData(AccessClientType.NoAccessCheck, true, true, false, false)] + [BitAutoData(AccessClientType.NoAccessCheck, true, true, true, true)] public async Task ReplaceServiceAccountPeople_AccessCheck(AccessClientType accessClient, bool read, bool write, - bool expected, + bool manage, bool expected, SutProvider sutProvider, ServiceAccountPeopleAccessPolicies resource, ClaimsPrincipal claimsPrincipal, Guid userId) { var requirement = ServiceAccountPeopleAccessPoliciesOperations.Replace; - SetupUserPermission(sutProvider, accessClient, resource, userId, read, write); + SetupUserPermission(sutProvider, accessClient, resource, userId, read, write, manage); SetupOrganizationUsers(sutProvider, resource); SetupGroups(sutProvider, resource); diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Projects/ProjectAuthorizationHandlerTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Projects/ProjectAuthorizationHandlerTests.cs index aa84db43f447..7869497ea42c 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Projects/ProjectAuthorizationHandlerTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Projects/ProjectAuthorizationHandlerTests.cs @@ -171,7 +171,7 @@ public async Task CanUpdateProject_NullResource_DoesNotSucceed( SetupPermission(sutProvider, PermissionType.RunAsAdmin, project.OrganizationId); sutProvider.GetDependency() .AccessToProjectAsync(project.Id, userId, Arg.Any()) - .Returns((true, true)); + .Returns((true, true, false)); var requirement = ProjectOperations.Update; var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, null); @@ -221,7 +221,7 @@ public async Task CanUpdateProject_AccessCheck(PermissionType permissionType, bo SetupPermission(sutProvider, permissionType, project.OrganizationId, userId); sutProvider.GetDependency() .AccessToProjectAsync(project.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, project); @@ -304,7 +304,7 @@ public async Task CanDeleteProject_AccessCheck(PermissionType permissionType, bo SetupPermission(sutProvider, permissionType, project.OrganizationId, userId); sutProvider.GetDependency() .AccessToProjectAsync(project.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, project); diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/BulkSecretAuthorizationHandlerTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/BulkSecretAuthorizationHandlerTests.cs index a015b1a02acb..5c4a488211ed 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/BulkSecretAuthorizationHandlerTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/BulkSecretAuthorizationHandlerTests.cs @@ -95,7 +95,7 @@ public async Task Handler_NoAccessToSecrets_DoesNotSucceed( SetupSecretAccessRequest(sutProvider, resources, accessClientType, resources.First().OrganizationId); sutProvider.GetDependency() .AccessToSecretsAsync(Arg.Any>(), Arg.Any(), Arg.Any()) - .Returns(secretIds.ToDictionary(id => id, _ => (false, false))); + .Returns(secretIds.ToDictionary(id => id, _ => (false, false, false))); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, resources); @@ -119,8 +119,8 @@ public async Task Handler_HasAccessToSomeSecrets_DoesNotSucceed( var secretIds = SetupSecretAccessRequest(sutProvider, resources, accessClientType, resources.First().OrganizationId); - var accessResult = secretIds.ToDictionary(secretId => secretId, _ => (false, false)); - accessResult[secretIds.First()] = (true, true); + var accessResult = secretIds.ToDictionary(secretId => secretId, _ => (false, false, false)); + accessResult[secretIds.First()] = (true, true, false); sutProvider.GetDependency() .AccessToSecretsAsync(Arg.Any>(), Arg.Any(), Arg.Any()) .Returns(accessResult); @@ -147,7 +147,7 @@ public async Task Handler_PartialAccessReturn_DoesNotSucceed( var secretIds = SetupSecretAccessRequest(sutProvider, resources, accessClientType, resources.First().OrganizationId); - var accessResult = secretIds.ToDictionary(secretId => secretId, _ => (false, false)); + var accessResult = secretIds.ToDictionary(secretId => secretId, _ => (false, false, false)); accessResult.Remove(secretIds.First()); sutProvider.GetDependency() .AccessToSecretsAsync(Arg.Any>(), Arg.Any(), Arg.Any()) @@ -175,7 +175,7 @@ public async Task Handler_HasAccessToAllSecrets_Success( var secretIds = SetupSecretAccessRequest(sutProvider, resources, accessClientType, resources.First().OrganizationId); - var accessResult = secretIds.ToDictionary(secretId => secretId, _ => (true, true)); + var accessResult = secretIds.ToDictionary(secretId => secretId, _ => (true, true, false)); sutProvider.GetDependency() .AccessToSecretsAsync(Arg.Any>(), Arg.Any(), Arg.Any()) .Returns(accessResult); 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 feaaf2634efa..df361e946e52 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 @@ -118,7 +118,7 @@ public async Task CanCreateSecret_NotSupportedClientTypes_DoesNotSucceed(AccessC SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId, clientType); sutProvider.GetDependency() .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any()).Returns( - (true, true)); + (true, true, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); @@ -196,7 +196,7 @@ public async Task CanCreateSecret_DoesNotSucceed(PermissionType permissionType, SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); sutProvider.GetDependency() .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any()).ReturnsForAnyArgs( - (read, write)); + (read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); @@ -223,7 +223,7 @@ public async Task CanCreateSecret_Success(PermissionType permissionType, bool re SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); sutProvider.GetDependency() .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any()).ReturnsForAnyArgs( - (read, write)); + (read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); @@ -286,7 +286,7 @@ public async Task CanReadSecret_AccessCheck(PermissionType permissionType, bool SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); sutProvider.GetDependency() .AccessToSecretAsync(secret.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); @@ -322,7 +322,7 @@ public async Task CanUpdateSecret_NotSupportedClientTypes_DoesNotSucceed(AccessC SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId, clientType); sutProvider.GetDependency() .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any()).Returns( - (true, true)); + (true, true, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); @@ -361,7 +361,7 @@ public async Task CanUpdateSecret_ClearProjectsUser_DoesNotSucceed( var requirement = SecretOperations.Update; SetupPermission(sutProvider, PermissionType.RunAsUserWithPermission, secret.OrganizationId, userId); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, Arg.Any(), Arg.Any()).Returns( - (true, true)); + (true, true, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); @@ -408,7 +408,7 @@ public async Task CanUpdateSecret_NoProjectChanges_ReturnsExpected(PermissionTyp SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); sutProvider.GetDependency() .AccessToSecretAsync(secret.Id, userId, Arg.Any()).Returns( - (read, write)); + (read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); @@ -445,10 +445,10 @@ public async Task CanUpdateSecret_DoesNotSucceed(PermissionType permissionType, var requirement = SecretOperations.Update; SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, Arg.Any()).Returns( - (read, write)); + (read, write, false)); sutProvider.GetDependency() .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any()).Returns( - (projectRead, projectWrite)); + (projectRead, projectWrite, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); @@ -474,10 +474,10 @@ public async Task CanUpdateSecret_Success(PermissionType permissionType, bool re var requirement = SecretOperations.Update; SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, Arg.Any()).Returns( - (read, write)); + (read, write, false)); sutProvider.GetDependency() .AccessToProjectAsync(secret.Projects!.FirstOrDefault()!.Id, userId, Arg.Any()).Returns( - (read, write)); + (read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); @@ -540,7 +540,7 @@ public async Task CanDeleteSecret_AccessCheck(PermissionType permissionType, boo SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); sutProvider.GetDependency() .AccessToSecretAsync(secret.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); @@ -621,7 +621,7 @@ public async Task CanReadAccessPolicies_AccessCheck(PermissionType permissionTyp SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); sutProvider.GetDependency() .AccessToSecretAsync(secret.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, write)); // manage mirrors write: handler requires Manage for ReadAccessPolicies var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, secret); diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandlerTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandlerTests.cs index eec69095d5c8..9423b6a0d357 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandlerTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandlerTests.cs @@ -193,7 +193,7 @@ public async Task CanUpdateServiceAccount_AccessCheck(PermissionType permissionT SetupPermission(sutProvider, permissionType, serviceAccount.OrganizationId, userId); sutProvider.GetDependency() .AccessToServiceAccountAsync(serviceAccount.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, serviceAccount); @@ -253,7 +253,7 @@ public async Task CanReadServiceAccount_AccessCheck(PermissionType permissionTyp SetupPermission(sutProvider, permissionType, serviceAccount.OrganizationId, userId); sutProvider.GetDependency() .AccessToServiceAccountAsync(serviceAccount.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, serviceAccount); @@ -312,7 +312,7 @@ public async Task CanCreateAccessToken_AccessCheck(PermissionType permissionType SetupPermission(sutProvider, permissionType, serviceAccount.OrganizationId, userId); sutProvider.GetDependency() .AccessToServiceAccountAsync(serviceAccount.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, serviceAccount); @@ -371,7 +371,7 @@ public async Task CanReadAccessTokens_AccessCheck(PermissionType permissionType, SetupPermission(sutProvider, permissionType, serviceAccount.OrganizationId, userId); sutProvider.GetDependency() .AccessToServiceAccountAsync(serviceAccount.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, serviceAccount); @@ -430,7 +430,7 @@ public async Task CanRevokeAccessTokens_AccessCheck(PermissionType permissionTyp SetupPermission(sutProvider, permissionType, serviceAccount.OrganizationId, userId); sutProvider.GetDependency() .AccessToServiceAccountAsync(serviceAccount.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, serviceAccount); @@ -489,7 +489,7 @@ public async Task CanDeleteProject_AccessCheck(PermissionType permissionType, bo SetupPermission(sutProvider, permissionType, serviceAccount.OrganizationId, userId); sutProvider.GetDependency() .AccessToServiceAccountAsync(serviceAccount.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, serviceAccount); @@ -548,7 +548,7 @@ public async Task CanReadEvents_AccessCheck(PermissionType permissionType, bool SetupPermission(sutProvider, permissionType, serviceAccount.OrganizationId, userId); sutProvider.GetDependency() .AccessToServiceAccountAsync(serviceAccount.Id, userId, Arg.Any()) - .Returns((read, write)); + .Returns((read, write, false)); var authzContext = new AuthorizationHandlerContext(new List { requirement }, claimsPrincipal, serviceAccount); diff --git a/src/Api/AssemblyInfo.cs b/src/Api/AssemblyInfo.cs new file mode 100644 index 000000000000..32019d43c0b8 --- /dev/null +++ b/src/Api/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("Api.Test")] diff --git a/src/Api/SecretsManager/Controllers/AccessPoliciesController.cs b/src/Api/SecretsManager/Controllers/AccessPoliciesController.cs index ad5d5e092b9a..cdf334c3323a 100644 --- a/src/Api/SecretsManager/Controllers/AccessPoliciesController.cs +++ b/src/Api/SecretsManager/Controllers/AccessPoliciesController.cs @@ -7,6 +7,9 @@ using Bit.Core.SecretsManager.AuthorizationRequirements; using Bit.Core.SecretsManager.Commands.AccessPolicies.Interfaces; using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Enums.AccessPolicies; +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.Repositories; @@ -14,6 +17,8 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; +#nullable enable + namespace Bit.Api.SecretsManager.Controllers; [Authorize("secrets")] @@ -34,6 +39,8 @@ private readonly IProjectServiceAccountsAccessPoliciesUpdatesQuery _projectServiceAccountsAccessPoliciesUpdatesQuery; private readonly IUpdateProjectServiceAccountsAccessPoliciesCommand _updateProjectServiceAccountsAccessPoliciesCommand; + private readonly ISecretAccessPoliciesUpdatesQuery _secretAccessPoliciesUpdatesQuery; + private readonly IUpdateSecretAccessPoliciesCommand _updateSecretAccessPoliciesCommand; public AccessPoliciesController( @@ -49,6 +56,8 @@ public AccessPoliciesController( IProjectServiceAccountsAccessPoliciesUpdatesQuery projectServiceAccountsAccessPoliciesUpdatesQuery, IUpdateServiceAccountGrantedPoliciesCommand updateServiceAccountGrantedPoliciesCommand, IUpdateProjectServiceAccountsAccessPoliciesCommand updateProjectServiceAccountsAccessPoliciesCommand, + ISecretAccessPoliciesUpdatesQuery secretAccessPoliciesUpdatesQuery, + IUpdateSecretAccessPoliciesCommand updateSecretAccessPoliciesCommand, IEventService eventService) { _authorizationService = authorizationService; @@ -63,6 +72,8 @@ public AccessPoliciesController( _serviceAccountGrantedPolicyUpdatesQuery = serviceAccountGrantedPolicyUpdatesQuery; _projectServiceAccountsAccessPoliciesUpdatesQuery = projectServiceAccountsAccessPoliciesUpdatesQuery; _updateProjectServiceAccountsAccessPoliciesCommand = updateProjectServiceAccountsAccessPoliciesCommand; + _secretAccessPoliciesUpdatesQuery = secretAccessPoliciesUpdatesQuery; + _updateSecretAccessPoliciesCommand = updateSecretAccessPoliciesCommand; _eventService = eventService; } @@ -129,7 +140,7 @@ await _projectRepository.GetManyByOrganizationIdWriteAccessAsync(id, public async Task GetProjectPeopleAccessPoliciesAsync([FromRoute] Guid id) { var project = await _projectRepository.GetByIdAsync(id); - var (_, userId) = await CheckUserHasWriteAccessToProjectAsync(project); + var (_, userId) = await CheckUserHasManageAccessToProjectAsync(project); var results = await _accessPolicyRepository.GetPeoplePoliciesByGrantedProjectIdAsync(id, userId); return new ProjectPeopleAccessPoliciesResponseModel(results, userId); } @@ -154,7 +165,9 @@ public async Task PutProjectPeopleAcce } var userId = _userService.GetProperUserId(User)!.Value; + var currentPolicies = await _accessPolicyRepository.GetPeoplePoliciesByGrantedProjectIdAsync(id, userId) ?? []; var results = await _accessPolicyRepository.ReplaceProjectPeopleAsync(peopleAccessPolicies, userId); + await LogProjectPeopleAccessPolicyChangesAsync(currentPolicies, results, userId, project.OrganizationId); return new ProjectPeopleAccessPoliciesResponseModel(results, userId); } @@ -163,7 +176,7 @@ public async Task GetServiceAcc [FromRoute] Guid id) { var serviceAccount = await _serviceAccountRepository.GetByIdAsync(id); - var (_, userId) = await CheckUserHasWriteAccessToServiceAccountAsync(serviceAccount); + var (_, userId) = await CheckUserHasManageAccessToServiceAccountAsync(serviceAccount); var results = await _accessPolicyRepository.GetPeoplePoliciesByGrantedServiceAccountIdAsync(id, userId); return new ServiceAccountPeopleAccessPoliciesResponseModel(results, userId); } @@ -200,6 +213,11 @@ public async Task GetServiceAccountGrantedPoliciesAsync([FromRoute] Guid id) { var serviceAccount = await _serviceAccountRepository.GetByIdAsync(id); + if (serviceAccount == null) + { + throw new NotFoundException(); + } + var authorizationResult = await _authorizationService.AuthorizeAsync(User, serviceAccount, ServiceAccountOperations.Update); @@ -228,7 +246,9 @@ public async Task throw new NotFoundException(); } + var userId = _userService.GetProperUserId(User)!.Value; await _updateServiceAccountGrantedPoliciesCommand.UpdateAsync(grantedPoliciesUpdates); + await LogServiceAccountGrantedPolicyChangesAsync(grantedPoliciesUpdates, userId); return await GetServiceAccountGrantedPoliciesAsync(serviceAccount); } @@ -238,9 +258,10 @@ public async Task [FromRoute] Guid id) { var project = await _projectRepository.GetByIdAsync(id); - await CheckUserHasWriteAccessToProjectAsync(project); + await CheckUserHasManageAccessToProjectAsync(project); var results = - await _accessPolicyRepository.GetProjectServiceAccountsAccessPoliciesAsync(id); + await _accessPolicyRepository.GetProjectServiceAccountsAccessPoliciesAsync(id) + ?? new ProjectServiceAccountsAccessPolicies(); return new ProjectServiceAccountsAccessPoliciesResponseModel(results); } @@ -261,9 +282,12 @@ await _projectServiceAccountsAccessPoliciesUpdatesQuery.GetAsync( throw new NotFoundException(); } + var userId = _userService.GetProperUserId(User)!.Value; await _updateProjectServiceAccountsAccessPoliciesCommand.UpdateAsync(accessPoliciesUpdates); + await LogProjectServiceAccountAccessPolicyChangesAsync(accessPoliciesUpdates, userId); - var results = await _accessPolicyRepository.GetProjectServiceAccountsAccessPoliciesAsync(id); + var results = await _accessPolicyRepository.GetProjectServiceAccountsAccessPoliciesAsync(id) + ?? new ProjectServiceAccountsAccessPolicies(); return new ProjectServiceAccountsAccessPoliciesResponseModel(results); } @@ -271,6 +295,11 @@ await _projectServiceAccountsAccessPoliciesUpdatesQuery.GetAsync( public async Task GetSecretAccessPoliciesAsync(Guid secretId) { var secret = await _secretRepository.GetByIdAsync(secretId); + if (secret == null) + { + throw new NotFoundException(); + } + var authorizationResult = await _authorizationService.AuthorizeAsync(User, secret, SecretOperations.ReadAccessPolicies); if (!authorizationResult.Succeeded) @@ -279,11 +308,42 @@ public async Task GetSecretAccessPoliciesAsyn } var userId = _userService.GetProperUserId(User)!.Value; - var accessPolicies = await _accessPolicyRepository.GetSecretAccessPoliciesAsync(secretId, userId); + var accessPolicies = await _accessPolicyRepository.GetSecretAccessPoliciesAsync(secretId, userId) + ?? new SecretAccessPolicies { SecretId = secretId, OrganizationId = secret!.OrganizationId }; return new SecretAccessPoliciesResponseModel(accessPolicies, userId); } - private async Task<(AccessClientType AccessClientType, Guid UserId)> CheckUserHasWriteAccessToProjectAsync( + [HttpPut("/secrets/{secretId}/access-policies")] + public async Task PutSecretAccessPoliciesAsync( + [FromRoute] Guid secretId, + [FromBody] SecretAccessPoliciesRequestsModel request) + { + var secret = await _secretRepository.GetByIdAsync(secretId); + if (secret == null) + { + throw new NotFoundException(); + } + + var (_, userId) = await _accessClientQuery.GetAccessClientAsync(User, secret.OrganizationId); + var accessPoliciesUpdates = await _secretAccessPoliciesUpdatesQuery.GetAsync( + request.ToSecretAccessPolicies(secretId, secret.OrganizationId), userId); + + var authorizationResult = await _authorizationService.AuthorizeAsync( + User, accessPoliciesUpdates, SecretAccessPoliciesOperations.Updates); + if (!authorizationResult.Succeeded) + { + throw new NotFoundException(); + } + + await _updateSecretAccessPoliciesCommand.UpdateAsync(accessPoliciesUpdates); + await LogSecretAccessPolicyChangesAsync(accessPoliciesUpdates, userId); + + var results = await _accessPolicyRepository.GetSecretAccessPoliciesAsync(secretId, userId) + ?? new SecretAccessPolicies { SecretId = secretId, OrganizationId = secret.OrganizationId }; + return new SecretAccessPoliciesResponseModel(results, userId); + } + + private async Task<(AccessClientType AccessClientType, Guid UserId)> CheckUserHasManageAccessToProjectAsync( Project project) { if (project == null) @@ -299,7 +359,7 @@ public async Task GetSecretAccessPoliciesAsyn var (accessClient, userId) = await _accessClientQuery.GetAccessClientAsync(User, project.OrganizationId); var access = await _projectRepository.AccessToProjectAsync(project.Id, userId, accessClient); - if (!access.Write || accessClient == AccessClientType.ServiceAccount) + if (!access.Manage) { throw new NotFoundException(); } @@ -307,7 +367,7 @@ public async Task GetSecretAccessPoliciesAsyn return (accessClient, userId); } - private async Task<(AccessClientType AccessClientType, Guid UserId)> CheckUserHasWriteAccessToServiceAccountAsync( + private async Task<(AccessClientType AccessClientType, Guid UserId)> CheckUserHasManageAccessToServiceAccountAsync( ServiceAccount serviceAccount) { if (serviceAccount == null) @@ -324,7 +384,7 @@ public async Task GetSecretAccessPoliciesAsyn var access = await _serviceAccountRepository.AccessToServiceAccountAsync(serviceAccount.Id, userId, accessClient); - if (!access.Write) + if (!access.Manage) { throw new NotFoundException(); } @@ -342,7 +402,7 @@ await _accessPolicyRepository.GetServiceAccountGrantedPoliciesPermissionDetailsA return new ServiceAccountGrantedPoliciesPermissionDetailsResponseModel(results); } - public async Task LogAccessPolicyServiceAccountChanges(IEnumerable currentPolicies, IEnumerable updatedPolicies, Guid userId) + internal async Task LogAccessPolicyServiceAccountChanges(IEnumerable currentPolicies, IEnumerable updatedPolicies, Guid userId) { foreach (var current in currentPolicies.OfType()) { @@ -375,5 +435,180 @@ public async Task LogAccessPolicyServiceAccountChanges(IEnumerable()) + { + var existing = currentPolicies.OfType() + .FirstOrDefault(p => p.OrganizationUserId == policy.OrganizationUserId); + if (existing != null && PermissionsChanged(existing, policy)) + { + await _eventService.LogServiceAccountPeopleEventAsync(userId, policy, + EventType.ServiceAccount_UserPermissionUpdated, _currentContext.IdentityClientType); + } + } + + foreach (var policy in updatedPolicies.OfType()) + { + var existing = currentPolicies.OfType() + .FirstOrDefault(p => p.GroupId == policy.GroupId); + if (existing != null && PermissionsChanged(existing, policy)) + { + await _eventService.LogServiceAccountGroupEventAsync(userId, policy, + EventType.ServiceAccount_GroupPermissionUpdated, _currentContext.IdentityClientType); + } + } + } + + private async Task LogProjectPeopleAccessPolicyChangesAsync( + IEnumerable before, + IEnumerable after, + Guid userId, + Guid organizationId) + { + foreach (var policy in after.OfType()) + { + var existing = before.OfType() + .FirstOrDefault(p => p.OrganizationUserId == policy.OrganizationUserId); + var type = existing is null ? EventType.Project_UserAccessGranted + : PermissionsChanged(existing, policy) ? EventType.Project_UserAccessUpdated + : (EventType?)null; + if (type.HasValue) + { + await _eventService.LogProjectAccessPolicyEventAsync(userId, organizationId, policy, type.Value, + _currentContext.IdentityClientType); + } + } + + foreach (var policy in before.OfType()) + { + if (!after.OfType().Any(p => p.OrganizationUserId == policy.OrganizationUserId)) + { + await _eventService.LogProjectAccessPolicyEventAsync(userId, organizationId, policy, + EventType.Project_UserAccessRevoked, _currentContext.IdentityClientType); + } + } + + foreach (var policy in after.OfType()) + { + var existing = before.OfType() + .FirstOrDefault(p => p.GroupId == policy.GroupId); + var type = existing is null ? EventType.Project_GroupAccessGranted + : PermissionsChanged(existing, policy) ? EventType.Project_GroupAccessUpdated + : (EventType?)null; + if (type.HasValue) + { + await _eventService.LogProjectAccessPolicyEventAsync(userId, organizationId, policy, type.Value, + _currentContext.IdentityClientType); + } + } + + foreach (var policy in before.OfType()) + { + if (!after.OfType().Any(p => p.GroupId == policy.GroupId)) + { + await _eventService.LogProjectAccessPolicyEventAsync(userId, organizationId, policy, + EventType.Project_GroupAccessRevoked, _currentContext.IdentityClientType); + } + } } + + private async Task LogProjectServiceAccountAccessPolicyChangesAsync( + ProjectServiceAccountsAccessPoliciesUpdates? updates, + Guid userId) + { + if (updates is null) return; + foreach (var update in updates.ServiceAccountAccessPolicyUpdates) + { + var type = update.Operation switch + { + AccessPolicyOperation.Create => EventType.Project_ServiceAccountAccessGranted, + AccessPolicyOperation.Update => EventType.Project_ServiceAccountAccessUpdated, + AccessPolicyOperation.Delete => EventType.Project_ServiceAccountAccessRevoked, + _ => (EventType?)null + }; + if (type.HasValue) + { + await _eventService.LogProjectAccessPolicyEventAsync(userId, updates.OrganizationId, + update.AccessPolicy, type.Value, _currentContext.IdentityClientType); + } + } + } + + private async Task LogSecretAccessPolicyChangesAsync( + SecretAccessPoliciesUpdates updates, + Guid userId) + { + foreach (var update in updates.UserAccessPolicyUpdates) + { + var type = update.Operation switch + { + AccessPolicyOperation.Create => EventType.Secret_UserAccessGranted, + AccessPolicyOperation.Update => EventType.Secret_UserAccessUpdated, + AccessPolicyOperation.Delete => EventType.Secret_UserAccessRevoked, + _ => (EventType?)null + }; + if (type.HasValue) + { + await _eventService.LogSecretAccessPolicyEventAsync(userId, updates.OrganizationId, + update.AccessPolicy, type.Value, _currentContext.IdentityClientType); + } + } + + foreach (var update in updates.GroupAccessPolicyUpdates) + { + var type = update.Operation switch + { + AccessPolicyOperation.Create => EventType.Secret_GroupAccessGranted, + AccessPolicyOperation.Update => EventType.Secret_GroupAccessUpdated, + AccessPolicyOperation.Delete => EventType.Secret_GroupAccessRevoked, + _ => (EventType?)null + }; + if (type.HasValue) + { + await _eventService.LogSecretAccessPolicyEventAsync(userId, updates.OrganizationId, + update.AccessPolicy, type.Value, _currentContext.IdentityClientType); + } + } + + foreach (var update in updates.ServiceAccountAccessPolicyUpdates) + { + var type = update.Operation switch + { + AccessPolicyOperation.Create => EventType.Secret_ServiceAccountAccessGranted, + AccessPolicyOperation.Update => EventType.Secret_ServiceAccountAccessUpdated, + AccessPolicyOperation.Delete => EventType.Secret_ServiceAccountAccessRevoked, + _ => (EventType?)null + }; + if (type.HasValue) + { + await _eventService.LogSecretAccessPolicyEventAsync(userId, updates.OrganizationId, + update.AccessPolicy, type.Value, _currentContext.IdentityClientType); + } + } + } + + private async Task LogServiceAccountGrantedPolicyChangesAsync( + ServiceAccountGrantedPoliciesUpdates? updates, + Guid userId) + { + if (updates is null) return; + foreach (var update in updates.ProjectGrantedPolicyUpdates) + { + var type = update.Operation switch + { + AccessPolicyOperation.Create => EventType.Project_ServiceAccountAccessGranted, + AccessPolicyOperation.Update => EventType.Project_ServiceAccountAccessUpdated, + AccessPolicyOperation.Delete => EventType.Project_ServiceAccountAccessRevoked, + _ => (EventType?)null + }; + if (type.HasValue) + { + await _eventService.LogProjectAccessPolicyEventAsync(userId, updates.OrganizationId, + update.AccessPolicy, type.Value, _currentContext.IdentityClientType); + } + } + } + + private static bool PermissionsChanged(BaseAccessPolicy before, BaseAccessPolicy after) => + before.Read != after.Read || before.Write != after.Write || before.Manage != after.Manage; } diff --git a/src/Api/SecretsManager/Models/Request/AccessPolicyRequest.cs b/src/Api/SecretsManager/Models/Request/AccessPolicyRequest.cs index 75e944e3a6b3..16a0f4caea11 100644 --- a/src/Api/SecretsManager/Models/Request/AccessPolicyRequest.cs +++ b/src/Api/SecretsManager/Models/Request/AccessPolicyRequest.cs @@ -4,7 +4,7 @@ namespace Bit.Api.SecretsManager.Models.Request; -public class AccessPolicyRequest +public class AccessPolicyRequest : IValidatableObject { [Required] public Guid GranteeId { get; set; } @@ -15,6 +15,19 @@ public class AccessPolicyRequest [Required] public bool Write { get; set; } + [Required] + public bool Manage { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + if (Manage && !Write) + { + yield return new ValidationResult( + "Write must be true when Manage is true.", + [nameof(Write), nameof(Manage)]); + } + } + public UserProjectAccessPolicy ToUserProjectAccessPolicy(Guid projectId, Guid organizationId) => new() { @@ -22,7 +35,8 @@ public UserProjectAccessPolicy ToUserProjectAccessPolicy(Guid projectId, Guid or GrantedProjectId = projectId, GrantedProject = new Project { OrganizationId = organizationId, Id = projectId }, Read = Read, - Write = Write + Write = Write, + Manage = Manage }; public UserSecretAccessPolicy ToUserSecretAccessPolicy(Guid secretId, Guid organizationId) => @@ -32,7 +46,8 @@ public UserSecretAccessPolicy ToUserSecretAccessPolicy(Guid secretId, Guid organ GrantedSecretId = secretId, GrantedSecret = new Secret { OrganizationId = organizationId, Id = secretId }, Read = Read, - Write = Write + Write = Write, + Manage = Manage }; public GroupProjectAccessPolicy ToGroupProjectAccessPolicy(Guid projectId, Guid organizationId) => @@ -42,7 +57,8 @@ public GroupProjectAccessPolicy ToGroupProjectAccessPolicy(Guid projectId, Guid GrantedProjectId = projectId, GrantedProject = new Project { OrganizationId = organizationId, Id = projectId }, Read = Read, - Write = Write + Write = Write, + Manage = Manage }; public GroupSecretAccessPolicy ToGroupSecretAccessPolicy(Guid secretId, Guid organizationId) => @@ -52,7 +68,8 @@ public GroupSecretAccessPolicy ToGroupSecretAccessPolicy(Guid secretId, Guid org GrantedSecretId = secretId, GrantedSecret = new Secret { OrganizationId = organizationId, Id = secretId }, Read = Read, - Write = Write + Write = Write, + Manage = Manage }; public ServiceAccountProjectAccessPolicy ToServiceAccountProjectAccessPolicy(Guid projectId, Guid organizationId) => @@ -62,7 +79,8 @@ public ServiceAccountProjectAccessPolicy ToServiceAccountProjectAccessPolicy(Gui GrantedProjectId = projectId, GrantedProject = new Project { OrganizationId = organizationId, Id = projectId }, Read = Read, - Write = Write + Write = Write, + Manage = Manage }; public ServiceAccountSecretAccessPolicy ToServiceAccountSecretAccessPolicy(Guid secretId, Guid organizationId) => @@ -72,7 +90,8 @@ public ServiceAccountSecretAccessPolicy ToServiceAccountSecretAccessPolicy(Guid GrantedSecretId = secretId, GrantedSecret = new Secret { OrganizationId = organizationId, Id = secretId }, Read = Read, - Write = Write + Write = Write, + Manage = Manage }; public UserServiceAccountAccessPolicy ToUserServiceAccountAccessPolicy(Guid id, Guid organizationId) => @@ -82,7 +101,8 @@ public UserServiceAccountAccessPolicy ToUserServiceAccountAccessPolicy(Guid id, GrantedServiceAccountId = id, GrantedServiceAccount = new ServiceAccount() { OrganizationId = organizationId, Id = id }, Read = Read, - Write = Write + Write = Write, + Manage = Manage }; public GroupServiceAccountAccessPolicy ToGroupServiceAccountAccessPolicy(Guid id, Guid organizationId) => @@ -92,6 +112,7 @@ public GroupServiceAccountAccessPolicy ToGroupServiceAccountAccessPolicy(Guid id GrantedServiceAccountId = id, GrantedServiceAccount = new ServiceAccount() { OrganizationId = organizationId, Id = id }, Read = Read, - Write = Write + Write = Write, + Manage = Manage }; } diff --git a/src/Api/SecretsManager/Models/Request/GrantedAccessPolicyRequest.cs b/src/Api/SecretsManager/Models/Request/GrantedAccessPolicyRequest.cs index 763768608fb3..64d4dcd7be64 100644 --- a/src/Api/SecretsManager/Models/Request/GrantedAccessPolicyRequest.cs +++ b/src/Api/SecretsManager/Models/Request/GrantedAccessPolicyRequest.cs @@ -15,6 +15,9 @@ public class GrantedAccessPolicyRequest [Required] public bool Write { get; set; } + [Required] + public bool Manage { get; set; } + public ServiceAccountProjectAccessPolicy ToServiceAccountProjectAccessPolicy(Guid serviceAccountId, Guid organizationId) => new() { @@ -23,5 +26,6 @@ public ServiceAccountProjectAccessPolicy ToServiceAccountProjectAccessPolicy(Gui GrantedProjectId = GrantedId, Read = Read, Write = Write, + Manage = Manage, }; } diff --git a/src/Api/SecretsManager/Models/Response/AccessPolicyResponseModel.cs b/src/Api/SecretsManager/Models/Response/AccessPolicyResponseModel.cs index 5a795fdd4382..39734dc3bc94 100644 --- a/src/Api/SecretsManager/Models/Response/AccessPolicyResponseModel.cs +++ b/src/Api/SecretsManager/Models/Response/AccessPolicyResponseModel.cs @@ -11,10 +11,12 @@ protected BaseAccessPolicyResponseModel(BaseAccessPolicy baseAccessPolicy, strin { Read = baseAccessPolicy.Read; Write = baseAccessPolicy.Write; + Manage = baseAccessPolicy.Manage; } public bool Read { get; set; } public bool Write { get; set; } + public bool Manage { get; set; } protected static string? GetUserDisplayName(User? user) { diff --git a/src/Api/SecretsManager/Utilities/AccessPolicyHelpers.cs b/src/Api/SecretsManager/Utilities/AccessPolicyHelpers.cs index 9eec6b688ca1..c20b4a95a762 100644 --- a/src/Api/SecretsManager/Utilities/AccessPolicyHelpers.cs +++ b/src/Api/SecretsManager/Utilities/AccessPolicyHelpers.cs @@ -35,10 +35,16 @@ public static void CheckForDistinctAccessPolicies(IReadOnlyCollection accessPolicies) { - var accessPoliciesPermission = accessPolicies.All(policy => policy.Read); - if (!accessPoliciesPermission) + foreach (var policy in accessPolicies) { - throw new BadRequestException("Resources must be Read = true"); + if (!policy.Read) + { + throw new BadRequestException("Resources must be Read = true"); + } + if (policy.Manage && !policy.Write) + { + throw new BadRequestException("Manage permission requires Write permission."); + } } } } diff --git a/src/Core/AdminConsole/Services/IEventService.cs b/src/Core/AdminConsole/Services/IEventService.cs index 795c06e25454..6799a0910b37 100644 --- a/src/Core/AdminConsole/Services/IEventService.cs +++ b/src/Core/AdminConsole/Services/IEventService.cs @@ -41,4 +41,6 @@ public interface IEventService Task LogServiceAccountPeopleEventAsync(Guid userId, UserServiceAccountAccessPolicy policy, EventType type, IdentityClientType identityClientType, DateTime? date = null); Task LogServiceAccountGroupEventAsync(Guid userId, GroupServiceAccountAccessPolicy policy, EventType type, IdentityClientType identityClientType, DateTime? date = null); Task LogServiceAccountEventAsync(Guid userId, List serviceAccount, EventType type, IdentityClientType identityClientType, DateTime? date = null); + Task LogProjectAccessPolicyEventAsync(Guid actingUserId, Guid organizationId, BaseAccessPolicy policy, EventType type, IdentityClientType identityClientType, DateTime? date = null); + Task LogSecretAccessPolicyEventAsync(Guid actingUserId, Guid organizationId, BaseAccessPolicy policy, EventType type, IdentityClientType identityClientType, DateTime? date = null); } diff --git a/src/Core/Dirt/Enums/EventType.cs b/src/Core/Dirt/Enums/EventType.cs index 61372fc4e081..a437f948d46c 100644 --- a/src/Core/Dirt/Enums/EventType.cs +++ b/src/Core/Dirt/Enums/EventType.cs @@ -108,11 +108,29 @@ public enum EventType : int Secret_Deleted = 2103, Secret_Permanently_Deleted = 2104, Secret_Restored = 2105, + Secret_UserAccessGranted = 2106, + Secret_UserAccessRevoked = 2107, + Secret_UserAccessUpdated = 2108, + Secret_GroupAccessGranted = 2109, + Secret_GroupAccessRevoked = 2110, + Secret_GroupAccessUpdated = 2111, + Secret_ServiceAccountAccessGranted = 2112, + Secret_ServiceAccountAccessRevoked = 2113, + Secret_ServiceAccountAccessUpdated = 2114, Project_Retrieved = 2200, Project_Created = 2201, Project_Edited = 2202, Project_Deleted = 2203, + Project_UserAccessGranted = 2204, + Project_UserAccessRevoked = 2205, + Project_UserAccessUpdated = 2206, + Project_GroupAccessGranted = 2207, + Project_GroupAccessRevoked = 2208, + Project_GroupAccessUpdated = 2209, + Project_ServiceAccountAccessGranted = 2210, + Project_ServiceAccountAccessRevoked = 2211, + Project_ServiceAccountAccessUpdated = 2212, ServiceAccount_UserAdded = 2300, ServiceAccount_UserRemoved = 2301, @@ -120,4 +138,6 @@ public enum EventType : int ServiceAccount_GroupRemoved = 2303, ServiceAccount_Created = 2304, ServiceAccount_Deleted = 2305, + ServiceAccount_UserPermissionUpdated = 2306, + ServiceAccount_GroupPermissionUpdated = 2307, } diff --git a/src/Core/Dirt/Services/Implementations/EventService.cs b/src/Core/Dirt/Services/Implementations/EventService.cs index 77d481890e2e..e43564e76c13 100644 --- a/src/Core/Dirt/Services/Implementations/EventService.cs +++ b/src/Core/Dirt/Services/Implementations/EventService.cs @@ -559,7 +559,7 @@ public async Task LogServiceAccountGroupEventAsync(Guid userId, GroupServiceAcco var orgAbilities = await _applicationCacheService.GetOrganizationAbilitiesAsync(); var eventMessages = new List(); - if (!CanUseEvents(orgAbilities, policy.Group.OrganizationId)) + if (policy.Group == null || !CanUseEvents(orgAbilities, policy.Group.OrganizationId)) { return; } @@ -629,6 +629,96 @@ public async Task LogServiceAccountEventAsync(Guid userId, List } } + public async Task LogProjectAccessPolicyEventAsync(Guid actingUserId, Guid organizationId, BaseAccessPolicy policy, + EventType type, IdentityClientType identityClientType, DateTime? date = null) + { + var orgAbilities = await _applicationCacheService.GetOrganizationAbilitiesAsync(); + if (!CanUseEvents(orgAbilities, organizationId)) + { + return; + } + + var (userId, serviceAccountId) = MapIdentityClientType(actingUserId, identityClientType); + if (userId is null && serviceAccountId is null) + { + return; + } + + var e = new EventMessage(_currentContext) + { + OrganizationId = organizationId, + Type = type, + ActingUserId = userId, + ServiceAccountId = serviceAccountId, + Date = date.GetValueOrDefault(DateTime.UtcNow) + }; + + switch (policy) + { + case UserProjectAccessPolicy p: + e.ProjectId = p.GrantedProjectId; + e.OrganizationUserId = p.OrganizationUserId; + break; + case GroupProjectAccessPolicy p: + e.ProjectId = p.GrantedProjectId; + e.GroupId = p.GroupId; + break; + case ServiceAccountProjectAccessPolicy p: + e.ProjectId = p.GrantedProjectId; + e.GrantedServiceAccountId = p.ServiceAccountId; + break; + default: + return; + } + + await _eventWriteService.CreateAsync(e); + } + + public async Task LogSecretAccessPolicyEventAsync(Guid actingUserId, Guid organizationId, BaseAccessPolicy policy, + EventType type, IdentityClientType identityClientType, DateTime? date = null) + { + var orgAbilities = await _applicationCacheService.GetOrganizationAbilitiesAsync(); + if (!CanUseEvents(orgAbilities, organizationId)) + { + return; + } + + var (userId, serviceAccountId) = MapIdentityClientType(actingUserId, identityClientType); + if (userId is null && serviceAccountId is null) + { + return; + } + + var e = new EventMessage(_currentContext) + { + OrganizationId = organizationId, + Type = type, + ActingUserId = userId, + ServiceAccountId = serviceAccountId, + Date = date.GetValueOrDefault(DateTime.UtcNow) + }; + + switch (policy) + { + case UserSecretAccessPolicy p: + e.SecretId = p.GrantedSecretId; + e.OrganizationUserId = p.OrganizationUserId; + break; + case GroupSecretAccessPolicy p: + e.SecretId = p.GrantedSecretId; + e.GroupId = p.GroupId; + break; + case ServiceAccountSecretAccessPolicy p: + e.SecretId = p.GrantedSecretId; + e.GrantedServiceAccountId = p.ServiceAccountId; + break; + default: + return; + } + + await _eventWriteService.CreateAsync(e); + } + private (Guid? actingUserId, Guid? serviceAccountId) MapIdentityClientType( Guid userId, IdentityClientType identityClientType) { diff --git a/src/Core/Dirt/Services/NoopImplementations/NoopEventService.cs b/src/Core/Dirt/Services/NoopImplementations/NoopEventService.cs index 6ecea7d234b1..eccb653fa7db 100644 --- a/src/Core/Dirt/Services/NoopImplementations/NoopEventService.cs +++ b/src/Core/Dirt/Services/NoopImplementations/NoopEventService.cs @@ -155,4 +155,14 @@ public Task LogServiceAccountEventAsync(Guid userId, List servic { return Task.FromResult(0); } + + public Task LogProjectAccessPolicyEventAsync(Guid actingUserId, Guid organizationId, BaseAccessPolicy policy, EventType type, IdentityClientType identityClientType, DateTime? date = null) + { + return Task.FromResult(0); + } + + public Task LogSecretAccessPolicyEventAsync(Guid actingUserId, Guid organizationId, BaseAccessPolicy policy, EventType type, IdentityClientType identityClientType, DateTime? date = null) + { + return Task.FromResult(0); + } } diff --git a/src/Core/SecretsManager/Commands/AccessPolicies/Interfaces/IUpdateSecretAccessPoliciesCommand.cs b/src/Core/SecretsManager/Commands/AccessPolicies/Interfaces/IUpdateSecretAccessPoliciesCommand.cs new file mode 100644 index 000000000000..10f90159a017 --- /dev/null +++ b/src/Core/SecretsManager/Commands/AccessPolicies/Interfaces/IUpdateSecretAccessPoliciesCommand.cs @@ -0,0 +1,9 @@ +#nullable enable +using Bit.Core.SecretsManager.Models.Data.AccessPolicyUpdates; + +namespace Bit.Core.SecretsManager.Commands.AccessPolicies.Interfaces; + +public interface IUpdateSecretAccessPoliciesCommand +{ + Task UpdateAsync(SecretAccessPoliciesUpdates accessPoliciesUpdates); +} diff --git a/src/Core/SecretsManager/Entities/AccessPolicy.cs b/src/Core/SecretsManager/Entities/AccessPolicy.cs index 1a96929e1e77..0f7da1d96f2a 100644 --- a/src/Core/SecretsManager/Entities/AccessPolicy.cs +++ b/src/Core/SecretsManager/Entities/AccessPolicy.cs @@ -12,6 +12,7 @@ public abstract class BaseAccessPolicy // Access public bool Read { get; set; } public bool Write { get; set; } + public bool Manage { get; set; } public DateTime CreationDate { get; set; } = DateTime.UtcNow; public DateTime RevisionDate { get; set; } = DateTime.UtcNow; diff --git a/src/Core/SecretsManager/Entities/Project.cs b/src/Core/SecretsManager/Entities/Project.cs index f843ce62c58d..b059ee22083f 100644 --- a/src/Core/SecretsManager/Entities/Project.cs +++ b/src/Core/SecretsManager/Entities/Project.cs @@ -18,6 +18,8 @@ public class Project : ITableObject public DateTime? DeletedDate { get; set; } + public Guid? CreatedByServiceAccountId { get; set; } + public virtual ICollection? Secrets { get; set; } public void SetNewId() diff --git a/src/Core/SecretsManager/Models/Data/ProjectPermissionDetails.cs b/src/Core/SecretsManager/Models/Data/ProjectPermissionDetails.cs index 7f847c816dae..088e64993f4b 100644 --- a/src/Core/SecretsManager/Models/Data/ProjectPermissionDetails.cs +++ b/src/Core/SecretsManager/Models/Data/ProjectPermissionDetails.cs @@ -10,4 +10,5 @@ public class ProjectPermissionDetails public Project Project { get; set; } public bool Read { get; set; } public bool Write { get; set; } + public bool Manage { get; set; } } diff --git a/src/Core/SecretsManager/Models/Data/SecretAccessPolicies.cs b/src/Core/SecretsManager/Models/Data/SecretAccessPolicies.cs index d3fb0cba29da..58aba2bf26d5 100644 --- a/src/Core/SecretsManager/Models/Data/SecretAccessPolicies.cs +++ b/src/Core/SecretsManager/Models/Data/SecretAccessPolicies.cs @@ -122,7 +122,7 @@ private List GetUserIdsToBeUpdated(IEnumerable req .Where(currentAp => requested.Any(requestedAp => requestedAp.GrantedSecretId == currentAp.GrantedSecretId && requestedAp.OrganizationUserId == currentAp.OrganizationUserId && - (requestedAp.Write != currentAp.Write || requestedAp.Read != currentAp.Read))) + (requestedAp.Write != currentAp.Write || requestedAp.Read != currentAp.Read || requestedAp.Manage != currentAp.Manage))) .Select(ap => ap.OrganizationUserId!.Value) .ToList(); @@ -131,7 +131,7 @@ private List GetGroupIdsToBeUpdated(IEnumerable r .Where(currentAp => requested.Any(requestedAp => requestedAp.GrantedSecretId == currentAp.GrantedSecretId && requestedAp.GroupId == currentAp.GroupId && - (requestedAp.Write != currentAp.Write || requestedAp.Read != currentAp.Read))) + (requestedAp.Write != currentAp.Write || requestedAp.Read != currentAp.Read || requestedAp.Manage != currentAp.Manage))) .Select(ap => ap.GroupId!.Value) .ToList(); @@ -140,7 +140,7 @@ private List GetServiceAccountIdsToBeUpdated(IEnumerable requested.Any(requestedAp => requestedAp.GrantedSecretId == currentAp.GrantedSecretId && requestedAp.ServiceAccountId == currentAp.ServiceAccountId && - (requestedAp.Write != currentAp.Write || requestedAp.Read != currentAp.Read))) + (requestedAp.Write != currentAp.Write || requestedAp.Read != currentAp.Read || requestedAp.Manage != currentAp.Manage))) .Select(ap => ap.ServiceAccountId!.Value) .ToList(); } diff --git a/src/Core/SecretsManager/Models/Data/SecretPermissionDetails.cs b/src/Core/SecretsManager/Models/Data/SecretPermissionDetails.cs index 232a96629a21..168e4ed416e9 100644 --- a/src/Core/SecretsManager/Models/Data/SecretPermissionDetails.cs +++ b/src/Core/SecretsManager/Models/Data/SecretPermissionDetails.cs @@ -10,4 +10,5 @@ public class SecretPermissionDetails public Secret Secret { get; set; } public bool Read { get; set; } public bool Write { get; set; } + public bool Manage { get; set; } } diff --git a/src/Core/SecretsManager/Models/Data/ServiceAccountGrantedPolicies.cs b/src/Core/SecretsManager/Models/Data/ServiceAccountGrantedPolicies.cs index a69208a0c747..361cc765850d 100644 --- a/src/Core/SecretsManager/Models/Data/ServiceAccountGrantedPolicies.cs +++ b/src/Core/SecretsManager/Models/Data/ServiceAccountGrantedPolicies.cs @@ -68,7 +68,7 @@ private List GetProjectIdsToBeUpdated(ServiceAccountGrantedPolicies reques .Where(currentAp => requested.ProjectGrantedPolicies.Any(requestedAp => requestedAp.GrantedProjectId == currentAp.GrantedProjectId && requestedAp.ServiceAccountId == currentAp.ServiceAccountId && - (requestedAp.Write != currentAp.Write || requestedAp.Read != currentAp.Read))) + (requestedAp.Write != currentAp.Write || requestedAp.Read != currentAp.Read || requestedAp.Manage != currentAp.Manage))) .Select(ap => ap.GrantedProjectId!.Value) .ToList(); } diff --git a/src/Core/SecretsManager/Repositories/IAccessPolicyRepository.cs b/src/Core/SecretsManager/Repositories/IAccessPolicyRepository.cs index af474d8e6e0f..96f526a71f77 100644 --- a/src/Core/SecretsManager/Repositories/IAccessPolicyRepository.cs +++ b/src/Core/SecretsManager/Repositories/IAccessPolicyRepository.cs @@ -21,4 +21,5 @@ public interface IAccessPolicyRepository Task GetProjectServiceAccountsAccessPoliciesAsync(Guid projectId); Task UpdateProjectServiceAccountsAccessPoliciesAsync(ProjectServiceAccountsAccessPoliciesUpdates updates); Task GetSecretAccessPoliciesAsync(Guid secretId, Guid userId); + Task UpdateSecretAccessPoliciesAsync(SecretAccessPoliciesUpdates accessPoliciesUpdates); } diff --git a/src/Core/SecretsManager/Repositories/IProjectRepository.cs b/src/Core/SecretsManager/Repositories/IProjectRepository.cs index 93dabacb4968..3c5f82179ac1 100644 --- a/src/Core/SecretsManager/Repositories/IProjectRepository.cs +++ b/src/Core/SecretsManager/Repositories/IProjectRepository.cs @@ -17,11 +17,13 @@ public interface IProjectRepository Task ReplaceAsync(Project project); Task DeleteManyByIdAsync(IEnumerable ids); Task> ImportAsync(IEnumerable projects); - Task<(bool Read, bool Write)> AccessToProjectAsync(Guid id, Guid userId, AccessClientType accessType); + Task<(bool Read, bool Write, bool Manage)> AccessToProjectAsync(Guid id, Guid userId, AccessClientType accessType); Task ProjectsAreInOrganization(List projectIds, Guid organizationId); Task GetProjectCountByOrganizationIdAsync(Guid organizationId); Task GetProjectCountByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType); Task GetProjectCountsByIdAsync(Guid projectId, Guid userId, AccessClientType accessType); - Task> AccessToProjectsAsync(IEnumerable projectIds, Guid userId, + Task> AccessToProjectsAsync(IEnumerable projectIds, Guid userId, AccessClientType accessType); + Task GetProjectCreatorServiceAccountIdAsync(Guid projectId); + Task IsServiceAccountCreatorOfAnyProjectForSecretAsync(Guid secretId, Guid serviceAccountId); } diff --git a/src/Core/SecretsManager/Repositories/ISecretRepository.cs b/src/Core/SecretsManager/Repositories/ISecretRepository.cs index d491bf79d377..58a9ddd90835 100644 --- a/src/Core/SecretsManager/Repositories/ISecretRepository.cs +++ b/src/Core/SecretsManager/Repositories/ISecretRepository.cs @@ -24,8 +24,8 @@ public interface ISecretRepository Task HardDeleteManyByIdAsync(IEnumerable ids); Task RestoreManyByIdAsync(IEnumerable ids); Task> ImportAsync(IEnumerable secrets); - Task<(bool Read, bool Write)> AccessToSecretAsync(Guid id, Guid userId, AccessClientType accessType); - Task> AccessToSecretsAsync(IEnumerable ids, Guid userId, AccessClientType accessType); + Task<(bool Read, bool Write, bool Manage)> AccessToSecretAsync(Guid id, Guid userId, AccessClientType accessType); + Task> AccessToSecretsAsync(IEnumerable ids, Guid userId, AccessClientType accessType); Task EmptyTrash(DateTime nowTime, uint deleteAfterThisNumberOfDays); Task GetSecretsCountByOrganizationIdAsync(Guid organizationId); Task GetSecretsCountByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType); diff --git a/src/Core/SecretsManager/Repositories/IServiceAccountRepository.cs b/src/Core/SecretsManager/Repositories/IServiceAccountRepository.cs index 26f01a473773..07a0b4ad18ea 100644 --- a/src/Core/SecretsManager/Repositories/IServiceAccountRepository.cs +++ b/src/Core/SecretsManager/Repositories/IServiceAccountRepository.cs @@ -16,8 +16,8 @@ public interface IServiceAccountRepository Task ReplaceAsync(ServiceAccount serviceAccount); Task DeleteManyByIdAsync(IEnumerable ids); Task> GetManyByOrganizationIdWriteAccessAsync(Guid organizationId, Guid userId, AccessClientType accessType); - Task<(bool Read, bool Write)> AccessToServiceAccountAsync(Guid id, Guid userId, AccessClientType accessType); - Task> AccessToServiceAccountsAsync(IEnumerable ids, Guid userId, + Task<(bool Read, bool Write, bool Manage)> AccessToServiceAccountAsync(Guid id, Guid userId, AccessClientType accessType); + Task> AccessToServiceAccountsAsync(IEnumerable ids, Guid userId, AccessClientType accessType); Task GetServiceAccountCountByOrganizationIdAsync(Guid organizationId); Task GetServiceAccountCountByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType); diff --git a/src/Core/SecretsManager/Repositories/Noop/NoopProjectRepository.cs b/src/Core/SecretsManager/Repositories/Noop/NoopProjectRepository.cs index 043230a00901..83ef6dab5425 100644 --- a/src/Core/SecretsManager/Repositories/Noop/NoopProjectRepository.cs +++ b/src/Core/SecretsManager/Repositories/Noop/NoopProjectRepository.cs @@ -51,9 +51,9 @@ public Task> ImportAsync(IEnumerable projects) return Task.FromResult(null as IEnumerable); } - public Task<(bool Read, bool Write)> AccessToProjectAsync(Guid id, Guid userId, AccessClientType accessType) + public Task<(bool Read, bool Write, bool Manage)> AccessToProjectAsync(Guid id, Guid userId, AccessClientType accessType) { - return Task.FromResult((false, false)); + return Task.FromResult((false, false, false)); } public Task ProjectsAreInOrganization(List projectIds, Guid organizationId) @@ -74,12 +74,22 @@ public Task GetProjectCountByOrganizationIdAsync(Guid organizationId, Guid public Task GetProjectCountsByIdAsync(Guid projectId, Guid userId, AccessClientType accessType) { - return Task.FromResult(null as ProjectCounts); + return Task.FromResult(new ProjectCounts()); } - public Task> AccessToProjectsAsync(IEnumerable projectIds, + public Task> AccessToProjectsAsync(IEnumerable projectIds, Guid userId, AccessClientType accessType) { - return Task.FromResult(null as Dictionary); + return Task.FromResult(new Dictionary()); + } + + public Task GetProjectCreatorServiceAccountIdAsync(Guid projectId) + { + return Task.FromResult(null as Guid?); + } + + public Task IsServiceAccountCreatorOfAnyProjectForSecretAsync(Guid secretId, Guid serviceAccountId) + { + return Task.FromResult(false); } } diff --git a/src/Core/SecretsManager/Repositories/Noop/NoopSecretRepository.cs b/src/Core/SecretsManager/Repositories/Noop/NoopSecretRepository.cs index b54187f8deb1..82cd0a659364 100644 --- a/src/Core/SecretsManager/Repositories/Noop/NoopSecretRepository.cs +++ b/src/Core/SecretsManager/Repositories/Noop/NoopSecretRepository.cs @@ -79,15 +79,15 @@ public Task> ImportAsync(IEnumerable secrets) return Task.FromResult(null as IEnumerable); } - public Task<(bool Read, bool Write)> AccessToSecretAsync(Guid id, Guid userId, AccessClientType accessType) + public Task<(bool Read, bool Write, bool Manage)> AccessToSecretAsync(Guid id, Guid userId, AccessClientType accessType) { - return Task.FromResult((false, false)); + return Task.FromResult((false, false, false)); } - public Task> AccessToSecretsAsync(IEnumerable ids, + public Task> AccessToSecretsAsync(IEnumerable ids, Guid userId, AccessClientType accessType) { - return Task.FromResult(null as Dictionary); + return Task.FromResult(null as Dictionary); } public Task EmptyTrash(DateTime nowTime, uint deleteAfterThisNumberOfDays) diff --git a/src/Core/SecretsManager/Repositories/Noop/NoopServiceAccountRepository.cs b/src/Core/SecretsManager/Repositories/Noop/NoopServiceAccountRepository.cs index 335ec4b2e327..8adda96a938c 100644 --- a/src/Core/SecretsManager/Repositories/Noop/NoopServiceAccountRepository.cs +++ b/src/Core/SecretsManager/Repositories/Noop/NoopServiceAccountRepository.cs @@ -39,27 +39,20 @@ public Task DeleteManyByIdAsync(IEnumerable ids) return Task.FromResult(0); } - public Task UserHasReadAccessToServiceAccount(Guid id, Guid userId) + public Task> GetManyByOrganizationIdWriteAccessAsync(Guid organizationId, Guid userId, AccessClientType accessType) { - return Task.FromResult(false); - } - - public Task UserHasWriteAccessToServiceAccount(Guid id, Guid userId) - { - return Task.FromResult(false); + return Task.FromResult(Enumerable.Empty()); } - public Task> GetManyByOrganizationIdWriteAccessAsync(Guid organizationId, Guid userId, AccessClientType accessType) => throw new NotImplementedException(); - - public Task<(bool Read, bool Write)> AccessToServiceAccountAsync(Guid id, Guid userId, AccessClientType accessType) + public Task<(bool Read, bool Write, bool Manage)> AccessToServiceAccountAsync(Guid id, Guid userId, AccessClientType accessType) { - return Task.FromResult((false, false)); + return Task.FromResult((false, false, false)); } - public Task> AccessToServiceAccountsAsync(IEnumerable ids, + public Task> AccessToServiceAccountsAsync(IEnumerable ids, Guid userId, AccessClientType accessType) { - return Task.FromResult(null as Dictionary); + return Task.FromResult(new Dictionary()); } public Task GetServiceAccountCountByOrganizationIdAsync(Guid organizationId) diff --git a/src/Infrastructure.EntityFramework/SecretsManager/Configurations/AccessPolicyEntityTypeConfiguration.cs b/src/Infrastructure.EntityFramework/SecretsManager/Configurations/AccessPolicyEntityTypeConfiguration.cs index 435d809e35c7..af8ed41c991d 100644 --- a/src/Infrastructure.EntityFramework/SecretsManager/Configurations/AccessPolicyEntityTypeConfiguration.cs +++ b/src/Infrastructure.EntityFramework/SecretsManager/Configurations/AccessPolicyEntityTypeConfiguration.cs @@ -29,6 +29,8 @@ public void Configure(EntityTypeBuilder builder) .IsClustered(); builder.ToTable(nameof(AccessPolicy)); + builder.HasCheckConstraint("CK_AccessPolicy_ManageImpliesWrite", "[Manage] = 0 OR [Write] = 1"); + builder.HasCheckConstraint("CK_AccessPolicy_WriteImpliesRead", "[Write] = 0 OR [Read] = 1"); } } diff --git a/src/Infrastructure.EntityFramework/SecretsManager/Configurations/ProjectEntityTypeConfiguration.cs b/src/Infrastructure.EntityFramework/SecretsManager/Configurations/ProjectEntityTypeConfiguration.cs index 69d44d1c1b2c..cdddc55d68c7 100644 --- a/src/Infrastructure.EntityFramework/SecretsManager/Configurations/ProjectEntityTypeConfiguration.cs +++ b/src/Infrastructure.EntityFramework/SecretsManager/Configurations/ProjectEntityTypeConfiguration.cs @@ -24,6 +24,17 @@ public void Configure(EntityTypeBuilder builder) .HasIndex(s => s.OrganizationId) .IsClustered(false); + builder + .HasIndex(s => s.CreatedByServiceAccountId) + .IsClustered(false); + + builder + .HasOne(p => p.CreatedByServiceAccount) + .WithMany() + .HasForeignKey(p => p.CreatedByServiceAccountId) + .IsRequired(false) + .OnDelete(DeleteBehavior.SetNull); + builder.ToTable(nameof(Project)); } } diff --git a/src/Infrastructure.EntityFramework/SecretsManager/Models/Project.cs b/src/Infrastructure.EntityFramework/SecretsManager/Models/Project.cs index 05035b2f375b..627c8f822c81 100644 --- a/src/Infrastructure.EntityFramework/SecretsManager/Models/Project.cs +++ b/src/Infrastructure.EntityFramework/SecretsManager/Models/Project.cs @@ -13,6 +13,7 @@ public class Project : Core.SecretsManager.Entities.Project public virtual ICollection GroupAccessPolicies { get; set; } public virtual ICollection UserAccessPolicies { get; set; } public virtual ICollection ServiceAccountAccessPolicies { get; set; } + public virtual ServiceAccount CreatedByServiceAccount { get; set; } } public class ProjectMapperProfile : Profile diff --git a/test/Api.IntegrationTest/SecretsManager/Controllers/AccessPoliciesControllerTests.cs b/test/Api.IntegrationTest/SecretsManager/Controllers/AccessPoliciesControllerTests.cs index 77614574c178..d554ae7ee5ff 100644 --- a/test/Api.IntegrationTest/SecretsManager/Controllers/AccessPoliciesControllerTests.cs +++ b/test/Api.IntegrationTest/SecretsManager/Controllers/AccessPoliciesControllerTests.cs @@ -1121,6 +1121,277 @@ public async Task GetSecretAccessPoliciesAsync_Success(PermissionType permission Assert.Equal(currentOrgUser.Id, result.UserAccessPolicies.First().OrganizationUserId); } + [Fact] + public async Task PutProjectPeopleAccessPolicies_RemoveLastManageGrant_ReturnsBadRequest() + { + var (org, _) = await _organizationHelper.Initialize(true, true, true); + await _loginHelper.LoginAsync(_email); + + var project = await _projectRepository.CreateAsync(new Project + { + OrganizationId = org.Id, + Name = _mockEncryptedString + }); + + var (_, orgUser) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); + + await _accessPolicyRepository.CreateManyAsync( + [ + new UserProjectAccessPolicy + { + GrantedProjectId = project.Id, + OrganizationUserId = orgUser.Id, + Read = true, + Write = true, + Manage = true + } + ]); + + var request = new PeopleAccessPoliciesRequestModel + { + UserAccessPolicyRequests = + [ + new AccessPolicyRequest { GranteeId = orgUser.Id, Read = true, Write = true, Manage = false } + ] + }; + + var response = await _client.PutAsJsonAsync($"/projects/{project.Id}/access-policies/people", request); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task PutProjectPeopleAccessPolicies_RemoveSecondToLastManageGrant_ReturnsSuccess() + { + var (org, _) = await _organizationHelper.Initialize(true, true, true); + await _loginHelper.LoginAsync(_email); + + var project = await _projectRepository.CreateAsync(new Project + { + OrganizationId = org.Id, + Name = _mockEncryptedString + }); + + var (_, orgUserA) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); + var (_, orgUserB) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); + + await _accessPolicyRepository.CreateManyAsync( + [ + new UserProjectAccessPolicy + { + GrantedProjectId = project.Id, + OrganizationUserId = orgUserA.Id, + Read = true, + Write = true, + Manage = true + }, + new UserProjectAccessPolicy + { + GrantedProjectId = project.Id, + OrganizationUserId = orgUserB.Id, + Read = true, + Write = true, + Manage = true + } + ]); + + var request = new PeopleAccessPoliciesRequestModel + { + UserAccessPolicyRequests = + [ + new AccessPolicyRequest { GranteeId = orgUserA.Id, Read = true, Write = true, Manage = true } + ] + }; + + var response = await _client.PutAsJsonAsync($"/projects/{project.Id}/access-policies/people", request); + + response.EnsureSuccessStatusCode(); + } + + [Fact] + public async Task PutProjectPeopleAccessPolicies_ManageOmittedInRequest_SetsManageToFalse() + { + var (_, organizationUser) = await _organizationHelper.Initialize(true, true, true); + await _loginHelper.LoginAsync(_email); + + var project = await _projectRepository.CreateAsync(new Project + { + OrganizationId = organizationUser.OrganizationId, + Name = _mockEncryptedString + }); + + var (_, orgUserA) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); + var (_, orgUserB) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); + + await _accessPolicyRepository.CreateManyAsync( + [ + new UserProjectAccessPolicy + { + GrantedProjectId = project.Id, + OrganizationUserId = orgUserA.Id, + Read = true, + Write = true, + Manage = true + }, + new UserProjectAccessPolicy + { + GrantedProjectId = project.Id, + OrganizationUserId = orgUserB.Id, + Read = true, + Write = true, + Manage = false + } + ]); + + // Manage field intentionally omitted — should default to false per bool binding design + var requestBody = $@"{{""userAccessPolicyRequests"":[{{""granteeId"":""{orgUserA.Id}"",""read"":true,""write"":true,""manage"":true}},{{""granteeId"":""{orgUserB.Id}"",""read"":true,""write"":true}}]}}"; + var content = new StringContent(requestBody, System.Text.Encoding.UTF8, "application/json"); + + var response = await _client.PutAsync($"/projects/{project.Id}/access-policies/people", content); + + response.EnsureSuccessStatusCode(); + + var result = await response.Content.ReadFromJsonAsync(); + var orgUserBPolicy = result?.UserAccessPolicies.FirstOrDefault(p => p.OrganizationUserId == orgUserB.Id); + Assert.NotNull(orgUserBPolicy); + Assert.False(orgUserBPolicy.Manage, "Absent manage field must default to false — documented bool binding behavior"); + } + + [Fact] + public async Task GetThenPutProjectPeopleAccessPolicies_ManageTruePreserved() + { + var (_, organizationUser) = await _organizationHelper.Initialize(true, true, true); + await _loginHelper.LoginAsync(_email); + + var project = await _projectRepository.CreateAsync(new Project + { + OrganizationId = organizationUser.OrganizationId, + Name = _mockEncryptedString + }); + + var (_, orgUser) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); + + await _accessPolicyRepository.CreateManyAsync( + [ + new UserProjectAccessPolicy + { + GrantedProjectId = project.Id, + OrganizationUserId = orgUser.Id, + Read = true, + Write = true, + Manage = true + } + ]); + + var getResponse = await _client.GetAsync($"/projects/{project.Id}/access-policies/people"); + getResponse.EnsureSuccessStatusCode(); + var getResult = await getResponse.Content.ReadFromJsonAsync(); + Assert.NotNull(getResult); + Assert.True(getResult.UserAccessPolicies.First().Manage, "GET response must include manage:true"); + + var putRequest = new PeopleAccessPoliciesRequestModel + { + UserAccessPolicyRequests = + [ + new AccessPolicyRequest + { + GranteeId = orgUser.Id, + Read = getResult.UserAccessPolicies.First().Read, + Write = getResult.UserAccessPolicies.First().Write, + Manage = getResult.UserAccessPolicies.First().Manage + } + ] + }; + + var putResponse = await _client.PutAsJsonAsync($"/projects/{project.Id}/access-policies/people", putRequest); + putResponse.EnsureSuccessStatusCode(); + + var putResult = await putResponse.Content.ReadFromJsonAsync(); + Assert.NotNull(putResult); + Assert.True(putResult.UserAccessPolicies.First().Manage, "manage:true must survive a GET→PUT round-trip"); + } + + [Fact] + public async Task PutProjectPeopleAccessPolicies_WriteOnlyCallerWithoutManage_ReturnsNotFound() + { + var (org, _) = await _organizationHelper.Initialize(true, true, true); + + var project = await _projectRepository.CreateAsync(new Project + { + OrganizationId = org.Id, + Name = _mockEncryptedString + }); + + var (email, writeOnlyOrgUser) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); + await _loginHelper.LoginAsync(email); + + await _accessPolicyRepository.CreateManyAsync( + [ + new UserProjectAccessPolicy + { + GrantedProjectId = project.Id, + OrganizationUserId = writeOnlyOrgUser.Id, + Read = true, + Write = true, + Manage = false + } + ]); + + var request = new PeopleAccessPoliciesRequestModel + { + UserAccessPolicyRequests = + [ + new AccessPolicyRequest { GranteeId = writeOnlyOrgUser.Id, Read = true, Write = true, Manage = false } + ] + }; + + var response = await _client.PutAsJsonAsync($"/projects/{project.Id}/access-policies/people", request); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task PutSecretAccessPolicies_ValidRequest_Returns200() + { + var (secretId, _) = await SetupSecretAccessPoliciesTest(PermissionType.RunAsAdmin); + var (_, granteeOrgUser) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); + + var request = new SecretAccessPoliciesRequestsModel + { + UserAccessPolicyRequests = new List + { + new() { GranteeId = granteeOrgUser.Id, Read = true, Write = true, Manage = false } + }, + GroupAccessPolicyRequests = new List(), + ServiceAccountAccessPolicyRequests = new List() + }; + + var response = await _client.PutAsJsonAsync($"/secrets/{secretId}/access-policies", request); + response.EnsureSuccessStatusCode(); + + var result = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(result); + Assert.NotEmpty(result.UserAccessPolicies); + Assert.Equal(granteeOrgUser.Id, result.UserAccessPolicies.First().OrganizationUserId); + } + + [Fact] + public async Task PutSecretAccessPolicies_EmptyBody_Returns400() + { + var (secretId, _) = await SetupSecretAccessPoliciesTest(PermissionType.RunAsAdmin); + + var request = new SecretAccessPoliciesRequestsModel + { + UserAccessPolicyRequests = new List(), + GroupAccessPolicyRequests = new List(), + ServiceAccountAccessPolicyRequests = new List() + }; + + var response = await _client.PutAsJsonAsync($"/secrets/{secretId}/access-policies", request); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + private async Task<(Guid ProjectId, Guid ServiceAccountId)> CreateServiceAccountProjectAccessPolicyAsync( Guid organizationId) { @@ -1164,7 +1435,8 @@ await _accessPolicyRepository.CreateManyAsync( GrantedProjectId = project.Id, OrganizationUserId = organizationUser.Id, Read = true, - Write = true + Write = true, + Manage = true } }; await _accessPolicyRepository.CreateManyAsync(accessPolicies); @@ -1214,7 +1486,7 @@ await _accessPolicyRepository.CreateManyAsync( { UserAccessPolicyRequests = new List { - new() { GranteeId = currentUser.Id, Read = true, Write = true } + new() { GranteeId = currentUser.Id, Read = true, Write = true, Manage = true } } }; return (project, request); diff --git a/test/Api.Test/SecretsManager/Controllers/AccessPoliciesControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/AccessPoliciesControllerTests.cs index 6a4767958051..24053a313e73 100644 --- a/test/Api.Test/SecretsManager/Controllers/AccessPoliciesControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/AccessPoliciesControllerTests.cs @@ -2,13 +2,16 @@ using System.Security.Claims; using Bit.Api.SecretsManager.Controllers; using Bit.Api.SecretsManager.Models.Request; +using Bit.Core.Auth.Identity; using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.SecretsManager.Commands.AccessPolicies.Interfaces; using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Enums.AccessPolicies; 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.Repositories; using Bit.Core.Services; @@ -240,7 +243,7 @@ public async Task GetProjectPeopleAccessPolicies_UserWithoutPermission_ThrowsNot .ReturnsForAnyArgs(true); sutProvider.GetDependency() .AccessToProjectAsync(Arg.Any(), Arg.Any(), Arg.Any()) - .ReturnsForAnyArgs((canRead, false)); + .ReturnsForAnyArgs((canRead, false, false)); await Assert.ThrowsAsync(() => sutProvider.Sut.GetProjectPeopleAccessPoliciesAsync(data.Id)); @@ -346,6 +349,7 @@ public async Task PutProjectPeopleAccessPoliciesAsync_NoAccess_Throws( Project project, PeopleAccessPoliciesRequestModel request) { + request = SetupValidRequest(request); sutProvider.GetDependency().GetByIdAsync(default).ReturnsForAnyArgs(project); var peoplePolicies = request.ToProjectPeopleAccessPolicies(project.Id, project.OrganizationId); sutProvider.GetDependency() @@ -367,6 +371,7 @@ public async Task PutProjectPeopleAccessPoliciesAsync_Success( Project project, PeopleAccessPoliciesRequestModel request) { + request = SetupValidRequest(request); sutProvider.GetDependency().GetByIdAsync(default).ReturnsForAnyArgs(project); sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); var peoplePolicies = request.ToProjectPeopleAccessPolicies(project.Id, project.OrganizationId); @@ -428,7 +433,7 @@ public async Task GetServiceAccountPeopleAccessPoliciesAsync_UserWithoutPermissi .ReturnsForAnyArgs(true); sutProvider.GetDependency() .AccessToServiceAccountAsync(Arg.Any(), Arg.Any(), Arg.Any()) - .ReturnsForAnyArgs((canRead, false)); + .ReturnsForAnyArgs((canRead, false, false)); await Assert.ThrowsAsync(() => sutProvider.Sut.GetServiceAccountPeopleAccessPoliciesAsync(data.Id)); @@ -448,7 +453,7 @@ public async Task GetServiceAccountPeopleAccessPoliciesAsync_HasPermissionNoPoli .ReturnsForAnyArgs(true); sutProvider.GetDependency() .AccessToServiceAccountAsync(Arg.Any(), Arg.Any(), Arg.Any()) - .ReturnsForAnyArgs((true, true)); + .ReturnsForAnyArgs((true, true, true)); var result = await sutProvider.Sut.GetServiceAccountPeopleAccessPoliciesAsync(data.Id); @@ -472,7 +477,7 @@ public async Task GetServiceAccountPeopleAccessPoliciesAsync_Success( .ReturnsForAnyArgs(true); sutProvider.GetDependency() .AccessToServiceAccountAsync(Arg.Any(), Arg.Any(), Arg.Any()) - .ReturnsForAnyArgs((true, true)); + .ReturnsForAnyArgs((true, true, true)); sutProvider.GetDependency() .GetPeoplePoliciesByGrantedServiceAccountIdAsync(default, default) @@ -740,10 +745,12 @@ await sutProvider.GetDependency().D public async Task PutServiceAccountGrantedPoliciesAsync_Success( SutProvider sutProvider, ServiceAccount data, + Guid userId, ServiceAccountGrantedPoliciesRequestModel request) { request = SetupValidRequest(request); sutProvider.GetDependency().GetByIdAsync(data.Id).ReturnsForAnyArgs(data); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); sutProvider.GetDependency() .AuthorizeAsync(Arg.Any(), Arg.Any(), @@ -795,7 +802,7 @@ public async Task GetProjectServiceAccountsAccessPoliciesAsync_NoAccess_ThrowsNo sutProvider.GetDependency().GetByIdAsync(data.Id).Returns(data); sutProvider.GetDependency().AccessSecretsManager(default).ReturnsForAnyArgs(true); sutProvider.GetDependency().AccessToProjectAsync(default, default, default) - .ReturnsForAnyArgs((false, false)); + .ReturnsForAnyArgs((false, false, false)); await Assert.ThrowsAsync(() => sutProvider.Sut.GetProjectServiceAccountsAccessPoliciesAsync(data.Id)); @@ -937,10 +944,12 @@ await sutProvider.GetDependency sutProvider, Project data, + Guid userId, ProjectServiceAccountsAccessPoliciesRequestModel request) { request = SetupValidRequest(request); sutProvider.GetDependency().GetByIdAsync(data.Id).ReturnsForAnyArgs(data); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); sutProvider.GetDependency() .AuthorizeAsync(Arg.Any(), Arg.Any(), @@ -1030,6 +1039,7 @@ private static ServiceAccountGrantedPoliciesRequestModel SetupValidRequest( foreach (var policyRequest in request.ProjectGrantedPolicyRequests) { policyRequest.Read = true; + policyRequest.Manage = false; } return request; @@ -1041,6 +1051,24 @@ private static ProjectServiceAccountsAccessPoliciesRequestModel SetupValidReques foreach (var policyRequest in request.ServiceAccountAccessPolicyRequests) { policyRequest.Read = true; + policyRequest.Manage = false; + } + + return request; + } + + private static PeopleAccessPoliciesRequestModel SetupValidRequest(PeopleAccessPoliciesRequestModel request) + { + foreach (var policyRequest in request.UserAccessPolicyRequests ?? []) + { + policyRequest.Read = true; + policyRequest.Manage = false; + } + + foreach (var policyRequest in request.GroupAccessPolicyRequests ?? []) + { + policyRequest.Read = true; + policyRequest.Manage = false; } return request; @@ -1052,9 +1080,11 @@ private static void SetupProjectAccessPoliciesTest(SutProvider().GetByIdAsync(default).ReturnsForAnyArgs(data); sutProvider.GetDependency().AccessSecretsManager(Arg.Any()) .ReturnsForAnyArgs(true); + // ServiceAccount clients do not have Manage access to project people/SA policies + var hasManage = accessClientType != AccessClientType.ServiceAccount; sutProvider.GetDependency() .AccessToProjectAsync(Arg.Any(), Arg.Any(), Arg.Any()) - .ReturnsForAnyArgs((true, true)); + .ReturnsForAnyArgs((true, true, hasManage)); sutProvider.GetDependency() .GetAccessClientAsync(Arg.Any(), Arg.Any()) .ReturnsForAnyArgs((accessClientType, Guid.NewGuid())); @@ -1068,4 +1098,618 @@ private static void SetupSecretAccessPoliciesTest(SutProvider>()).ReturnsForAnyArgs(AuthorizationResult.Success()); sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(Guid.NewGuid()); } + + // --- Event logging tests --- + + [Theory] + [BitAutoData] + public async Task LogAccessPolicyServiceAccountChanges_UserPermissionChanged_EmitsUpdatedEvent( + SutProvider sutProvider, + Guid userId, + UserServiceAccountAccessPolicy policy) + { + policy.Read = true; + policy.Write = true; + policy.Manage = false; + + var updatedPolicy = new UserServiceAccountAccessPolicy + { + Id = policy.Id, + OrganizationUserId = policy.OrganizationUserId, + GrantedServiceAccountId = policy.GrantedServiceAccountId, + Read = true, + Write = true, + Manage = true + }; + + var before = new List { policy }; + var after = new List { updatedPolicy }; + + await sutProvider.Sut.LogAccessPolicyServiceAccountChanges(before, after, userId); + + await sutProvider.GetDependency().Received(1) + .LogServiceAccountPeopleEventAsync(userId, updatedPolicy, + EventType.ServiceAccount_UserPermissionUpdated, Arg.Any()); + await sutProvider.GetDependency().DidNotReceive() + .LogServiceAccountPeopleEventAsync(Arg.Any(), Arg.Any(), + EventType.ServiceAccount_UserAdded, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task LogAccessPolicyServiceAccountChanges_UserPermissionUnchanged_DoesNotEmitUpdatedEvent( + SutProvider sutProvider, + Guid userId, + UserServiceAccountAccessPolicy policy) + { + policy.Read = true; + policy.Write = true; + policy.Manage = false; + + var samePolicy = new UserServiceAccountAccessPolicy + { + Id = policy.Id, + OrganizationUserId = policy.OrganizationUserId, + GrantedServiceAccountId = policy.GrantedServiceAccountId, + Read = true, + Write = true, + Manage = false + }; + + var before = new List { policy }; + var after = new List { samePolicy }; + + await sutProvider.Sut.LogAccessPolicyServiceAccountChanges(before, after, userId); + + await sutProvider.GetDependency().DidNotReceive() + .LogServiceAccountPeopleEventAsync(Arg.Any(), Arg.Any(), + EventType.ServiceAccount_UserPermissionUpdated, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task LogAccessPolicyServiceAccountChanges_GroupPermissionChanged_EmitsUpdatedEvent( + SutProvider sutProvider, + Guid userId, + GroupServiceAccountAccessPolicy policy) + { + policy.Read = true; + policy.Write = true; + policy.Manage = false; + + var updatedPolicy = new GroupServiceAccountAccessPolicy + { + Id = policy.Id, + GroupId = policy.GroupId, + Group = policy.Group, + GrantedServiceAccountId = policy.GrantedServiceAccountId, + Read = true, + Write = true, + Manage = true + }; + + var before = new List { policy }; + var after = new List { updatedPolicy }; + + await sutProvider.Sut.LogAccessPolicyServiceAccountChanges(before, after, userId); + + await sutProvider.GetDependency().Received(1) + .LogServiceAccountGroupEventAsync(userId, updatedPolicy, + EventType.ServiceAccount_GroupPermissionUpdated, Arg.Any()); + await sutProvider.GetDependency().DidNotReceive() + .LogServiceAccountGroupEventAsync(Arg.Any(), Arg.Any(), + EventType.ServiceAccount_GroupAdded, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutProjectPeopleAccessPoliciesAsync_UserAdded_EmitsGrantedEvent( + SutProvider sutProvider, + Guid userId, + Project project, + UserProjectAccessPolicy newPolicy, + PeopleAccessPoliciesRequestModel request) + { + request = SetupValidRequest(request); + sutProvider.GetDependency().GetByIdAsync(default).ReturnsForAnyArgs(project); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + sutProvider.GetDependency() + .GetPeoplePoliciesByGrantedProjectIdAsync(Arg.Any(), Arg.Any()) + .Returns(new List()); + sutProvider.GetDependency() + .ReplaceProjectPeopleAsync(Arg.Any(), Arg.Any()) + .Returns(new List { newPolicy }); + + await sutProvider.Sut.PutProjectPeopleAccessPoliciesAsync(project.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogProjectAccessPolicyEventAsync(userId, project.OrganizationId, newPolicy, + EventType.Project_UserAccessGranted, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutProjectPeopleAccessPoliciesAsync_UserRemoved_EmitsRevokedEvent( + SutProvider sutProvider, + Guid userId, + Project project, + UserProjectAccessPolicy existingPolicy, + PeopleAccessPoliciesRequestModel request) + { + request = SetupValidRequest(request); + sutProvider.GetDependency().GetByIdAsync(default).ReturnsForAnyArgs(project); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + sutProvider.GetDependency() + .GetPeoplePoliciesByGrantedProjectIdAsync(Arg.Any(), Arg.Any()) + .Returns(new List { existingPolicy }); + sutProvider.GetDependency() + .ReplaceProjectPeopleAsync(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await sutProvider.Sut.PutProjectPeopleAccessPoliciesAsync(project.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogProjectAccessPolicyEventAsync(userId, project.OrganizationId, existingPolicy, + EventType.Project_UserAccessRevoked, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutProjectPeopleAccessPoliciesAsync_UserPermissionChanged_EmitsUpdatedEvent( + SutProvider sutProvider, + Guid userId, + Project project, + UserProjectAccessPolicy existingPolicy, + PeopleAccessPoliciesRequestModel request) + { + existingPolicy.Read = true; + existingPolicy.Write = true; + existingPolicy.Manage = false; + request = SetupValidRequest(request); + + var updatedPolicy = new UserProjectAccessPolicy + { + Id = existingPolicy.Id, + OrganizationUserId = existingPolicy.OrganizationUserId, + GrantedProjectId = existingPolicy.GrantedProjectId, + Read = true, + Write = true, + Manage = true + }; + + sutProvider.GetDependency().GetByIdAsync(default).ReturnsForAnyArgs(project); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + sutProvider.GetDependency() + .GetPeoplePoliciesByGrantedProjectIdAsync(Arg.Any(), Arg.Any()) + .Returns(new List { existingPolicy }); + sutProvider.GetDependency() + .ReplaceProjectPeopleAsync(Arg.Any(), Arg.Any()) + .Returns(new List { updatedPolicy }); + + await sutProvider.Sut.PutProjectPeopleAccessPoliciesAsync(project.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogProjectAccessPolicyEventAsync(userId, project.OrganizationId, updatedPolicy, + EventType.Project_UserAccessUpdated, Arg.Any()); + await sutProvider.GetDependency().DidNotReceive() + .LogProjectAccessPolicyEventAsync(Arg.Any(), Arg.Any(), Arg.Any(), + EventType.Project_UserAccessGranted, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutProjectPeopleAccessPoliciesAsync_UserPermissionUnchanged_DoesNotEmitEvent( + SutProvider sutProvider, + Guid userId, + Project project, + UserProjectAccessPolicy existingPolicy, + PeopleAccessPoliciesRequestModel request) + { + var samePolicy = new UserProjectAccessPolicy + { + Id = existingPolicy.Id, + OrganizationUserId = existingPolicy.OrganizationUserId, + GrantedProjectId = existingPolicy.GrantedProjectId, + Read = existingPolicy.Read, + Write = existingPolicy.Write, + Manage = existingPolicy.Manage + }; + + request = SetupValidRequest(request); + sutProvider.GetDependency().GetByIdAsync(default).ReturnsForAnyArgs(project); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + sutProvider.GetDependency() + .GetPeoplePoliciesByGrantedProjectIdAsync(Arg.Any(), Arg.Any()) + .Returns(new List { existingPolicy }); + sutProvider.GetDependency() + .ReplaceProjectPeopleAsync(Arg.Any(), Arg.Any()) + .Returns(new List { samePolicy }); + + await sutProvider.Sut.PutProjectPeopleAccessPoliciesAsync(project.Id, request); + + await sutProvider.GetDependency().DidNotReceive() + .LogProjectAccessPolicyEventAsync(Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutProjectServiceAccountsAccessPoliciesAsync_ServiceAccountGranted_EmitsGrantedEvent( + SutProvider sutProvider, + Project data, + Guid userId, + ServiceAccountProjectAccessPolicy grantedPolicy, + ProjectServiceAccountsAccessPoliciesRequestModel request) + { + request = SetupValidRequest(request); + var updates = new ProjectServiceAccountsAccessPoliciesUpdates + { + ProjectId = data.Id, + OrganizationId = data.OrganizationId, + ServiceAccountAccessPolicyUpdates = new[] + { + new ServiceAccountProjectAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Create, + AccessPolicy = grantedPolicy + } + } + }; + sutProvider.GetDependency().GetByIdAsync(data.Id).ReturnsForAnyArgs(data); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .GetAsync(Arg.Any()).Returns(updates); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + await sutProvider.Sut.PutProjectServiceAccountsAccessPoliciesAsync(data.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogProjectAccessPolicyEventAsync(userId, data.OrganizationId, grantedPolicy, + EventType.Project_ServiceAccountAccessGranted, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutProjectServiceAccountsAccessPoliciesAsync_ServiceAccountRevoked_EmitsRevokedEvent( + SutProvider sutProvider, + Project data, + Guid userId, + ServiceAccountProjectAccessPolicy revokedPolicy, + ProjectServiceAccountsAccessPoliciesRequestModel request) + { + request = SetupValidRequest(request); + var updates = new ProjectServiceAccountsAccessPoliciesUpdates + { + ProjectId = data.Id, + OrganizationId = data.OrganizationId, + ServiceAccountAccessPolicyUpdates = new[] + { + new ServiceAccountProjectAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Delete, + AccessPolicy = revokedPolicy + } + } + }; + sutProvider.GetDependency().GetByIdAsync(data.Id).ReturnsForAnyArgs(data); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .GetAsync(Arg.Any()).Returns(updates); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + await sutProvider.Sut.PutProjectServiceAccountsAccessPoliciesAsync(data.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogProjectAccessPolicyEventAsync(userId, data.OrganizationId, revokedPolicy, + EventType.Project_ServiceAccountAccessRevoked, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutProjectServiceAccountsAccessPoliciesAsync_ServiceAccountUpdated_EmitsUpdatedEvent( + SutProvider sutProvider, + Project data, + Guid userId, + ServiceAccountProjectAccessPolicy updatedPolicy, + ProjectServiceAccountsAccessPoliciesRequestModel request) + { + request = SetupValidRequest(request); + var updates = new ProjectServiceAccountsAccessPoliciesUpdates + { + ProjectId = data.Id, + OrganizationId = data.OrganizationId, + ServiceAccountAccessPolicyUpdates = new[] + { + new ServiceAccountProjectAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Update, + AccessPolicy = updatedPolicy + } + } + }; + sutProvider.GetDependency().GetByIdAsync(data.Id).ReturnsForAnyArgs(data); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .GetAsync(Arg.Any()).Returns(updates); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + await sutProvider.Sut.PutProjectServiceAccountsAccessPoliciesAsync(data.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogProjectAccessPolicyEventAsync(userId, data.OrganizationId, updatedPolicy, + EventType.Project_ServiceAccountAccessUpdated, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutServiceAccountGrantedPoliciesAsync_ProjectGranted_EmitsGrantedEvent( + SutProvider sutProvider, + ServiceAccount data, + Guid userId, + ServiceAccountProjectAccessPolicy grantedPolicy, + ServiceAccountGrantedPoliciesRequestModel request) + { + request = SetupValidRequest(request); + var updates = new ServiceAccountGrantedPoliciesUpdates + { + ServiceAccountId = data.Id, + OrganizationId = data.OrganizationId, + ProjectGrantedPolicyUpdates = new[] + { + new ServiceAccountProjectAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Create, + AccessPolicy = grantedPolicy + } + } + }; + sutProvider.GetDependency().GetByIdAsync(data.Id).ReturnsForAnyArgs(data); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .GetAsync(Arg.Any()).Returns(updates); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + await sutProvider.Sut.PutServiceAccountGrantedPoliciesAsync(data.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogProjectAccessPolicyEventAsync(userId, data.OrganizationId, grantedPolicy, + EventType.Project_ServiceAccountAccessGranted, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutServiceAccountGrantedPoliciesAsync_ProjectRevoked_EmitsRevokedEvent( + SutProvider sutProvider, + ServiceAccount data, + Guid userId, + ServiceAccountProjectAccessPolicy revokedPolicy, + ServiceAccountGrantedPoliciesRequestModel request) + { + request = SetupValidRequest(request); + var updates = new ServiceAccountGrantedPoliciesUpdates + { + ServiceAccountId = data.Id, + OrganizationId = data.OrganizationId, + ProjectGrantedPolicyUpdates = new[] + { + new ServiceAccountProjectAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Delete, + AccessPolicy = revokedPolicy + } + } + }; + sutProvider.GetDependency().GetByIdAsync(data.Id).ReturnsForAnyArgs(data); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .GetAsync(Arg.Any()).Returns(updates); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + await sutProvider.Sut.PutServiceAccountGrantedPoliciesAsync(data.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogProjectAccessPolicyEventAsync(userId, data.OrganizationId, revokedPolicy, + EventType.Project_ServiceAccountAccessRevoked, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutServiceAccountGrantedPoliciesAsync_ProjectUpdated_EmitsUpdatedEvent( + SutProvider sutProvider, + ServiceAccount data, + Guid userId, + ServiceAccountProjectAccessPolicy updatedPolicy, + ServiceAccountGrantedPoliciesRequestModel request) + { + request = SetupValidRequest(request); + var updates = new ServiceAccountGrantedPoliciesUpdates + { + ServiceAccountId = data.Id, + OrganizationId = data.OrganizationId, + ProjectGrantedPolicyUpdates = new[] + { + new ServiceAccountProjectAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Update, + AccessPolicy = updatedPolicy + } + } + }; + sutProvider.GetDependency().GetByIdAsync(data.Id).ReturnsForAnyArgs(data); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .GetAsync(Arg.Any()).Returns(updates); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + await sutProvider.Sut.PutServiceAccountGrantedPoliciesAsync(data.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogProjectAccessPolicyEventAsync(userId, data.OrganizationId, updatedPolicy, + EventType.Project_ServiceAccountAccessUpdated, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutSecretAccessPoliciesAsync_UserGranted_EmitsGrantedEvent( + SutProvider sutProvider, + Secret secret, + Guid userId, + UserSecretAccessPolicy grantedPolicy, + SecretAccessPoliciesRequestsModel request) + { + request.UserAccessPolicyRequests = new[] { new AccessPolicyRequest { GranteeId = Guid.NewGuid(), Read = true, Write = true } }; + request.GroupAccessPolicyRequests = Array.Empty(); + request.ServiceAccountAccessPolicyRequests = Array.Empty(); + + var updates = new SecretAccessPoliciesUpdates + { + SecretId = secret.Id, + OrganizationId = secret.OrganizationId, + UserAccessPolicyUpdates = new[] + { + new UserSecretAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Create, + AccessPolicy = grantedPolicy + } + }, + GroupAccessPolicyUpdates = Array.Empty(), + ServiceAccountAccessPolicyUpdates = Array.Empty() + }; + + sutProvider.GetDependency().GetByIdAsync(secret.Id).Returns(secret); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .GetAccessClientAsync(Arg.Any(), Arg.Any()) + .Returns((AccessClientType.User, userId)); + sutProvider.GetDependency() + .GetAsync(Arg.Any(), Arg.Any()).Returns(updates); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + await sutProvider.Sut.PutSecretAccessPoliciesAsync(secret.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogSecretAccessPolicyEventAsync(userId, secret.OrganizationId, grantedPolicy, + EventType.Secret_UserAccessGranted, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutSecretAccessPoliciesAsync_UserRevoked_EmitsRevokedEvent( + SutProvider sutProvider, + Secret secret, + Guid userId, + UserSecretAccessPolicy revokedPolicy, + SecretAccessPoliciesRequestsModel request) + { + request.UserAccessPolicyRequests = new[] { new AccessPolicyRequest { GranteeId = Guid.NewGuid(), Read = true, Write = true } }; + request.GroupAccessPolicyRequests = Array.Empty(); + request.ServiceAccountAccessPolicyRequests = Array.Empty(); + + var updates = new SecretAccessPoliciesUpdates + { + SecretId = secret.Id, + OrganizationId = secret.OrganizationId, + UserAccessPolicyUpdates = new[] + { + new UserSecretAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Delete, + AccessPolicy = revokedPolicy + } + }, + GroupAccessPolicyUpdates = Array.Empty(), + ServiceAccountAccessPolicyUpdates = Array.Empty() + }; + + sutProvider.GetDependency().GetByIdAsync(secret.Id).Returns(secret); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .GetAccessClientAsync(Arg.Any(), Arg.Any()) + .Returns((AccessClientType.User, userId)); + sutProvider.GetDependency() + .GetAsync(Arg.Any(), Arg.Any()).Returns(updates); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + await sutProvider.Sut.PutSecretAccessPoliciesAsync(secret.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogSecretAccessPolicyEventAsync(userId, secret.OrganizationId, revokedPolicy, + EventType.Secret_UserAccessRevoked, Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PutSecretAccessPoliciesAsync_ServiceAccountUpdated_EmitsUpdatedEvent( + SutProvider sutProvider, + Secret secret, + Guid userId, + ServiceAccountSecretAccessPolicy updatedPolicy, + SecretAccessPoliciesRequestsModel request) + { + request.UserAccessPolicyRequests = new[] { new AccessPolicyRequest { GranteeId = Guid.NewGuid(), Read = true, Write = true } }; + request.GroupAccessPolicyRequests = Array.Empty(); + request.ServiceAccountAccessPolicyRequests = Array.Empty(); + + var updates = new SecretAccessPoliciesUpdates + { + SecretId = secret.Id, + OrganizationId = secret.OrganizationId, + UserAccessPolicyUpdates = Array.Empty(), + GroupAccessPolicyUpdates = Array.Empty(), + ServiceAccountAccessPolicyUpdates = new[] + { + new ServiceAccountSecretAccessPolicyUpdate + { + Operation = AccessPolicyOperation.Update, + AccessPolicy = updatedPolicy + } + } + }; + + sutProvider.GetDependency().GetByIdAsync(secret.Id).Returns(secret); + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + sutProvider.GetDependency() + .GetAccessClientAsync(Arg.Any(), Arg.Any()) + .Returns((AccessClientType.User, userId)); + sutProvider.GetDependency() + .GetAsync(Arg.Any(), Arg.Any()).Returns(updates); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), Arg.Any(), + Arg.Any>()).Returns(AuthorizationResult.Success()); + + await sutProvider.Sut.PutSecretAccessPoliciesAsync(secret.Id, request); + + await sutProvider.GetDependency().Received(1) + .LogSecretAccessPolicyEventAsync(userId, secret.OrganizationId, updatedPolicy, + EventType.Secret_ServiceAccountAccessUpdated, Arg.Any()); + } } diff --git a/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs index 9ff4a5e19b03..ed9b250eae9f 100644 --- a/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs @@ -234,7 +234,7 @@ public async Task Get_Success(PermissionType permissionType, SutProvider().AccessToProjectAsync(default, default, default) - .Returns((true, true)); + .Returns((true, true, false)); break; } @@ -242,7 +242,7 @@ public async Task Get_Success(PermissionType permissionType, SutProvider().AccessToProjectAsync(default, default, default) - .ReturnsForAnyArgs((true, false)); + .ReturnsForAnyArgs((true, false, false)); await sutProvider.Sut.GetAsync(data); @@ -257,7 +257,7 @@ public async Task Get_UserWithoutPermission_Throws(SutProvider().AccessToProjectAsync(default, default, default) - .Returns((false, false)); + .Returns((false, false, false)); sutProvider.GetDependency().GetByIdAsync(Arg.Is(data)) .ReturnsForAnyArgs(new Project { Id = data, OrganizationId = orgId }); diff --git a/test/Api.Test/SecretsManager/Controllers/SecretVersionsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/SecretVersionsControllerTests.cs index 79a339fcbad4..faeaccd4921f 100644 --- a/test/Api.Test/SecretsManager/Controllers/SecretVersionsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/SecretVersionsControllerTests.cs @@ -58,7 +58,7 @@ public async Task GetVersionsBySecretId_NoReadAccess_Throws( sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().OrganizationAdmin(secret.OrganizationId).Returns(false); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, default) - .ReturnsForAnyArgs((false, false)); + .ReturnsForAnyArgs((false, false, false)); await Assert.ThrowsAsync(() => sutProvider.Sut.GetVersionsBySecretIdAsync(secret.Id)); @@ -77,7 +77,7 @@ public async Task GetVersionsBySecretId_Success( sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().OrganizationAdmin(secret.OrganizationId).Returns(false); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, default) - .ReturnsForAnyArgs((true, false)); + .ReturnsForAnyArgs((true, false, false)); foreach (var version in versions) { @@ -119,7 +119,7 @@ public async Task GetById_Success( sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().OrganizationAdmin(secret.OrganizationId).Returns(false); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, default) - .ReturnsForAnyArgs((true, false)); + .ReturnsForAnyArgs((true, false, false)); var result = await sutProvider.Sut.GetByIdAsync(version.Id); @@ -144,7 +144,7 @@ public async Task RestoreVersion_NoWriteAccess_Throws( sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().OrganizationAdmin(secret.OrganizationId).Returns(false); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, default) - .ReturnsForAnyArgs((true, false)); + .ReturnsForAnyArgs((true, false, false)); await Assert.ThrowsAsync(() => sutProvider.Sut.RestoreVersionAsync(secret.Id, request)); @@ -163,7 +163,7 @@ public async Task RestoreVersion_VersionNotFound_Throws( sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().OrganizationAdmin(secret.OrganizationId).Returns(true); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, default) - .ReturnsForAnyArgs((true, true)); + .ReturnsForAnyArgs((true, true, false)); sutProvider.GetDependency().GetByIdAsync(request.VersionId).Returns((SecretVersion?)null); await Assert.ThrowsAsync(() => @@ -187,7 +187,7 @@ public async Task RestoreVersion_VersionBelongsToDifferentSecret_Throws( sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().OrganizationAdmin(secret.OrganizationId).Returns(true); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, default) - .ReturnsForAnyArgs((true, true)); + .ReturnsForAnyArgs((true, true, false)); sutProvider.GetDependency().GetByIdAsync(request.VersionId).Returns(version); await Assert.ThrowsAsync(() => @@ -215,7 +215,7 @@ public async Task RestoreVersion_Success( sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().OrganizationAdmin(secret.OrganizationId).Returns(true); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, default) - .ReturnsForAnyArgs((true, true)); + .ReturnsForAnyArgs((true, true, false)); sutProvider.GetDependency().GetByIdAsync(request.VersionId).Returns(version); sutProvider.GetDependency() .GetByOrganizationAsync(secret.OrganizationId, userId).Returns(organizationUser); @@ -269,7 +269,7 @@ public async Task BulkDelete_NoWriteAccess_Throws( sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().OrganizationAdmin(secret.OrganizationId).Returns(false); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, default) - .ReturnsForAnyArgs((true, false)); + .ReturnsForAnyArgs((true, false, false)); await Assert.ThrowsAsync(() => sutProvider.Sut.BulkDeleteAsync(ids)); @@ -297,7 +297,7 @@ public async Task BulkDelete_Success( sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); sutProvider.GetDependency().OrganizationAdmin(secret.OrganizationId).Returns(true); sutProvider.GetDependency().AccessToSecretAsync(secret.Id, userId, default) - .ReturnsForAnyArgs((true, true)); + .ReturnsForAnyArgs((true, true, false)); await sutProvider.Sut.BulkDeleteAsync(ids); diff --git a/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs index 51f61ad7c1a4..65c422e2bff5 100644 --- a/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs @@ -70,7 +70,7 @@ public async Task GetSecretsByOrganization_Success(PermissionType permissionType resultSecret.Projects = new List() { mockProject }; sutProvider.GetDependency().OrganizationAdmin(organizationId).Returns(false); sutProvider.GetDependency().AccessToProjectAsync(default, default, default) - .Returns((true, true)); + .Returns((true, true, false)); } @@ -110,20 +110,20 @@ public async Task GetSecret_Success(PermissionType permissionType, SutProvider().GetByIdAsync(default).ReturnsForAnyArgs(resultSecret); sutProvider.GetDependency().AccessToSecretAsync(default, default, default) - .ReturnsForAnyArgs(Task.FromResult((true, true))); + .ReturnsForAnyArgs(Task.FromResult((true, true, false))); if (permissionType == PermissionType.RunAsAdmin) { resultSecret.OrganizationId = organizationId; sutProvider.GetDependency().OrganizationAdmin(organizationId).Returns(true); sutProvider.GetDependency().AccessToProjectAsync(Arg.Any(), Arg.Any(), AccessClientType.NoAccessCheck) - .Returns((true, true)); + .Returns((true, true, false)); } else { sutProvider.GetDependency().OrganizationAdmin(organizationId).Returns(false); sutProvider.GetDependency().AccessToProjectAsync(Arg.Any(), Arg.Any(), AccessClientType.User) - .Returns((true, true)); + .Returns((true, true, false)); } await sutProvider.Sut.GetAsync(resultSecret.Id); @@ -572,6 +572,10 @@ private static SecretCreateRequestModel SetupSecretCreateRequest(SutProvider() .CreateAsync(Arg.Any(), Arg.Any()) @@ -596,10 +600,33 @@ private static SecretUpdateRequestModel SetupSecretUpdateRequest(SecretUpdateReq { data.AccessPoliciesRequests = null; } + else if (data.AccessPoliciesRequests != null) + { + SanitizeSecretAccessPoliciesRequests(data.AccessPoliciesRequests); + } return data; } + private static void SanitizeSecretAccessPoliciesRequests(SecretAccessPoliciesRequestsModel requests) + { + foreach (var r in requests.UserAccessPolicyRequests) + { + r.Read = true; + r.Manage = false; + } + foreach (var r in requests.GroupAccessPolicyRequests) + { + r.Read = true; + r.Manage = false; + } + foreach (var r in requests.ServiceAccountAccessPolicyRequests) + { + r.Read = true; + r.Manage = false; + } + } + private static SecretUpdateRequestModel SetupSecretUpdateAccessPoliciesRequest(SutProvider sutProvider, SecretUpdateRequestModel data, Secret currentSecret, SecretAccessPoliciesUpdates accessPoliciesUpdates) { data = SetupSecretUpdateRequest(data, true); diff --git a/test/Core.Test/SecretsManager/Models/SecretAccessPoliciesTests.cs b/test/Core.Test/SecretsManager/Models/SecretAccessPoliciesTests.cs index fa8deff50bf2..c684698c6bca 100644 --- a/test/Core.Test/SecretsManager/Models/SecretAccessPoliciesTests.cs +++ b/test/Core.Test/SecretsManager/Models/SecretAccessPoliciesTests.cs @@ -116,4 +116,39 @@ public void GetPolicyUpdates_ReturnsCorrectPolicyChanges() Assert.DoesNotContain(unChangedId, result.ServiceAccountAccessPolicyUpdates .Select(pu => pu.AccessPolicy.ServiceAccountId!.Value)); } + + [Fact] + public void GetPolicyUpdates_ManageOnlyChange_IsDetectedAsUpdate() + { + var secretId = Guid.NewGuid(); + var policyId = Guid.NewGuid(); + + var existing = new SecretAccessPolicies + { + UserAccessPolicies = new List + { + new() { OrganizationUserId = policyId, GrantedSecretId = secretId, Read = true, Write = true, Manage = false } + }, + GroupAccessPolicies = new List(), + ServiceAccountAccessPolicies = new List() + }; + + var requested = new SecretAccessPolicies + { + UserAccessPolicies = new List + { + new() { OrganizationUserId = policyId, GrantedSecretId = secretId, Read = true, Write = true, Manage = true } + }, + GroupAccessPolicies = new List(), + ServiceAccountAccessPolicies = new List() + }; + + var result = existing.GetPolicyUpdates(requested); + + Assert.Single(result.UserAccessPolicyUpdates); + Assert.Equal(AccessPolicyOperation.Update, result.UserAccessPolicyUpdates.Single().Operation); + Assert.Equal(policyId, result.UserAccessPolicyUpdates.Single().AccessPolicy.OrganizationUserId); + Assert.Empty(result.GroupAccessPolicyUpdates); + Assert.Empty(result.ServiceAccountAccessPolicyUpdates); + } } diff --git a/util/Migrator/DbScripts/2026-02-19_00_AddManagePermissionAndCreatedByServiceAccount.sql b/util/Migrator/DbScripts/2026-02-19_00_AddManagePermissionAndCreatedByServiceAccount.sql new file mode 100644 index 000000000000..d144871e7e12 --- /dev/null +++ b/util/Migrator/DbScripts/2026-02-19_00_AddManagePermissionAndCreatedByServiceAccount.sql @@ -0,0 +1,53 @@ +-- Add Manage column to AccessPolicy and CreatedByServiceAccountId to Project + +-- Step 1: Add the column first (separate batch to avoid compile-time "invalid column" on subsequent DML) +IF OBJECT_ID('[dbo].[AccessPolicy]') IS NOT NULL AND COL_LENGTH('[dbo].[AccessPolicy]', 'Manage') IS NULL +BEGIN + ALTER TABLE [dbo].[AccessPolicy] + ADD [Manage] BIT NOT NULL DEFAULT (0); +END +GO + +-- Step 2: Backfill and add constraints (compiled after column exists) +IF OBJECT_ID('[dbo].[AccessPolicy]') IS NOT NULL AND COL_LENGTH('[dbo].[AccessPolicy]', 'Manage') IS NOT NULL +BEGIN + -- Backfill: preserve current behavior — user and group policies with Write=1 get Manage=1 + -- Service account policies remain Manage=0 per spec + UPDATE [dbo].[AccessPolicy] + SET [Manage] = 1 + WHERE [Write] = 1 + AND [Discriminator] IN ( + 'user_project', + 'user_service_account', + 'user_secret', + 'group_project', + 'group_service_account', + 'group_secret' + ); + + -- Enforce permission hierarchy at DB level + IF NOT EXISTS (SELECT 1 FROM sys.check_constraints WHERE name = 'CK_AccessPolicy_ManageImpliesWrite' AND parent_object_id = OBJECT_ID('[dbo].[AccessPolicy]')) + ALTER TABLE [dbo].[AccessPolicy] + ADD CONSTRAINT [CK_AccessPolicy_ManageImpliesWrite] CHECK ([Manage] = 0 OR [Write] = 1); + + IF NOT EXISTS (SELECT 1 FROM sys.check_constraints WHERE name = 'CK_AccessPolicy_WriteImpliesRead' AND parent_object_id = OBJECT_ID('[dbo].[AccessPolicy]')) + ALTER TABLE [dbo].[AccessPolicy] + ADD CONSTRAINT [CK_AccessPolicy_WriteImpliesRead] CHECK ([Write] = 0 OR [Read] = 1); +END +GO + +IF OBJECT_ID('[dbo].[Project]') IS NOT NULL AND COL_LENGTH('[dbo].[Project]', 'CreatedByServiceAccountId') IS NULL +BEGIN + ALTER TABLE [dbo].[Project] + ADD [CreatedByServiceAccountId] UNIQUEIDENTIFIER NULL; + + CREATE INDEX [IX_Project_CreatedByServiceAccountId] + ON [dbo].[Project] ([CreatedByServiceAccountId]); + + ALTER TABLE [dbo].[Project] + ADD CONSTRAINT [FK_Project_ServiceAccount_CreatedByServiceAccountId] + FOREIGN KEY ([CreatedByServiceAccountId]) + REFERENCES [dbo].[ServiceAccount] ([Id]) + ON DELETE SET NULL; +END +GO diff --git a/util/MySqlMigrations/Migrations/20260219000000_AddManagePermissionAndCreatedByServiceAccount.cs b/util/MySqlMigrations/Migrations/20260219000000_AddManagePermissionAndCreatedByServiceAccount.cs new file mode 100644 index 000000000000..09b5abf2866e --- /dev/null +++ b/util/MySqlMigrations/Migrations/20260219000000_AddManagePermissionAndCreatedByServiceAccount.cs @@ -0,0 +1,87 @@ +using System; +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Bit.MySqlMigrations.Migrations; + +/// +public partial class AddManagePermissionAndCreatedByServiceAccount : Migration +{ + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.AddColumn( + name: "Manage", + table: "AccessPolicy", + type: "tinyint(1)", + nullable: false, + defaultValue: false); + + // Backfill: preserve current behavior — user and group policies with Write=1 get Manage=1 + // Service account policies remain Manage=false per spec + migrationBuilder.Sql(@" + UPDATE `AccessPolicy` + SET `Manage` = 1 + WHERE `Write` = 1 + AND `Discriminator` IN ( + 'user_project', + 'user_service_account', + 'user_secret', + 'group_project', + 'group_service_account', + 'group_secret' + )"); + + // Enforce permission hierarchy at the DB level + migrationBuilder.Sql(@" + ALTER TABLE `AccessPolicy` + ADD CONSTRAINT `CK_AccessPolicy_ManageImpliesWrite` CHECK (`Manage` = 0 OR `Write` = 1), + ADD CONSTRAINT `CK_AccessPolicy_WriteImpliesRead` CHECK (`Write` = 0 OR `Read` = 1)"); + + migrationBuilder.AddColumn( + name: "CreatedByServiceAccountId", + table: "Project", + type: "char(36)", + nullable: true, + collation: "ascii_general_ci"); + + migrationBuilder.CreateIndex( + name: "IX_Project_CreatedByServiceAccountId", + table: "Project", + column: "CreatedByServiceAccountId"); + + migrationBuilder.AddForeignKey( + name: "FK_Project_ServiceAccount_CreatedByServiceAccountId", + table: "Project", + column: "CreatedByServiceAccountId", + principalTable: "ServiceAccount", + principalColumn: "Id", + onDelete: ReferentialAction.SetNull); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropForeignKey( + name: "FK_Project_ServiceAccount_CreatedByServiceAccountId", + table: "Project"); + + migrationBuilder.DropIndex( + name: "IX_Project_CreatedByServiceAccountId", + table: "Project"); + + migrationBuilder.DropColumn( + name: "CreatedByServiceAccountId", + table: "Project"); + + migrationBuilder.Sql(@" + ALTER TABLE `AccessPolicy` + DROP CONSTRAINT IF EXISTS `CK_AccessPolicy_ManageImpliesWrite`, + DROP CONSTRAINT IF EXISTS `CK_AccessPolicy_WriteImpliesRead`"); + + migrationBuilder.DropColumn( + name: "Manage", + table: "AccessPolicy"); + } +} diff --git a/util/MySqlMigrations/Migrations/DatabaseContextModelSnapshot.cs b/util/MySqlMigrations/Migrations/DatabaseContextModelSnapshot.cs index ce4c21637530..ecc06cfa0145 100644 --- a/util/MySqlMigrations/Migrations/DatabaseContextModelSnapshot.cs +++ b/util/MySqlMigrations/Migrations/DatabaseContextModelSnapshot.cs @@ -2234,6 +2234,10 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Property("Write") .HasColumnType("tinyint(1)"); + b.Property("Manage") + .HasColumnType("tinyint(1)") + .HasDefaultValue(false); + b.HasKey("Id") .HasAnnotation("SqlServer:Clustered", true); @@ -2313,9 +2317,15 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Property("RevisionDate") .HasColumnType("datetime(6)"); + b.Property("CreatedByServiceAccountId") + .HasColumnType("char(36)"); + b.HasKey("Id") .HasAnnotation("SqlServer:Clustered", true); + b.HasIndex("CreatedByServiceAccountId") + .HasAnnotation("SqlServer:Clustered", false); + b.HasIndex("DeletedDate") .HasAnnotation("SqlServer:Clustered", false); @@ -3232,6 +3242,11 @@ protected override void BuildModel(ModelBuilder modelBuilder) modelBuilder.Entity("Bit.Infrastructure.EntityFramework.SecretsManager.Models.Project", b => { + b.HasOne("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", "CreatedByServiceAccount") + .WithMany() + .HasForeignKey("CreatedByServiceAccountId") + .OnDelete(DeleteBehavior.SetNull); + b.HasOne("Bit.Infrastructure.EntityFramework.AdminConsole.Models.Organization", "Organization") .WithMany() .HasForeignKey("OrganizationId") diff --git a/util/PostgresMigrations/Migrations/20260219000000_AddManagePermissionAndCreatedByServiceAccount.cs b/util/PostgresMigrations/Migrations/20260219000000_AddManagePermissionAndCreatedByServiceAccount.cs new file mode 100644 index 000000000000..66e7862ce8e7 --- /dev/null +++ b/util/PostgresMigrations/Migrations/20260219000000_AddManagePermissionAndCreatedByServiceAccount.cs @@ -0,0 +1,92 @@ +using System; +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Bit.PostgresMigrations.Migrations; + +/// +public partial class AddManagePermissionAndCreatedByServiceAccount : Migration +{ + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.Sql("SET lock_timeout = '5s'"); + migrationBuilder.AddColumn( + name: "Manage", + table: "AccessPolicy", + type: "boolean", + nullable: false, + defaultValue: false); + migrationBuilder.Sql("RESET lock_timeout"); + + // Backfill: preserve current behavior — user and group policies with Write=true get Manage=true + // Service account policies remain Manage=false per spec + migrationBuilder.Sql(@" + UPDATE ""AccessPolicy"" + SET ""Manage"" = TRUE + WHERE ""Write"" = TRUE + AND ""Discriminator"" IN ( + 'user_project', + 'user_service_account', + 'user_secret', + 'group_project', + 'group_service_account', + 'group_secret' + )"); + + // Enforce permission hierarchy at the DB level + migrationBuilder.Sql("SET lock_timeout = '5s'"); + migrationBuilder.Sql(@" + ALTER TABLE ""AccessPolicy"" + ADD CONSTRAINT ""CK_AccessPolicy_ManageImpliesWrite"" CHECK (""Manage"" = FALSE OR ""Write"" = TRUE), + ADD CONSTRAINT ""CK_AccessPolicy_WriteImpliesRead"" CHECK (""Write"" = FALSE OR ""Read"" = TRUE)"); + migrationBuilder.Sql("RESET lock_timeout"); + + migrationBuilder.Sql("SET lock_timeout = '5s'"); + migrationBuilder.AddColumn( + name: "CreatedByServiceAccountId", + table: "Project", + type: "uuid", + nullable: true); + migrationBuilder.Sql("RESET lock_timeout"); + + migrationBuilder.CreateIndex( + name: "IX_Project_CreatedByServiceAccountId", + table: "Project", + column: "CreatedByServiceAccountId"); + + migrationBuilder.AddForeignKey( + name: "FK_Project_ServiceAccount_CreatedByServiceAccountId", + table: "Project", + column: "CreatedByServiceAccountId", + principalTable: "ServiceAccount", + principalColumn: "Id", + onDelete: ReferentialAction.SetNull); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropForeignKey( + name: "FK_Project_ServiceAccount_CreatedByServiceAccountId", + table: "Project"); + + migrationBuilder.DropIndex( + name: "IX_Project_CreatedByServiceAccountId", + table: "Project"); + + migrationBuilder.DropColumn( + name: "CreatedByServiceAccountId", + table: "Project"); + + migrationBuilder.Sql(@" + ALTER TABLE ""AccessPolicy"" + DROP CONSTRAINT IF EXISTS ""CK_AccessPolicy_ManageImpliesWrite"", + DROP CONSTRAINT IF EXISTS ""CK_AccessPolicy_WriteImpliesRead"""); + + migrationBuilder.DropColumn( + name: "Manage", + table: "AccessPolicy"); + } +} diff --git a/util/PostgresMigrations/Migrations/DatabaseContextModelSnapshot.cs b/util/PostgresMigrations/Migrations/DatabaseContextModelSnapshot.cs index 56088da8f50f..340f4a990ce1 100644 --- a/util/PostgresMigrations/Migrations/DatabaseContextModelSnapshot.cs +++ b/util/PostgresMigrations/Migrations/DatabaseContextModelSnapshot.cs @@ -2240,6 +2240,10 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Property("Write") .HasColumnType("boolean"); + b.Property("Manage") + .HasColumnType("boolean") + .HasDefaultValue(false); + b.HasKey("Id") .HasAnnotation("SqlServer:Clustered", true); @@ -2319,9 +2323,15 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Property("RevisionDate") .HasColumnType("timestamp with time zone"); + b.Property("CreatedByServiceAccountId") + .HasColumnType("uuid"); + b.HasKey("Id") .HasAnnotation("SqlServer:Clustered", true); + b.HasIndex("CreatedByServiceAccountId") + .HasAnnotation("SqlServer:Clustered", false); + b.HasIndex("DeletedDate") .HasAnnotation("SqlServer:Clustered", false); @@ -3238,6 +3248,11 @@ protected override void BuildModel(ModelBuilder modelBuilder) modelBuilder.Entity("Bit.Infrastructure.EntityFramework.SecretsManager.Models.Project", b => { + b.HasOne("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", "CreatedByServiceAccount") + .WithMany() + .HasForeignKey("CreatedByServiceAccountId") + .OnDelete(DeleteBehavior.SetNull); + b.HasOne("Bit.Infrastructure.EntityFramework.AdminConsole.Models.Organization", "Organization") .WithMany() .HasForeignKey("OrganizationId") diff --git a/util/SqliteMigrations/Migrations/20260219000000_AddManagePermissionAndCreatedByServiceAccount.cs b/util/SqliteMigrations/Migrations/20260219000000_AddManagePermissionAndCreatedByServiceAccount.cs new file mode 100644 index 000000000000..dbce4f7eb15e --- /dev/null +++ b/util/SqliteMigrations/Migrations/20260219000000_AddManagePermissionAndCreatedByServiceAccount.cs @@ -0,0 +1,83 @@ +using System; +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace Bit.SqliteMigrations.Migrations; + +/// +public partial class AddManagePermissionAndCreatedByServiceAccount : Migration +{ + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.AddColumn( + name: "Manage", + table: "AccessPolicy", + type: "INTEGER", + nullable: false, + defaultValue: false); + + // Backfill: preserve current behavior — user and group policies with Write=1 get Manage=1 + // Service account policies remain Manage=false per spec + migrationBuilder.Sql(@" + UPDATE ""AccessPolicy"" + SET ""Manage"" = 1 + WHERE ""Write"" = 1 + AND ""Discriminator"" IN ( + 'user_project', + 'user_service_account', + 'user_secret', + 'group_project', + 'group_service_account', + 'group_secret' + )"); + + // SQLite does not support ALTER TABLE ... ADD CONSTRAINT. + // Check constraints on existing tables require a full table rebuild (12-step SQLite procedure), + // which is not used in this project. The equivalent constraints are: + // CK_AccessPolicy_ManageImpliesWrite: "Manage" = 0 OR "Write" = 1 + // CK_AccessPolicy_WriteImpliesRead: "Write" = 0 OR "Read" = 1 + // These are enforced via HasCheckConstraint in AccessPolicyEntityTypeConfiguration, + // which applies them at database creation time for SQLite. + + migrationBuilder.AddColumn( + name: "CreatedByServiceAccountId", + table: "Project", + type: "TEXT", + nullable: true); + + migrationBuilder.CreateIndex( + name: "IX_Project_CreatedByServiceAccountId", + table: "Project", + column: "CreatedByServiceAccountId"); + + migrationBuilder.AddForeignKey( + name: "FK_Project_ServiceAccount_CreatedByServiceAccountId", + table: "Project", + column: "CreatedByServiceAccountId", + principalTable: "ServiceAccount", + principalColumn: "Id", + onDelete: ReferentialAction.SetNull); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.DropForeignKey( + name: "FK_Project_ServiceAccount_CreatedByServiceAccountId", + table: "Project"); + + migrationBuilder.DropIndex( + name: "IX_Project_CreatedByServiceAccountId", + table: "Project"); + + migrationBuilder.DropColumn( + name: "CreatedByServiceAccountId", + table: "Project"); + + migrationBuilder.DropColumn( + name: "Manage", + table: "AccessPolicy"); + } +} diff --git a/util/SqliteMigrations/Migrations/DatabaseContextModelSnapshot.cs b/util/SqliteMigrations/Migrations/DatabaseContextModelSnapshot.cs index bfcfe6e26aa2..c567532cfa5b 100644 --- a/util/SqliteMigrations/Migrations/DatabaseContextModelSnapshot.cs +++ b/util/SqliteMigrations/Migrations/DatabaseContextModelSnapshot.cs @@ -2223,6 +2223,10 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Property("Write") .HasColumnType("INTEGER"); + b.Property("Manage") + .HasColumnType("INTEGER") + .HasDefaultValue(false); + b.HasKey("Id") .HasAnnotation("SqlServer:Clustered", true); @@ -2302,9 +2306,15 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Property("RevisionDate") .HasColumnType("TEXT"); + b.Property("CreatedByServiceAccountId") + .HasColumnType("TEXT"); + b.HasKey("Id") .HasAnnotation("SqlServer:Clustered", true); + b.HasIndex("CreatedByServiceAccountId") + .HasAnnotation("SqlServer:Clustered", false); + b.HasIndex("DeletedDate") .HasAnnotation("SqlServer:Clustered", false); @@ -3221,6 +3231,11 @@ protected override void BuildModel(ModelBuilder modelBuilder) modelBuilder.Entity("Bit.Infrastructure.EntityFramework.SecretsManager.Models.Project", b => { + b.HasOne("Bit.Infrastructure.EntityFramework.SecretsManager.Models.ServiceAccount", "CreatedByServiceAccount") + .WithMany() + .HasForeignKey("CreatedByServiceAccountId") + .OnDelete(DeleteBehavior.SetNull); + b.HasOne("Bit.Infrastructure.EntityFramework.AdminConsole.Models.Organization", "Organization") .WithMany() .HasForeignKey("OrganizationId")