From 73c6421bd3a216f91c39e930d99f8a6bf2553534 Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Thu, 3 Aug 2023 10:06:34 -0500 Subject: [PATCH 1/2] [SM-722] Add optional access to secrets for service account lists (#3074) * Add access to secret count to service account list * dotnet format * refactor into query * Remove duplicate * Add new method to noop --- .../ServiceAccountSecretsDetailsQuery.cs | 36 +++++++++++++ .../SecretsManagerCollectionExtensions.cs | 3 ++ .../Repositories/ServiceAccountRepository.cs | 39 ++++++++++++++ .../ServiceAccountSecretsDetailsQueryTests.cs | 52 +++++++++++++++++++ .../Controllers/ServiceAccountsController.cs | 18 ++++--- .../Response/ServiceAccountResponseModel.cs | 16 ++++++ .../Data/ServiceAccountSecretsDetails.cs | 9 ++++ .../IServiceAccountSecretsDetailsQuery.cs | 10 ++++ .../Repositories/IServiceAccountRepository.cs | 2 + .../Noop/NoopServiceAccountRepository.cs | 3 ++ .../ServiceAccountsControllerTests.cs | 21 ++++---- 11 files changed, 192 insertions(+), 17 deletions(-) create mode 100644 bitwarden_license/src/Commercial.Core/SecretsManager/Queries/ServiceAccounts/ServiceAccountSecretsDetailsQuery.cs create mode 100644 bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/ServiceAccounts/ServiceAccountSecretsDetailsQueryTests.cs create mode 100644 src/Core/SecretsManager/Models/Data/ServiceAccountSecretsDetails.cs create mode 100644 src/Core/SecretsManager/Queries/ServiceAccounts/Interfaces/IServiceAccountSecretsDetailsQuery.cs diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/ServiceAccounts/ServiceAccountSecretsDetailsQuery.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/ServiceAccounts/ServiceAccountSecretsDetailsQuery.cs new file mode 100644 index 000000000000..5bf3ba354e53 --- /dev/null +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/ServiceAccounts/ServiceAccountSecretsDetailsQuery.cs @@ -0,0 +1,36 @@ +using Bit.Core.Enums; +using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Queries.ServiceAccounts.Interfaces; +using Bit.Core.SecretsManager.Repositories; + +namespace Bit.Commercial.Core.SecretsManager.Queries.ServiceAccounts; + +public class ServiceAccountSecretsDetailsQuery : IServiceAccountSecretsDetailsQuery +{ + private readonly IServiceAccountRepository _serviceAccountRepository; + + public ServiceAccountSecretsDetailsQuery(IServiceAccountRepository serviceAccountRepository) + { + _serviceAccountRepository = serviceAccountRepository; + } + + public async Task> GetManyByOrganizationIdAsync( + Guid organizationId, Guid userId, AccessClientType accessClient, bool includeAccessToSecrets) + { + if (includeAccessToSecrets) + { + return await _serviceAccountRepository.GetManyByOrganizationIdWithSecretsDetailsAsync(organizationId, + userId, + accessClient); + } + + var serviceAccounts = + await _serviceAccountRepository.GetManyByOrganizationIdAsync(organizationId, userId, accessClient); + + return serviceAccounts.Select(sa => new ServiceAccountSecretsDetails + { + ServiceAccount = sa, + AccessToSecrets = 0, + }); + } +} diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs index 614bd4610539..5858ef9b55b6 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs @@ -10,6 +10,7 @@ using Bit.Commercial.Core.SecretsManager.Commands.ServiceAccounts; using Bit.Commercial.Core.SecretsManager.Commands.Trash; using Bit.Commercial.Core.SecretsManager.Queries; +using Bit.Commercial.Core.SecretsManager.Queries.ServiceAccounts; using Bit.Core.SecretsManager.Commands.AccessPolicies.Interfaces; using Bit.Core.SecretsManager.Commands.AccessTokens.Interfaces; using Bit.Core.SecretsManager.Commands.Porting.Interfaces; @@ -18,6 +19,7 @@ using Bit.Core.SecretsManager.Commands.ServiceAccounts.Interfaces; using Bit.Core.SecretsManager.Commands.Trash.Interfaces; using Bit.Core.SecretsManager.Queries.Interfaces; +using Bit.Core.SecretsManager.Queries.ServiceAccounts.Interfaces; using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.DependencyInjection; @@ -32,6 +34,7 @@ public static void AddSecretsManagerServices(this IServiceCollection services) services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); 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 d880f5d02434..e9a7c183b567 100644 --- a/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ServiceAccountRepository.cs +++ b/bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ServiceAccountRepository.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.Repositories; using Bit.Infrastructure.EntityFramework.SecretsManager.Models; @@ -140,6 +141,44 @@ public async Task GetServiceAccountCountByOrganizationIdAsync(Guid organiza } } + public async Task> GetManyByOrganizationIdWithSecretsDetailsAsync( + Guid organizationId, Guid userId, AccessClientType accessType) + { + using var scope = ServiceScopeFactory.CreateScope(); + 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 + { + 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 => + ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Read))), + _ => throw new ArgumentOutOfRangeException(nameof(accessType), accessType, null), + }; + + var results = (await query.ToListAsync()) + .GroupBy(g => g.ServiceAccount) + .Select(g => + new ServiceAccountSecretsDetails + { + ServiceAccount = Mapper.Map(g.Key), + AccessToSecrets = g.Sum(x => x.AccessToSecrets), + }).OrderBy(c => c.ServiceAccount.RevisionDate).ToList(); + + return results; + } + private static Expression> UserHasReadAccessToServiceAccount(Guid userId) => sa => sa.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Read) || sa.GroupAccessPolicies.Any(ap => ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Read)); diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/ServiceAccounts/ServiceAccountSecretsDetailsQueryTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/ServiceAccounts/ServiceAccountSecretsDetailsQueryTests.cs new file mode 100644 index 000000000000..0f926ceae9cb --- /dev/null +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/ServiceAccounts/ServiceAccountSecretsDetailsQueryTests.cs @@ -0,0 +1,52 @@ +using Bit.Commercial.Core.SecretsManager.Queries.ServiceAccounts; +using Bit.Core.Enums; +using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Repositories; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Bit.Test.Common.Helpers; +using NSubstitute; +using Xunit; + +namespace Bit.Commercial.Core.Test.SecretsManager.Queries.ServiceAccounts; + +[SutProviderCustomize] +public class ServiceAccountSecretsDetailsQueryTests +{ + [Theory] + [BitAutoData(false)] + [BitAutoData(true)] + public async Task GetManyByOrganizationId_CallsDifferentRepoMethods( + bool includeAccessToSecrets, + SutProvider sutProvider, + Guid organizationId, + Guid userId, + AccessClientType accessClient, + ServiceAccount mockSa, + ServiceAccountSecretsDetails mockSaDetails) + { + sutProvider.GetDependency().GetManyByOrganizationIdAsync(default, default, default) + .ReturnsForAnyArgs(new List { mockSa }); + + sutProvider.GetDependency().GetManyByOrganizationIdWithSecretsDetailsAsync(default, default, default) + .ReturnsForAnyArgs(new List { mockSaDetails }); + + + var result = await sutProvider.Sut.GetManyByOrganizationIdAsync(organizationId, userId, accessClient, includeAccessToSecrets); + + if (includeAccessToSecrets) + { + await sutProvider.GetDependency().Received(1) + .GetManyByOrganizationIdWithSecretsDetailsAsync(Arg.Is(AssertHelper.AssertPropertyEqual(mockSaDetails.ServiceAccount.OrganizationId)), + Arg.Any(), Arg.Any()); + } + else + { + await sutProvider.GetDependency().Received(1) + .GetManyByOrganizationIdAsync(Arg.Is(AssertHelper.AssertPropertyEqual(mockSa.OrganizationId)), + Arg.Any(), Arg.Any()); + Assert.Equal(0, result.First().AccessToSecrets); + } + } +} diff --git a/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs b/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs index 1b7307b8a283..4a7993f0bcaa 100644 --- a/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs +++ b/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs @@ -8,6 +8,7 @@ using Bit.Core.SecretsManager.Commands.AccessTokens.Interfaces; using Bit.Core.SecretsManager.Commands.ServiceAccounts.Interfaces; using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Queries.ServiceAccounts.Interfaces; using Bit.Core.SecretsManager.Repositories; using Bit.Core.Services; using Bit.Core.Utilities; @@ -26,6 +27,7 @@ public class ServiceAccountsController : Controller private readonly IAuthorizationService _authorizationService; private readonly IServiceAccountRepository _serviceAccountRepository; private readonly IApiKeyRepository _apiKeyRepository; + private readonly IServiceAccountSecretsDetailsQuery _serviceAccountSecretsDetailsQuery; private readonly ICreateAccessTokenCommand _createAccessTokenCommand; private readonly ICreateServiceAccountCommand _createServiceAccountCommand; private readonly IUpdateServiceAccountCommand _updateServiceAccountCommand; @@ -38,6 +40,7 @@ public ServiceAccountsController( IAuthorizationService authorizationService, IServiceAccountRepository serviceAccountRepository, IApiKeyRepository apiKeyRepository, + IServiceAccountSecretsDetailsQuery serviceAccountSecretsDetailsQuery, ICreateAccessTokenCommand createAccessTokenCommand, ICreateServiceAccountCommand createServiceAccountCommand, IUpdateServiceAccountCommand updateServiceAccountCommand, @@ -49,6 +52,7 @@ public ServiceAccountsController( _authorizationService = authorizationService; _serviceAccountRepository = serviceAccountRepository; _apiKeyRepository = apiKeyRepository; + _serviceAccountSecretsDetailsQuery = serviceAccountSecretsDetailsQuery; _createServiceAccountCommand = createServiceAccountCommand; _updateServiceAccountCommand = updateServiceAccountCommand; _deleteServiceAccountsCommand = deleteServiceAccountsCommand; @@ -57,8 +61,8 @@ public ServiceAccountsController( } [HttpGet("/organizations/{organizationId}/service-accounts")] - public async Task> ListByOrganizationAsync( - [FromRoute] Guid organizationId) + public async Task> ListByOrganizationAsync( + [FromRoute] Guid organizationId, [FromQuery] bool includeAccessToSecrets = false) { if (!_currentContext.AccessSecretsManager(organizationId)) { @@ -69,11 +73,11 @@ public async Task> ListByOrganiza var orgAdmin = await _currentContext.OrganizationAdmin(organizationId); var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin); - var serviceAccounts = - await _serviceAccountRepository.GetManyByOrganizationIdAsync(organizationId, userId, accessClient); - - var responses = serviceAccounts.Select(serviceAccount => new ServiceAccountResponseModel(serviceAccount)); - return new ListResponseModel(responses); + var results = + await _serviceAccountSecretsDetailsQuery.GetManyByOrganizationIdAsync(organizationId, userId, accessClient, + includeAccessToSecrets); + var responses = results.Select(r => new ServiceAccountSecretsDetailsResponseModel(r)); + return new ListResponseModel(responses); } [HttpGet("{id}")] diff --git a/src/Api/SecretsManager/Models/Response/ServiceAccountResponseModel.cs b/src/Api/SecretsManager/Models/Response/ServiceAccountResponseModel.cs index c8306b61e75a..868c241ec228 100644 --- a/src/Api/SecretsManager/Models/Response/ServiceAccountResponseModel.cs +++ b/src/Api/SecretsManager/Models/Response/ServiceAccountResponseModel.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; @@ -35,3 +36,18 @@ public ServiceAccountResponseModel() : base(_objectName) public DateTime RevisionDate { get; set; } } + +public class ServiceAccountSecretsDetailsResponseModel : ServiceAccountResponseModel +{ + public ServiceAccountSecretsDetailsResponseModel(ServiceAccountSecretsDetails serviceAccountDetails) : base(serviceAccountDetails.ServiceAccount) + { + if (serviceAccountDetails == null) + { + throw new ArgumentNullException(nameof(serviceAccountDetails)); + } + + AccessToSecrets = serviceAccountDetails.AccessToSecrets; + } + + public int AccessToSecrets { get; set; } +} diff --git a/src/Core/SecretsManager/Models/Data/ServiceAccountSecretsDetails.cs b/src/Core/SecretsManager/Models/Data/ServiceAccountSecretsDetails.cs new file mode 100644 index 000000000000..67a369f02e1e --- /dev/null +++ b/src/Core/SecretsManager/Models/Data/ServiceAccountSecretsDetails.cs @@ -0,0 +1,9 @@ +using Bit.Core.SecretsManager.Entities; + +namespace Bit.Core.SecretsManager.Models.Data; + +public class ServiceAccountSecretsDetails +{ + public ServiceAccount ServiceAccount { get; set; } + public int AccessToSecrets { get; set; } +} diff --git a/src/Core/SecretsManager/Queries/ServiceAccounts/Interfaces/IServiceAccountSecretsDetailsQuery.cs b/src/Core/SecretsManager/Queries/ServiceAccounts/Interfaces/IServiceAccountSecretsDetailsQuery.cs new file mode 100644 index 000000000000..5af9f4d56d47 --- /dev/null +++ b/src/Core/SecretsManager/Queries/ServiceAccounts/Interfaces/IServiceAccountSecretsDetailsQuery.cs @@ -0,0 +1,10 @@ +using Bit.Core.Enums; +using Bit.Core.SecretsManager.Models.Data; + +namespace Bit.Core.SecretsManager.Queries.ServiceAccounts.Interfaces; + +public interface IServiceAccountSecretsDetailsQuery +{ + public Task> GetManyByOrganizationIdAsync( + Guid organizationId, Guid userId, AccessClientType accessClient, bool includeAccessToSecrets); +} diff --git a/src/Core/SecretsManager/Repositories/IServiceAccountRepository.cs b/src/Core/SecretsManager/Repositories/IServiceAccountRepository.cs index b362a5676b89..40f9cbfdd66b 100644 --- a/src/Core/SecretsManager/Repositories/IServiceAccountRepository.cs +++ b/src/Core/SecretsManager/Repositories/IServiceAccountRepository.cs @@ -1,5 +1,6 @@ using Bit.Core.Enums; using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Models.Data; namespace Bit.Core.SecretsManager.Repositories; @@ -16,4 +17,5 @@ public interface IServiceAccountRepository Task> GetManyByOrganizationIdWriteAccessAsync(Guid organizationId, Guid userId, AccessClientType accessType); Task<(bool Read, bool Write)> AccessToServiceAccountAsync(Guid id, Guid userId, AccessClientType accessType); Task GetServiceAccountCountByOrganizationIdAsync(Guid organizationId); + Task> GetManyByOrganizationIdWithSecretsDetailsAsync(Guid organizationId, Guid userId, AccessClientType accessType); } diff --git a/src/Core/SecretsManager/Repositories/Noop/NoopServiceAccountRepository.cs b/src/Core/SecretsManager/Repositories/Noop/NoopServiceAccountRepository.cs index eab0b069b9ea..dc5c5e209d62 100644 --- a/src/Core/SecretsManager/Repositories/Noop/NoopServiceAccountRepository.cs +++ b/src/Core/SecretsManager/Repositories/Noop/NoopServiceAccountRepository.cs @@ -1,5 +1,6 @@ using Bit.Core.Enums; using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Models.Data; namespace Bit.Core.SecretsManager.Repositories.Noop; @@ -56,4 +57,6 @@ public Task GetServiceAccountCountByOrganizationIdAsync(Guid organizationId { return Task.FromResult(0); } + + public Task> GetManyByOrganizationIdWithSecretsDetailsAsync(Guid organizationId, Guid userId, AccessClientType accessType) => throw new NotImplementedException(); } diff --git a/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs index f075d0ec31ae..0847c5c377a3 100644 --- a/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs @@ -8,6 +8,7 @@ using Bit.Core.SecretsManager.Commands.ServiceAccounts.Interfaces; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Queries.ServiceAccounts.Interfaces; using Bit.Core.SecretsManager.Repositories; using Bit.Core.Services; using Bit.Test.Common.AutoFixture; @@ -33,9 +34,9 @@ public async void GetServiceAccountsByOrganization_ReturnsEmptyList( sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); var result = await sutProvider.Sut.ListByOrganizationAsync(id); - await sutProvider.GetDependency().Received(1) - .GetManyByOrganizationIdAsync(Arg.Is(AssertHelper.AssertPropertyEqual(id)), Arg.Any(), - Arg.Any()); + await sutProvider.GetDependency().Received(1) + .GetManyByOrganizationIdAsync(Arg.Is(AssertHelper.AssertPropertyEqual(id)), + Arg.Any(), Arg.Any(), Arg.Any()); Assert.Empty(result.Data); } @@ -43,18 +44,18 @@ await sutProvider.GetDependency().Received(1) [Theory] [BitAutoData] public async void GetServiceAccountsByOrganization_Success(SutProvider sutProvider, - ServiceAccount resultServiceAccount) + ServiceAccountSecretsDetails resultServiceAccount) { sutProvider.GetDependency().AccessSecretsManager(default).ReturnsForAnyArgs(true); sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); - sutProvider.GetDependency().GetManyByOrganizationIdAsync(default, default, default) - .ReturnsForAnyArgs(new List { resultServiceAccount }); + sutProvider.GetDependency().GetManyByOrganizationIdAsync(default, default, default, default) + .ReturnsForAnyArgs(new List { resultServiceAccount }); - var result = await sutProvider.Sut.ListByOrganizationAsync(resultServiceAccount.OrganizationId); + var result = await sutProvider.Sut.ListByOrganizationAsync(resultServiceAccount.ServiceAccount.OrganizationId); - await sutProvider.GetDependency().Received(1) - .GetManyByOrganizationIdAsync(Arg.Is(AssertHelper.AssertPropertyEqual(resultServiceAccount.OrganizationId)), - Arg.Any(), Arg.Any()); + await sutProvider.GetDependency().Received(1) + .GetManyByOrganizationIdAsync(Arg.Is(AssertHelper.AssertPropertyEqual(resultServiceAccount.ServiceAccount.OrganizationId)), + Arg.Any(), Arg.Any(), Arg.Any()); Assert.NotEmpty(result.Data); Assert.Single(result.Data); } From 78588d0246d21cfbd8bb01b36d6cec380647c9d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Thu, 3 Aug 2023 18:36:47 +0100 Subject: [PATCH 2/2] [PM-3007] Caching user policies on PolicyService variable (#3117) * [PM-3007] Caching user policies on PolicyService variable * [PM-3007] Added missing newlines on sql files --- .../IOrganizationUserRepository.cs | 2 +- .../Services/Implementations/PolicyService.cs | 17 +++++++--- .../OrganizationUserRepository.cs | 4 +-- .../OrganizationUserRepository.cs | 5 ++- ...tionUser_ReadByUserIdWithPolicyDetails.sql | 30 ++++++++---------- test/Core.Test/Services/PolicyServiceTests.cs | 10 ++---- .../OrganizationUserRepositoryTests.cs | 2 +- ...ationUserReadByUserIdWithPolicyDetails.sql | 31 +++++++++++++++++++ 8 files changed, 65 insertions(+), 36 deletions(-) create mode 100644 util/Migrator/DbScripts/2023-07-18_00_OrganizationUserReadByUserIdWithPolicyDetails.sql diff --git a/src/Core/Repositories/IOrganizationUserRepository.cs b/src/Core/Repositories/IOrganizationUserRepository.cs index f9dfa12c2cbf..fb012c62362f 100644 --- a/src/Core/Repositories/IOrganizationUserRepository.cs +++ b/src/Core/Repositories/IOrganizationUserRepository.cs @@ -39,6 +39,6 @@ Task GetDetailsByUserAsync(Guid userId, Gui Task> GetManyByMinimumRoleAsync(Guid organizationId, OrganizationUserType minRole); Task RevokeAsync(Guid id); Task RestoreAsync(Guid id, OrganizationUserStatusType status); - Task> GetByUserIdWithPolicyDetailsAsync(Guid userId, PolicyType policyType); + Task> GetByUserIdWithPolicyDetailsAsync(Guid userId); Task GetOccupiedSmSeatCountByOrganizationIdAsync(Guid organizationId); } diff --git a/src/Core/Services/Implementations/PolicyService.cs b/src/Core/Services/Implementations/PolicyService.cs index 6b1009093c1c..595a798e7307 100644 --- a/src/Core/Services/Implementations/PolicyService.cs +++ b/src/Core/Services/Implementations/PolicyService.cs @@ -20,6 +20,8 @@ public class PolicyService : IPolicyService private readonly IMailService _mailService; private readonly GlobalSettings _globalSettings; + private IEnumerable _cachedOrganizationUserPolicyDetails; + public PolicyService( IEventService eventService, IOrganizationRepository organizationRepository, @@ -194,18 +196,25 @@ public async Task AnyPoliciesApplicableToUserAsync(Guid userId, PolicyType return result.Any(); } - private async Task> QueryOrganizationUserPolicyDetailsAsync(Guid userId, PolicyType policyType, OrganizationUserStatusType minStatus = OrganizationUserStatusType.Accepted) + private async Task> QueryOrganizationUserPolicyDetailsAsync(Guid userId, PolicyType? policyType, OrganizationUserStatusType minStatus = OrganizationUserStatusType.Accepted) { - var organizationUserPolicyDetails = await _organizationUserRepository.GetByUserIdWithPolicyDetailsAsync(userId, policyType); + // Check if the cached policies are available + if (_cachedOrganizationUserPolicyDetails == null) + { + // Cached policies not available, retrieve from the repository + _cachedOrganizationUserPolicyDetails = await _organizationUserRepository.GetByUserIdWithPolicyDetailsAsync(userId); + } + var excludedUserTypes = GetUserTypesExcludedFromPolicy(policyType); - return organizationUserPolicyDetails.Where(o => + return _cachedOrganizationUserPolicyDetails.Where(o => + (policyType == null || o.PolicyType == policyType) && o.PolicyEnabled && !excludedUserTypes.Contains(o.OrganizationUserType) && o.OrganizationUserStatus >= minStatus && !o.IsProvider); } - private OrganizationUserType[] GetUserTypesExcludedFromPolicy(PolicyType policyType) + private OrganizationUserType[] GetUserTypesExcludedFromPolicy(PolicyType? policyType) { switch (policyType) { diff --git a/src/Infrastructure.Dapper/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.Dapper/Repositories/OrganizationUserRepository.cs index 008242c26c2b..27410fb3f7a1 100644 --- a/src/Infrastructure.Dapper/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/OrganizationUserRepository.cs @@ -505,13 +505,13 @@ public async Task RestoreAsync(Guid id, OrganizationUserStatusType status) } } - public async Task> GetByUserIdWithPolicyDetailsAsync(Guid userId, PolicyType policyType) + public async Task> GetByUserIdWithPolicyDetailsAsync(Guid userId) { using (var connection = new SqlConnection(ConnectionString)) { var results = await connection.QueryAsync( $"[{Schema}].[{Table}_ReadByUserIdWithPolicyDetails]", - new { UserId = userId, PolicyType = policyType }, + new { UserId = userId }, commandType: CommandType.StoredProcedure); return results.ToList(); diff --git a/src/Infrastructure.EntityFramework/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.EntityFramework/Repositories/OrganizationUserRepository.cs index 8256696d9686..f5ba2b9c1b66 100644 --- a/src/Infrastructure.EntityFramework/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/OrganizationUserRepository.cs @@ -588,7 +588,7 @@ public async Task RestoreAsync(Guid id, OrganizationUserStatusType status) } } - public async Task> GetByUserIdWithPolicyDetailsAsync(Guid userId, PolicyType policyType) + public async Task> GetByUserIdWithPolicyDetailsAsync(Guid userId) { using (var scope = ServiceScopeFactory.CreateScope()) { @@ -604,8 +604,7 @@ on pu.ProviderId equals po.ProviderId join ou in dbContext.OrganizationUsers on p.OrganizationId equals ou.OrganizationId let email = dbContext.Users.Find(userId).Email // Invited orgUsers do not have a UserId associated with them, so we have to match up their email - where p.Type == policyType && - (ou.UserId == userId || ou.Email == email) + where ou.UserId == userId || ou.Email == email select new OrganizationUserPolicyDetails { OrganizationUserId = ou.Id, diff --git a/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByUserIdWithPolicyDetails.sql b/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByUserIdWithPolicyDetails.sql index c2bc690a2787..b4968e2cd9d8 100644 --- a/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByUserIdWithPolicyDetails.sql +++ b/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByUserIdWithPolicyDetails.sql @@ -1,6 +1,5 @@ CREATE PROCEDURE [dbo].[OrganizationUser_ReadByUserIdWithPolicyDetails] - @UserId UNIQUEIDENTIFIER, - @PolicyType TINYINT + @UserId UNIQUEIDENTIFIER AS BEGIN SET NOCOUNT ON @@ -13,22 +12,19 @@ SELECT OU.[Type] AS OrganizationUserType, OU.[Status] AS OrganizationUserStatus, OU.[Permissions] AS OrganizationUserPermissionsData, - CASE WHEN EXISTS ( - SELECT 1 - FROM [dbo].[ProviderUserView] PU - INNER JOIN [dbo].[ProviderOrganizationView] PO ON PO.[ProviderId] = PU.[ProviderId] - WHERE PU.[UserId] = OU.[UserId] AND PO.[OrganizationId] = P.[OrganizationId] - ) THEN 1 ELSE 0 END AS IsProvider + CASE WHEN PU.[ProviderId] IS NOT NULL THEN 1 ELSE 0 END AS IsProvider FROM [dbo].[PolicyView] P INNER JOIN [dbo].[OrganizationUserView] OU ON P.[OrganizationId] = OU.[OrganizationId] -WHERE P.[Type] = @PolicyType AND - ( - (OU.[Status] != 0 AND OU.[UserId] = @UserId) -- OrgUsers who have accepted their invite and are linked to a UserId - OR EXISTS ( - SELECT 1 - FROM [dbo].[UserView] U - WHERE U.[Id] = @UserId AND OU.[Email] = U.[Email] AND OU.[Status] = 0 -- 'Invited' OrgUsers are not linked to a UserId yet, so we have to look up their email - ) +LEFT JOIN [dbo].[ProviderUserView] PU + ON PU.[UserId] = OU.[UserId] +LEFT JOIN [dbo].[ProviderOrganizationView] PO + ON PO.[ProviderId] = PU.[ProviderId] AND PO.[OrganizationId] = P.[OrganizationId] +WHERE + (OU.[Status] != 0 AND OU.[UserId] = @UserId) -- OrgUsers who have accepted their invite and are linked to a UserId + OR EXISTS ( + SELECT 1 + FROM [dbo].[UserView] U + WHERE U.[Id] = @UserId AND OU.[Email] = U.[Email] AND OU.[Status] = 0 -- 'Invited' OrgUsers are not linked to a UserId yet, so we have to look up their email ) -END \ No newline at end of file +END diff --git a/test/Core.Test/Services/PolicyServiceTests.cs b/test/Core.Test/Services/PolicyServiceTests.cs index 6c1218c84a7f..a85c2a7c491b 100644 --- a/test/Core.Test/Services/PolicyServiceTests.cs +++ b/test/Core.Test/Services/PolicyServiceTests.cs @@ -624,18 +624,12 @@ private static void SetupOrg(SutProvider sutProvider, Guid organi private static void SetupUserPolicies(Guid userId, SutProvider sutProvider) { sutProvider.GetDependency() - .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.RequireSso) + .GetByUserIdWithPolicyDetailsAsync(userId) .Returns(new List { new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = false, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false}, new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = false }, - new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = true } - }); - - sutProvider.GetDependency() - .GetByUserIdWithPolicyDetailsAsync(userId, PolicyType.DisableSend) - .Returns(new List - { + new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.RequireSso, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.Owner, OrganizationUserStatus = OrganizationUserStatusType.Confirmed, IsProvider = true }, new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Invited, IsProvider = false }, new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Invited, IsProvider = true } }); diff --git a/test/Infrastructure.EFIntegration.Test/Repositories/OrganizationUserRepositoryTests.cs b/test/Infrastructure.EFIntegration.Test/Repositories/OrganizationUserRepositoryTests.cs index 4f3b912b6ec0..b1ee1cbc9bee 100644 --- a/test/Infrastructure.EFIntegration.Test/Repositories/OrganizationUserRepositoryTests.cs +++ b/test/Infrastructure.EFIntegration.Test/Repositories/OrganizationUserRepositoryTests.cs @@ -274,7 +274,7 @@ SqlRepo.ProviderUserRepository sqlProviderUserRepo } // Act - var result = await orgUserRepos[i].GetByUserIdWithPolicyDetailsAsync(savedUser.Id, policy.Type); + var result = await orgUserRepos[i].GetByUserIdWithPolicyDetailsAsync(savedUser.Id); results.Add(result.FirstOrDefault()); } diff --git a/util/Migrator/DbScripts/2023-07-18_00_OrganizationUserReadByUserIdWithPolicyDetails.sql b/util/Migrator/DbScripts/2023-07-18_00_OrganizationUserReadByUserIdWithPolicyDetails.sql new file mode 100644 index 000000000000..a56d8cec566b --- /dev/null +++ b/util/Migrator/DbScripts/2023-07-18_00_OrganizationUserReadByUserIdWithPolicyDetails.sql @@ -0,0 +1,31 @@ +CREATE OR ALTER PROCEDURE [dbo].[OrganizationUser_ReadByUserIdWithPolicyDetails] + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON +SELECT + OU.[Id] AS OrganizationUserId, + P.[OrganizationId], + P.[Type] AS PolicyType, + P.[Enabled] AS PolicyEnabled, + P.[Data] AS PolicyData, + OU.[Type] AS OrganizationUserType, + OU.[Status] AS OrganizationUserStatus, + OU.[Permissions] AS OrganizationUserPermissionsData, + CASE WHEN PU.[ProviderId] IS NOT NULL THEN 1 ELSE 0 END AS IsProvider +FROM [dbo].[PolicyView] P +INNER JOIN [dbo].[OrganizationUserView] OU + ON P.[OrganizationId] = OU.[OrganizationId] +LEFT JOIN [dbo].[ProviderUserView] PU + ON PU.[UserId] = OU.[UserId] +LEFT JOIN [dbo].[ProviderOrganizationView] PO + ON PO.[ProviderId] = PU.[ProviderId] AND PO.[OrganizationId] = P.[OrganizationId] +WHERE + (OU.[Status] != 0 AND OU.[UserId] = @UserId) -- OrgUsers who have accepted their invite and are linked to a UserId + OR EXISTS ( + SELECT 1 + FROM [dbo].[UserView] U + WHERE U.[Id] = @UserId AND OU.[Email] = U.[Email] AND OU.[Status] = 0 -- 'Invited' OrgUsers are not linked to a UserId yet, so we have to look up their email + ) +END +GO