diff --git a/src/Core/AdminConsole/Services/Implementations/PolicyServicevNext.cs b/src/Core/AdminConsole/Services/Implementations/PolicyServicevNext.cs index 63c1cdafe..6c04df015 100644 --- a/src/Core/AdminConsole/Services/Implementations/PolicyServicevNext.cs +++ b/src/Core/AdminConsole/Services/Implementations/PolicyServicevNext.cs @@ -90,7 +90,7 @@ public class PolicyServicevNext : IPolicyServicevNext // Run other validation var validationError = await policyDefinition.ValidateAsync(currentPolicy, policy); - if (validationError != null) + if (!string.IsNullOrEmpty(validationError)) { throw new BadRequestException(validationError); } diff --git a/test/Core.Test/AdminConsole/AutoFixture/PolicyFixtures.cs b/test/Core.Test/AdminConsole/AutoFixture/PolicyFixtures.cs index f70fd579e..09b112c43 100644 --- a/test/Core.Test/AdminConsole/AutoFixture/PolicyFixtures.cs +++ b/test/Core.Test/AdminConsole/AutoFixture/PolicyFixtures.cs @@ -9,10 +9,12 @@ namespace Bit.Core.Test.AdminConsole.AutoFixture; internal class PolicyCustomization : ICustomization { public PolicyType Type { get; set; } + public bool Enabled { get; set; } - public PolicyCustomization(PolicyType type) + public PolicyCustomization(PolicyType type, bool enabled) { Type = type; + Enabled = enabled; } public void Customize(IFixture fixture) @@ -20,21 +22,23 @@ internal class PolicyCustomization : ICustomization fixture.Customize(composer => composer .With(o => o.OrganizationId, Guid.NewGuid()) .With(o => o.Type, Type) - .With(o => o.Enabled, true)); + .With(o => o.Enabled, Enabled)); } } public class PolicyAttribute : CustomizeAttribute { private readonly PolicyType _type; + private readonly bool _enabled; - public PolicyAttribute(PolicyType type) + public PolicyAttribute(PolicyType type, bool enabled = true) { _type = type; + _enabled = enabled; } public override ICustomization GetCustomization(ParameterInfo parameter) { - return new PolicyCustomization(_type); + return new PolicyCustomization(_type, _enabled); } } diff --git a/test/Core.Test/AdminConsole/Services/PolicyDefinitionFixtures.cs b/test/Core.Test/AdminConsole/Services/PolicyDefinitionFixtures.cs index f0047c83b..a6e102201 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; @@ -11,10 +12,10 @@ 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 readonly Func> ValidateAsyncMock = Substitute.For>>(); + public readonly Action OnSaveSideEffectsAsyncMock = Substitute.For>(); - public Task? ValidateAsync(Policy? currentPolicy, Policy modifiedPolicy) + public Task ValidateAsync(Policy? currentPolicy, Policy modifiedPolicy) { return ValidateAsyncMock(currentPolicy, modifiedPolicy); } @@ -25,3 +26,18 @@ public class FakeSingleOrgPolicyDefinition : IPolicyDefinition return Task.FromResult(0); } } +public class FakeRequireSsoPolicyDefinition : IPolicyDefinition +{ + public PolicyType Type => PolicyType.RequireSso; + public IEnumerable RequiredPolicies => [PolicyType.SingleOrg]; + + public Task ValidateAsync(Policy? currentPolicy, Policy modifiedPolicy) + { + return Task.FromResult((string)null); + } + + public Task OnSaveSideEffectsAsync(Policy? currentPolicy, Policy modifiedPolicy) + { + return Task.FromResult(0); + } +} diff --git a/test/Core.Test/AdminConsole/Services/PolicyServicevNextTests.cs b/test/Core.Test/AdminConsole/Services/PolicyServicevNextTests.cs index 4ad8828ad..2477a355c 100644 --- a/test/Core.Test/AdminConsole/Services/PolicyServicevNextTests.cs +++ b/test/Core.Test/AdminConsole/Services/PolicyServicevNextTests.cs @@ -1,4 +1,6 @@ -using AutoFixture; +#nullable enable + +using AutoFixture; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; @@ -107,6 +109,78 @@ public class PolicyServicevNextTests await AssertPolicyNotSavedAsync(sutProvider); } + [Theory, BitAutoData] + public async Task SaveAsync_RequiredPolicyIsNull_Throws( + [Policy(PolicyType.RequireSso)] Policy policy) + { + var sutProvider = SutProviderFactory([ + new FakeRequireSsoPolicyDefinition(), + new FakeSingleOrgPolicyDefinition() + ]); + + ArrangeOrganization(sutProvider, policy); + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(policy.OrganizationId) + .Returns([]); + + var badRequestException = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveAsync(policy, + Substitute.For(), + Substitute.For(), + Guid.NewGuid())); + + Assert.Contains("Policy requires PolicyType SingleOrg to be enabled", badRequestException.Message, StringComparison.OrdinalIgnoreCase); + await AssertPolicyNotSavedAsync(sutProvider); + } + + [Theory, BitAutoData] + public async Task SaveAsync_RequiredPolicyNotEnabled_Throws( + [Policy(PolicyType.SingleOrg, false)] Policy singleOrgPolicy, + [Policy(PolicyType.RequireSso)] Policy policy) + { + var sutProvider = SutProviderFactory([ + new FakeRequireSsoPolicyDefinition(), + new FakeSingleOrgPolicyDefinition() + ]); + + ArrangeOrganization(sutProvider, policy); + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(policy.OrganizationId) + .Returns([singleOrgPolicy]); + + var badRequestException = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveAsync(policy, + Substitute.For(), + Substitute.For(), + Guid.NewGuid())); + + Assert.Contains("Policy requires PolicyType SingleOrg to be enabled", badRequestException.Message, StringComparison.OrdinalIgnoreCase); + await AssertPolicyNotSavedAsync(sutProvider); + } + + [Theory, BitAutoData] + public async Task SaveAsync_RequiredPolicyEnabled_Success( + [Policy(PolicyType.SingleOrg, true)] Policy singleOrgPolicy, + [Policy(PolicyType.RequireSso)] Policy policy) + { + var sutProvider = SutProviderFactory([ + new FakeRequireSsoPolicyDefinition(), + new FakeSingleOrgPolicyDefinition() + ]); + + ArrangeOrganization(sutProvider, policy); + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(policy.OrganizationId) + .Returns([singleOrgPolicy]); + + await sutProvider.Sut.SaveAsync(policy, + Substitute.For(), + Substitute.For(), + Guid.NewGuid()); + + await sutProvider.GetDependency().Received(1).UpsertAsync(policy); + } + /// /// Returns a new SutProvider with the PolicyDefinitions registered in the Sut. ///