From 5cd571df640688ddc0a786dd79bcb934e8e7240d Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Fri, 20 Jan 2023 16:33:11 +0100 Subject: [PATCH] [SM-380] Access checks for listing projects (#2496) * Add project access checks for listing --- .editorconfig | 4 + .../Projects/DeleteProjectCommand.cs | 57 ++++++++++---- .../Projects/UpdateProjectCommand.cs | 33 ++++++-- .../Repositories/ProjectRepository.cs | 53 ++++++++++--- .../Projects/DeleteProjectCommandTests.cs | 68 +++++++++++++---- .../Projects/UpdateProjectCommandTests.cs | 75 +++++++++++++++++++ src/Api/Controllers/ProjectsController.cs | 46 ++++++++++-- src/Core/Context/CurrentContext.cs | 8 ++ src/Core/Context/ICurrentContext.cs | 2 + src/Core/Enums/AccessClientType.cs | 30 ++++++++ src/Core/Identity/Claims.cs | 3 + src/Core/Identity/ClientType.cs | 8 ++ src/Core/Repositories/IProjectRepository.cs | 5 +- .../Interfaces/IDeleteProjectCommand.cs | 2 +- .../Interfaces/IUpdateProjectCommand.cs | 2 +- src/Identity/IdentityServer/ClientStore.cs | 3 + .../Controllers/ProjectsControllerTest.cs | 74 +++++++++++++++--- .../Factories/ApiApplicationFactory.cs | 8 ++ .../Helpers/OrganizationTestHelpers.cs | 31 +++++++- .../Controllers/ProjectsControllerTests.cs | 9 ++- 20 files changed, 452 insertions(+), 69 deletions(-) create mode 100644 bitwarden_license/test/Commercial.Core.Test/SecretManagerFeatures/Projects/UpdateProjectCommandTests.cs create mode 100644 src/Core/Enums/AccessClientType.cs create mode 100644 src/Core/Identity/ClientType.cs diff --git a/.editorconfig b/.editorconfig index 40931fa5e..e53c670fa 100644 --- 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 index 3d2061418..cf1644c92 100644 --- 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>> DeleteProjects(List ids) + public async Task>> DeleteProjects(List 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>(projects.Count); + var deleteIds = new List(); + + foreach (var project in projects) + { + var hasAccess = accessClient switch { - throw new NotFoundException(); + AccessClientType.NoAccessCheck => true, + AccessClientType.User => await _projectRepository.UserHasWriteAccessToProject(project.Id, userId), + _ => false, + }; + + if (!hasAccess) + { + results.Add(new Tuple(project, "access denied")); } - // TODO Once permissions are implemented add check for each project here. else { - return new Tuple(project, ""); + results.Add(new Tuple(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 index 1e0163ecc..11cd04d85 100644 --- 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 UpdateAsync(Project project) + public async Task 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/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/Repositories/ProjectRepository.cs b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/Repositories/ProjectRepository.cs index b500a5734..4023d7319 100644 --- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/Repositories/ProjectRepository.cs +++ b/bitwarden_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 class ProjectRepository : Repository> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId) + public async Task> 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>(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>(projects); } - private static Expression> UserHasAccessToProject(Guid userId) => p => + private static Expression> 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> 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> ServiceAccountHasReadAccessToProject(Guid serviceAccountId) => p => + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccount.Id == serviceAccountId && ap.Read); + public async Task DeleteManyByIdAsync(IEnumerable 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 @@ public class ProjectRepository : Repository>(projects); } + } + public async Task 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 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/bitwarden_license/test/Commercial.Core.Test/SecretManagerFeatures/Projects/DeleteProjectCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretManagerFeatures/Projects/DeleteProjectCommandTests.cs index c9dca6242..2652663fd 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretManagerFeatures/Projects/DeleteProjectCommandTests.cs +++ b/bitwarden_license/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 data, + public async Task DeleteProjects_Throws_NotFoundException(List data, Guid userId, SutProvider sutProvider) { sutProvider.GetDependency().GetManyByIds(data).Returns(new List()); - var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteProjects(data)); + await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteProjects(data, userId)); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().DeleteManyByIdAsync(default); } [Theory] [BitAutoData] - public async Task DeleteSecrets_OneIdNotFound_Throws_NotFoundException(List data, + public async Task DeleteSecrets_OneIdNotFound_Throws_NotFoundException(List data, Guid userId, SutProvider sutProvider) { var project = new Project() @@ -35,35 +37,69 @@ public class DeleteProjectCommandTests }; sutProvider.GetDependency().GetManyByIds(data).Returns(new List() { project }); - var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteProjects(data)); + await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteProjects(data, userId)); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().DeleteManyByIdAsync(default); } [Theory] [BitAutoData] - public async Task DeleteSecrets_Success(List data, - SutProvider sutProvider) + public async Task DeleteSecrets_User_Success(List data, Guid userId, Guid organizationId, + SutProvider sutProvider) { - var projects = new List(); - foreach (Guid id in data) + var projects = data.Select(id => new Project { Id = id, OrganizationId = organizationId }).ToList(); + + sutProvider.GetDependency().ClientType = ClientType.User; + sutProvider.GetDependency().GetManyByIds(data).Returns(projects); + sutProvider.GetDependency().UserHasWriteAccessToProject(Arg.Any(), 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().Received(1).DeleteManyByIdAsync(Arg.Is>(d => d.SequenceEqual(data))); + } + + [Theory] + [BitAutoData] + public async Task DeleteSecrets_User_No_Permission(List data, Guid userId, Guid organizationId, + SutProvider sutProvider) + { + var projects = data.Select(id => new Project { Id = id, OrganizationId = organizationId }).ToList(); + + sutProvider.GetDependency().ClientType = ClientType.User; + sutProvider.GetDependency().GetManyByIds(data).Returns(projects); + sutProvider.GetDependency().UserHasWriteAccessToProject(userId, userId).Returns(false); + + var results = await sutProvider.Sut.DeleteProjects(data, userId); + + foreach (var result in results) + { + Assert.Equal("access denied", result.Item2); + } + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().DeleteManyByIdAsync(default); + } + + [Theory] + [BitAutoData] + public async Task DeleteSecrets_OrganizationAdmin_Success(List data, Guid userId, Guid organizationId, + SutProvider sutProvider) + { + var projects = data.Select(id => new Project { Id = id, OrganizationId = organizationId }).ToList(); + + sutProvider.GetDependency().OrganizationAdmin(organizationId).Returns(true); sutProvider.GetDependency().GetManyByIds(data).Returns(projects); - var results = await sutProvider.Sut.DeleteProjects(data); + var results = await sutProvider.Sut.DeleteProjects(data, userId); - await sutProvider.GetDependency().Received(1).DeleteManyByIdAsync(Arg.Is(data)); + await sutProvider.GetDependency().Received(1).DeleteManyByIdAsync(Arg.Is>(d => d.SequenceEqual(data))); foreach (var result in results) { Assert.Equal("", result.Item2); } } } - diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretManagerFeatures/Projects/UpdateProjectCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretManagerFeatures/Projects/UpdateProjectCommandTests.cs new file mode 100644 index 000000000..c85068d99 --- /dev/null +++ b/bitwarden_license/test/Commercial.Core.Test/SecretManagerFeatures/Projects/UpdateProjectCommandTests.cs @@ -0,0 +1,75 @@ +using Bit.Commercial.Core.SecretManagerFeatures.Projects; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Core.Test.AutoFixture.ProjectsFixture; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Bit.Test.Common.Helpers; +using NSubstitute; +using NSubstitute.ReturnsExtensions; +using Xunit; + +namespace Bit.Commercial.Core.Test.SecretManagerFeatures.Projects; + +[SutProviderCustomize] +[ProjectCustomize] +public class UpdateProjectCommandTests +{ + [Theory] + [BitAutoData] + public async Task UpdateAsync_Throws_NotFoundException(Project project, Guid userId, SutProvider sutProvider) + { + sutProvider.GetDependency().GetByIdAsync(project.Id).ReturnsNull(); + + await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateAsync(project, userId)); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().ReplaceAsync(default); + } + + [Theory] + [BitAutoData] + public async Task UpdateAsync_Admin_Succeeds(Project project, Guid userId, SutProvider sutProvider) + { + sutProvider.GetDependency().GetByIdAsync(project.Id).Returns(project); + sutProvider.GetDependency().OrganizationAdmin(project.OrganizationId).Returns(true); + + var project2 = new Project { Id = project.Id, Name = "newName" }; + var result = await sutProvider.Sut.UpdateAsync(project2, userId); + + Assert.NotNull(result); + Assert.Equal("newName", result.Name); + AssertHelper.AssertRecent(result.RevisionDate); + + await sutProvider.GetDependency().ReceivedWithAnyArgs(1).ReplaceAsync(default); + } + + [Theory] + [BitAutoData] + public async Task UpdateAsync_User_NoAccess(Project project, Guid userId, SutProvider sutProvider) + { + sutProvider.GetDependency().GetByIdAsync(project.Id).Returns(project); + sutProvider.GetDependency().UserHasWriteAccessToProject(project.Id, userId).Returns(false); + + await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateAsync(project, userId)); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().ReplaceAsync(default); + } + + [Theory] + [BitAutoData] + public async Task UpdateAsync_User_Success(Project project, Guid userId, SutProvider sutProvider) + { + sutProvider.GetDependency().GetByIdAsync(project.Id).Returns(project); + sutProvider.GetDependency().UserHasWriteAccessToProject(project.Id, userId).Returns(true); + + var project2 = new Project { Id = project.Id, Name = "newName" }; + var result = await sutProvider.Sut.UpdateAsync(project2, userId); + + Assert.NotNull(result); + Assert.Equal("newName", result.Name); + + await sutProvider.GetDependency().ReceivedWithAnyArgs(1).ReplaceAsync(default); + } +} diff --git a/src/Api/Controllers/ProjectsController.cs b/src/Api/Controllers/ProjectsController.cs index 8b706d42d..fa714da80 100644 --- a/src/Api/Controllers/ProjectsController.cs +++ b/src/Api/Controllers/ProjectsController.cs @@ -2,6 +2,8 @@ using Bit.Api.SecretManagerFeatures.Models.Request; using Bit.Api.SecretManagerFeatures.Models.Response; using Bit.Api.Utilities; +using Bit.Core.Context; +using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.SecretManagerFeatures.Projects.Interfaces; @@ -18,24 +20,32 @@ public class ProjectsController : Controller private readonly ICreateProjectCommand _createProjectCommand; private readonly IUpdateProjectCommand _updateProjectCommand; private readonly IDeleteProjectCommand _deleteProjectCommand; + private readonly ICurrentContext _currentContext; public ProjectsController( IUserService userService, IProjectRepository projectRepository, ICreateProjectCommand createProjectCommand, IUpdateProjectCommand updateProjectCommand, - IDeleteProjectCommand deleteProjectCommand) + IDeleteProjectCommand deleteProjectCommand, + ICurrentContext currentContext) { _userService = userService; _projectRepository = projectRepository; _createProjectCommand = createProjectCommand; _updateProjectCommand = updateProjectCommand; _deleteProjectCommand = deleteProjectCommand; + _currentContext = currentContext; } [HttpPost("organizations/{organizationId}/projects")] public async Task CreateAsync([FromRoute] Guid organizationId, [FromBody] ProjectCreateRequestModel createRequest) { + if (!await _currentContext.OrganizationUser(organizationId)) + { + throw new NotFoundException(); + } + var result = await _createProjectCommand.CreateAsync(createRequest.ToProject(organizationId)); return new ProjectResponseModel(result); } @@ -43,15 +53,22 @@ public class ProjectsController : Controller [HttpPut("projects/{id}")] public async Task UpdateProjectAsync([FromRoute] Guid id, [FromBody] ProjectUpdateRequestModel updateRequest) { - var result = await _updateProjectCommand.UpdateAsync(updateRequest.ToProject(id)); + var userId = _userService.GetProperUserId(User).Value; + + var result = await _updateProjectCommand.UpdateAsync(updateRequest.ToProject(id), userId); return new ProjectResponseModel(result); } [HttpGet("organizations/{organizationId}/projects")] - public async Task> GetProjectsByOrganizationAsync([FromRoute] Guid organizationId) + public async Task> GetProjectsByOrganizationAsync( + [FromRoute] Guid organizationId) { var userId = _userService.GetProperUserId(User).Value; - var projects = await _projectRepository.GetManyByOrganizationIdAsync(organizationId, userId); + var orgAdmin = await _currentContext.OrganizationAdmin(organizationId); + var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin); + + var projects = await _projectRepository.GetManyByOrganizationIdAsync(organizationId, userId, accessClient); + var responses = projects.Select(project => new ProjectResponseModel(project)); return new ListResponseModel(responses); } @@ -64,13 +81,32 @@ public class ProjectsController : Controller { throw new NotFoundException(); } + + var userId = _userService.GetProperUserId(User).Value; + 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.UserHasReadAccessToProject(id, userId), + _ => false, + }; + + if (!hasAccess) + { + throw new NotFoundException(); + } + return new ProjectResponseModel(project); } [HttpPost("projects/delete")] public async Task> BulkDeleteProjectsAsync([FromBody] List ids) { - var results = await _deleteProjectCommand.DeleteProjects(ids); + var userId = _userService.GetProperUserId(User).Value; + + var results = await _deleteProjectCommand.DeleteProjects(ids, userId); var responses = results.Select(r => new BulkDeleteResponseModel(r.Item1.Id, r.Item2)); return new ListResponseModel(responses); } diff --git a/src/Core/Context/CurrentContext.cs b/src/Core/Context/CurrentContext.cs index 5a74b35c9..68e4246d0 100644 --- a/src/Core/Context/CurrentContext.cs +++ b/src/Core/Context/CurrentContext.cs @@ -34,6 +34,7 @@ public class CurrentContext : ICurrentContext public virtual int? BotScore { get; set; } public virtual string ClientId { get; set; } public virtual Version ClientVersion { get; set; } + public virtual ClientType ClientType { get; set; } public CurrentContext(IProviderUserRepository providerUserRepository) { @@ -138,6 +139,13 @@ public class CurrentContext : ICurrentContext } } + var clientType = GetClaimValue(claimsDict, Claims.Type); + if (clientType != null) + { + Enum.TryParse(clientType, out ClientType c); + ClientType = c; + } + DeviceIdentifier = GetClaimValue(claimsDict, Claims.Device); Organizations = GetOrganizations(claimsDict, orgApi); diff --git a/src/Core/Context/ICurrentContext.cs b/src/Core/Context/ICurrentContext.cs index 19917fe4f..d5ea35060 100644 --- a/src/Core/Context/ICurrentContext.cs +++ b/src/Core/Context/ICurrentContext.cs @@ -1,6 +1,7 @@ using System.Security.Claims; using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.Identity; using Bit.Core.Repositories; using Bit.Core.Settings; using Microsoft.AspNetCore.Http; @@ -18,6 +19,7 @@ public interface ICurrentContext List Organizations { get; set; } Guid? InstallationId { get; set; } Guid? OrganizationId { get; set; } + ClientType ClientType { get; set; } bool IsBot { get; set; } bool MaybeBot { get; set; } int? BotScore { get; set; } diff --git a/src/Core/Enums/AccessClientType.cs b/src/Core/Enums/AccessClientType.cs new file mode 100644 index 000000000..b2b198616 --- /dev/null +++ b/src/Core/Enums/AccessClientType.cs @@ -0,0 +1,30 @@ +using Bit.Core.Identity; + +namespace Bit.Core.Enums; + +public enum AccessClientType +{ + NoAccessCheck = -1, + User = 0, + Organization = 1, + ServiceAccount = 2, +} + +public static class AccessClientHelper +{ + public static AccessClientType ToAccessClient(ClientType clientType, bool bypassAccessCheck = false) + { + if (bypassAccessCheck) + { + return AccessClientType.NoAccessCheck; + } + + return clientType switch + { + ClientType.User => AccessClientType.User, + ClientType.Organization => AccessClientType.Organization, + ClientType.ServiceAccount => AccessClientType.ServiceAccount, + _ => throw new ArgumentOutOfRangeException(nameof(clientType), clientType, null), + }; + } +} diff --git a/src/Core/Identity/Claims.cs b/src/Core/Identity/Claims.cs index d069e16ae..b56b23ad8 100644 --- a/src/Core/Identity/Claims.cs +++ b/src/Core/Identity/Claims.cs @@ -16,4 +16,7 @@ public static class Claims // Service Account public const string Organization = "organization"; + + // General + public const string Type = "type"; } diff --git a/src/Core/Identity/ClientType.cs b/src/Core/Identity/ClientType.cs new file mode 100644 index 000000000..8952657df --- /dev/null +++ b/src/Core/Identity/ClientType.cs @@ -0,0 +1,8 @@ +namespace Bit.Core.Identity; + +public enum ClientType : byte +{ + User = 0, + Organization = 1, + ServiceAccount = 2, +} diff --git a/src/Core/Repositories/IProjectRepository.cs b/src/Core/Repositories/IProjectRepository.cs index 80a3a6242..ef2491462 100644 --- a/src/Core/Repositories/IProjectRepository.cs +++ b/src/Core/Repositories/IProjectRepository.cs @@ -1,13 +1,16 @@ using Bit.Core.Entities; +using Bit.Core.Enums; namespace Bit.Core.Repositories; public interface IProjectRepository { - Task> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId); + Task> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType); Task> GetManyByIds(IEnumerable ids); Task GetByIdAsync(Guid id); Task CreateAsync(Project project); Task ReplaceAsync(Project project); Task DeleteManyByIdAsync(IEnumerable ids); + Task UserHasReadAccessToProject(Guid id, Guid userId); + Task UserHasWriteAccessToProject(Guid id, Guid userId); } diff --git a/src/Core/SecretManagerFeatures/Projects/Interfaces/IDeleteProjectCommand.cs b/src/Core/SecretManagerFeatures/Projects/Interfaces/IDeleteProjectCommand.cs index c587fd5c7..e03566e6f 100644 --- a/src/Core/SecretManagerFeatures/Projects/Interfaces/IDeleteProjectCommand.cs +++ b/src/Core/SecretManagerFeatures/Projects/Interfaces/IDeleteProjectCommand.cs @@ -4,6 +4,6 @@ namespace Bit.Core.SecretManagerFeatures.Projects.Interfaces; public interface IDeleteProjectCommand { - Task>> DeleteProjects(List ids); + Task>> DeleteProjects(List ids, Guid userId); } diff --git a/src/Core/SecretManagerFeatures/Projects/Interfaces/IUpdateProjectCommand.cs b/src/Core/SecretManagerFeatures/Projects/Interfaces/IUpdateProjectCommand.cs index 06058cdd8..2032204b0 100644 --- a/src/Core/SecretManagerFeatures/Projects/Interfaces/IUpdateProjectCommand.cs +++ b/src/Core/SecretManagerFeatures/Projects/Interfaces/IUpdateProjectCommand.cs @@ -4,5 +4,5 @@ namespace Bit.Core.SecretManagerFeatures.Projects.Interfaces; public interface IUpdateProjectCommand { - Task UpdateAsync(Project project); + Task UpdateAsync(Project updatedProject, Guid userId); } diff --git a/src/Identity/IdentityServer/ClientStore.cs b/src/Identity/IdentityServer/ClientStore.cs index b73498f9c..5400f3116 100644 --- a/src/Identity/IdentityServer/ClientStore.cs +++ b/src/Identity/IdentityServer/ClientStore.cs @@ -110,6 +110,7 @@ public class ClientStore : IClientStore Claims = new List { new(JwtClaimTypes.Subject, apiKey.ServiceAccountId.ToString()), + new(Claims.Type, ClientType.ServiceAccount.ToString()), }, }; @@ -141,6 +142,7 @@ public class ClientStore : IClientStore { new(JwtClaimTypes.Subject, user.Id.ToString()), new(JwtClaimTypes.AuthenticationMethod, "Application", "external"), + new(Claims.Type, ClientType.User.ToString()), }; var orgs = await _currentContext.OrganizationMembershipAsync(_organizationUserRepository, user.Id); var providers = await _currentContext.ProviderMembershipAsync(_providerUserRepository, user.Id); @@ -198,6 +200,7 @@ public class ClientStore : IClientStore Claims = new List { new(JwtClaimTypes.Subject, org.Id.ToString()), + new(Claims.Type, ClientType.Organization.ToString()), }, }; } diff --git a/test/Api.IntegrationTest/Controllers/ProjectsControllerTest.cs b/test/Api.IntegrationTest/Controllers/ProjectsControllerTest.cs index 8b23203ac..8c4c81e59 100644 --- a/test/Api.IntegrationTest/Controllers/ProjectsControllerTest.cs +++ b/test/Api.IntegrationTest/Controllers/ProjectsControllerTest.cs @@ -1,10 +1,12 @@ -using System.Net.Http.Headers; +using System.Net; +using System.Net.Http.Headers; using Bit.Api.IntegrationTest.Factories; using Bit.Api.IntegrationTest.Helpers; using Bit.Api.Models.Response; using Bit.Api.SecretManagerFeatures.Models.Request; using Bit.Api.SecretManagerFeatures.Models.Response; using Bit.Core.Entities; +using Bit.Core.Enums; using Bit.Core.Repositories; using Bit.Test.Common.Helpers; using Xunit; @@ -31,10 +33,19 @@ public class ProjectsControllerTest : IClassFixture, IAsy public async Task InitializeAsync() { var ownerEmail = $"integration-test{Guid.NewGuid()}@bitwarden.com"; - var tokens = await _factory.LoginWithNewAccount(ownerEmail); - var (organization, _) = await OrganizationTestHelpers.SignUpAsync(_factory, ownerEmail: ownerEmail, billingEmail: ownerEmail); + await _factory.LoginWithNewAccount(ownerEmail); + (_organization, _) = await OrganizationTestHelpers.SignUpAsync(_factory, ownerEmail: ownerEmail, billingEmail: ownerEmail); + var tokens = await _factory.LoginAsync(ownerEmail); + _client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", tokens.Token); + } + + public async Task LoginAsNewOrgUser(OrganizationUserType type = OrganizationUserType.User) + { + var email = $"integration-test{Guid.NewGuid()}@bitwarden.com"; + await _factory.LoginWithNewAccount(email); + await OrganizationTestHelpers.CreateUserAsync(_factory, _organization.Id, email, type); + var tokens = await _factory.LoginAsync(email); _client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", tokens.Token); - _organization = organization; } public Task DisposeAsync() @@ -44,12 +55,9 @@ public class ProjectsControllerTest : IClassFixture, IAsy } [Fact] - public async Task CreateProject() + public async Task CreateProject_Success() { - var request = new ProjectCreateRequestModel() - { - Name = _mockEncryptedString - }; + var request = new ProjectCreateRequestModel { Name = _mockEncryptedString }; var response = await _client.PostAsJsonAsync($"/organizations/{_organization.Id}/projects", request); response.EnsureSuccessStatusCode(); @@ -69,7 +77,17 @@ public class ProjectsControllerTest : IClassFixture, IAsy } [Fact] - public async Task UpdateProject() + public async Task CreateProject_NoPermission() + { + var request = new ProjectCreateRequestModel { Name = _mockEncryptedString }; + + var response = await _client.PostAsJsonAsync("/organizations/911d9106-7cf1-4d55-a3f9-f9abdeadecb3/projects", request); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task UpdateProject_Success() { var initialProject = await _projectRepository.CreateAsync(new Project { @@ -101,6 +119,42 @@ public class ProjectsControllerTest : IClassFixture, IAsy Assert.NotEqual(initialProject.RevisionDate, updatedProject.RevisionDate); } + [Fact] + public async Task UpdateProject_NotFound() + { + var request = new ProjectUpdateRequestModel() + { + Name = "2.3Uk+WNBIoU5xzmVFNcoWzz==|1MsPIYuRfdOHfu/0uY6H2Q==|/98xy4wb6pHP1VTZ9JcNCYgQjEUMFPlqJgCwRk1YXKg=", + }; + + var response = await _client.PutAsJsonAsync("/projects/c53de509-4581-402c-8cbd-f26d2c516fba", request); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task UpdateProject_MissingPermission() + { + // Create a new account as a user + await LoginAsNewOrgUser(); + + var project = await _projectRepository.CreateAsync(new Project + { + OrganizationId = _organization.Id, + Name = _mockEncryptedString + }); + + + var request = new ProjectUpdateRequestModel() + { + Name = "2.3Uk+WNBIoU5xzmVFNcoWzz==|1MsPIYuRfdOHfu/0uY6H2Q==|/98xy4wb6pHP1VTZ9JcNCYgQjEUMFPlqJgCwRk1YXKg=", + }; + + var response = await _client.PutAsJsonAsync($"/projects/{project.Id}", request); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + [Fact] public async Task GetProject() { diff --git a/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs b/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs index 94f5568f4..27e655060 100644 --- a/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs +++ b/test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs @@ -45,4 +45,12 @@ public class ApiApplicationFactory : WebApplicationFactoryBase return await _identityApplicationFactory.TokenFromPasswordAsync(email, masterPasswordHash); } + + /// + /// Helper for logging in to an account + /// + public async Task<(string Token, string RefreshToken)> LoginAsync(string email = "integration-test@bitwarden.com", string masterPasswordHash = "master_password_hash") + { + return await _identityApplicationFactory.TokenFromPasswordAsync(email, masterPasswordHash); + } } diff --git a/test/Api.IntegrationTest/Helpers/OrganizationTestHelpers.cs b/test/Api.IntegrationTest/Helpers/OrganizationTestHelpers.cs index ac2317cf4..7163d3e93 100644 --- a/test/Api.IntegrationTest/Helpers/OrganizationTestHelpers.cs +++ b/test/Api.IntegrationTest/Helpers/OrganizationTestHelpers.cs @@ -9,7 +9,8 @@ namespace Bit.Api.IntegrationTest.Helpers; public static class OrganizationTestHelpers { - public static async Task> SignUpAsync(WebApplicationFactoryBase factory, + public static async Task> SignUpAsync( + WebApplicationFactoryBase factory, PlanType plan = PlanType.Free, string ownerEmail = "integration-test@bitwarden.com", string name = "Integration Test Org", @@ -30,4 +31,32 @@ public static class OrganizationTestHelpers Owner = owner, }); } + + public static async Task CreateUserAsync( + WebApplicationFactoryBase factory, + Guid organizationId, + string userEmail, + OrganizationUserType type + ) where T : class + { + var userRepository = factory.GetService(); + var organizationUserRepository = factory.GetService(); + + var user = await userRepository.GetByEmailAsync(userEmail); + + var orgUser = new OrganizationUser + { + OrganizationId = organizationId, + UserId = user.Id, + Key = null, + Type = type, + Status = OrganizationUserStatusType.Invited, + AccessAll = false, + ExternalId = null, + }; + + await organizationUserRepository.CreateAsync(orgUser); + + return orgUser; + } } diff --git a/test/Api.Test/Controllers/ProjectsControllerTests.cs b/test/Api.Test/Controllers/ProjectsControllerTests.cs index f81713704..0ffefe7c5 100644 --- a/test/Api.Test/Controllers/ProjectsControllerTests.cs +++ b/test/Api.Test/Controllers/ProjectsControllerTests.cs @@ -1,6 +1,7 @@ using Bit.Api.Controllers; using Bit.Core.Entities; using Bit.Core.SecretManagerFeatures.Projects.Interfaces; +using Bit.Core.Services; using Bit.Core.Test.AutoFixture.ProjectsFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -19,17 +20,18 @@ public class ProjectsControllerTests [BitAutoData] public async void BulkDeleteProjects_Success(SutProvider sutProvider, List data) { - var ids = data.Select(project => project.Id).ToList(); + sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); + var ids = data.Select(project => project.Id)?.ToList(); var mockResult = new List>(); foreach (var project in data) { mockResult.Add(new Tuple(project, "")); } - sutProvider.GetDependency().DeleteProjects(ids).ReturnsForAnyArgs(mockResult); + sutProvider.GetDependency().DeleteProjects(ids, default).ReturnsForAnyArgs(mockResult); var results = await sutProvider.Sut.BulkDeleteProjectsAsync(ids); await sutProvider.GetDependency().Received(1) - .DeleteProjects(Arg.Is(ids)); + .DeleteProjects(Arg.Is(ids), Arg.Any()); Assert.Equal(data.Count, results.Data.Count()); } @@ -37,6 +39,7 @@ public class ProjectsControllerTests [BitAutoData] public async void BulkDeleteProjects_NoGuids_ThrowsArgumentNullException(SutProvider sutProvider) { + sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); await Assert.ThrowsAsync(() => sutProvider.Sut.BulkDeleteProjectsAsync(new List())); } }