From 716e52f6ff1d58b5d14aecaff30864ade923f5cf Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Fri, 24 Sep 2021 09:51:02 +1000 Subject: [PATCH] Move policy checks inside PolicyService (#1533) * Move policy checks inside PolicyService * Remove leftover code * Remove duplicate code * Reorder code for consistency --- src/Android/Autofill/AutofillService.cs | 20 +++----------- src/App/Pages/Vault/AddEditPageViewModel.cs | 28 ++++++-------------- src/App/Utilities/AppHelpers.cs | 29 +++------------------ src/Core/Models/Domain/Organization.cs | 2 +- src/Core/Services/PolicyService.cs | 27 ++++++++++--------- 5 files changed, 30 insertions(+), 76 deletions(-) diff --git a/src/Android/Autofill/AutofillService.cs b/src/Android/Autofill/AutofillService.cs index 4b55b005d..38ecbe4a9 100644 --- a/src/Android/Autofill/AutofillService.cs +++ b/src/Android/Autofill/AutofillService.cs @@ -94,24 +94,12 @@ namespace Bit.Droid.Autofill _policyService ??= ServiceContainer.Resolve("policyService"); - var personalOwnershipPolicies = await _policyService.GetAll(PolicyType.PersonalOwnership); - if (personalOwnershipPolicies != null) + var personalOwnershipPolicyApplies = await _policyService.PolicyAppliesToUser(PolicyType.PersonalOwnership); + if (personalOwnershipPolicyApplies) { - _userService ??= ServiceContainer.Resolve("userService"); - foreach (var policy in personalOwnershipPolicies) - { - if (policy.Enabled) - { - var org = await _userService.GetOrganizationAsync(policy.OrganizationId); - if (org != null && org.Enabled && org.UsePolicies && !org.canManagePolicies - && org.Status == OrganizationUserStatusType.Confirmed) - { - return; - } - } - } + return; } - + var parser = new Parser(structure, ApplicationContext); parser.Parse(); diff --git a/src/App/Pages/Vault/AddEditPageViewModel.cs b/src/App/Pages/Vault/AddEditPageViewModel.cs index 86df1dd28..4950b6c28 100644 --- a/src/App/Pages/Vault/AddEditPageViewModel.cs +++ b/src/App/Pages/Vault/AddEditPageViewModel.cs @@ -309,7 +309,6 @@ namespace Bit.App.Pages public async Task LoadAsync(AppOptions appOptions = null) { - var policies = (await _policyService.GetAll(PolicyType.PersonalOwnership))?.ToList(); var myEmail = await _userService.GetEmailAsync(); OwnershipOptions.Add(new KeyValuePair(myEmail, null)); var orgs = await _userService.GetAllOrganizationAsync(); @@ -318,28 +317,17 @@ namespace Bit.App.Pages if (org.Enabled && org.Status == OrganizationUserStatusType.Confirmed) { OwnershipOptions.Add(new KeyValuePair(org.Name, org.Id)); - if ((!EditMode || CloneMode) && policies != null && org.UsePolicies && !org.canManagePolicies && - AllowPersonal) - { - foreach (var policy in policies) - { - if (policy.OrganizationId == org.Id && policy.Enabled) - { - AllowPersonal = false; - // Remove personal ownership - OwnershipOptions.RemoveAt(0); - // Default to the organization who owns this policy for now (if necessary) - if (string.IsNullOrWhiteSpace(OrganizationId)) - { - OrganizationId = org.Id; - } - break; - } - } - } } } + var personalOwnershipPolicyApplies = await _policyService.PolicyAppliesToUser(PolicyType.PersonalOwnership); + if (personalOwnershipPolicyApplies && (!EditMode || CloneMode)) + { + AllowPersonal = false; + // Remove personal ownership + OwnershipOptions.RemoveAt(0); + } + var allCollections = await _collectionService.GetAllDecryptedAsync(); _writeableCollections = allCollections.Where(c => !c.ReadOnly).ToList(); if (CollectionIds?.Any() ?? false) diff --git a/src/App/Utilities/AppHelpers.cs b/src/App/Utilities/AppHelpers.cs index ace0719e8..6f605aafa 100644 --- a/src/App/Utilities/AppHelpers.cs +++ b/src/App/Utilities/AppHelpers.cs @@ -315,38 +315,15 @@ namespace Bit.App.Utilities public static async Task IsSendDisabledByPolicyAsync() { var policyService = ServiceContainer.Resolve("policyService"); - var userService = ServiceContainer.Resolve("userService"); - - var policies = await policyService.GetAll(PolicyType.DisableSend); - var organizations = await userService.GetAllOrganizationAsync(); - return organizations.Any(o => - { - return o.Enabled && - o.Status == OrganizationUserStatusType.Confirmed && - o.UsePolicies && - !o.canManagePolicies && - policies.Any(p => p.OrganizationId == o.Id && p.Enabled); - }); + return await policyService.PolicyAppliesToUser(PolicyType.DisableSend); } public static async Task IsHideEmailDisabledByPolicyAsync() { var policyService = ServiceContainer.Resolve("policyService"); - var userService = ServiceContainer.Resolve("userService"); - var policies = await policyService.GetAll(PolicyType.SendOptions); - var organizations = await userService.GetAllOrganizationAsync(); - return organizations.Any(o => - { - return o.Enabled && - o.Status == OrganizationUserStatusType.Confirmed && - o.UsePolicies && - !o.canManagePolicies && - policies.Any(p => p.OrganizationId == o.Id && - p.Enabled && - p.Data.ContainsKey("disableHideEmail") && - (bool)p.Data["disableHideEmail"]); - }); + return await policyService.PolicyAppliesToUser(PolicyType.SendOptions, + policy => policy.Data.ContainsKey("disableHideEmail") && (bool)policy.Data["disableHideEmail"]); } public static async Task PerformUpdateTasksAsync(ISyncService syncService, diff --git a/src/Core/Models/Domain/Organization.cs b/src/Core/Models/Domain/Organization.cs index 8d3cebae4..53ca13302 100644 --- a/src/Core/Models/Domain/Organization.cs +++ b/src/Core/Models/Domain/Organization.cs @@ -91,6 +91,6 @@ namespace Bit.Core.Models.Domain public bool canManageGroups => IsAdmin || Permissions.ManageGroups; public bool canManagePolicies => IsAdmin || Permissions.ManagePolicies; public bool canManageUser => IsAdmin || Permissions.ManageUsers; - public bool IsExemptFromPolicies => canManagePolicies; + public bool isExemptFromPolicies => canManagePolicies; } } diff --git a/src/Core/Services/PolicyService.cs b/src/Core/Services/PolicyService.cs index a890bc7e3..f76afa667 100644 --- a/src/Core/Services/PolicyService.cs +++ b/src/Core/Services/PolicyService.cs @@ -198,29 +198,30 @@ namespace Bit.Core.Services return new Tuple(resetPasswordPolicyOptions, policy != null); } - public async Task PolicyAppliesToUser(PolicyType policyType, Func policyFilter = null) + public async Task PolicyAppliesToUser(PolicyType policyType, Func policyFilter) { - if (policyFilter == null) { - policyFilter = _ => true; - } - var policies = await GetAll(policyType); var organizations = await _userService.GetAllOrganizationAsync(); - var filteredPolicies = policies.Where(p => - p.Enabled && - p.Type == policyType && - policyFilter(p)) - .Select(p => p.OrganizationId); + IEnumerable filteredPolicies; - var policySet = filteredPolicies.Distinct(); + if (policyFilter != null) + { + filteredPolicies = policies.Where(p => p.Enabled && policyFilter(p)); + } + else + { + filteredPolicies = policies.Where(p => p.Enabled); + } + + var policySet = new HashSet(filteredPolicies.Select(p => p.OrganizationId)); return organizations.Any(o => o.Enabled && o.Status >= OrganizationUserStatusType.Accepted && o.UsePolicies && - !o.IsExemptFromPolicies && - policySet.Distinct().Contains(o.Id)); + !o.isExemptFromPolicies && + policySet.Contains(o.Id)); } public int? GetPolicyInt(Policy policy, string key)