[SM-380] Access checks for listing projects (#2496)

* Add project access checks for listing
bitwarden · Jan 20, 2023 · 5cd571d · 5cd571d
1 parent a7c2ff9
commit 5cd571d
Show file tree

Hide file tree

Showing 20 changed files with 452 additions and 69 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -117,6 +117,10 @@ csharp_new_line_before_members_in_anonymous_types = true
 # Namespace settings
 csharp_style_namespace_declarations = file_scoped:warning
 
+# Switch expression
+dotnet_diagnostic.CS8509.severity = error # missing switch case for named enum value
+dotnet_diagnostic.CS8524.severity = none # missing switch case for unnamed enum value
+
 # All files
 [*]
 guidelines = 120
diff --git a/bitwarden_license/src/Commercial.Core/SecretManagerFeatures/Projects/DeleteProjectCommand.cs b/bitwarden_license/src/Commercial.Core/SecretManagerFeatures/Projects/DeleteProjectCommand.cs
@@ -1,4 +1,6 @@
-using Bit.Core.Entities;
+using Bit.Core.Context;
+using Bit.Core.Entities;
+using Bit.Core.Enums;
 using Bit.Core.Exceptions;
 using Bit.Core.Repositories;
 using Bit.Core.SecretManagerFeatures.Projects.Interfaces;
@@ -8,36 +10,65 @@ namespace Bit.Commercial.Core.SecretManagerFeatures.Projects;
 public class DeleteProjectCommand : IDeleteProjectCommand
 {
     private readonly IProjectRepository _projectRepository;
+    private readonly ICurrentContext _currentContext;
 
-    public DeleteProjectCommand(IProjectRepository projectRepository)
+    public DeleteProjectCommand(IProjectRepository projectRepository, ICurrentContext currentContext)
     {
         _projectRepository = projectRepository;
+        _currentContext = currentContext;
     }
 
-    public async Task<List<Tuple<Project, string>>> DeleteProjects(List<Guid> ids)
+    public async Task<List<Tuple<Project, string>>> DeleteProjects(List<Guid> ids, Guid userId)
     {
-        var projects = await _projectRepository.GetManyByIds(ids);
+        if (ids.Any() != true || userId == new Guid())
+        {
+            throw new ArgumentNullException();
+        }
 
-        if (projects?.Any() != true)
+        var projects = (await _projectRepository.GetManyByIds(ids))?.ToList();
+
+        if (projects?.Any() != true || projects.Count != ids.Count)
         {
             throw new NotFoundException();
         }
 
-        var results = ids.Select(id =>
+        // Ensure all projects belongs to the same organization
+        var organizationId = projects.First().OrganizationId;
+        if (projects.Any(p => p.OrganizationId != organizationId))
         {
-            var project = projects.FirstOrDefault(project => project.Id == id);
-            if (project == null)
+            throw new UnauthorizedAccessException();
+        }
+
+        var orgAdmin = await _currentContext.OrganizationAdmin(organizationId);
+        var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin);
+
+        var results = new List<Tuple<Project, String>>(projects.Count);
+        var deleteIds = new List<Guid>();
+
+        foreach (var project in projects)
+        {
+            var hasAccess = accessClient switch
+            {
+                AccessClientType.NoAccessCheck => true,
+                AccessClientType.User => await _projectRepository.UserHasWriteAccessToProject(project.Id, userId),
+                _ => false,
+            };
+
+            if (!hasAccess)
             {
-                throw new NotFoundException();
+                results.Add(new Tuple<Project, string>(project, "access denied"));
             }
-            // TODO Once permissions are implemented add check for each project here.
             else
             {
-                return new Tuple<Project, string>(project, "");
+                results.Add(new Tuple<Project, string>(project, ""));
+                deleteIds.Add(project.Id);
             }
-        }).ToList();
+        }
 
-        await _projectRepository.DeleteManyByIdAsync(ids);
+        if (deleteIds.Count > 0)
+        {
+            await _projectRepository.DeleteManyByIdAsync(deleteIds);
+        }
         return results;
     }
 }

