diff --git a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs index 63316dd71..d25af115c 100644 --- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs +++ b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs @@ -1,6 +1,7 @@ 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; using Bit.Infrastructure.EntityFramework.Repositories; @@ -35,40 +36,25 @@ public class SecretRepository : Repository ids.Contains(c.Id) && c.DeletedDate == null) - .Include(c => c.Projects) - .ToListAsync(); + .Where(c => ids.Contains(c.Id) && c.DeletedDate == null) + .Include(c => c.Projects) + .ToListAsync(); return Mapper.Map>(secrets); } } - private static Expression> ServiceAccountHasReadAccessToSecret(Guid serviceAccountId) => s => - s.Projects.Any(p => - p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccount.Id == serviceAccountId && ap.Read)); - - private static Expression> UserHasReadAccessToSecret(Guid userId) => s => - s.Projects.Any(p => - p.UserAccessPolicies.Any(ap => ap.OrganizationUser.UserId == userId && ap.Read) || - p.GroupAccessPolicies.Any(ap => - ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.UserId == userId && ap.Read))); - - - public async Task> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType) + public async Task> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType) { using var scope = ServiceScopeFactory.CreateScope(); var dbContext = GetDatabaseContext(scope); - var query = dbContext.Secret.Include(c => c.Projects).Where(c => c.OrganizationId == organizationId && c.DeletedDate == null); + var query = dbContext.Secret + .Include(c => c.Projects) + .Where(c => c.OrganizationId == organizationId && c.DeletedDate == null) + .OrderBy(s => s.RevisionDate); - query = accessType switch - { - AccessClientType.NoAccessCheck => query, - AccessClientType.User => query.Where(UserHasReadAccessToSecret(userId)), - AccessClientType.ServiceAccount => query.Where(ServiceAccountHasReadAccessToSecret(userId)), - _ => throw new ArgumentOutOfRangeException(nameof(accessType), accessType, null), - }; + var secrets = SecretToPermissionDetails(query, userId, accessType); - var secrets = await query.OrderBy(c => c.RevisionDate).ToListAsync(); - return Mapper.Map>(secrets); + return await secrets.ToListAsync(); } public async Task> GetManyByOrganizationIdInTrashByIdsAsync(Guid organizationId, IEnumerable ids) @@ -86,7 +72,7 @@ public class SecretRepository : Repository> GetManyByOrganizationIdInTrashAsync(Guid organizationId) + public async Task> GetManyByOrganizationIdInTrashAsync(Guid organizationId) { using (var scope = ServiceScopeFactory.CreateScope()) { @@ -97,29 +83,26 @@ public class SecretRepository : Repository c.RevisionDate) .ToListAsync(); - return Mapper.Map>(secrets); + // This should be changed if/when we allow non admins to access trashed items + return Mapper.Map>(secrets).Select(s => new SecretPermissionDetails + { + Secret = s, + Read = true, + Write = true, + }); } } - public async Task> GetManyByProjectIdAsync(Guid projectId, Guid userId, AccessClientType accessType) + public async Task> GetManyByProjectIdAsync(Guid projectId, Guid userId, AccessClientType accessType) { - using (var scope = ServiceScopeFactory.CreateScope()) - { - var dbContext = GetDatabaseContext(scope); - var query = dbContext.Secret.Include(s => s.Projects) - .Where(s => s.Projects.Any(p => p.Id == projectId) && s.DeletedDate == null); + using var scope = ServiceScopeFactory.CreateScope(); + var dbContext = GetDatabaseContext(scope); + var query = dbContext.Secret.Include(s => s.Projects) + .Where(s => s.Projects.Any(p => p.Id == projectId) && s.DeletedDate == null); - query = accessType switch - { - AccessClientType.NoAccessCheck => query, - AccessClientType.User => query.Where(UserHasReadAccessToSecret(userId)), - AccessClientType.ServiceAccount => query.Where(ServiceAccountHasReadAccessToSecret(userId)), - _ => throw new ArgumentOutOfRangeException(nameof(accessType), accessType, null), - }; + var secrets = SecretToPermissionDetails(query, userId, accessType); - var secrets = await query.OrderBy(s => s.RevisionDate).ToListAsync(); - return Mapper.Map>(secrets); - } + return await secrets.ToListAsync(); } public override async Task CreateAsync(Core.SecretsManager.Entities.Secret secret) @@ -272,4 +255,85 @@ public class SecretRepository : Repository AccessToSecretAsync(Guid id, Guid userId, AccessClientType accessType) + { + using var scope = ServiceScopeFactory.CreateScope(); + var dbContext = GetDatabaseContext(scope); + + var secret = dbContext.Secret + .Where(s => s.Id == id); + + var query = accessType switch + { + AccessClientType.NoAccessCheck => secret.Select(_ => new { Read = true, Write = true }), + AccessClientType.User => secret.Select(s => new + { + Read = s.Projects.Any(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))), + Write = s.Projects.Any(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))), + }), + AccessClientType.ServiceAccount => secret.Select(s => new + { + Read = s.Projects.Any(p => + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Read)), + Write = s.Projects.Any(p => + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Write)), + }), + _ => secret.Select(_ => new { Read = false, Write = false }), + }; + + var policy = await query.FirstOrDefaultAsync(); + + return (policy.Read, policy.Write); + } + + private IQueryable SecretToPermissionDetails(IQueryable query, Guid userId, AccessClientType accessType) + { + var secrets = accessType switch + { + AccessClientType.NoAccessCheck => query.Select(s => new SecretPermissionDetails + { + Secret = Mapper.Map(s), + Read = true, + Write = true, + }), + AccessClientType.User => query.Where(UserHasReadAccessToSecret(userId)).Select(SecretToPermissionsUser(userId, true)), + AccessClientType.ServiceAccount => query.Where(ServiceAccountHasReadAccessToSecret(userId)).Select(s => + new SecretPermissionDetails + { + Secret = Mapper.Map(s), + Read = true, + Write = false, + }), + _ => throw new ArgumentOutOfRangeException(nameof(accessType), accessType, null), + }; + return secrets; + } + + private Expression> SecretToPermissionsUser(Guid userId, bool read) => + s => new SecretPermissionDetails + { + Secret = Mapper.Map(s), + Read = read, + Write = s.Projects.Any(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> ServiceAccountHasReadAccessToSecret(Guid serviceAccountId) => s => + s.Projects.Any(p => + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccount.Id == serviceAccountId && ap.Read)); + + private static Expression> UserHasReadAccessToSecret(Guid userId) => s => + s.Projects.Any(p => + p.UserAccessPolicies.Any(ap => ap.OrganizationUser.UserId == userId && ap.Read) || + p.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.UserId == userId && ap.Read))); } diff --git a/src/Api/SecretsManager/Controllers/SecretsController.cs b/src/Api/SecretsManager/Controllers/SecretsController.cs index afa792b33..fd60c15fb 100644 --- a/src/Api/SecretsManager/Controllers/SecretsController.cs +++ b/src/Api/SecretsManager/Controllers/SecretsController.cs @@ -8,7 +8,6 @@ using Bit.Core.Identity; using Bit.Core.Models.Business; using Bit.Core.Repositories; using Bit.Core.SecretsManager.Commands.Secrets.Interfaces; -using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Repositories; using Bit.Core.Services; using Microsoft.AspNetCore.Authorization; @@ -83,7 +82,9 @@ public class SecretsController : Controller var userId = _userService.GetProperUserId(User).Value; var result = await _createSecretCommand.CreateAsync(createRequest.ToSecret(organizationId), userId); - return new SecretResponseModel(result); + + // Creating a secret means you have read & write permission. + return new SecretResponseModel(result, true, true); } [HttpGet("secrets/{id}")] @@ -96,21 +97,26 @@ public class SecretsController : Controller throw new NotFoundException(); } - if (!await UserHasReadAccessToSecret(secret)) + var userId = _userService.GetProperUserId(User).Value; + var orgAdmin = await _currentContext.OrganizationAdmin(secret.OrganizationId); + var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin); + + var access = await _secretRepository.AccessToSecretAsync(id, userId, accessClient); + + if (!access.Read) { throw new NotFoundException(); } if (_currentContext.ClientType == ClientType.ServiceAccount) { - var userId = _userService.GetProperUserId(User).Value; await _eventService.LogServiceAccountSecretEventAsync(userId, secret, EventType.Secret_Retrieved); var org = await _organizationRepository.GetByIdAsync(secret.OrganizationId); await _referenceEventService.RaiseEventAsync(new ReferenceEvent(ReferenceEventType.SmServiceAccountAccessedSecret, org)); } - return new SecretResponseModel(secret); + return new SecretResponseModel(secret, access.Read, access.Write); } [HttpGet("projects/{projectId}/secrets")] @@ -137,7 +143,9 @@ public class SecretsController : Controller var userId = _userService.GetProperUserId(User).Value; var secret = updateRequest.ToSecret(id); var result = await _updateSecretCommand.UpdateAsync(secret, userId); - return new SecretResponseModel(result); + + // Updating a secret means you have read & write permission. + return new SecretResponseModel(result, true, true); } [HttpPost("secrets/delete")] @@ -148,26 +156,4 @@ public class SecretsController : Controller var responses = results.Select(r => new BulkDeleteResponseModel(r.Item1.Id, r.Item2)); return new ListResponseModel(responses); } - - public async Task UserHasReadAccessToSecret(Secret secret) - { - var userId = _userService.GetProperUserId(User).Value; - var orgAdmin = await _currentContext.OrganizationAdmin(secret.OrganizationId); - var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin); - var hasAccess = orgAdmin; - - if (secret.Projects?.Count > 0) - { - Guid projectId = secret.Projects.FirstOrDefault().Id; - hasAccess = accessClient switch - { - AccessClientType.NoAccessCheck => true, - AccessClientType.User => await _projectRepository.UserHasReadAccessToProject(projectId, userId), - AccessClientType.ServiceAccount => await _projectRepository.ServiceAccountHasReadAccessToProject(projectId, userId), - _ => false, - }; - } - - return hasAccess; - } } diff --git a/src/Api/SecretsManager/Controllers/SecretsManagerPortingController.cs b/src/Api/SecretsManager/Controllers/SecretsManagerPortingController.cs index 86e3ed182..a74c2fa48 100644 --- a/src/Api/SecretsManager/Controllers/SecretsManagerPortingController.cs +++ b/src/Api/SecretsManager/Controllers/SecretsManagerPortingController.cs @@ -47,7 +47,7 @@ public class SecretsManagerPortingController : Controller throw new NotFoundException(); } - return new SMExportResponseModel(projects, secrets); + return new SMExportResponseModel(projects, secrets.Select(s => s.Secret)); } [HttpPost("sm/{organizationId}/import")] diff --git a/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs b/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs index db2ee3729..15f46434c 100644 --- a/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs +++ b/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs @@ -7,7 +7,7 @@ public class SecretResponseModel : ResponseModel { private const string _objectName = "secret"; - public SecretResponseModel(Secret secret) : base(_objectName) + public SecretResponseModel(Secret secret, bool read, bool write) : base(_objectName) { if (secret == null) { @@ -22,6 +22,9 @@ public class SecretResponseModel : ResponseModel CreationDate = secret.CreationDate; RevisionDate = secret.RevisionDate; Projects = secret.Projects?.Select(p => new InnerProject(p)); + + Read = read; + Write = write; } public SecretResponseModel() : base(_objectName) @@ -44,6 +47,10 @@ public class SecretResponseModel : ResponseModel public IEnumerable Projects { get; set; } + public bool Read { get; set; } + + public bool Write { get; set; } + public class InnerProject { public InnerProject(Project project) diff --git a/src/Api/SecretsManager/Models/Response/SecretWithProjectsListResponseModel.cs b/src/Api/SecretsManager/Models/Response/SecretWithProjectsListResponseModel.cs index ec841b901..ff7d82002 100644 --- a/src/Api/SecretsManager/Models/Response/SecretWithProjectsListResponseModel.cs +++ b/src/Api/SecretsManager/Models/Response/SecretWithProjectsListResponseModel.cs @@ -1,5 +1,6 @@ using Bit.Core.Models.Api; using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Models.Data; namespace Bit.Api.SecretsManager.Models.Response; @@ -7,10 +8,10 @@ public class SecretWithProjectsListResponseModel : ResponseModel { private const string _objectName = "SecretsWithProjectsList"; - public SecretWithProjectsListResponseModel(IEnumerable secrets) : base(_objectName) + public SecretWithProjectsListResponseModel(IEnumerable secrets) : base(_objectName) { Secrets = secrets.Select(s => new InnerSecret(s)); - Projects = secrets.SelectMany(s => s.Projects).DistinctBy(p => p.Id).Select(p => new InnerProject(p)); + Projects = secrets.SelectMany(s => s.Secret.Projects).DistinctBy(p => p.Id).Select(p => new InnerProject(p)); } public SecretWithProjectsListResponseModel() : base(_objectName) @@ -38,14 +39,16 @@ public class SecretWithProjectsListResponseModel : ResponseModel public class InnerSecret { - public InnerSecret(Secret secret) + public InnerSecret(SecretPermissionDetails secret) { - Id = secret.Id.ToString(); - OrganizationId = secret.OrganizationId.ToString(); - Key = secret.Key; - CreationDate = secret.CreationDate; - RevisionDate = secret.RevisionDate; - Projects = secret.Projects?.Select(p => new InnerProject(p)); + Id = secret.Secret.Id.ToString(); + OrganizationId = secret.Secret.OrganizationId.ToString(); + Key = secret.Secret.Key; + CreationDate = secret.Secret.CreationDate; + RevisionDate = secret.Secret.RevisionDate; + Projects = secret.Secret.Projects?.Select(p => new InnerProject(p)); + Read = secret.Read; + Write = secret.Write; } public InnerSecret() @@ -63,6 +66,8 @@ public class SecretWithProjectsListResponseModel : ResponseModel public DateTime RevisionDate { get; set; } public IEnumerable Projects { get; set; } + public bool Read { get; set; } + public bool Write { get; set; } } } diff --git a/src/Core/SecretsManager/Models/Data/SecretPermissionDetails.cs b/src/Core/SecretsManager/Models/Data/SecretPermissionDetails.cs new file mode 100644 index 000000000..3a3c8d29c --- /dev/null +++ b/src/Core/SecretsManager/Models/Data/SecretPermissionDetails.cs @@ -0,0 +1,10 @@ +using Bit.Core.SecretsManager.Entities; + +namespace Bit.Core.SecretsManager.Models.Data; + +public class SecretPermissionDetails +{ + public Secret Secret; + public bool Read { get; set; } + public bool Write { get; set; } +} diff --git a/src/Core/SecretsManager/Repositories/ISecretRepository.cs b/src/Core/SecretsManager/Repositories/ISecretRepository.cs index 44b704177..5b40a3327 100644 --- a/src/Core/SecretsManager/Repositories/ISecretRepository.cs +++ b/src/Core/SecretsManager/Repositories/ISecretRepository.cs @@ -1,15 +1,16 @@ using Bit.Core.Enums; using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Models.Data; namespace Bit.Core.SecretsManager.Repositories; public interface ISecretRepository { - Task> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType); - Task> GetManyByOrganizationIdInTrashAsync(Guid organizationId); + Task> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType); + Task> GetManyByOrganizationIdInTrashAsync(Guid organizationId); Task> GetManyByOrganizationIdInTrashByIdsAsync(Guid organizationId, IEnumerable ids); Task> GetManyByIds(IEnumerable ids); - Task> GetManyByProjectIdAsync(Guid projectId, Guid userId, AccessClientType accessType); + Task> GetManyByProjectIdAsync(Guid projectId, Guid userId, AccessClientType accessType); Task GetByIdAsync(Guid id); Task CreateAsync(Secret secret); Task UpdateAsync(Secret secret); @@ -18,4 +19,5 @@ public interface ISecretRepository Task RestoreManyByIdAsync(IEnumerable ids); Task> ImportAsync(IEnumerable secrets); Task UpdateRevisionDates(IEnumerable ids); + Task<(bool Read, bool Write)> AccessToSecretAsync(Guid id, Guid userId, AccessClientType accessType); } diff --git a/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs b/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs index 9f90335ea..1ea7c4049 100644 --- a/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs +++ b/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs @@ -244,7 +244,8 @@ public class SecretsControllerTests : IClassFixture, IAsy secretResponse.EnsureSuccessStatusCode(); var secretResult = await secretResponse.Content.ReadFromJsonAsync(); - var secret = (await _secretRepository.GetManyByProjectIdAsync(project.Id, orgUserId, accessType)).First(); + var result = (await _secretRepository.GetManyByProjectIdAsync(project.Id, orgUserId, accessType)).First(); + var secret = result.Secret; Assert.NotNull(secretResult); Assert.Equal(secret.Id.ToString(), secretResult!.Id); diff --git a/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs index ab020f751..6877c2871 100644 --- a/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs @@ -6,6 +6,7 @@ using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.SecretsManager.Commands.Secrets.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.SecretsFixture; @@ -45,7 +46,11 @@ public class SecretsControllerTests public async void GetSecretsByOrganization_Success(PermissionType permissionType, SutProvider sutProvider, Core.SecretsManager.Entities.Secret resultSecret, Guid organizationId, Guid userId, Core.SecretsManager.Entities.Project mockProject, AccessClientType accessType) { sutProvider.GetDependency().AccessSecretsManager(default).ReturnsForAnyArgs(true); - sutProvider.GetDependency().GetManyByOrganizationIdAsync(default, default, default).ReturnsForAnyArgs(new List { resultSecret }); + sutProvider.GetDependency().GetManyByOrganizationIdAsync(default, default, default) + .ReturnsForAnyArgs(new List + { + new() { Secret = resultSecret, Read = true, Write = true }, + }); sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); if (permissionType == PermissionType.RunAsAdmin) @@ -95,6 +100,8 @@ public class SecretsControllerTests resultSecret.OrganizationId = organizationId; sutProvider.GetDependency().GetByIdAsync(default).ReturnsForAnyArgs(resultSecret); + sutProvider.GetDependency().AccessToSecretAsync(default, default, default) + .ReturnsForAnyArgs(Task.FromResult((true, true))); if (permissionType == PermissionType.RunAsAdmin) { @@ -107,7 +114,7 @@ public class SecretsControllerTests sutProvider.GetDependency().UserHasReadAccessToProject(mockProject.Id, userId).Returns(true); } - var result = await sutProvider.Sut.GetAsync(resultSecret.Id); + await sutProvider.Sut.GetAsync(resultSecret.Id); await sutProvider.GetDependency().Received(1) .GetByIdAsync(Arg.Is(AssertHelper.AssertPropertyEqual(resultSecret.Id)));