[SM-909] Add service-account people access policy management endpoints (#3324)

* refactoring replace logic * model for policies + authz handler + unit tests * update AP repository * add new endpoints to controller * update unit tests and integration tests --------- Co-authored-by: cd-bitwarden <106776772+cd-bitwarden@users.noreply.github.com>
2026-01-07 11:03:37 +00:00 · 2023-12-07 15:35:16 -06:00
parent a589af3588
commit f9232bcbb0
19 changed files with 1154 additions and 626 deletions
--- a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectPeopleAccessPoliciesAuthorizationHandler.cs
+++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ProjectPeopleAccessPoliciesAuthorizationHandler.cs
@@ -1,9 +1,8 @@
-using Bit.Core.AdminConsole.Repositories;
-using Bit.Core.Context;
+using Bit.Core.Context;
 using Bit.Core.Enums;
-using Bit.Core.Repositories;
 using Bit.Core.SecretsManager.AuthorizationRequirements;
 using Bit.Core.SecretsManager.Models.Data;
+using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces;
 using Bit.Core.SecretsManager.Queries.Interfaces;
 using Bit.Core.SecretsManager.Repositories;
 using Microsoft.AspNetCore.Authorization;
@@ -11,25 +10,23 @@ using Microsoft.AspNetCore.Authorization;
 namespace Bit.Commercial.Core.SecretsManager.AuthorizationHandlers.AccessPolicies;

 public class
-    ProjectPeopleAccessPoliciesAuthorizationHandler : AuthorizationHandler<ProjectPeopleAccessPoliciesOperationRequirement,
+    ProjectPeopleAccessPoliciesAuthorizationHandler : AuthorizationHandler<
+        ProjectPeopleAccessPoliciesOperationRequirement,
        ProjectPeopleAccessPolicies>
 {
    private readonly IAccessClientQuery _accessClientQuery;
    private readonly ICurrentContext _currentContext;
-    private readonly IGroupRepository _groupRepository;
-    private readonly IOrganizationUserRepository _organizationUserRepository;
    private readonly IProjectRepository _projectRepository;
+    private readonly ISameOrganizationQuery _sameOrganizationQuery;

    public ProjectPeopleAccessPoliciesAuthorizationHandler(ICurrentContext currentContext,
        IAccessClientQuery accessClientQuery,
-        IGroupRepository groupRepository,
-        IOrganizationUserRepository organizationUserRepository,
+        ISameOrganizationQuery sameOrganizationQuery,
        IProjectRepository projectRepository)
    {
        _currentContext = currentContext;
        _accessClientQuery = accessClientQuery;
-        _groupRepository = groupRepository;
-        _organizationUserRepository = organizationUserRepository;
+        _sameOrganizationQuery = sameOrganizationQuery;
        _projectRepository = projectRepository;
    }

@@ -71,9 +68,7 @@ public class
            if (resource.UserAccessPolicies != null && resource.UserAccessPolicies.Any())
            {
                var orgUserIds = resource.UserAccessPolicies.Select(ap => ap.OrganizationUserId!.Value).ToList();
-                var users = await _organizationUserRepository.GetManyAsync(orgUserIds);
-                if (users.Any(user => user.OrganizationId != resource.OrganizationId) ||
-                    users.Count != orgUserIds.Count)
+                if (!await _sameOrganizationQuery.OrgUsersInTheSameOrgAsync(orgUserIds, resource.OrganizationId))
                {
                    return;
                }
@@ -82,9 +77,7 @@ public class
            if (resource.GroupAccessPolicies != null && resource.GroupAccessPolicies.Any())
            {
                var groupIds = resource.GroupAccessPolicies.Select(ap => ap.GroupId!.Value).ToList();
-                var groups = await _groupRepository.GetManyByManyIds(groupIds);
-                if (groups.Any(group => group.OrganizationId != resource.OrganizationId) ||
-                    groups.Count != groupIds.Count)
+                if (!await _sameOrganizationQuery.GroupsInTheSameOrgAsync(groupIds, resource.OrganizationId))
                {
                    return;
                }
--- a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountPeopleAccessPoliciesAuthorizationHandler.cs
+++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/AccessPolicies/ServiceAccountPeopleAccessPoliciesAuthorizationHandler.cs
@@ -0,0 +1,89 @@
+using Bit.Core.Context;
+using Bit.Core.Enums;
+using Bit.Core.SecretsManager.AuthorizationRequirements;
+using Bit.Core.SecretsManager.Models.Data;
+using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces;
+using Bit.Core.SecretsManager.Queries.Interfaces;
+using Bit.Core.SecretsManager.Repositories;
+using Microsoft.AspNetCore.Authorization;
+
+namespace Bit.Commercial.Core.SecretsManager.AuthorizationHandlers.AccessPolicies;
+
+public class
+    ServiceAccountPeopleAccessPoliciesAuthorizationHandler : AuthorizationHandler<
+        ServiceAccountPeopleAccessPoliciesOperationRequirement,
+        ServiceAccountPeopleAccessPolicies>
+{
+    private readonly IAccessClientQuery _accessClientQuery;
+    private readonly ICurrentContext _currentContext;
+    private readonly ISameOrganizationQuery _sameOrganizationQuery;
+    private readonly IServiceAccountRepository _serviceAccountRepository;
+
+    public ServiceAccountPeopleAccessPoliciesAuthorizationHandler(ICurrentContext currentContext,
+        IAccessClientQuery accessClientQuery,
+        ISameOrganizationQuery sameOrganizationQuery,
+        IServiceAccountRepository serviceAccountRepository)
+    {
+        _currentContext = currentContext;
+        _accessClientQuery = accessClientQuery;
+        _sameOrganizationQuery = sameOrganizationQuery;
+        _serviceAccountRepository = serviceAccountRepository;
+    }
+
+    protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context,
+        ServiceAccountPeopleAccessPoliciesOperationRequirement requirement,
+        ServiceAccountPeopleAccessPolicies resource)
+    {
+        if (!_currentContext.AccessSecretsManager(resource.OrganizationId))
+        {
+            return;
+        }
+
+        // Only users and admins should be able to manipulate access policies
+        var (accessClient, userId) =
+            await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId);
+        if (accessClient != AccessClientType.User && accessClient != AccessClientType.NoAccessCheck)
+        {
+            return;
+        }
+
+        switch (requirement)
+        {
+            case not null when requirement == ServiceAccountPeopleAccessPoliciesOperations.Replace:
+                await CanReplaceServiceAccountPeopleAsync(context, requirement, resource, accessClient, userId);
+                break;
+            default:
+                throw new ArgumentException("Unsupported operation requirement type provided.",
+                    nameof(requirement));
+        }
+    }
+
+    private async Task CanReplaceServiceAccountPeopleAsync(AuthorizationHandlerContext context,
+        ServiceAccountPeopleAccessPoliciesOperationRequirement requirement, ServiceAccountPeopleAccessPolicies resource,
+        AccessClientType accessClient, Guid userId)
+    {
+        var access = await _serviceAccountRepository.AccessToServiceAccountAsync(resource.Id, userId, accessClient);
+        if (access.Write)
+        {
+            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);
+        }
+    }
+}
--- a/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/AccessPolicies/SameOrganizationQuery.cs
+++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/AccessPolicies/SameOrganizationQuery.cs
@@ -0,0 +1,32 @@
+using Bit.Core.AdminConsole.Repositories;
+using Bit.Core.Repositories;
+using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces;
+
+namespace Bit.Commercial.Core.SecretsManager.Queries.AccessPolicies;
+
+public class SameOrganizationQuery : ISameOrganizationQuery
+{
+    private readonly IGroupRepository _groupRepository;
+    private readonly IOrganizationUserRepository _organizationUserRepository;
+
+    public SameOrganizationQuery(IOrganizationUserRepository organizationUserRepository,
+        IGroupRepository groupRepository)
+    {
+        _organizationUserRepository = organizationUserRepository;
+        _groupRepository = groupRepository;
+    }
+
+    public async Task<bool> OrgUsersInTheSameOrgAsync(List<Guid> organizationUserIds, Guid organizationId)
+    {
+        var users = await _organizationUserRepository.GetManyAsync(organizationUserIds);
+        return users.All(user => user.OrganizationId == organizationId) &&
+               users.Count == organizationUserIds.Count;
+    }
+
+    public async Task<bool> GroupsInTheSameOrgAsync(List<Guid> groupIds, Guid organizationId)
+    {
+        var groups = await _groupRepository.GetManyByManyIds(groupIds);
+        return groups.All(group => group.OrganizationId == organizationId) &&
+               groups.Count == groupIds.Count;
+    }
+}
--- a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs
+++ b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs
@@ -10,6 +10,7 @@ using Bit.Commercial.Core.SecretsManager.Commands.Secrets;
 using Bit.Commercial.Core.SecretsManager.Commands.ServiceAccounts;
 using Bit.Commercial.Core.SecretsManager.Commands.Trash;
 using Bit.Commercial.Core.SecretsManager.Queries;
+using Bit.Commercial.Core.SecretsManager.Queries.AccessPolicies;
 using Bit.Commercial.Core.SecretsManager.Queries.Projects;
 using Bit.Commercial.Core.SecretsManager.Queries.ServiceAccounts;
 using Bit.Core.SecretsManager.Commands.AccessPolicies.Interfaces;
@@ -19,6 +20,7 @@ using Bit.Core.SecretsManager.Commands.Projects.Interfaces;
 using Bit.Core.SecretsManager.Commands.Secrets.Interfaces;
 using Bit.Core.SecretsManager.Commands.ServiceAccounts.Interfaces;
 using Bit.Core.SecretsManager.Commands.Trash.Interfaces;
+using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces;
 using Bit.Core.SecretsManager.Queries.Interfaces;
 using Bit.Core.SecretsManager.Queries.Projects.Interfaces;
 using Bit.Core.SecretsManager.Queries.ServiceAccounts.Interfaces;
@@ -36,8 +38,10 @@ public static class SecretsManagerCollectionExtensions
        services.AddScoped<IAuthorizationHandler, ServiceAccountAuthorizationHandler>();
        services.AddScoped<IAuthorizationHandler, AccessPolicyAuthorizationHandler>();
        services.AddScoped<IAuthorizationHandler, ProjectPeopleAccessPoliciesAuthorizationHandler>();
+        services.AddScoped<IAuthorizationHandler, ServiceAccountPeopleAccessPoliciesAuthorizationHandler>();
        services.AddScoped<IAccessClientQuery, AccessClientQuery>();
        services.AddScoped<IMaxProjectsQuery, MaxProjectsQuery>();
+        services.AddScoped<ISameOrganizationQuery, SameOrganizationQuery>();
        services.AddScoped<IServiceAccountSecretsDetailsQuery, ServiceAccountSecretsDetailsQuery>();
        services.AddScoped<ICreateSecretCommand, CreateSecretCommand>();
        services.AddScoped<IUpdateSecretCommand, UpdateSecretCommand>();
--- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/AccessPolicyRepository.cs
+++ b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/AccessPolicyRepository.cs
@@ -183,28 +183,6 @@ public class AccessPolicyRepository : BaseEntityFrameworkRepository, IAccessPoli
        return entities.Select(e => MapToCore(e.ap, e.CurrentUserInGroup));
    }