diff --git a/bitwarden_license/src/Commercial.Core/SecretManagerFeatures/Projects/UpdateProjectCommand.cs b/bitwarden_license/src/Commercial.Core/SecretManagerFeatures/Projects/UpdateProjectCommand.cs
@@ -1,4 +1,6 @@
-using Bit.Core.Entities;
+using Bit.Core.Context;
+using Bit.Core.Entities;
+using Bit.Core.Enums;
 using Bit.Core.Exceptions;
 using Bit.Core.Repositories;
 using Bit.Core.SecretManagerFeatures.Projects.Interfaces;
@@ -8,23 +10,38 @@ namespace Bit.Commercial.Core.SecretManagerFeatures.Projects;
 public class UpdateProjectCommand : IUpdateProjectCommand
 {
     private readonly IProjectRepository _projectRepository;
+    private readonly ICurrentContext _currentContext;
 
-    public UpdateProjectCommand(IProjectRepository projectRepository)
+    public UpdateProjectCommand(IProjectRepository projectRepository, ICurrentContext currentContext)
     {
         _projectRepository = projectRepository;
+        _currentContext = currentContext;
     }
 
-    public async Task<Project> UpdateAsync(Project project)
+    public async Task<Project> UpdateAsync(Project updatedProject, Guid userId)
     {
-        var existingProject = await _projectRepository.GetByIdAsync(project.Id);
-        if (existingProject == null)
+        var project = await _projectRepository.GetByIdAsync(updatedProject.Id);
+        if (project == null)
         {
             throw new NotFoundException();
         }
 
-        project.OrganizationId = existingProject.OrganizationId;
-        project.CreationDate = existingProject.CreationDate;
-        project.DeletedDate = existingProject.DeletedDate;
+        var orgAdmin = await _currentContext.OrganizationAdmin(project.OrganizationId);
+        var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin);
+
+        var hasAccess = accessClient switch
+        {
+            AccessClientType.NoAccessCheck => true,
+            AccessClientType.User => await _projectRepository.UserHasWriteAccessToProject(updatedProject.Id, userId),
+            _ => false,
+        };
+
+        if (!hasAccess)
+        {
+            throw new UnauthorizedAccessException();
+        }
+
+        project.Name = updatedProject.Name;
         project.RevisionDate = DateTime.UtcNow;
 
         await _projectRepository.ReplaceAsync(project);

diff --git a/...n_license/src/Commercial.Infrastructure.EntityFramework/Repositories/ProjectRepository.cs b/...n_license/src/Commercial.Infrastructure.EntityFramework/Repositories/ProjectRepository.cs
@@ -1,5 +1,6 @@
 using System.Linq.Expressions;
 using AutoMapper;
+using Bit.Core.Enums;
 using Bit.Core.Repositories;
 using Bit.Infrastructure.EntityFramework.Models;
 using Bit.Infrastructure.EntityFramework.Repositories;
@@ -26,29 +27,40 @@ public ProjectRepository(IServiceScopeFactory serviceScopeFactory, IMapper mappe
         }
     }
 
-    public async Task<IEnumerable<Core.Entities.Project>> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId)
+    public async Task<IEnumerable<Core.Entities.Project>> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType)
     {
         using var scope = ServiceScopeFactory.CreateScope();
         var dbContext = GetDatabaseContext(scope);
-        var project = await dbContext.Project
-            .Where(p => p.OrganizationId == organizationId && p.DeletedDate == null)
-            // TODO: Enable this + Handle Admins
-            //.Where(UserHasAccessToProject(userId))
-            .OrderBy(p => p.RevisionDate)
-            .ToListAsync();
-        return Mapper.Map<List<Core.Entities.Project>>(project);
+        var query = dbContext.Project.Where(p => p.OrganizationId == organizationId && p.DeletedDate == null);
+
+        query = accessType switch
+        {
+            AccessClientType.NoAccessCheck => query,
+            AccessClientType.User => query.Where(UserHasReadAccessToProject(userId)),
+            AccessClientType.ServiceAccount => query.Where(ServiceAccountHasReadAccessToProject(userId)),
+            _ => throw new ArgumentOutOfRangeException(nameof(accessType), accessType, null),
+        };
+
+        var projects = await query.OrderBy(p => p.RevisionDate).ToListAsync();
+        return Mapper.Map<List<Core.Entities.Project>>(projects);
     }
 
