From 66629b2f1c3fdea38d63b4e03c6e2f9cfd362938 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Tue, 28 Sep 2021 06:54:28 +1000 Subject: [PATCH] Refactor policy checks (#1536) * Move policy checking logic inside PolicyService * Refactor to use currentContext.ManagePolicies * Make orgUser status check more semantic * Fix single org user checks * Use CoreHelper implementation to deserialize json * Refactor policy checks to use db query * Use new db query for enforcing 2FA Policy * Add Policy_ReadByTypeApplicableToUser * Stub out EF implementations * Refactor: use PolicyRepository only * Refactor tests * Copy SQL queries to proj and update sqlproj file * Refactor importCiphersAsync to use new method * Add EF implementations and tests * Refactor SQL to remove unnecessary operations --- .../EntityFramework/PolicyRepository.cs | 25 ++++ .../PolicyReadByTypeApplicableToUserQuery.cs | 45 ++++++ src/Core/Repositories/IPolicyRepository.cs | 4 + .../SqlServer/PolicyRepository.cs | 28 ++++ .../Services/Implementations/CipherService.cs | 44 ++---- .../Implementations/OrganizationService.cs | 61 +++----- .../Services/Implementations/SendService.cs | 33 ++--- .../Services/Implementations/UserService.cs | 28 ++-- src/Sql/Sql.sqlproj | 3 + .../dbo/Functions/PolicyApplicableToUser.sql | 35 +++++ .../Policy_CountByTypeApplicableToUser.sql | 11 ++ .../Policy_ReadByTypeApplicableToUser.sql | 11 ++ .../EntityFrameworkRepositoryFixtures.cs | 4 + test/Core.Test/AutoFixture/PolicyFixtures.cs | 24 ++++ .../EqualityComparers/PolicyCompare.cs | 9 ++ .../EntityFramework/PolicyRepositoryTests.cs | 132 +++++++++++++++++- test/Core.Test/Services/SendServiceTests.cs | 123 ++++++---------- .../2021-09-16_00_PolicyApplicableToUser.sql | 82 +++++++++++ 18 files changed, 505 insertions(+), 197 deletions(-) create mode 100644 src/Core/Repositories/EntityFramework/Queries/PolicyReadByTypeApplicableToUserQuery.cs create mode 100644 src/Sql/dbo/Functions/PolicyApplicableToUser.sql create mode 100644 src/Sql/dbo/Stored Procedures/Policy_CountByTypeApplicableToUser.sql create mode 100644 src/Sql/dbo/Stored Procedures/Policy_ReadByTypeApplicableToUser.sql create mode 100644 util/Migrator/DbScripts/2021-09-16_00_PolicyApplicableToUser.sql diff --git a/src/Core/Repositories/EntityFramework/PolicyRepository.cs b/src/Core/Repositories/EntityFramework/PolicyRepository.cs index 8ad345000f..9e8c95634a 100644 --- a/src/Core/Repositories/EntityFramework/PolicyRepository.cs +++ b/src/Core/Repositories/EntityFramework/PolicyRepository.cs @@ -54,5 +54,30 @@ namespace Bit.Core.Repositories.EntityFramework return Mapper.Map>(results); } } + + public async Task> GetManyByTypeApplicableToUserIdAsync(Guid userId, PolicyType policyType, + OrganizationUserStatusType minStatus) + { + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + + var query = new PolicyReadByTypeApplicableToUserQuery(userId, policyType, minStatus); + var results = await query.Run(dbContext).ToListAsync(); + return Mapper.Map>(results); + } + } + + public async Task GetCountByTypeApplicableToUserIdAsync(Guid userId, PolicyType policyType, + OrganizationUserStatusType minStatus) + { + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + + var query = new PolicyReadByTypeApplicableToUserQuery(userId, policyType, minStatus); + return await GetCountFromQuery(query); + } + } } } diff --git a/src/Core/Repositories/EntityFramework/Queries/PolicyReadByTypeApplicableToUserQuery.cs b/src/Core/Repositories/EntityFramework/Queries/PolicyReadByTypeApplicableToUserQuery.cs new file mode 100644 index 0000000000..57e52455db --- /dev/null +++ b/src/Core/Repositories/EntityFramework/Queries/PolicyReadByTypeApplicableToUserQuery.cs @@ -0,0 +1,45 @@ +using System.Collections.Generic; +using System.Linq; +using Bit.Core.Enums; +using Bit.Core.Models.EntityFramework; +using System; + +namespace Bit.Core.Repositories.EntityFramework.Queries +{ + public class PolicyReadByTypeApplicableToUserQuery : IQuery + { + private readonly Guid _userId; + private readonly PolicyType _policyType; + private readonly OrganizationUserStatusType _minimumStatus; + + public PolicyReadByTypeApplicableToUserQuery(Guid userId, PolicyType policyType, OrganizationUserStatusType minimumStatus) + { + _userId = userId; + _policyType = policyType; + _minimumStatus = minimumStatus; + } + + public IQueryable Run(DatabaseContext dbContext) + { + var providerOrganizations = from pu in dbContext.ProviderUsers + where pu.UserId == _userId + join po in dbContext.ProviderOrganizations + on pu.ProviderId equals po.ProviderId + select po; + + var query = from p in dbContext.Policies + join ou in dbContext.OrganizationUsers + on p.OrganizationId equals ou.OrganizationId + where ou.UserId == _userId && + p.Type == _policyType && + p.Enabled && + ou.Status >= _minimumStatus && + ou.Type >= OrganizationUserType.User && + (ou.Permissions == null || + ou.Permissions.Contains($"\"managePolicies\":false")) && + !providerOrganizations.Any(po => po.OrganizationId == p.OrganizationId) + select p; + return query; + } + } +} diff --git a/src/Core/Repositories/IPolicyRepository.cs b/src/Core/Repositories/IPolicyRepository.cs index 185b631a3b..cf80d32b54 100644 --- a/src/Core/Repositories/IPolicyRepository.cs +++ b/src/Core/Repositories/IPolicyRepository.cs @@ -11,5 +11,9 @@ namespace Bit.Core.Repositories Task GetByOrganizationIdTypeAsync(Guid organizationId, PolicyType type); Task> GetManyByOrganizationIdAsync(Guid organizationId); Task> GetManyByUserIdAsync(Guid userId); + Task> GetManyByTypeApplicableToUserIdAsync(Guid userId, PolicyType policyType, + OrganizationUserStatusType minStatus = OrganizationUserStatusType.Accepted); + Task GetCountByTypeApplicableToUserIdAsync(Guid userId, PolicyType policyType, + OrganizationUserStatusType minStatus = OrganizationUserStatusType.Accepted); } } diff --git a/src/Core/Repositories/SqlServer/PolicyRepository.cs b/src/Core/Repositories/SqlServer/PolicyRepository.cs index acde732cc7..c95f63cfff 100644 --- a/src/Core/Repositories/SqlServer/PolicyRepository.cs +++ b/src/Core/Repositories/SqlServer/PolicyRepository.cs @@ -58,5 +58,33 @@ namespace Bit.Core.Repositories.SqlServer return results.ToList(); } } + + public async Task> GetManyByTypeApplicableToUserIdAsync(Guid userId, PolicyType policyType, + OrganizationUserStatusType minStatus) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.QueryAsync( + $"[{Schema}].[{Table}_ReadByTypeApplicableToUser]", + new { UserId = userId, PolicyType = policyType, MinimumStatus = minStatus }, + commandType: CommandType.StoredProcedure); + + return results.ToList(); + } + } + + public async Task GetCountByTypeApplicableToUserIdAsync(Guid userId, PolicyType policyType, + OrganizationUserStatusType minStatus) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var result = await connection.ExecuteScalarAsync( + $"[{Schema}].[{Table}_CountByTypeApplicableToUser]", + new { UserId = userId, PolicyType = policyType, MinimumStatus = minStatus }, + commandType: CommandType.StoredProcedure); + + return result; + } + } } } diff --git a/src/Core/Services/Implementations/CipherService.cs b/src/Core/Services/Implementations/CipherService.cs index 3e22f6ddf0..5ffb7ae893 100644 --- a/src/Core/Services/Implementations/CipherService.cs +++ b/src/Core/Services/Implementations/CipherService.cs @@ -26,7 +26,6 @@ namespace Bit.Core.Services private readonly ICollectionRepository _collectionRepository; private readonly IUserRepository _userRepository; private readonly IOrganizationRepository _organizationRepository; - private readonly IOrganizationUserRepository _organizationUserRepository; private readonly ICollectionCipherRepository _collectionCipherRepository; private readonly IPushNotificationService _pushService; private readonly IAttachmentStorageService _attachmentStorageService; @@ -43,7 +42,6 @@ namespace Bit.Core.Services ICollectionRepository collectionRepository, IUserRepository userRepository, IOrganizationRepository organizationRepository, - IOrganizationUserRepository organizationUserRepository, ICollectionCipherRepository collectionCipherRepository, IPushNotificationService pushService, IAttachmentStorageService attachmentStorageService, @@ -58,7 +56,6 @@ namespace Bit.Core.Services _collectionRepository = collectionRepository; _userRepository = userRepository; _organizationRepository = organizationRepository; - _organizationUserRepository = organizationUserRepository; _collectionCipherRepository = collectionCipherRepository; _pushService = pushService; _attachmentStorageService = attachmentStorageService; @@ -139,19 +136,11 @@ namespace Bit.Core.Services else { // Make sure the user can save new ciphers to their personal vault - var userPolicies = await _policyRepository.GetManyByUserIdAsync(savingUserId); - if (userPolicies != null) + var personalOwnershipPolicyCount = await _policyRepository.GetCountByTypeApplicableToUserIdAsync(savingUserId, + PolicyType.PersonalOwnership); + if (personalOwnershipPolicyCount > 0) { - foreach (var policy in userPolicies.Where(p => p.Enabled && p.Type == PolicyType.PersonalOwnership)) - { - var org = await _organizationUserRepository.GetDetailsByUserAsync(savingUserId, policy.OrganizationId, - OrganizationUserStatusType.Confirmed); - if (org != null && org.Enabled && org.UsePolicies - && org.Type != OrganizationUserType.Admin && org.Type != OrganizationUserType.Owner) - { - throw new BadRequestException("Due to an Enterprise Policy, you are restricted from saving items to your personal vault."); - } - } + throw new BadRequestException("Due to an Enterprise Policy, you are restricted from saving items to your personal vault."); } await _cipherRepository.CreateAsync(cipher); } @@ -688,26 +677,13 @@ namespace Bit.Core.Services { var userId = folders.FirstOrDefault()?.UserId ?? ciphers.FirstOrDefault()?.UserId; - // Check user is allowed to import to personal vault - if (userId.HasValue) + // Make sure the user can save new ciphers to their personal vault + var personalOwnershipPolicyCount = await _policyRepository.GetCountByTypeApplicableToUserIdAsync(userId.Value, + PolicyType.PersonalOwnership); + if (personalOwnershipPolicyCount > 0) { - var policies = await _policyRepository.GetManyByUserIdAsync(userId.Value); - var allOrgUsers = await _organizationUserRepository.GetManyByUserAsync(userId.Value); - - var orgsWithBlockingPolicy = policies - .Where(p => p.Enabled && p.Type == PolicyType.PersonalOwnership) - .Select(p => p.OrganizationId); - var blockedByPolicy = allOrgUsers.Any(ou => - ou.Type != OrganizationUserType.Owner && - ou.Type != OrganizationUserType.Admin && - ou.Status != OrganizationUserStatusType.Invited && - orgsWithBlockingPolicy.Contains(ou.OrganizationId)); - - if (blockedByPolicy) - { - throw new BadRequestException("You cannot import items into your personal vault because you are " + - "a member of an organization which forbids it."); - } + throw new BadRequestException("You cannot import items into your personal vault because you are " + + "a member of an organization which forbids it."); } foreach (var cipher in ciphers) diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index 256a94b336..bbbd65239a 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -639,16 +639,8 @@ namespace Bit.Core.Services private async Task ValidateSignUpPoliciesAsync(Guid ownerId) { - var policies = await _policyRepository.GetManyByUserIdAsync(ownerId); - var orgUsers = await _organizationUserRepository.GetManyByUserAsync(ownerId); - - var orgsWithSingleOrgPolicy = policies.Where(p => p.Enabled && p.Type == PolicyType.SingleOrg) - .Select(p => p.OrganizationId); - var blockedBySingleOrgPolicy = orgUsers.Any(ou => ou is { Type: OrganizationUserType.Owner } && - ou.Type != OrganizationUserType.Admin && - ou.Status != OrganizationUserStatusType.Invited && - orgsWithSingleOrgPolicy.Contains(ou.OrganizationId)); - if (blockedBySingleOrgPolicy) + var singleOrgPolicyCount = await _policyRepository.GetCountByTypeApplicableToUserIdAsync(ownerId, PolicyType.SingleOrg); + if (singleOrgPolicyCount > 0) { throw new BadRequestException("You may not create an organization. You belong to an organization " + "which has a policy that prohibits you from being a member of any other organization."); @@ -1324,48 +1316,37 @@ namespace Bit.Core.Services } } - bool notExempt(OrganizationUser organizationUser) - { - return organizationUser.Type != OrganizationUserType.Owner && - organizationUser.Type != OrganizationUserType.Admin; - } - - var allOrgUsers = await _organizationUserRepository.GetManyByUserAsync(user.Id); - // Enforce Single Organization Policy of organization user is trying to join - var thisSingleOrgPolicy = await _policyRepository.GetByOrganizationIdTypeAsync(orgUser.OrganizationId, PolicyType.SingleOrg); - if (thisSingleOrgPolicy != null && - thisSingleOrgPolicy.Enabled && - notExempt(orgUser) && - allOrgUsers.Any(ou => ou.OrganizationId != orgUser.OrganizationId)) + var allOrgUsers = await _organizationUserRepository.GetManyByUserAsync(user.Id); + var hasOtherOrgs = allOrgUsers.Any(ou => ou.OrganizationId != orgUser.OrganizationId); + var invitedSingleOrgPolicies = await _policyRepository.GetManyByTypeApplicableToUserIdAsync(user.Id, + PolicyType.SingleOrg, OrganizationUserStatusType.Invited); + + if (hasOtherOrgs && invitedSingleOrgPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) { throw new BadRequestException("You may not join this organization until you leave or remove " + "all other organizations."); } // Enforce Single Organization Policy of other organizations user is a member of - var policies = await _policyRepository.GetManyByUserIdAsync(user.Id); - - var orgsWithSingleOrgPolicy = policies.Where(p => p.Enabled && p.Type == PolicyType.SingleOrg) - .Select(p => p.OrganizationId); - var blockedBySingleOrgPolicy = allOrgUsers.Any(ou => notExempt(ou) && - ou.Status != OrganizationUserStatusType.Invited && - orgsWithSingleOrgPolicy.Contains(ou.OrganizationId)); - - if (blockedBySingleOrgPolicy) + var singleOrgPolicyCount = await _policyRepository.GetCountByTypeApplicableToUserIdAsync(user.Id, + PolicyType.SingleOrg); + if (singleOrgPolicyCount > 0) { throw new BadRequestException("You cannot join this organization because you are a member of " + - "an organization which forbids it"); + "another organization which forbids it"); } - var twoFactorPolicy = await _policyRepository.GetByOrganizationIdTypeAsync(orgUser.OrganizationId, PolicyType.TwoFactorAuthentication); - if (!await userService.TwoFactorIsEnabledAsync(user) && - twoFactorPolicy != null && - twoFactorPolicy.Enabled && - notExempt(orgUser)) + // Enforce Two Factor Authentication Policy of organization user is trying to join + if (!await userService.TwoFactorIsEnabledAsync(user)) { - throw new BadRequestException("You cannot join this organization until you enable " + - "two-step login on your user account."); + var invitedTwoFactorPolicies = await _policyRepository.GetManyByTypeApplicableToUserIdAsync(user.Id, + PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited); + if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) + { + throw new BadRequestException("You cannot join this organization until you enable " + + "two-step login on your user account."); + } } orgUser.Status = OrganizationUserStatusType.Accepted; diff --git a/src/Core/Services/Implementations/SendService.cs b/src/Core/Services/Implementations/SendService.cs index 918d816170..327c73259e 100644 --- a/src/Core/Services/Implementations/SendService.cs +++ b/src/Core/Services/Implementations/SendService.cs @@ -10,6 +10,7 @@ using Bit.Core.Models.Data; using Bit.Core.Models.Table; using Bit.Core.Repositories; using Bit.Core.Settings; +using Bit.Core.Utilities; using Microsoft.AspNetCore.Identity; using Newtonsoft.Json; @@ -280,40 +281,24 @@ namespace Bit.Core.Services return; } - var policies = await _policyRepository.GetManyByUserIdAsync(userId.Value); - - if (policies == null) + var disableSendPolicyCount = await _policyRepository.GetCountByTypeApplicableToUserIdAsync(userId.Value, + PolicyType.DisableSend); + if (disableSendPolicyCount > 0) { - return; - } - - foreach (var policy in policies.Where(p => p.Enabled && p.Type == PolicyType.DisableSend)) - { - if (!await _currentContext.ManagePolicies(policy.OrganizationId)) - { - throw new BadRequestException("Due to an Enterprise Policy, you are only able to delete an existing Send."); - } + throw new BadRequestException("Due to an Enterprise Policy, you are only able to delete an existing Send."); } if (send.HideEmail.GetValueOrDefault()) { - foreach (var policy in policies.Where(p => p.Enabled && p.Type == PolicyType.SendOptions)) + var sendOptionsPolicies = await _policyRepository.GetManyByTypeApplicableToUserIdAsync(userId.Value, PolicyType.SendOptions); + foreach (var policy in sendOptionsPolicies) { - if (await _currentContext.ManagePolicies(policy.OrganizationId)) - { - continue; - } - - SendOptionsPolicyData data = null; - if (policy.Data != null) - { - data = JsonConvert.DeserializeObject(policy.Data); - } - + var data = CoreHelpers.LoadClassFromJsonData(policy.Data); if (data?.DisableHideEmail ?? false) { throw new BadRequestException("Due to an Enterprise Policy, you are not allowed to hide your email address from recipients when creating or editing a Send."); } + } } } diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index d034f75ad9..c62d75ba17 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -1303,24 +1303,18 @@ namespace Bit.Core.Services private async Task CheckPoliciesOnTwoFactorRemovalAsync(User user, IOrganizationService organizationService) { - var policies = await _policyRepository.GetManyByUserIdAsync(user.Id); - var twoFactorPolicies = policies.Where(p => p.Type == PolicyType.TwoFactorAuthentication && p.Enabled); - if (twoFactorPolicies.Any()) + var twoFactorPolicies = await _policyRepository.GetManyByTypeApplicableToUserIdAsync(user.Id, + PolicyType.TwoFactorAuthentication); + + var removeOrgUserTasks = twoFactorPolicies.Select(async p => { - var userOrgs = await _organizationUserRepository.GetManyByUserAsync(user.Id); - var ownerOrgs = userOrgs.Where(o => o.Type == OrganizationUserType.Owner) - .Select(o => o.OrganizationId).ToHashSet(); - foreach (var policy in twoFactorPolicies) - { - if (!ownerOrgs.Contains(policy.OrganizationId)) - { - await organizationService.DeleteUserAsync(policy.OrganizationId, user.Id); - var organization = await _organizationRepository.GetByIdAsync(policy.OrganizationId); - await _mailService.SendOrganizationUserRemovedForPolicyTwoStepEmailAsync( - organization.Name, user.Email); - } - } - } + await organizationService.DeleteUserAsync(p.OrganizationId, user.Id); + var organization = await _organizationRepository.GetByIdAsync(p.OrganizationId); + await _mailService.SendOrganizationUserRemovedForPolicyTwoStepEmailAsync( + organization.Name, user.Email); + }).ToArray(); + + await Task.WhenAll(removeOrgUserTasks); } public override async Task ConfirmEmailAsync(User user, string token) diff --git a/src/Sql/Sql.sqlproj b/src/Sql/Sql.sqlproj index 774f1f7e5b..53a5fde097 100644 --- a/src/Sql/Sql.sqlproj +++ b/src/Sql/Sql.sqlproj @@ -368,5 +368,8 @@ + + + diff --git a/src/Sql/dbo/Functions/PolicyApplicableToUser.sql b/src/Sql/dbo/Functions/PolicyApplicableToUser.sql new file mode 100644 index 0000000000..5dd2a49be9 --- /dev/null +++ b/src/Sql/dbo/Functions/PolicyApplicableToUser.sql @@ -0,0 +1,35 @@ +CREATE FUNCTION [dbo].[PolicyApplicableToUser] +( + @UserId UNIQUEIDENTIFIER, + @PolicyType TINYINT, + @MinimumStatus TINYINT +) +RETURNS TABLE +AS RETURN +SELECT + P.* +FROM + [dbo].[PolicyView] P +INNER JOIN + [dbo].[OrganizationUserView] OU ON P.[OrganizationId] = OU.[OrganizationId] +LEFT JOIN + (SELECT + PU.UserId, + PO.OrganizationId + FROM + [dbo].[ProviderUserView] PU + INNER JOIN + [ProviderOrganizationView] PO ON PO.[ProviderId] = PU.[ProviderId]) PUPO + ON PUPO.UserId = OU.UserId + AND PUPO.OrganizationId = P.OrganizationId +WHERE + OU.[UserId] = @UserId + AND P.[Type] = @PolicyType + AND P.[Enabled] = 1 + AND OU.[Status] >= @MinimumStatus + AND OU.[Type] >= 2 -- Not an owner (0) or admin (1) + AND ( -- Can't manage policies + OU.[Permissions] IS NULL + OR COALESCE(JSON_VALUE(OU.[Permissions], '$.managePolicies'), 'false') = 'false' + ) + AND PUPO.[UserId] IS NULL -- Not a provider diff --git a/src/Sql/dbo/Stored Procedures/Policy_CountByTypeApplicableToUser.sql b/src/Sql/dbo/Stored Procedures/Policy_CountByTypeApplicableToUser.sql new file mode 100644 index 0000000000..0f75909804 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/Policy_CountByTypeApplicableToUser.sql @@ -0,0 +1,11 @@ +CREATE PROCEDURE [dbo].[Policy_CountByTypeApplicableToUser] + @UserId UNIQUEIDENTIFIER, + @PolicyType TINYINT, + @MinimumStatus TINYINT +AS +BEGIN + SET NOCOUNT ON + + SELECT COUNT(1) + FROM [dbo].[PolicyApplicableToUser](@UserId, @PolicyType, @MinimumStatus) +END diff --git a/src/Sql/dbo/Stored Procedures/Policy_ReadByTypeApplicableToUser.sql b/src/Sql/dbo/Stored Procedures/Policy_ReadByTypeApplicableToUser.sql new file mode 100644 index 0000000000..db0b4040d1 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/Policy_ReadByTypeApplicableToUser.sql @@ -0,0 +1,11 @@ +CREATE PROCEDURE [dbo].[Policy_ReadByTypeApplicableToUser] + @UserId UNIQUEIDENTIFIER, + @PolicyType TINYINT, + @MinimumStatus TINYINT +AS +BEGIN + SET NOCOUNT ON + + SELECT * + FROM [dbo].[PolicyApplicableToUser](@UserId, @PolicyType, @MinimumStatus) +END diff --git a/test/Core.Test/AutoFixture/EntityFrameworkRepositoryFixtures.cs b/test/Core.Test/AutoFixture/EntityFrameworkRepositoryFixtures.cs index fdc5e79fe5..88bf72168d 100644 --- a/test/Core.Test/AutoFixture/EntityFrameworkRepositoryFixtures.cs +++ b/test/Core.Test/AutoFixture/EntityFrameworkRepositoryFixtures.cs @@ -1,6 +1,7 @@ using AutoFixture; using AutoMapper; using Bit.Core.Models.EntityFramework; +using Bit.Core.Models.EntityFramework.Provider; using System.Collections.Generic; using AutoFixture.Kernel; using System; @@ -76,6 +77,9 @@ namespace Bit.Core.Test.AutoFixture.EntityFrameworkRepositoryFixtures cfg.AddProfile(); cfg.AddProfile(); cfg.AddProfile(); + cfg.AddProfile(); + cfg.AddProfile(); + cfg.AddProfile(); cfg.AddProfile(); cfg.AddProfile(); cfg.AddProfile(); diff --git a/test/Core.Test/AutoFixture/PolicyFixtures.cs b/test/Core.Test/AutoFixture/PolicyFixtures.cs index f53cd271cb..a5c5f85b86 100644 --- a/test/Core.Test/AutoFixture/PolicyFixtures.cs +++ b/test/Core.Test/AutoFixture/PolicyFixtures.cs @@ -86,12 +86,36 @@ namespace Bit.Core.Test.AutoFixture.PolicyFixtures } } + internal class EfPolicyApplicableToUser : ICustomization + { + public void Customize(IFixture fixture) + { + fixture.Customizations.Add(new IgnoreVirtualMembersCustomization()); + fixture.Customizations.Add(new GlobalSettingsBuilder()); + fixture.Customizations.Add(new PolicyBuilder()); + fixture.Customizations.Add(new OrganizationBuilder()); + fixture.Customizations.Add(new EfRepositoryListBuilder()); + fixture.Customizations.Add(new EfRepositoryListBuilder()); + fixture.Customizations.Add(new EfRepositoryListBuilder()); + fixture.Customizations.Add(new EfRepositoryListBuilder()); + fixture.Customizations.Add(new EfRepositoryListBuilder()); + fixture.Customizations.Add(new EfRepositoryListBuilder()); + fixture.Customizations.Add(new EfRepositoryListBuilder()); + } + } + internal class EfPolicyAutoDataAttribute : CustomAutoDataAttribute { public EfPolicyAutoDataAttribute() : base(new SutProviderCustomization(), new EfPolicy()) { } } + internal class EfPolicyApplicableToUserInlineAutoDataAttribute : InlineCustomAutoDataAttribute + { + public EfPolicyApplicableToUserInlineAutoDataAttribute(params object[] values) : base(new[] { typeof(SutProviderCustomization), typeof(EfPolicyApplicableToUser) }, values) + { } + } + internal class InlineEfPolicyAutoDataAttribute : InlineCustomAutoDataAttribute { public InlineEfPolicyAutoDataAttribute(params object[] values) : base(new[] { typeof(SutProviderCustomization), diff --git a/test/Core.Test/Repositories/EntityFramework/EqualityComparers/PolicyCompare.cs b/test/Core.Test/Repositories/EntityFramework/EqualityComparers/PolicyCompare.cs index f17d63b8b5..ee54af9ccd 100644 --- a/test/Core.Test/Repositories/EntityFramework/EqualityComparers/PolicyCompare.cs +++ b/test/Core.Test/Repositories/EntityFramework/EqualityComparers/PolicyCompare.cs @@ -18,4 +18,13 @@ namespace Bit.Core.Test.Repositories.EntityFramework.EqualityComparers return base.GetHashCode(); } } + + public class PolicyCompareIncludingOrganization: PolicyCompare + { + public new bool Equals(Policy x, Policy y) + { + return base.Equals(x, y) && + x.OrganizationId == y.OrganizationId; + } + } } diff --git a/test/Core.Test/Repositories/EntityFramework/PolicyRepositoryTests.cs b/test/Core.Test/Repositories/EntityFramework/PolicyRepositoryTests.cs index b6fe5b83b5..737835b801 100644 --- a/test/Core.Test/Repositories/EntityFramework/PolicyRepositoryTests.cs +++ b/test/Core.Test/Repositories/EntityFramework/PolicyRepositoryTests.cs @@ -2,6 +2,7 @@ using Bit.Core.Repositories.EntityFramework; using Bit.Core.Test.AutoFixture; using Bit.Core.Test.AutoFixture.Attributes; using Bit.Core.Test.AutoFixture.PolicyFixtures; +using Bit.Core.Test.AutoFixture.OrganizationUserFixtures; using Microsoft.EntityFrameworkCore; using Xunit; using TableModel = Bit.Core.Models.Table; @@ -10,6 +11,11 @@ using System.Collections.Generic; using EfRepo = Bit.Core.Repositories.EntityFramework; using SqlRepo = Bit.Core.Repositories.SqlServer; using Bit.Core.Test.Repositories.EntityFramework.EqualityComparers; +using Bit.Core.Models.Data; +using System.Text.Json; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using System.Threading.Tasks; namespace Bit.Core.Test.Repositories.EntityFramework { @@ -51,6 +57,130 @@ namespace Bit.Core.Test.Repositories.EntityFramework var distinctItems = savedPolicys.Distinct(equalityComparer); Assert.True(!distinctItems.Skip(1).Any()); - } + } + + [CiSkippedTheory] + [EfPolicyApplicableToUserInlineAutoData(OrganizationUserType.User, false, OrganizationUserStatusType.Confirmed, true, true, false)] // Ordinary user + [EfPolicyApplicableToUserInlineAutoData(OrganizationUserType.Owner, false, OrganizationUserStatusType.Confirmed, true, true, false)] // Owner + [EfPolicyApplicableToUserInlineAutoData(OrganizationUserType.Admin, false, OrganizationUserStatusType.Confirmed, true, true, false)] // Admin + [EfPolicyApplicableToUserInlineAutoData(OrganizationUserType.User, true, OrganizationUserStatusType.Confirmed, true, true, false)] // canManagePolicies + [EfPolicyApplicableToUserInlineAutoData(OrganizationUserType.User, false, OrganizationUserStatusType.Confirmed, true, true, true)] // Provider + [EfPolicyApplicableToUserInlineAutoData(OrganizationUserType.User, false, OrganizationUserStatusType.Confirmed, false, true, false)] // Policy disabled + [EfPolicyApplicableToUserInlineAutoData(OrganizationUserType.User, false, OrganizationUserStatusType.Confirmed, true, false, false)] // No policy of Type + [EfPolicyApplicableToUserInlineAutoData(OrganizationUserType.User, false, OrganizationUserStatusType.Invited, true, true, false)] // User not minStatus + public async void GetManyByTypeApplicableToUser_Works_DataMatches_Corre( + // Inline data + OrganizationUserType userType, + bool canManagePolicies, + OrganizationUserStatusType orgUserStatus, + bool policyEnabled, + bool policySameType, + bool isProvider, + + // Auto data - models + TableModel.Policy policy, + TableModel.User user, + TableModel.Organization organization, + TableModel.OrganizationUser orgUser, + TableModel.Provider.Provider provider, + TableModel.Provider.ProviderOrganization providerOrganization, + TableModel.Provider.ProviderUser providerUser, + PolicyCompareIncludingOrganization equalityComparer, + + // Auto data - EF repos + List suts, + List efUserRepository, + List efOrganizationRepository, + List efOrganizationUserRepository, + List efProviderRepository, + List efProviderOrganizationRepository, + List efProviderUserRepository, + + // Auto data - SQL repos + SqlRepo.PolicyRepository sqlPolicyRepo, + SqlRepo.UserRepository sqlUserRepo, + SqlRepo.OrganizationRepository sqlOrganizationRepo, + SqlRepo.ProviderRepository sqlProviderRepo, + SqlRepo.OrganizationUserRepository sqlOrganizationUserRepo, + SqlRepo.ProviderOrganizationRepository sqlProviderOrganizationRepo, + SqlRepo.ProviderUserRepository sqlProviderUserRepo + ) + { + // Combine EF and SQL repos into one list per type + var policyRepos = suts.ToList(); + policyRepos.Add(sqlPolicyRepo); + var userRepos = efUserRepository.ToList(); + userRepos.Add(sqlUserRepo); + var orgRepos = efOrganizationRepository.ToList(); + orgRepos.Add(sqlOrganizationRepo); + var orgUserRepos = efOrganizationUserRepository.ToList(); + orgUserRepos.Add(sqlOrganizationUserRepo); + var providerRepos = efProviderRepository.ToList(); + providerRepos.Add(sqlProviderRepo); + var providerOrgRepos = efProviderOrganizationRepository.ToList(); + providerOrgRepos.Add(sqlProviderOrganizationRepo); + var providerUserRepos = efProviderUserRepository.ToList(); + providerUserRepos.Add(sqlProviderUserRepo); + + // Arrange data + var savedPolicyType = PolicyType.SingleOrg; + var queriedPolicyType = policySameType ? savedPolicyType : PolicyType.DisableSend; + + orgUser.Type = userType; + orgUser.Status = orgUserStatus; + var permissionsData = new Permissions { ManagePolicies = canManagePolicies }; + orgUser.Permissions = JsonSerializer.Serialize(permissionsData, new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }); + + policy.Enabled = policyEnabled; + policy.Type = savedPolicyType; + + var results = new List(); + + foreach (var policyRepo in policyRepos) + { + var i = policyRepos.IndexOf(policyRepo); + + // Seed database + var savedUser = await userRepos[i].CreateAsync(user); + var savedOrg = await orgRepos[i].CreateAsync(organization); + + orgUser.UserId = savedUser.Id; + orgUser.OrganizationId = savedOrg.Id; + await orgUserRepos[i].CreateAsync(orgUser); + + if (isProvider) + { + var savedProvider = await providerRepos[i].CreateAsync(provider); + + providerOrganization.OrganizationId = savedOrg.Id; + providerOrganization.ProviderId = savedProvider.Id; + await providerOrgRepos[i].CreateAsync(providerOrganization); + + providerUser.UserId = savedUser.Id; + providerUser.ProviderId = savedProvider.Id; + await providerUserRepos[i].CreateAsync(providerUser); + } + + policy.OrganizationId = savedOrg.Id; + await policyRepo.CreateAsync(policy); + if (suts.Contains(policyRepo)) + { + (policyRepo as BaseEntityFrameworkRepository).ClearChangeTracking(); + } + + // Act + var result = await policyRepo.GetManyByTypeApplicableToUserIdAsync(savedUser.Id, queriedPolicyType, OrganizationUserStatusType.Accepted); + results.Add(result.FirstOrDefault()); + } + + // Assert + var distinctItems = results.Distinct(equalityComparer); + + Assert.True(results.All(r => r == null) || + !distinctItems.Skip(1).Any()); + } } } diff --git a/test/Core.Test/Services/SendServiceTests.cs b/test/Core.Test/Services/SendServiceTests.cs index d24780473f..28fb68b0e5 100644 --- a/test/Core.Test/Services/SendServiceTests.cs +++ b/test/Core.Test/Services/SendServiceTests.cs @@ -12,34 +12,31 @@ using Bit.Core.Test.AutoFixture; using Bit.Core.Test.AutoFixture.SendFixtures; using NSubstitute; using Xunit; -using Newtonsoft.Json; +using System.Text.Json; namespace Bit.Core.Test.Services { public class SendServiceTests { + private void SaveSendAsync_Setup(SendType sendType, bool disableSendPolicyAppliesToUser, + SutProvider sutProvider, Send send) + { + send.Id = default; + send.Type = sendType; + + sutProvider.GetDependency().GetCountByTypeApplicableToUserIdAsync( + Arg.Any(), PolicyType.DisableSend).Returns(disableSendPolicyAppliesToUser ? 1 : 0); + } + // Disable Send policy check - private void SaveSendAsync_DisableSend_Setup(SendType sendType, bool canManagePolicies, - SutProvider sutProvider, Send send, List policies) - { - send.Id = default; - send.Type = sendType; - - policies.First().Type = PolicyType.DisableSend; - policies.First().Enabled = true; - - sutProvider.GetDependency().GetManyByUserIdAsync(send.UserId.Value).Returns(policies); - sutProvider.GetDependency().ManagePolicies(Arg.Any()).Returns(canManagePolicies); - } - [Theory] [InlineUserSendAutoData(SendType.File)] [InlineUserSendAutoData(SendType.Text)] - public async void SaveSendAsync_DisableSend_CantManagePolicies_throws(SendType sendType, - SutProvider sutProvider, Send send, List policies) + public async void SaveSendAsync_DisableSend_Applies_throws(SendType sendType, + SutProvider sutProvider, Send send) { - SaveSendAsync_DisableSend_Setup(sendType, canManagePolicies: false, sutProvider, send, policies); + SaveSendAsync_Setup(sendType, disableSendPolicyAppliesToUser: true, sutProvider, send); await Assert.ThrowsAsync(() => sutProvider.Sut.SaveSendAsync(send)); } @@ -47,61 +44,47 @@ namespace Bit.Core.Test.Services [Theory] [InlineUserSendAutoData(SendType.File)] [InlineUserSendAutoData(SendType.Text)] - public async void SaveSendAsync_DisableSend_DisabledPolicy_CantManagePolicies_success(SendType sendType, - SutProvider sutProvider, Send send, List policies) + public async void SaveSendAsync_DisableSend_DoesntApply_success(SendType sendType, + SutProvider sutProvider, Send send) { - SaveSendAsync_DisableSend_Setup(sendType, canManagePolicies: false, sutProvider, send, policies); - foreach (var policy in policies.Where(p => p.Type == PolicyType.DisableSend)) - { - policy.Enabled = false; - } + SaveSendAsync_Setup(sendType, disableSendPolicyAppliesToUser: false, sutProvider, send); await sutProvider.Sut.SaveSendAsync(send); await sutProvider.GetDependency().Received(1).CreateAsync(send); } - [Theory] - [InlineUserSendAutoData(SendType.File)] - [InlineUserSendAutoData(SendType.Text)] - public async void SaveSendAsync_DisableSend_CanManagePolicies_success(SendType sendType, - SutProvider sutProvider, Send send, List policies) + // Send Options Policy - Disable Hide Email check + + private void SaveSendAsync_HideEmail_Setup(bool disableHideEmailAppliesToUser, + SutProvider sutProvider, Send send, Policy policy) { - SaveSendAsync_DisableSend_Setup(sendType, canManagePolicies: true, sutProvider, send, policies); - - await sutProvider.Sut.SaveSendAsync(send); - - await sutProvider.GetDependency().Received(1).CreateAsync(send); - } - - // SendOptionsPolicy.DisableHideEmail check - - private void SaveSendAsync_DisableHideEmail_Setup(SendType sendType, bool canManagePolicies, - SutProvider sutProvider, Send send, List policies) - { - send.Id = default; - send.Type = sendType; send.HideEmail = true; - var dataObj = new SendOptionsPolicyData(); - dataObj.DisableHideEmail = true; + var sendOptions = new SendOptionsPolicyData + { + DisableHideEmail = disableHideEmailAppliesToUser + }; + policy.Data = JsonSerializer.Serialize(sendOptions, new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }); - policies.First().Type = PolicyType.SendOptions; - policies.First().Enabled = true; - policies.First().Data = JsonConvert.SerializeObject(dataObj); - - sutProvider.GetDependency().GetManyByUserIdAsync(send.UserId.Value).Returns(policies); - sutProvider.GetDependency().ManagePolicies(Arg.Any()).Returns(canManagePolicies); + sutProvider.GetDependency().GetManyByTypeApplicableToUserIdAsync( + Arg.Any(), PolicyType.SendOptions).Returns(new List + { + policy, + }); } - [Theory] [InlineUserSendAutoData(SendType.File)] [InlineUserSendAutoData(SendType.Text)] - public async void SaveSendAsync_DisableHideEmail_CantManagePolicies_throws(SendType sendType, - SutProvider sutProvider, Send send, List policies) + public async void SaveSendAsync_DisableHideEmail_Applies_throws(SendType sendType, + SutProvider sutProvider, Send send, Policy policy) { - SaveSendAsync_DisableHideEmail_Setup(sendType, canManagePolicies: false, sutProvider, send, policies); + SaveSendAsync_Setup(sendType, false, sutProvider, send); + SaveSendAsync_HideEmail_Setup(true, sutProvider, send, policy); await Assert.ThrowsAsync(() => sutProvider.Sut.SaveSendAsync(send)); } @@ -109,33 +92,11 @@ namespace Bit.Core.Test.Services [Theory] [InlineUserSendAutoData(SendType.File)] [InlineUserSendAutoData(SendType.Text)] - public async void SaveSendAsync_DisableHideEmail_CantManagePolicies_success(SendType sendType, - SutProvider sutProvider, Send send, List policies) + public async void SaveSendAsync_DisableHideEmail_DoesntApply_success(SendType sendType, + SutProvider sutProvider, Send send, Policy policy) { - SaveSendAsync_DisableHideEmail_Setup(sendType, canManagePolicies: false, sutProvider, send, policies); - - var policyData = new SendOptionsPolicyData(); - policyData.DisableHideEmail = false; - var policyDataSerialized = JsonConvert.SerializeObject(policyData); - - foreach (var policy in policies.Where(p => p.Type == PolicyType.SendOptions)) - { - policies.First().Enabled = true; - policies.First().Data = policyDataSerialized; - } - - await sutProvider.Sut.SaveSendAsync(send); - - await sutProvider.GetDependency().Received(1).CreateAsync(send); - } - - [Theory] - [InlineUserSendAutoData(SendType.File)] - [InlineUserSendAutoData(SendType.Text)] - public async void SaveSendAsync_DisableHideEmail_CanManagePolicies_success(SendType sendType, - SutProvider sutProvider, Send send, List policies) - { - SaveSendAsync_DisableHideEmail_Setup(sendType, canManagePolicies: true, sutProvider, send, policies); + SaveSendAsync_Setup(sendType, false, sutProvider, send); + SaveSendAsync_HideEmail_Setup(false, sutProvider, send, policy); await sutProvider.Sut.SaveSendAsync(send); diff --git a/util/Migrator/DbScripts/2021-09-16_00_PolicyApplicableToUser.sql b/util/Migrator/DbScripts/2021-09-16_00_PolicyApplicableToUser.sql new file mode 100644 index 0000000000..1ce8dcb719 --- /dev/null +++ b/util/Migrator/DbScripts/2021-09-16_00_PolicyApplicableToUser.sql @@ -0,0 +1,82 @@ +-- PolicyApplicableToUser +IF OBJECT_ID('[dbo].[PolicyApplicableToUser]') IS NOT NULL +BEGIN + DROP FUNCTION [dbo].[PolicyApplicableToUser] +END +GO + +CREATE FUNCTION [dbo].[PolicyApplicableToUser] +( + @UserId UNIQUEIDENTIFIER, + @PolicyType TINYINT, + @MinimumStatus TINYINT +) +RETURNS TABLE +AS RETURN +SELECT + P.* +FROM + [dbo].[PolicyView] P +INNER JOIN + [dbo].[OrganizationUserView] OU ON P.[OrganizationId] = OU.[OrganizationId] +LEFT JOIN + (SELECT + PU.UserId, + PO.OrganizationId + FROM + [dbo].[ProviderUserView] PU + INNER JOIN + [ProviderOrganizationView] PO ON PO.[ProviderId] = PU.[ProviderId]) PUPO + ON PUPO.UserId = OU.UserId + AND PUPO.OrganizationId = P.OrganizationId +WHERE + OU.[UserId] = @UserId + AND P.[Type] = @PolicyType + AND P.[Enabled] = 1 + AND OU.[Status] >= @MinimumStatus + AND OU.[Type] >= 2 -- Not an owner (0) or admin (1) + AND ( -- Can't manage policies + OU.[Permissions] IS NULL + OR COALESCE(JSON_VALUE(OU.[Permissions], '$.managePolicies'), 'false') = 'false' + ) + AND PUPO.[UserId] IS NULL -- Not a provider +GO + +-- Policy_ReadByTypeApplicableToUser +IF OBJECT_ID('[dbo].[Policy_ReadByTypeApplicableToUser]') IS NOT NULL +BEGIN + DROP PROCEDURE [dbo].[Policy_ReadByTypeApplicableToUser] +END +GO + +CREATE PROCEDURE [dbo].[Policy_ReadByTypeApplicableToUser] + @UserId UNIQUEIDENTIFIER, + @PolicyType TINYINT, + @MinimumStatus TINYINT +AS +BEGIN + SET NOCOUNT ON + + SELECT * + FROM [dbo].[PolicyApplicableToUser](@UserId, @PolicyType, @MinimumStatus) +END +GO + +-- Policy_CountByTypeApplicableToUser +IF OBJECT_ID('[dbo].[Policy_CountByTypeApplicableToUser]') IS NOT NULL +BEGIN + DROP PROCEDURE [dbo].[Policy_CountByTypeApplicableToUser] +END +GO + +CREATE PROCEDURE [dbo].[Policy_CountByTypeApplicableToUser] + @UserId UNIQUEIDENTIFIER, + @PolicyType TINYINT, + @MinimumStatus TINYINT +AS +BEGIN + SET NOCOUNT ON + + SELECT COUNT(1) + FROM [dbo].[PolicyApplicableToUser](@UserId, @PolicyType, @MinimumStatus) +END