[SM-707] Refactor authorization for Access Policy Commands (#2905)

* Extract authorization from access policy commands

* Use auto mapper to ignore unwanted properties

---------

src/Api/SecretsManager/Controllers/AccessPoliciesController.cs

@@ -5,6 +5,7 @@ using Bit.Core.Context;
 using Bit.Core.Enums;
 using Bit.Core.Exceptions;
 using Bit.Core.Repositories;
+using Bit.Core.SecretsManager.AuthorizationRequirements;
 using Bit.Core.SecretsManager.Commands.AccessPolicies.Interfaces;
 using Bit.Core.SecretsManager.Entities;
 using Bit.Core.SecretsManager.Repositories;
@@ -31,8 +32,10 @@ public class AccessPoliciesController : Controller
     private readonly IServiceAccountRepository _serviceAccountRepository;
     private readonly IUpdateAccessPolicyCommand _updateAccessPolicyCommand;
     private readonly IUserService _userService;
+    private readonly IAuthorizationService _authorizationService;
 
     public AccessPoliciesController(
+        IAuthorizationService authorizationService,
         IUserService userService,
         ICurrentContext currentContext,
         IAccessPolicyRepository accessPolicyRepository,
@@ -44,6 +47,7 @@ public class AccessPoliciesController : Controller
         IDeleteAccessPolicyCommand deleteAccessPolicyCommand,
         IUpdateAccessPolicyCommand updateAccessPolicyCommand)
     {
+        _authorizationService = authorizationService;
         _userService = userService;
         _currentContext = currentContext;
         _serviceAccountRepository = serviceAccountRepository;
@@ -71,9 +75,17 @@ public class AccessPoliciesController : Controller
             throw new NotFoundException();
         }
 
-        var (accessClient, userId) = await GetAccessClientTypeAsync(project.OrganizationId);
-        var policies = request.ToBaseAccessPoliciesForProject(id);
-        var results = await _createAccessPoliciesCommand.CreateManyAsync(policies, userId, accessClient);
+        var policies = request.ToBaseAccessPoliciesForProject(id, project.OrganizationId);
+        foreach (var policy in policies)
+        {
+            var authorizationResult = await _authorizationService.AuthorizeAsync(User, policy, AccessPolicyOperations.Create);
+            if (!authorizationResult.Succeeded)
+            {
+                throw new NotFoundException();
+            }
+        }
+
+        var results = await _createAccessPoliciesCommand.CreateManyAsync(policies);
         return new ProjectAccessPoliciesResponseModel(results);
     }
 
@@ -102,9 +114,17 @@ public class AccessPoliciesController : Controller
             throw new NotFoundException();
         }
 
-        var (accessClient, userId) = await GetAccessClientTypeAsync(serviceAccount.OrganizationId);
-        var policies = request.ToBaseAccessPoliciesForServiceAccount(id);
-        var results = await _createAccessPoliciesCommand.CreateManyAsync(policies, userId, accessClient);
+        var policies = request.ToBaseAccessPoliciesForServiceAccount(id, serviceAccount.OrganizationId);
+        foreach (var policy in policies)
+        {
+            var authorizationResult = await _authorizationService.AuthorizeAsync(User, policy, AccessPolicyOperations.Create);
+            if (!authorizationResult.Succeeded)
+            {
+                throw new NotFoundException();
+            }
+        }
+
+        var results = await _createAccessPoliciesCommand.CreateManyAsync(policies);
         return new ServiceAccountAccessPoliciesResponseModel(results);
     }
 
@@ -128,17 +148,29 @@ public class AccessPoliciesController : Controller
             throw new BadRequestException($"Can process no more than {_maxBulkCreation} creation requests at once.");
         }
 
+        if (requests.Count != requests.DistinctBy(request => request.GrantedId).Count())
+        {
+            throw new BadRequestException("Resources must be unique");
+        }
+
         var serviceAccount = await _serviceAccountRepository.GetByIdAsync(id);
         if (serviceAccount == null)
         {
             throw new NotFoundException();
         }
 