-    public async Task<IEnumerable<Core.SecretsManager.Entities.BaseAccessPolicy>> GetManyByGrantedServiceAccountIdAsync(Guid id, Guid userId)
-    {
-        using var scope = ServiceScopeFactory.CreateScope();
-        var dbContext = GetDatabaseContext(scope);
-
-        var entities = await dbContext.AccessPolicies.Where(ap =>
-                ((UserServiceAccountAccessPolicy)ap).GrantedServiceAccountId == id ||
-                ((GroupServiceAccountAccessPolicy)ap).GrantedServiceAccountId == id)
-            .Include(ap => ((UserServiceAccountAccessPolicy)ap).OrganizationUser.User)
-            .Include(ap => ((GroupServiceAccountAccessPolicy)ap).Group)
-            .Select(ap => new
-            {
-                ap,
-                CurrentUserInGroup = ap is GroupServiceAccountAccessPolicy &&
-                                     ((GroupServiceAccountAccessPolicy)ap).Group.GroupUsers.Any(g =>
-                                         g.OrganizationUser.User.Id == userId),
-            })
-            .ToListAsync();
-
-        return entities.Select(e => MapToCore(e.ap, e.CurrentUserInGroup));
-    }
-
    public async Task DeleteAsync(Guid id)
    {
        using var scope = ServiceScopeFactory.CreateScope();
@@ -352,6 +330,81 @@ public class AccessPolicyRepository : BaseEntityFrameworkRepository, IAccessPoli
        return await GetPeoplePoliciesByGrantedProjectIdAsync(peopleAccessPolicies.Id, userId);
    }

