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 cb69db5e2d..333719c42e 100644 --- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs +++ b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/SecretRepository.cs @@ -289,35 +289,13 @@ public class SecretRepository : Repository AccessToSecretAsync(Guid id, Guid userId, AccessClientType accessType) { - using var scope = ServiceScopeFactory.CreateScope(); + await using var scope = ServiceScopeFactory.CreateAsyncScope(); 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 query = BuildSecretAccessQuery(secret, userId, accessType); var policy = await query.FirstOrDefaultAsync(); @@ -361,19 +339,27 @@ public class SecretRepository : Repository> SecretToPermissionsUser(Guid userId, bool read) => s => new SecretPermissionDetails { - Secret = Mapper.Map(s), + 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))), + Write = + s.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Write) || + s.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.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.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == serviceAccountId && ap.Read) || s.Projects.Any(p => p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccount.Id == serviceAccountId && ap.Read)); private static Expression> UserHasReadAccessToSecret(Guid userId) => s => + s.UserAccessPolicies.Any(ap => ap.OrganizationUser.UserId == userId && ap.Read) || + s.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.UserId == userId && ap.Read)) || s.Projects.Any(p => p.UserAccessPolicies.Any(ap => ap.OrganizationUser.UserId == userId && ap.Read) || p.GroupAccessPolicies.Any(ap => @@ -434,4 +420,40 @@ public class SecretRepository : Repository setters.SetProperty(b => b.RevisionDate, utcNow)); } } + + private static IQueryable BuildSecretAccessQuery(IQueryable secrets, Guid accessClientId, + AccessClientType accessType) => + accessType switch + { + AccessClientType.NoAccessCheck => secrets.Select(s => new SecretAccess(s.Id, true, true)), + AccessClientType.User => secrets.Select(s => new SecretAccess( + s.Id, + s.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == accessClientId && ap.Read) || + s.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == accessClientId && ap.Read)) || + s.Projects.Any(p => + p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == accessClientId && ap.Read) || + p.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == accessClientId && ap.Read))), + s.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == accessClientId && ap.Write) || + s.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == accessClientId && ap.Write)) || + s.Projects.Any(p => + p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == accessClientId && ap.Write) || + p.GroupAccessPolicies.Any(ap => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == accessClientId && ap.Write))) + )), + AccessClientType.ServiceAccount => secrets.Select(s => new SecretAccess( + s.Id, + s.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == accessClientId && ap.Read) || + s.Projects.Any(p => + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == accessClientId && ap.Read)), + s.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == accessClientId && ap.Write) || + s.Projects.Any(p => + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == accessClientId && ap.Write)) + )), + _ => secrets.Select(s => new SecretAccess(s.Id, false, false)) + }; + + private record SecretAccess(Guid Id, bool Read, bool Write); } diff --git a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ServiceAccountRepository.cs b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ServiceAccountRepository.cs index 10fe0bea4f..ffeb939e2d 100644 --- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ServiceAccountRepository.cs +++ b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ServiceAccountRepository.cs @@ -135,38 +135,37 @@ public class ServiceAccountRepository : Repository> GetManyByOrganizationIdWithSecretsDetailsAsync( - Guid organizationId, Guid userId, AccessClientType accessType) + Guid organizationId, Guid userId, AccessClientType accessType) { - using var scope = ServiceScopeFactory.CreateScope(); + await using var scope = ServiceScopeFactory.CreateAsyncScope(); var dbContext = GetDatabaseContext(scope); - var query = from sa in dbContext.ServiceAccount - join ap in dbContext.ServiceAccountProjectAccessPolicy - on sa.Id equals ap.ServiceAccountId into grouping - from ap in grouping.DefaultIfEmpty() - where sa.OrganizationId == organizationId - select new - { - ServiceAccount = sa, - AccessToSecrets = ap.GrantedProject.Secrets.Count(s => s.DeletedDate == null) - }; - query = accessType switch + var serviceAccountQuery = dbContext.ServiceAccount.Where(c => c.OrganizationId == organizationId); + serviceAccountQuery = accessType switch { - AccessClientType.NoAccessCheck => query, - AccessClientType.User => query.Where(c => - c.ServiceAccount.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Read) || - c.ServiceAccount.GroupAccessPolicies.Any(ap => + AccessClientType.NoAccessCheck => serviceAccountQuery, + AccessClientType.User => serviceAccountQuery.Where(c => + c.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Read) || + c.GroupAccessPolicies.Any(ap => ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Read))), _ => throw new ArgumentOutOfRangeException(nameof(accessType), accessType, null), }; - var results = (await query.ToListAsync()) + var projectSecretsAccessQuery = BuildProjectSecretsAccessQuery(dbContext, serviceAccountQuery); + var directSecretAccessQuery = BuildDirectSecretAccessQuery(dbContext, serviceAccountQuery); + + var projectSecretsAccessResults = await projectSecretsAccessQuery.ToListAsync(); + var directSecretAccessResults = await directSecretAccessQuery.ToListAsync(); + + var applicableDirectSecretAccessResults = FilterDirectSecretAccessResults(projectSecretsAccessResults, directSecretAccessResults); + + var results = projectSecretsAccessResults.Concat(applicableDirectSecretAccessResults) .GroupBy(g => g.ServiceAccount) .Select(g => new ServiceAccountSecretsDetails { ServiceAccount = Mapper.Map(g.Key), - AccessToSecrets = g.Sum(x => x.AccessToSecrets), + AccessToSecrets = g.Sum(x => x.SecretIds.Count()) }).OrderBy(c => c.ServiceAccount.RevisionDate).ToList(); return results; @@ -200,4 +199,46 @@ public class ServiceAccountRepository : Repository> UserHasWriteAccessToServiceAccount(Guid userId) => sa => sa.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Write) || sa.GroupAccessPolicies.Any(ap => ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Write)); + + private static IQueryable BuildProjectSecretsAccessQuery(DatabaseContext dbContext, + IQueryable serviceAccountQuery) => + from sa in serviceAccountQuery + join ap in dbContext.ServiceAccountProjectAccessPolicy + on sa.Id equals ap.ServiceAccountId into grouping + from ap in grouping.DefaultIfEmpty() + select new ServiceAccountSecretsAccess + ( + sa, ap.GrantedProject.Secrets.Where(s => s.DeletedDate == null).Select(s => s.Id) + ); + + private static IQueryable BuildDirectSecretAccessQuery( + DatabaseContext dbContext, + IQueryable serviceAccountQuery) => + from sa in serviceAccountQuery + join ap in dbContext.ServiceAccountSecretAccessPolicy + on sa.Id equals ap.ServiceAccountId into grouping + from ap in grouping.DefaultIfEmpty() + where ap.GrantedSecret.DeletedDate == null && + ap.GrantedSecretId != null + select new ServiceAccountSecretsAccess(sa, + new List { ap.GrantedSecretId.Value }); + + private static List FilterDirectSecretAccessResults( + List projectSecretsAccessResults, + List directSecretAccessResults) => + directSecretAccessResults.Where(directSecretAccessResult => + { + var serviceAccountId = directSecretAccessResult.ServiceAccount.Id; + var secretId = directSecretAccessResult.SecretIds.FirstOrDefault(); + if (secretId == Guid.Empty) + { + return false; + } + + return !projectSecretsAccessResults + .Where(x => x.ServiceAccount.Id == serviceAccountId) + .Any(x => x.SecretIds.Contains(secretId)); + }).ToList(); + + private record ServiceAccountSecretsAccess(ServiceAccount ServiceAccount, IEnumerable SecretIds); } diff --git a/src/Api/SecretsManager/Models/Response/ServiceAccountResponseModel.cs b/src/Api/SecretsManager/Models/Response/ServiceAccountResponseModel.cs index 868c241ec2..570c91fd08 100644 --- a/src/Api/SecretsManager/Models/Response/ServiceAccountResponseModel.cs +++ b/src/Api/SecretsManager/Models/Response/ServiceAccountResponseModel.cs @@ -49,5 +49,9 @@ public class ServiceAccountSecretsDetailsResponseModel : ServiceAccountResponseM AccessToSecrets = serviceAccountDetails.AccessToSecrets; } + public ServiceAccountSecretsDetailsResponseModel() : base(new ServiceAccount()) + { + } + public int AccessToSecrets { get; set; } } diff --git a/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs b/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs index f25005b269..ae7b522750 100644 --- a/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs +++ b/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs @@ -29,6 +29,8 @@ public class ServiceAccountsControllerTests : IClassFixture(); _accessPolicyRepository = _factory.GetService(); _apiKeyRepository = _factory.GetService(); + _secretRepository = _factory.GetService(); + _projectRepository = _factory.GetService(); _loginHelper = new LoginHelper(_factory, _client); } @@ -73,51 +77,90 @@ public class ServiceAccountsControllerTests : IClassFixture>(); + var result = await response.Content + .ReadFromJsonAsync>(); Assert.NotNull(result); Assert.NotEmpty(result.Data); Assert.Equal(serviceAccountIds.Count, result.Data.Count()); + Assert.DoesNotContain(result.Data, x => x.AccessToSecrets != 0); } - [Fact] - public async Task ListByOrganization_User_Success() + [Theory] + [InlineData(PermissionType.RunAsAdmin)] + [InlineData(PermissionType.RunAsUserWithPermission)] + public async Task ListByOrganization_SecretAccess_Success(PermissionType permissionType) { - var (org, _) = await _organizationHelper.Initialize(true, true, true); - var (email, orgUser) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); - await _loginHelper.LoginAsync(email); + var (orgId, serviceAccountIds) = await SetupListByOrganizationRequestAsync(permissionType); + var expectedAccess = await SetupServiceAccountSecretAccessAsync(serviceAccountIds, orgId); - var serviceAccountIds = await SetupGetServiceAccountsByOrganizationAsync(org); - - // Setup access for two - var accessPolicies = serviceAccountIds.Take(2).Select( - id => new UserServiceAccountAccessPolicy - { - OrganizationUserId = orgUser.Id, - GrantedServiceAccountId = id, - Read = true, - Write = false, - }).Cast().ToList(); - - await _accessPolicyRepository.CreateManyAsync(accessPolicies); - - var response = await _client.GetAsync($"/organizations/{org.Id}/service-accounts"); + var response = await _client.GetAsync($"/organizations/{orgId}/service-accounts?includeAccessToSecrets=true"); response.EnsureSuccessStatusCode(); - var result = await response.Content.ReadFromJsonAsync>(); + var result = await response.Content + .ReadFromJsonAsync>(); Assert.NotNull(result); Assert.NotEmpty(result.Data); - Assert.Equal(2, result.Data.Count()); + Assert.Equal(serviceAccountIds.Count, result.Data.Count()); + + foreach (var item in expectedAccess) + { + var serviceAccountResult = result.Data.FirstOrDefault(x => x.Id == item.Key); + Assert.NotNull(serviceAccountResult); + Assert.Equal(item.Value, serviceAccountResult.AccessToSecrets); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ListByOrganization_UserPartialAccess_ReturnsServiceAccountsUserHasAccessTo( + bool includeAccessToSecrets) + { + var (orgId, serviceAccountIds) = + await SetupListByOrganizationRequestAsync(PermissionType.RunAsUserWithPermission); + var expectedAccess = await SetupServiceAccountSecretAccessAsync(serviceAccountIds, orgId); + + var serviceAccountWithoutAccess = await _serviceAccountRepository.CreateAsync(new ServiceAccount + { + OrganizationId = orgId, + Name = _mockEncryptedString + }); + + var response = + await _client.GetAsync( + $"/organizations/{orgId}/service-accounts?includeAccessToSecrets={includeAccessToSecrets}"); + response.EnsureSuccessStatusCode(); + var result = await response.Content + .ReadFromJsonAsync>(); + + Assert.NotNull(result); + Assert.NotEmpty(result.Data); + Assert.Equal(serviceAccountIds.Count, result.Data.Count()); + Assert.DoesNotContain(result.Data, x => x.Id == serviceAccountWithoutAccess.Id); + + if (includeAccessToSecrets) + { + foreach (var item in expectedAccess) + { + var serviceAccountResult = result.Data.FirstOrDefault(x => x.Id == item.Key); + Assert.NotNull(serviceAccountResult); + Assert.Equal(item.Value, serviceAccountResult.AccessToSecrets); + } + } + else + { + Assert.Contains(result.Data, x => x.AccessToSecrets == 0); + } } [Theory] @@ -824,10 +867,10 @@ public class ServiceAccountsControllerTests : IClassFixture { policy }); } - private async Task> SetupGetServiceAccountsByOrganizationAsync(Organization org) + private async Task> CreateServiceAccountsInOrganizationAsync(Organization org) { var serviceAccountIds = new List(); - for (var i = 0; i < 3; i++) + for (var i = 0; i < 4; i++) { var serviceAccount = await _serviceAccountRepository.CreateAsync(new ServiceAccount { @@ -870,4 +913,109 @@ public class ServiceAccountsControllerTests : IClassFixture ServiceAccountIds)> SetupListByOrganizationRequestAsync(PermissionType permissionType) + { + var (org, _) = await _organizationHelper.Initialize(true, true, true); + await _loginHelper.LoginAsync(_email); + + var serviceAccountIds = await CreateServiceAccountsInOrganizationAsync(org); + + if (permissionType == PermissionType.RunAsAdmin) + { + return (org.Id, serviceAccountIds); + } + + var (email, orgUser) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); + await _loginHelper.LoginAsync(email); + + var accessPolicies = serviceAccountIds.Select( + id => new UserServiceAccountAccessPolicy + { + OrganizationUserId = orgUser.Id, + GrantedServiceAccountId = id, + Read = true, + Write = false, + }).Cast().ToList(); + + await _accessPolicyRepository.CreateManyAsync(accessPolicies); + + return (org.Id, serviceAccountIds); + } + + private async Task> SetupServiceAccountSecretAccessAsync(List serviceAccountIds, + Guid organizationId) + { + var project = + await _projectRepository.CreateAsync(new Project + { + Name = _mockEncryptedString, + OrganizationId = organizationId + }); + + var secret = await _secretRepository.CreateAsync(new Secret + { + Key = _mockEncryptedString, + Value = _mockEncryptedString, + OrganizationId = organizationId, + Projects = [project] + }); + + var secretNoProject = await _secretRepository.CreateAsync(new Secret + { + Key = _mockEncryptedString, + Value = _mockEncryptedString, + OrganizationId = organizationId + }); + + var serviceAccountWithProjectAccess = serviceAccountIds[0]; + var serviceAccountWithProjectAndSecretAccess = serviceAccountIds[1]; + var serviceAccountWithSecretAccess = serviceAccountIds[2]; + var serviceAccountWithNoAccess = serviceAccountIds[3]; + await _accessPolicyRepository.CreateManyAsync([ + new ServiceAccountProjectAccessPolicy + { + ServiceAccountId = serviceAccountWithProjectAccess, + GrantedProjectId = project.Id, + Read = true, + Write = true + }, + new ServiceAccountProjectAccessPolicy + { + ServiceAccountId = serviceAccountWithProjectAndSecretAccess, + GrantedProjectId = project.Id, + Read = true, + Write = true + }, + new ServiceAccountSecretAccessPolicy + { + ServiceAccountId = serviceAccountWithProjectAndSecretAccess, + GrantedSecretId = secret.Id, + Read = true, + Write = true + }, + new ServiceAccountSecretAccessPolicy + { + ServiceAccountId = serviceAccountWithProjectAndSecretAccess, + GrantedSecretId = secretNoProject.Id, + Read = true, + Write = true + }, + new ServiceAccountSecretAccessPolicy + { + ServiceAccountId = serviceAccountWithSecretAccess, + GrantedSecretId = secretNoProject.Id, + Read = true, + Write = true + } + ]); + + return new Dictionary + { + { serviceAccountWithProjectAccess, 1 }, + { serviceAccountWithProjectAndSecretAccess, 2 }, + { serviceAccountWithSecretAccess, 1 }, + { serviceAccountWithNoAccess, 0 } + }; + } }