[SM-567] Change how project permission is resolved (bitwarden#2791)

* Change how project permission is resolved * Fix tests --------- Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>
peckercode · Mar 9, 2023 · 6a6b15f · 6a6b15f
1 parent 15954fb
commit 6a6b15f
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 54 deletions.
diff --git a/...ommercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ProjectRepository.cs b/...ommercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ProjectRepository.cs
@@ -1,7 +1,6 @@
 using System.Linq.Expressions;
 using AutoMapper;
 using Bit.Core.Enums;
-using Bit.Core.SecretsManager.Models.Data;
 using Bit.Core.SecretsManager.Repositories;
 using Bit.Infrastructure.EntityFramework.Repositories;
 using Bit.Infrastructure.EntityFramework.SecretsManager.Models;
@@ -28,31 +27,6 @@ public ProjectRepository(IServiceScopeFactory serviceScopeFactory, IMapper mappe
         }
     }
 
-    public async Task<ProjectPermissionDetails> GetPermissionDetailsByIdAsync(Guid id, Guid userId)
-    {
-        using var scope = ServiceScopeFactory.CreateScope();
-        var dbContext = GetDatabaseContext(scope);
-
-        var project = await dbContext.Project
-            .Where(c => c.Id == id && c.DeletedDate == null)
-            .Select(p => new ProjectPermissionDetails
-            {
-                Id = p.Id,
-                OrganizationId = p.OrganizationId,
-                Name = p.Name,
-                CreationDate = p.CreationDate,
-                RevisionDate = p.RevisionDate,
-                DeletedDate = p.DeletedDate,
-                Read = p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Read)
-                       || p.GroupAccessPolicies.Any(ap =>
-                           ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Read)),
-                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)),
-            }).FirstOrDefaultAsync();
-        return project;
-    }
-
     public async Task<IEnumerable<Core.SecretsManager.Entities.Project>> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType)
     {
         using var scope = ServiceScopeFactory.CreateScope();
@@ -182,4 +156,37 @@ public async Task<bool> UserHasWriteAccessToProject(Guid id, Guid userId)
         await dbContext.SaveChangesAsync();
         return projects;
     }
+
+    public async Task<(bool Read, bool Write)> AccessToProjectAsync(Guid id, Guid userId, AccessClientType accessType)
+    {
+        using var scope = ServiceScopeFactory.CreateScope();
+        var dbContext = GetDatabaseContext(scope);
+
+        var projectQuery = dbContext.Project
+            .Where(s => s.Id == id);
+
+        var query = accessType switch
+        {
+            AccessClientType.NoAccessCheck => projectQuery.Select(_ => new { Read = true, Write = true }),
+            AccessClientType.User => projectQuery.Select(p => new
+            {
+                Read = p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Read)
+                       || p.GroupAccessPolicies.Any(ap =>
+                           ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Read)),
+                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)),
+            }),
+            AccessClientType.ServiceAccount => projectQuery.Select(p => new
+            {
+                Read = p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Read),
+                Write = p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Write),
+            }),
+            _ => projectQuery.Select(_ => new { Read = false, Write = false }),
+        };
+
+        var policy = await query.FirstOrDefaultAsync();
+
+        return (policy.Read, policy.Write);
+    }
 }