+    public async Task<IEnumerable<Core.SecretsManager.Entities.BaseAccessPolicy>>
+        GetPeoplePoliciesByGrantedServiceAccountIdAsync(Guid id, Guid userId)
+    {
+        using var scope = ServiceScopeFactory.CreateScope();
+        var dbContext = GetDatabaseContext(scope);
+
+        var entities = await dbContext.AccessPolicies.Where(ap =>
+                ((UserServiceAccountAccessPolicy)ap).GrantedServiceAccountId == id ||
+                ((GroupServiceAccountAccessPolicy)ap).GrantedServiceAccountId == id)
+            .Include(ap => ((UserServiceAccountAccessPolicy)ap).OrganizationUser.User)
+            .Include(ap => ((GroupServiceAccountAccessPolicy)ap).Group)
+            .Select(ap => new
+            {
+                ap,
+                CurrentUserInGroup = ap is GroupServiceAccountAccessPolicy &&
+                                     ((GroupServiceAccountAccessPolicy)ap).Group.GroupUsers.Any(g =>
+                                         g.OrganizationUser.UserId == userId)
+            })
+            .ToListAsync();
+
+        return entities.Select(e => MapToCore(e.ap, e.CurrentUserInGroup));
+    }
+
+    public async Task<IEnumerable<Core.SecretsManager.Entities.BaseAccessPolicy>> ReplaceServiceAccountPeopleAsync(
+        ServiceAccountPeopleAccessPolicies peopleAccessPolicies, Guid userId)
+    {
+        using var scope = ServiceScopeFactory.CreateScope();
+        var dbContext = GetDatabaseContext(scope);
+        var peoplePolicyEntities = await dbContext.AccessPolicies.Where(ap =>
+            ((UserServiceAccountAccessPolicy)ap).GrantedServiceAccountId == peopleAccessPolicies.Id ||
+            ((GroupServiceAccountAccessPolicy)ap).GrantedServiceAccountId == peopleAccessPolicies.Id).ToListAsync();
+
+        var userPolicyEntities =
+            peoplePolicyEntities.Where(ap => ap.GetType() == typeof(UserServiceAccountAccessPolicy)).ToList();
+        var groupPolicyEntities =
+            peoplePolicyEntities.Where(ap => ap.GetType() == typeof(GroupServiceAccountAccessPolicy)).ToList();
+
+
+        if (peopleAccessPolicies.UserAccessPolicies == null || !peopleAccessPolicies.UserAccessPolicies.Any())
+        {
+            dbContext.RemoveRange(userPolicyEntities);
+        }
+        else
+        {
+            foreach (var userPolicyEntity in userPolicyEntities.Where(entity =>
+                         peopleAccessPolicies.UserAccessPolicies.All(ap =>
+                             ((Core.SecretsManager.Entities.UserServiceAccountAccessPolicy)ap).OrganizationUserId !=
+                             ((UserServiceAccountAccessPolicy)entity).OrganizationUserId)))
+            {
+                dbContext.Remove(userPolicyEntity);
+            }
+        }
+
+        if (peopleAccessPolicies.GroupAccessPolicies == null || !peopleAccessPolicies.GroupAccessPolicies.Any())
+        {
+            dbContext.RemoveRange(groupPolicyEntities);
+        }
+        else
+        {
+            foreach (var groupPolicyEntity in groupPolicyEntities.Where(entity =>
+                         peopleAccessPolicies.GroupAccessPolicies.All(ap =>
+                             ((Core.SecretsManager.Entities.GroupServiceAccountAccessPolicy)ap).GroupId !=
+                             ((GroupServiceAccountAccessPolicy)entity).GroupId)))
+            {
+                dbContext.Remove(groupPolicyEntity);
+            }
+        }
+
+        await UpsertPeoplePoliciesAsync(dbContext,
+            peopleAccessPolicies.ToBaseAccessPolicies().Select(MapToEntity).ToList(), userPolicyEntities,
+            groupPolicyEntities);
+        await dbContext.SaveChangesAsync();
+        return await GetPeoplePoliciesByGrantedServiceAccountIdAsync(peopleAccessPolicies.Id, userId);
+    }
+
    private static async Task UpsertPeoplePoliciesAsync(DatabaseContext dbContext,
        List<BaseAccessPolicy> policies, IReadOnlyCollection<AccessPolicy> userPolicyEntities,
        IReadOnlyCollection<AccessPolicy> groupPolicyEntities)