-    private static Expression<Func<Project, bool>> UserHasAccessToProject(Guid userId) => p =>
+    private static Expression<Func<Project, bool>> UserHasReadAccessToProject(Guid userId) => p =>
         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));
 
+    private static Expression<Func<Project, bool>> UserHasWriteAccessToProject(Guid userId) => 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));
+
+    private static Expression<Func<Project, bool>> ServiceAccountHasReadAccessToProject(Guid serviceAccountId) => p =>
+        p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccount.Id == serviceAccountId && ap.Read);
+
     public async Task DeleteManyByIdAsync(IEnumerable<Guid> ids)
     {
         using (var scope = ServiceScopeFactory.CreateScope())
         {
             var dbContext = GetDatabaseContext(scope);
-            var utcNow = DateTime.UtcNow;
             var projects = dbContext.Project.Where(c => ids.Contains(c.Id));
             await projects.ForEachAsync(project =>
             {
@@ -68,6 +80,27 @@ await projects.ForEachAsync(project =>
                                     .ToListAsync();
             return Mapper.Map<List<Core.Entities.Project>>(projects);
         }
+    }
+
+    public async Task<bool> UserHasReadAccessToProject(Guid id, Guid userId)
+    {
+        using var scope = ServiceScopeFactory.CreateScope();
+        var dbContext = GetDatabaseContext(scope);
+        var query = dbContext.Project
+            .Where(p => p.Id == id)
+            .Where(UserHasReadAccessToProject(userId));
+
+        return await query.AnyAsync();
+    }
+
+    public async Task<bool> UserHasWriteAccessToProject(Guid id, Guid userId)
+    {
+        using var scope = ServiceScopeFactory.CreateScope();
+        var dbContext = GetDatabaseContext(scope);
+        var query = dbContext.Project
+            .Where(p => p.Id == id)
+            .Where(UserHasWriteAccessToProject(userId));
 
+        return await query.AnyAsync();
     }
 }
diff --git a/...nse/test/Commercial.Core.Test/SecretManagerFeatures/Projects/DeleteProjectCommandTests.cs b/...nse/test/Commercial.Core.Test/SecretManagerFeatures/Projects/DeleteProjectCommandTests.cs
@@ -1,6 +1,8 @@
 using Bit.Commercial.Core.SecretManagerFeatures.Projects;
+using Bit.Core.Context;
 using Bit.Core.Entities;
 using Bit.Core.Exceptions;
+using Bit.Core.Identity;
 using Bit.Core.Repositories;
 using Bit.Test.Common.AutoFixture;
 using Bit.Test.Common.AutoFixture.Attributes;
@@ -14,19 +16,19 @@ public class DeleteProjectCommandTests
 {
     [Theory]
     [BitAutoData]
-    public async Task DeleteProjects_Throws_NotFoundException(List<Guid> data,
+    public async Task DeleteProjects_Throws_NotFoundException(List<Guid> data, Guid userId,
       SutProvider<DeleteProjectCommand> sutProvider)
     {
         sutProvider.GetDependency<IProjectRepository>().GetManyByIds(data).Returns(new List<Project>());
 
-        var exception = await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.DeleteProjects(data));
+        await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.DeleteProjects(data, userId));
 
         await sutProvider.GetDependency<IProjectRepository>().DidNotReceiveWithAnyArgs().DeleteManyByIdAsync(default);
     }
 
     [Theory]
     [BitAutoData]
-    public async Task DeleteSecrets_OneIdNotFound_Throws_NotFoundException(List<Guid> data,
+    public async Task DeleteSecrets_OneIdNotFound_Throws_NotFoundException(List<Guid> data, Guid userId,
       SutProvider<DeleteProjectCommand> sutProvider)
     {
         var project = new Project()
@@ -35,35 +37,69 @@ public async Task DeleteSecrets_OneIdNotFound_Throws_NotFoundException(List<Guid
         };
         sutProvider.GetDependency<IProjectRepository>().GetManyByIds(data).Returns(new List<Project>() { project });
 
-        var exception = await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.DeleteProjects(data));
+        await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.DeleteProjects(data, userId));
 
         await sutProvider.GetDependency<IProjectRepository>().DidNotReceiveWithAnyArgs().DeleteManyByIdAsync(default);
     }
 
     [Theory]
     [BitAutoData]
-    public async Task DeleteSecrets_Success(List<Guid> data,
-      SutProvider<DeleteProjectCommand> sutProvider)
+    public async Task DeleteSecrets_User_Success(List<Guid> data, Guid userId, Guid organizationId,
+        SutProvider<DeleteProjectCommand> sutProvider)
     {
-        var projects = new List<Project>();
-        foreach (Guid id in data)
+        var projects = data.Select(id => new Project { Id = id, OrganizationId = organizationId }).ToList();
+
+        sutProvider.GetDependency<ICurrentContext>().ClientType = ClientType.User;
+        sutProvider.GetDependency<IProjectRepository>().GetManyByIds(data).Returns(projects);
+        sutProvider.GetDependency<IProjectRepository>().UserHasWriteAccessToProject(Arg.Any<Guid>(), userId).Returns(true);
+
+        var results = await sutProvider.Sut.DeleteProjects(data, userId);
+
+        foreach (var result in results)
         {
-            var project = new Project()
-            {
-                Id = id
-            };
-            projects.Add(project);
+            Assert.Equal("", result.Item2);
         }
 
+        await sutProvider.GetDependency<IProjectRepository>().Received(1).DeleteManyByIdAsync(Arg.Is<List<Guid>>(d => d.SequenceEqual(data)));
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async Task DeleteSecrets_User_No_Permission(List<Guid> data, Guid userId, Guid organizationId,
+        SutProvider<DeleteProjectCommand> sutProvider)
+    {
+        var projects = data.Select(id => new Project { Id = id, OrganizationId = organizationId }).ToList();
+
+        sutProvider.GetDependency<ICurrentContext>().ClientType = ClientType.User;
         sutProvider.GetDependency<IProjectRepository>().GetManyByIds(data).Returns(projects);
+        sutProvider.GetDependency<IProjectRepository>().UserHasWriteAccessToProject(userId, userId).Returns(false);
 
-        var results = await sutProvider.Sut.DeleteProjects(data);
+        var results = await sutProvider.Sut.DeleteProjects(data, userId);
 
-        await sutProvider.GetDependency<IProjectRepository>().Received(1).DeleteManyByIdAsync(Arg.Is(data));
+        foreach (var result in results)
+        {
+            Assert.Equal("access denied", result.Item2);
+        }
+
+        await sutProvider.GetDependency<IProjectRepository>().DidNotReceiveWithAnyArgs().DeleteManyByIdAsync(default);
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async Task DeleteSecrets_OrganizationAdmin_Success(List<Guid> data, Guid userId, Guid organizationId,
+      SutProvider<DeleteProjectCommand> sutProvider)
+    {
+        var projects = data.Select(id => new Project { Id = id, OrganizationId = organizationId }).ToList();
+
+        sutProvider.GetDependency<ICurrentContext>().OrganizationAdmin(organizationId).Returns(true);
+        sutProvider.GetDependency<IProjectRepository>().GetManyByIds(data).Returns(projects);
+
+        var results = await sutProvider.Sut.DeleteProjects(data, userId);
+
+        await sutProvider.GetDependency<IProjectRepository>().Received(1).DeleteManyByIdAsync(Arg.Is<List<Guid>>(d => d.SequenceEqual(data)));
         foreach (var result in results)
         {
             Assert.Equal("", result.Item2);
         }
     }
 }
-