diff --git a/src/Api/SecretsManager/Controllers/ProjectsController.cs b/src/Api/SecretsManager/Controllers/ProjectsController.cs
@@ -82,8 +82,7 @@ public async Task<ProjectResponseModel> UpdateAsync([FromRoute] Guid id, [FromBo
     [HttpGet("projects/{id}")]
     public async Task<ProjectPermissionDetailsResponseModel> GetAsync([FromRoute] Guid id)
     {
-        var userId = _userService.GetProperUserId(User).Value;
-        var project = await _projectRepository.GetPermissionDetailsByIdAsync(id, userId);
+        var project = await _projectRepository.GetByIdAsync(id);
         if (project == null)
         {
             throw new NotFoundException();
@@ -94,34 +93,18 @@ public async Task<ProjectPermissionDetailsResponseModel> GetAsync([FromRoute] Gu
             throw new NotFoundException();
         }
 
+        var userId = _userService.GetProperUserId(User).Value;
         var orgAdmin = await _currentContext.OrganizationAdmin(project.OrganizationId);
         var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin);
 
-        bool hasAccess;
-        var read = project.Read;
-        var write = project.Write;
-
-        switch (accessClient)
-        {
-            case AccessClientType.NoAccessCheck:
-                hasAccess = true;
-                write = true;
-                read = true;
-                break;
-            case AccessClientType.User:
-                hasAccess = project.Read;
-                break;
-            default:
-                hasAccess = false;
-                break;
-        }
+        var access = await _projectRepository.AccessToProjectAsync(id, userId, accessClient);
 
-        if (!hasAccess)
+        if (!access.Read)
         {
             throw new NotFoundException();
         }
 
-        return new ProjectPermissionDetailsResponseModel(project, read, write);
+        return new ProjectPermissionDetailsResponseModel(project, access.Read, access.Write);
     }
 
     [HttpPost("projects/delete")]

diff --git a/src/Core/SecretsManager/Repositories/IProjectRepository.cs b/src/Core/SecretsManager/Repositories/IProjectRepository.cs
@@ -1,6 +1,5 @@
 using Bit.Core.Enums;
 using Bit.Core.SecretsManager.Entities;
-using Bit.Core.SecretsManager.Models.Data;
 
 namespace Bit.Core.SecretsManager.Repositories;
 
@@ -9,7 +8,6 @@ public interface IProjectRepository
     Task<IEnumerable<Project>> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType);
     Task<IEnumerable<Project>> GetManyByOrganizationIdWriteAccessAsync(Guid organizationId, Guid userId, AccessClientType accessType);
     Task<IEnumerable<Project>> GetManyByIds(IEnumerable<Guid> ids);
-    Task<ProjectPermissionDetails> GetPermissionDetailsByIdAsync(Guid id, Guid userId);
     Task<Project> GetByIdAsync(Guid id);
     Task<Project> CreateAsync(Project project);
     Task ReplaceAsync(Project project);
@@ -19,4 +17,5 @@ public interface IProjectRepository
     Task<bool> UserHasWriteAccessToProject(Guid id, Guid userId);
     Task<bool> ServiceAccountHasWriteAccessToProject(Guid id, Guid userId);
     Task<bool> ServiceAccountHasReadAccessToProject(Guid id, Guid userId);
+    Task<(bool Read, bool Write)> AccessToProjectAsync(Guid id, Guid userId, AccessClientType accessType);
 }
diff --git a/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs
@@ -6,7 +6,6 @@
 using Bit.Core.Exceptions;
 using Bit.Core.SecretsManager.Commands.Projects.Interfaces;
 using Bit.Core.SecretsManager.Entities;
-using Bit.Core.SecretsManager.Models.Data;
 using Bit.Core.SecretsManager.Repositories;
 using Bit.Core.Services;
 using Bit.Core.Test.SecretsManager.AutoFixture.ProjectsFixture;
@@ -199,13 +198,16 @@ public async void Get_Success(PermissionType permissionType, SutProvider<Project
                 break;
         }
 
-        sutProvider.GetDependency<IProjectRepository>().GetPermissionDetailsByIdAsync(Arg.Is(data), Arg.Any<Guid>())
-            .ReturnsForAnyArgs(new ProjectPermissionDetails() { Id = data, OrganizationId = orgId, Read = true, Write = true });
+        sutProvider.GetDependency<IProjectRepository>().GetByIdAsync(Arg.Is(data))
+            .ReturnsForAnyArgs(new Project { Id = data, OrganizationId = orgId });
+
+        sutProvider.GetDependency<IProjectRepository>().AccessToProjectAsync(default, default, default)
+            .ReturnsForAnyArgs((true, false));
 
         await sutProvider.Sut.GetAsync(data);
 
         await sutProvider.GetDependency<IProjectRepository>().Received(1)
-            .GetPermissionDetailsByIdAsync(Arg.Is(data), Arg.Any<Guid>());
+            .GetByIdAsync(Arg.Is(data));
     }
 
     [Theory]