From 4983e413e468675506f9189ae9aa5e7826da91bf Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Mon, 7 Oct 2024 09:04:33 +1000 Subject: [PATCH] Use delegates to simplify mocking --- .../Policies/IPolicyDefinition.cs | 5 ++- .../SingleOrgPolicyDefinition.cs | 34 +++++++++---------- .../Implementations/PolicyServicevNext.cs | 8 ++--- .../PolicyServicevNextCustomization.cs | 2 -- .../Services/PolicyDefinitionFixtures.cs | 30 ++++++++-------- .../Services/PolicyServicevNextTests.cs | 6 ++-- 6 files changed, 39 insertions(+), 46 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/IPolicyDefinition.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/IPolicyDefinition.cs index c5c1b1257..82cf3d64a 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/IPolicyDefinition.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/IPolicyDefinition.cs @@ -2,7 +2,6 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; -using Bit.Core.Entities; namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies; @@ -26,7 +25,7 @@ public interface IPolicyDefinition /// The current policy, if any /// The modified policy to be saved /// A sequence of validation errors if validation was unsuccessful - public Task ValidateAsync(Policy? currentPolicy, Policy modifiedPolicy); + public Func> Validate { get; } /// /// Optionally performs side effects after a policy is validated but before it is saved. @@ -34,5 +33,5 @@ public interface IPolicyDefinition /// /// The current policy, if any /// The modified policy to be saved - public Task OnSaveSideEffectsAsync(Policy? currentPolicy, Policy modifiedPolicy); + public Func OnSaveSideEffects { get; } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/SingleOrgPolicyDefinition.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/SingleOrgPolicyDefinition.cs index 9b59d2790..46a6b1389 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/SingleOrgPolicyDefinition.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/SingleOrgPolicyDefinition.cs @@ -1,13 +1,10 @@ #nullable enable -using System.ComponentModel; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; -using Bit.Core.AdminConsole.Repositories; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Repositories; using Bit.Core.Context; -using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; @@ -40,13 +37,27 @@ public class SingleOrgPolicyDefinition : IPolicyDefinition _currentContext = currentContext; } - public async Task OnSaveSideEffectsAsync(Policy? currentPolicy, Policy modifiedPolicy) + public Func> Validate => async (currentPolicy, modifiedPolicy) => + { + var organizationId = modifiedPolicy.OrganizationId; + + // Do not allow this policy to be disabled if Key Connector is being used + var ssoConfig = await _ssoConfigRepository.GetByOrganizationIdAsync(organizationId); + if (ssoConfig?.GetData()?.MemberDecryptionType == MemberDecryptionType.KeyConnector) + { + return "Key Connector is enabled."; + } + + return null; + }; + + public Func OnSaveSideEffects => async (currentPolicy, modifiedPolicy) => { if (currentPolicy is null or { Enabled: false } && modifiedPolicy is { Enabled: true }) { await RemoveNonCompliantUsersAsync(modifiedPolicy.OrganizationId); } - } + }; private async Task RemoveNonCompliantUsersAsync(Guid organizationId) { @@ -85,17 +96,4 @@ public class SingleOrgPolicyDefinition : IPolicyDefinition } } - public async Task ValidateAsync(Policy? currentPolicy, Policy modifiedPolicy) - { - var organizationId = modifiedPolicy.OrganizationId; - - // Do not allow this policy to be disabled if Key Connector is being used - var ssoConfig = await _ssoConfigRepository.GetByOrganizationIdAsync(organizationId); - if (ssoConfig?.GetData()?.MemberDecryptionType == MemberDecryptionType.KeyConnector) - { - return "Key Connector is enabled."; - } - - return null; - } } diff --git a/src/Core/AdminConsole/Services/Implementations/PolicyServicevNext.cs b/src/Core/AdminConsole/Services/Implementations/PolicyServicevNext.cs index 63c1cdafe..a13884c1e 100644 --- a/src/Core/AdminConsole/Services/Implementations/PolicyServicevNext.cs +++ b/src/Core/AdminConsole/Services/Implementations/PolicyServicevNext.cs @@ -82,21 +82,21 @@ public class PolicyServicevNext : IPolicyServicevNext .Where(otherPolicy => otherPolicy is { Enabled: true }) .ToList(); - if (dependentPolicies is { Count: > 0}) + if (dependentPolicies is { Count: > 0 }) { - throw new BadRequestException("This policy is required by " + dependentPolicies.First() + ". Try disabling that policy first." ); + throw new BadRequestException("This policy is required by " + dependentPolicies.First() + ". Try disabling that policy first."); } } // Run other validation - var validationError = await policyDefinition.ValidateAsync(currentPolicy, policy); + var validationError = await policyDefinition.Validate(currentPolicy, policy); if (validationError != null) { throw new BadRequestException(validationError); } // Run side effects - await policyDefinition.OnSaveSideEffectsAsync(currentPolicy, policy); + await policyDefinition.OnSaveSideEffects(currentPolicy, policy); var now = DateTime.UtcNow; if (policy.Id == default) diff --git a/test/Core.Test/AdminConsole/AutoFixture/PolicyServicevNextCustomization.cs b/test/Core.Test/AdminConsole/AutoFixture/PolicyServicevNextCustomization.cs index 797f4ec60..10154a82f 100644 --- a/test/Core.Test/AdminConsole/AutoFixture/PolicyServicevNextCustomization.cs +++ b/test/Core.Test/AdminConsole/AutoFixture/PolicyServicevNextCustomization.cs @@ -1,11 +1,9 @@ #nullable enable using System.Reflection; -using AutoFixture; using AutoFixture.Kernel; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.Services.Implementations; -using Bit.Test.Common.AutoFixture.Attributes; namespace Bit.Core.Test.AdminConsole.AutoFixture; diff --git a/test/Core.Test/AdminConsole/Services/PolicyDefinitionFixtures.cs b/test/Core.Test/AdminConsole/Services/PolicyDefinitionFixtures.cs index f0047c83b..4e5dce9f9 100644 --- a/test/Core.Test/AdminConsole/Services/PolicyDefinitionFixtures.cs +++ b/test/Core.Test/AdminConsole/Services/PolicyDefinitionFixtures.cs @@ -1,8 +1,9 @@ -using Bit.Core.AdminConsole.Entities; +#nullable enable + +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using NSubstitute; -using NSubstitute.ExceptionExtensions; namespace Bit.Core.Test.AdminConsole.Services; @@ -10,18 +11,15 @@ public class FakeSingleOrgPolicyDefinition : IPolicyDefinition { public PolicyType Type => PolicyType.SingleOrg; public IEnumerable RequiredPolicies => Array.Empty(); - - public readonly Func> ValidateAsyncMock = Substitute.For>>(); - public readonly Action OnSaveSideEffectsAsyncMock = Substitute.For>(); - - public Task? ValidateAsync(Policy? currentPolicy, Policy modifiedPolicy) - { - return ValidateAsyncMock(currentPolicy, modifiedPolicy); - } - - public Task OnSaveSideEffectsAsync(Policy? currentPolicy, Policy modifiedPolicy) - { - OnSaveSideEffectsAsyncMock(currentPolicy, modifiedPolicy); - return Task.FromResult(0); - } + public Func> Validate => Substitute.For>>(); + public Func OnSaveSideEffects => Substitute.For>(); } + +public class FakeRequireSsoPolicyDefinition : IPolicyDefinition +{ + public PolicyType Type => PolicyType.RequireSso; + public IEnumerable RequiredPolicies => [PolicyType.SingleOrg]; + public Func> Validate => Substitute.For>>(); + public Func OnSaveSideEffects => Substitute.For>(); +} + diff --git a/test/Core.Test/AdminConsole/Services/PolicyServicevNextTests.cs b/test/Core.Test/AdminConsole/Services/PolicyServicevNextTests.cs index 4ad8828ad..ea994045f 100644 --- a/test/Core.Test/AdminConsole/Services/PolicyServicevNextTests.cs +++ b/test/Core.Test/AdminConsole/Services/PolicyServicevNextTests.cs @@ -10,9 +10,9 @@ using Bit.Core.Services; using Bit.Core.Test.AdminConsole.AutoFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; -using AdminConsoleFixtures = Bit.Core.Test.AdminConsole.AutoFixture; using NSubstitute; using Xunit; +using AdminConsoleFixtures = Bit.Core.Test.AdminConsole.AutoFixture; namespace Bit.Core.Test.AdminConsole.Services; @@ -72,7 +72,7 @@ public class PolicyServicevNextTests } [Theory, BitAutoData] - public async Task SaveAsync_PolicyDefinitionNotFound_Throws([Policy(PolicyType.SingleOrg)]Policy policy) + public async Task SaveAsync_PolicyDefinitionNotFound_Throws([Policy(PolicyType.SingleOrg)] Policy policy) { var sutProvider = SutProviderFactory(); ArrangeOrganization(sutProvider, policy); @@ -91,7 +91,7 @@ public class PolicyServicevNextTests public async Task SaveAsync_ThrowsOnValidationError([AdminConsoleFixtures.Policy(PolicyType.SingleOrg)] Policy policy) { var fakePolicyDefinition = new FakeSingleOrgPolicyDefinition(); - fakePolicyDefinition.ValidateAsyncMock(null, policy).Returns("Validation error!"); + fakePolicyDefinition.Validate(null, policy).Returns("Validation error!"); var sutProvider = SutProviderFactory([fakePolicyDefinition]); ArrangeOrganization(sutProvider, policy);