-        var (accessClient, userId) = await GetAccessClientTypeAsync(serviceAccount.OrganizationId);
-        var policies = requests.Select(request => request.ToServiceAccountProjectAccessPolicy(id));
+        var policies = requests.Select(request => request.ToServiceAccountProjectAccessPolicy(id, serviceAccount.OrganizationId)).ToList();
+        foreach (var policy in policies)
+        {
+            var authorizationResult = await _authorizationService.AuthorizeAsync(User, policy, AccessPolicyOperations.Create);
+            if (!authorizationResult.Succeeded)
+            {
+                throw new NotFoundException();
+            }
+        }
+
         var results =
-            await _createAccessPoliciesCommand.CreateManyAsync(new List<BaseAccessPolicy>(policies), userId,
-                accessClient);
+            await _createAccessPoliciesCommand.CreateManyAsync(new List<BaseAccessPolicy>(policies));
         var responses = results.Select(ap =>
             new ServiceAccountProjectAccessPolicyResponseModel((ServiceAccountProjectAccessPolicy)ap));
         return new ListResponseModel<ServiceAccountProjectAccessPolicyResponseModel>(responses);
@@ -165,8 +197,15 @@ public class AccessPoliciesController : Controller
     public async Task<BaseAccessPolicyResponseModel> UpdateAccessPolicyAsync([FromRoute] Guid id,
         [FromBody] AccessPolicyUpdateRequest request)
     {
-        var userId = _userService.GetProperUserId(User).Value;
-        var result = await _updateAccessPolicyCommand.UpdateAsync(id, request.Read, request.Write, userId);
+        var ap = await _accessPolicyRepository.GetByIdAsync(id);
+        var authorizationResult =
+            await _authorizationService.AuthorizeAsync(User, ap, AccessPolicyOperations.Update);
+        if (!authorizationResult.Succeeded)
+        {
+            throw new NotFoundException();
+        }
+
+        var result = await _updateAccessPolicyCommand.UpdateAsync(id, request.Read, request.Write);
 
         return result switch
         {
@@ -185,8 +224,15 @@ public class AccessPoliciesController : Controller
     [HttpDelete("{id}")]
     public async Task DeleteAccessPolicyAsync([FromRoute] Guid id)
     {
-        var userId = _userService.GetProperUserId(User).Value;
-        await _deleteAccessPolicyCommand.DeleteAsync(id, userId);
+        var ap = await _accessPolicyRepository.GetByIdAsync(id);
+        var authorizationResult =
+            await _authorizationService.AuthorizeAsync(User, ap, AccessPolicyOperations.Delete);
+        if (!authorizationResult.Succeeded)
+        {
+            throw new NotFoundException();
+        }
+
+        await _deleteAccessPolicyCommand.DeleteAsync(id);
     }
 
     [HttpGet("/organizations/{id}/access-policies/people/potential-grantees")]

src/Api/SecretsManager/Models/Request/AccessPoliciesCreateRequest.cs

@@ -7,13 +7,36 @@ namespace Bit.Api.SecretsManager.Models.Request;
 
 public class AccessPoliciesCreateRequest
 {
+    private static void CheckForDistinctAccessPolicies(IReadOnlyCollection<BaseAccessPolicy> accessPolicies)
+    {
+        var distinctAccessPolicies = accessPolicies.DistinctBy(baseAccessPolicy =>
+        {
+            return baseAccessPolicy switch
+            {
+                UserProjectAccessPolicy ap => new Tuple<Guid?, Guid?>(ap.OrganizationUserId, ap.GrantedProjectId),
+                GroupProjectAccessPolicy ap => new Tuple<Guid?, Guid?>(ap.GroupId, ap.GrantedProjectId),
+                ServiceAccountProjectAccessPolicy ap => new Tuple<Guid?, Guid?>(ap.ServiceAccountId,
+                    ap.GrantedProjectId),
+                UserServiceAccountAccessPolicy ap => new Tuple<Guid?, Guid?>(ap.OrganizationUserId,
+                    ap.GrantedServiceAccountId),
+                GroupServiceAccountAccessPolicy ap => new Tuple<Guid?, Guid?>(ap.GroupId, ap.GrantedServiceAccountId),
+                _ => throw new ArgumentException("Unsupported access policy type provided.", nameof(baseAccessPolicy)),
+            };
+        }).ToList();
+
+        if (accessPolicies.Count != distinctAccessPolicies.Count)
+        {
+            throw new BadRequestException("Resources must be unique");
+        }
+    }
+
     public IEnumerable<AccessPolicyRequest>? UserAccessPolicyRequests { get; set; }
 
     public IEnumerable<AccessPolicyRequest>? GroupAccessPolicyRequests { get; set; }
 
     public IEnumerable<AccessPolicyRequest>? ServiceAccountAccessPolicyRequests { get; set; }
 
-    public List<BaseAccessPolicy> ToBaseAccessPoliciesForProject(Guid grantedProjectId)
+    public List<BaseAccessPolicy> ToBaseAccessPoliciesForProject(Guid grantedProjectId, Guid organizationId)
     {
         if (UserAccessPolicyRequests == null && GroupAccessPolicyRequests == null && ServiceAccountAccessPolicyRequests == null)
         {
@@ -21,13 +44,13 @@ public class AccessPoliciesCreateRequest
         }
 
         var userAccessPolicies = UserAccessPolicyRequests?
-            .Select(x => x.ToUserProjectAccessPolicy(grantedProjectId)).ToList();
+            .Select(x => x.ToUserProjectAccessPolicy(grantedProjectId, organizationId)).ToList();
 
         var groupAccessPolicies = GroupAccessPolicyRequests?
-            .Select(x => x.ToGroupProjectAccessPolicy(grantedProjectId)).ToList();
+            .Select(x => x.ToGroupProjectAccessPolicy(grantedProjectId, organizationId)).ToList();
 
         var serviceAccountAccessPolicies = ServiceAccountAccessPolicyRequests?
-            .Select(x => x.ToServiceAccountProjectAccessPolicy(grantedProjectId)).ToList();
+            .Select(x => x.ToServiceAccountProjectAccessPolicy(grantedProjectId, organizationId)).ToList();
 
         var policies = new List<BaseAccessPolicy>();
         if (userAccessPolicies != null)
@@ -44,10 +67,12 @@ public class AccessPoliciesCreateRequest
         {
             policies.AddRange(serviceAccountAccessPolicies);
         }
+
+        CheckForDistinctAccessPolicies(policies);
         return policies;
     }
 
-    public List<BaseAccessPolicy> ToBaseAccessPoliciesForServiceAccount(Guid grantedServiceAccountId)
+    public List<BaseAccessPolicy> ToBaseAccessPoliciesForServiceAccount(Guid grantedServiceAccountId, Guid organizationId)
     {
         if (UserAccessPolicyRequests == null && GroupAccessPolicyRequests == null)
         {
@@ -55,10 +80,10 @@ public class AccessPoliciesCreateRequest
         }
 
         var userAccessPolicies = UserAccessPolicyRequests?
-            .Select(x => x.ToUserServiceAccountAccessPolicy(grantedServiceAccountId)).ToList();
+            .Select(x => x.ToUserServiceAccountAccessPolicy(grantedServiceAccountId, organizationId)).ToList();
 
         var groupAccessPolicies = GroupAccessPolicyRequests?
-            .Select(x => x.ToGroupServiceAccountAccessPolicy(grantedServiceAccountId)).ToList();
+            .Select(x => x.ToGroupServiceAccountAccessPolicy(grantedServiceAccountId, organizationId)).ToList();
 
         var policies = new List<BaseAccessPolicy>();
         if (userAccessPolicies != null)
@@ -70,6 +95,8 @@ public class AccessPoliciesCreateRequest
         {
             policies.AddRange(groupAccessPolicies);
         }
+
+        CheckForDistinctAccessPolicies(policies);
         return policies;
     }
 
@@ -105,47 +132,52 @@ public class AccessPolicyRequest
     [Required]
     public bool Write { get; set; }
 
-    public UserProjectAccessPolicy ToUserProjectAccessPolicy(Guid projectId) =>
+    public UserProjectAccessPolicy ToUserProjectAccessPolicy(Guid projectId, Guid organizationId) =>
         new()
         {
             OrganizationUserId = GranteeId,
             GrantedProjectId = projectId,
+            GrantedProject = new Project { OrganizationId = organizationId, Id = projectId },
             Read = Read,
             Write = Write
         };
 
-    public GroupProjectAccessPolicy ToGroupProjectAccessPolicy(Guid projectId) =>
+    public GroupProjectAccessPolicy ToGroupProjectAccessPolicy(Guid projectId, Guid organizationId) =>
         new()
         {
             GroupId = GranteeId,
             GrantedProjectId = projectId,
+            GrantedProject = new Project { OrganizationId = organizationId, Id = projectId },
             Read = Read,
             Write = Write
         };
 
-    public ServiceAccountProjectAccessPolicy ToServiceAccountProjectAccessPolicy(Guid projectId) =>
+    public ServiceAccountProjectAccessPolicy ToServiceAccountProjectAccessPolicy(Guid projectId, Guid organizationId) =>
         new()
         {
             ServiceAccountId = GranteeId,
             GrantedProjectId = projectId,
+            GrantedProject = new Project { OrganizationId = organizationId, Id = projectId },
             Read = Read,
             Write = Write
         };
 
-    public UserServiceAccountAccessPolicy ToUserServiceAccountAccessPolicy(Guid id) =>
+    public UserServiceAccountAccessPolicy ToUserServiceAccountAccessPolicy(Guid id, Guid organizationId) =>
         new()
         {
             OrganizationUserId = GranteeId,
             GrantedServiceAccountId = id,
+            GrantedServiceAccount = new ServiceAccount() { OrganizationId = organizationId, Id = id },
             Read = Read,
             Write = Write
         };
 
-    public GroupServiceAccountAccessPolicy ToGroupServiceAccountAccessPolicy(Guid id) =>
+    public GroupServiceAccountAccessPolicy ToGroupServiceAccountAccessPolicy(Guid id, Guid organizationId) =>
         new()
         {
             GroupId = GranteeId,
             GrantedServiceAccountId = id,
+            GrantedServiceAccount = new ServiceAccount() { OrganizationId = organizationId, Id = id },
             Read = Read,
             Write = Write
         };

src/Api/SecretsManager/Models/Request/GrantedAccessPolicyRequest.cs

@@ -1,5 +1,6 @@
 using System.ComponentModel.DataAnnotations;
-using Bit.Core.SecretsManager.Entities;
+using Bit.Infrastructure.EntityFramework.SecretsManager.Models;
+using ServiceAccountProjectAccessPolicy = Bit.Core.SecretsManager.Entities.ServiceAccountProjectAccessPolicy;
 
 namespace Bit.Api.SecretsManager.Models.Request;
 
@@ -14,10 +15,11 @@ public class GrantedAccessPolicyRequest
     [Required]
     public bool Write { get; set; }
 
-    public ServiceAccountProjectAccessPolicy ToServiceAccountProjectAccessPolicy(Guid serviceAccountId) =>
+    public ServiceAccountProjectAccessPolicy ToServiceAccountProjectAccessPolicy(Guid serviceAccountId, Guid organizationId) =>
         new()
         {
             ServiceAccountId = serviceAccountId,
+            ServiceAccount = new ServiceAccount() { Id = serviceAccountId, OrganizationId = organizationId },
             GrantedProjectId = GrantedId,
             Read = Read,
             Write = Write,

src/Core/SecretsManager/AuthorizationRequirements/AccessPolicyOperationRequirement.cs

@@ -0,0 +1,14 @@
+using Microsoft.AspNetCore.Authorization.Infrastructure;
+
+namespace Bit.Core.SecretsManager.AuthorizationRequirements;
+
+public class AccessPolicyOperationRequirement : OperationAuthorizationRequirement
+{
+}
+
+public static class AccessPolicyOperations
+{
+    public static readonly AccessPolicyOperationRequirement Create = new() { Name = nameof(Create) };
+    public static readonly AccessPolicyOperationRequirement Update = new() { Name = nameof(Update) };
+    public static readonly AccessPolicyOperationRequirement Delete = new() { Name = nameof(Delete) };
+}

src/Core/SecretsManager/Commands/AccessPolicies/Interfaces/ICreateAccessPoliciesCommand.cs

@@ -1,9 +1,8 @@
-using Bit.Core.Enums;
-using Bit.Core.SecretsManager.Entities;
+using Bit.Core.SecretsManager.Entities;
 
 namespace Bit.Core.SecretsManager.Commands.AccessPolicies.Interfaces;
 
 public interface ICreateAccessPoliciesCommand
 {
-    Task<IEnumerable<BaseAccessPolicy>> CreateManyAsync(List<BaseAccessPolicy> accessPolicies, Guid userId, AccessClientType accessType);
+    Task<IEnumerable<BaseAccessPolicy>> CreateManyAsync(List<BaseAccessPolicy> accessPolicies);
 }

src/Core/SecretsManager/Commands/AccessPolicies/Interfaces/IDeleteAccessPolicyCommand.cs

@@ -2,5 +2,5 @@
 
 public interface IDeleteAccessPolicyCommand
 {
-    Task DeleteAsync(Guid id, Guid userId);
+    Task DeleteAsync(Guid id);
 }

src/Core/SecretsManager/Commands/AccessPolicies/Interfaces/IUpdateAccessPolicyCommand.cs

@@ -4,5 +4,5 @@ namespace Bit.Core.SecretsManager.Commands.AccessPolicies.Interfaces;
 
 public interface IUpdateAccessPolicyCommand
 {
-    public Task<BaseAccessPolicy> UpdateAsync(Guid id, bool read, bool write, Guid userId);
+    public Task<BaseAccessPolicy> UpdateAsync(Guid id, bool read, bool write);
 }

src/Infrastructure.EntityFramework/SecretsManager/Models/AccessPolicy.cs

@@ -12,13 +12,32 @@ public class AccessPolicyMapperProfile : Profile
 {
     public AccessPolicyMapperProfile()
     {
-        CreateMap<Core.SecretsManager.Entities.UserProjectAccessPolicy, UserProjectAccessPolicy>().ReverseMap()
+        CreateMap<Core.SecretsManager.Entities.UserProjectAccessPolicy, UserProjectAccessPolicy>()
+            .ForMember(dst => dst.GrantedProject, opt => opt.Ignore())
+            .ForMember(dst => dst.OrganizationUser, opt => opt.Ignore())
+            .ReverseMap()
             .ForMember(dst => dst.User, opt => opt.MapFrom(src => src.OrganizationUser.User));
-        CreateMap<Core.SecretsManager.Entities.UserServiceAccountAccessPolicy, UserServiceAccountAccessPolicy>().ReverseMap()
+
+        CreateMap<Core.SecretsManager.Entities.UserServiceAccountAccessPolicy, UserServiceAccountAccessPolicy>()
+            .ForMember(dst => dst.GrantedServiceAccount, opt => opt.Ignore())
+            .ForMember(dst => dst.OrganizationUser, opt => opt.Ignore())
+            .ReverseMap()
             .ForMember(dst => dst.User, opt => opt.MapFrom(src => src.OrganizationUser.User));
-        CreateMap<Core.SecretsManager.Entities.GroupProjectAccessPolicy, GroupProjectAccessPolicy>().ReverseMap();
-        CreateMap<Core.SecretsManager.Entities.GroupServiceAccountAccessPolicy, GroupServiceAccountAccessPolicy>().ReverseMap();
-        CreateMap<Core.SecretsManager.Entities.ServiceAccountProjectAccessPolicy, ServiceAccountProjectAccessPolicy>().ReverseMap();
+
+        CreateMap<Core.SecretsManager.Entities.GroupProjectAccessPolicy, GroupProjectAccessPolicy>()
+            .ForMember(dst => dst.GrantedProject, opt => opt.Ignore())
+            .ForMember(dst => dst.Group, opt => opt.Ignore())
+            .ReverseMap();
+
+        CreateMap<Core.SecretsManager.Entities.GroupServiceAccountAccessPolicy, GroupServiceAccountAccessPolicy>()
+            .ForMember(dst => dst.GrantedServiceAccount, opt => opt.Ignore())
+            .ForMember(dst => dst.Group, opt => opt.Ignore())
+            .ReverseMap();
+
+        CreateMap<Core.SecretsManager.Entities.ServiceAccountProjectAccessPolicy, ServiceAccountProjectAccessPolicy>()
+            .ForMember(dst => dst.GrantedProject, opt => opt.Ignore())
+            .ForMember(dst => dst.ServiceAccount, opt => opt.Ignore())
+            .ReverseMap();
     }
 }