mirror of
https://github.com/bitwarden/server.git
synced 2025-07-02 08:32:50 -05:00
[PM-18876] Refine PolicyRequirements API (#5445)
* make the PolicyRequirements API more granular, e.g. replace factory methods with a factory interface * update Send to use the new API
This commit is contained in:
Thomas Rittson
@ -0,0 +1,23 @@
|
||||
using Bit.Core.AdminConsole.Enums;
|
||||
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
|
||||
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
|
||||
|
||||
namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies;
|
||||
|
||||
/// <summary>
|
||||
/// Intentionally simplified PolicyRequirement that just holds the input PolicyDetails for us to assert against.
|
||||
/// </summary>
|
||||
public class TestPolicyRequirement : IPolicyRequirement
|
||||
{
|
||||
public IEnumerable<PolicyDetails> Policies { get; init; } = [];
|
||||
}
|
||||
|
||||
public class TestPolicyRequirementFactory(Func<PolicyDetails, bool> enforce) : IPolicyRequirementFactory<TestPolicyRequirement>
|
||||
{
|
||||
public PolicyType PolicyType => PolicyType.SingleOrg;
|
||||
|
||||
public bool Enforce(PolicyDetails policyDetails) => enforce(policyDetails);
|
||||
|
||||
public TestPolicyRequirement Create(IEnumerable<PolicyDetails> policyDetails)
|
||||
=> new() { Policies = policyDetails };
|
||||
}
|
||||
@ -1,6 +1,6 @@
|
||||
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
|
||||
using Bit.Core.AdminConsole.Enums;
|
||||
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
|
||||
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Implementations;
|
||||
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
|
||||
using Bit.Core.AdminConsole.Repositories;
|
||||
using Bit.Test.Common.AutoFixture.Attributes;
|
||||
using NSubstitute;
|
||||
@ -11,50 +11,72 @@ namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies;
|
||||
[SutProviderCustomize]
|
||||
public class PolicyRequirementQueryTests
|
||||
{
|
||||
/// <summary>
|
||||
/// Tests that the query correctly registers, retrieves and instantiates arbitrary IPolicyRequirements
|
||||
/// according to their provided CreateRequirement delegate.
|
||||
/// </summary>
|
||||
[Theory, BitAutoData]
|
||||
public async Task GetAsync_Works(Guid userId, Guid organizationId)
|
||||
public async Task GetAsync_IgnoresOtherPolicyTypes(Guid userId)
|
||||
{
|
||||
var thisPolicy = new PolicyDetails { PolicyType = PolicyType.SingleOrg };
|
||||
var otherPolicy = new PolicyDetails { PolicyType = PolicyType.RequireSso };
|
||||
var policyRepository = Substitute.For<IPolicyRepository>();
|
||||
var factories = new List<RequirementFactory<IPolicyRequirement>>
|
||||
{
|
||||
// In prod this cast is handled when the CreateRequirement delegate is registered in DI
|
||||
(RequirementFactory<TestPolicyRequirement>)TestPolicyRequirement.Create
|
||||
};
|
||||
policyRepository.GetPolicyDetailsByUserId(userId).Returns([otherPolicy, thisPolicy]);
|
||||
|
||||
var sut = new PolicyRequirementQuery(policyRepository, factories);
|
||||
policyRepository.GetPolicyDetailsByUserId(userId).Returns([
|
||||
new PolicyDetails
|
||||
{
|
||||
OrganizationId = organizationId
|
||||
}
|
||||
]);
|
||||
var factory = new TestPolicyRequirementFactory(_ => true);
|
||||
var sut = new PolicyRequirementQuery(policyRepository, [factory]);
|
||||
|
||||
var requirement = await sut.GetAsync<TestPolicyRequirement>(userId);
|
||||
Assert.Equal(organizationId, requirement.OrganizationId);
|
||||
|
||||
Assert.Contains(thisPolicy, requirement.Policies);
|
||||
Assert.DoesNotContain(otherPolicy, requirement.Policies);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task GetAsync_ThrowsIfNoRequirementRegistered(Guid userId)
|
||||
public async Task GetAsync_CallsEnforceCallback(Guid userId)
|
||||
{
|
||||
// Arrange policies
|
||||
var policyRepository = Substitute.For<IPolicyRepository>();
|
||||
var thisPolicy = new PolicyDetails { PolicyType = PolicyType.SingleOrg };
|
||||
var otherPolicy = new PolicyDetails { PolicyType = PolicyType.SingleOrg };
|
||||
policyRepository.GetPolicyDetailsByUserId(userId).Returns([thisPolicy, otherPolicy]);
|
||||
|
||||
// Arrange a substitute Enforce function so that we can inspect the received calls
|
||||
var callback = Substitute.For<Func<PolicyDetails, bool>>();
|
||||
callback(Arg.Any<PolicyDetails>()).Returns(x => x.Arg<PolicyDetails>() == thisPolicy);
|
||||
|
||||
// Arrange the sut
|
||||
var factory = new TestPolicyRequirementFactory(callback);
|
||||
var sut = new PolicyRequirementQuery(policyRepository, [factory]);
|
||||
|
||||
// Act
|
||||
var requirement = await sut.GetAsync<TestPolicyRequirement>(userId);
|
||||
|
||||
// Assert
|
||||
Assert.Contains(thisPolicy, requirement.Policies);
|
||||
Assert.DoesNotContain(otherPolicy, requirement.Policies);
|
||||
callback.Received()(Arg.Is(thisPolicy));
|
||||
callback.Received()(Arg.Is(otherPolicy));
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task GetAsync_ThrowsIfNoFactoryRegistered(Guid userId)
|
||||
{
|
||||
var policyRepository = Substitute.For<IPolicyRepository>();
|
||||
var sut = new PolicyRequirementQuery(policyRepository, []);
|
||||
|
||||
var exception = await Assert.ThrowsAsync<NotImplementedException>(()
|
||||
=> sut.GetAsync<TestPolicyRequirement>(userId));
|
||||
Assert.Contains("No Policy Requirement found", exception.Message);
|
||||
Assert.Contains("No Requirement Factory found", exception.Message);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Intentionally simplified PolicyRequirement that just holds the Policy.OrganizationId for us to assert against.
|
||||
/// </summary>
|
||||
private class TestPolicyRequirement : IPolicyRequirement
|
||||
[Theory, BitAutoData]
|
||||
public async Task GetAsync_HandlesNoPolicies(Guid userId)
|
||||
{
|
||||
public Guid OrganizationId { get; init; }
|
||||
public static TestPolicyRequirement Create(IEnumerable<PolicyDetails> policyDetails)
|
||||
=> new() { OrganizationId = policyDetails.Single().OrganizationId };
|
||||
var policyRepository = Substitute.For<IPolicyRepository>();
|
||||
policyRepository.GetPolicyDetailsByUserId(userId).Returns([]);
|
||||
|
||||
var factory = new TestPolicyRequirementFactory(x => x.IsProvider);
|
||||
var sut = new PolicyRequirementQuery(policyRepository, [factory]);
|
||||
|
||||
var requirement = await sut.GetAsync<TestPolicyRequirement>(userId);
|
||||
|
||||
Assert.Empty(requirement.Policies);
|
||||
}
|
||||
}
|
||||
|
||||
@ -0,0 +1,90 @@
|
||||
using AutoFixture.Xunit2;
|
||||
using Bit.Core.AdminConsole.Enums;
|
||||
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
|
||||
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
|
||||
using Bit.Core.Enums;
|
||||
using Bit.Core.Test.AdminConsole.AutoFixture;
|
||||
using Xunit;
|
||||
|
||||
namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
|
||||
|
||||
public class BasePolicyRequirementFactoryTests
|
||||
{
|
||||
[Theory, AutoData]
|
||||
public void ExemptRoles_DoesNotEnforceAgainstThoseRoles(
|
||||
[PolicyDetails(PolicyType.SingleOrg, OrganizationUserType.Owner)] PolicyDetails ownerPolicy,
|
||||
[PolicyDetails(PolicyType.SingleOrg, OrganizationUserType.Admin)] PolicyDetails adminPolicy,
|
||||
[PolicyDetails(PolicyType.SingleOrg, OrganizationUserType.Custom)] PolicyDetails customPolicy,
|
||||
[PolicyDetails(PolicyType.SingleOrg)] PolicyDetails userPolicy)
|
||||
{
|
||||
var sut = new TestPolicyRequirementFactory(
|
||||
// These exempt roles are intentionally unusual to make sure we're properly testing the sut
|
||||
[OrganizationUserType.User, OrganizationUserType.Custom],
|
||||
[],
|
||||
false);
|
||||
|
||||
Assert.True(sut.Enforce(ownerPolicy));
|
||||
Assert.True(sut.Enforce(adminPolicy));
|
||||
Assert.False(sut.Enforce(customPolicy));
|
||||
Assert.False(sut.Enforce(userPolicy));
|
||||
}
|
||||
|
||||
[Theory, AutoData]
|
||||
public void ExemptStatuses_DoesNotEnforceAgainstThoseStatuses(
|
||||
[PolicyDetails(PolicyType.SingleOrg, userStatus: OrganizationUserStatusType.Invited)] PolicyDetails invitedPolicy,
|
||||
[PolicyDetails(PolicyType.SingleOrg, userStatus: OrganizationUserStatusType.Accepted)] PolicyDetails acceptedPolicy,
|
||||
[PolicyDetails(PolicyType.SingleOrg, userStatus: OrganizationUserStatusType.Confirmed)] PolicyDetails confirmedPolicy,
|
||||
[PolicyDetails(PolicyType.SingleOrg, userStatus: OrganizationUserStatusType.Revoked)] PolicyDetails revokedPolicy)
|
||||
{
|
||||
var sut = new TestPolicyRequirementFactory(
|
||||
[],
|
||||
// These exempt statuses are intentionally unusual to make sure we're properly testing the sut
|
||||
[OrganizationUserStatusType.Confirmed, OrganizationUserStatusType.Accepted],
|
||||
false);
|
||||
|
||||
Assert.True(sut.Enforce(invitedPolicy));
|
||||
Assert.True(sut.Enforce(revokedPolicy));
|
||||
Assert.False(sut.Enforce(confirmedPolicy));
|
||||
Assert.False(sut.Enforce(acceptedPolicy));
|
||||
}
|
||||
|
||||
[Theory, AutoData]
|
||||
public void ExemptProviders_DoesNotEnforceAgainstProviders(
|
||||
[PolicyDetails(PolicyType.SingleOrg, isProvider: true)] PolicyDetails policy)
|
||||
{
|
||||
var sut = new TestPolicyRequirementFactory(
|
||||
[],
|
||||
[],
|
||||
true);
|
||||
|
||||
Assert.False(sut.Enforce(policy));
|
||||
}
|
||||
|
||||
[Theory, AutoData]
|
||||
public void NoExemptions_EnforcesAgainstAdminsAndProviders(
|
||||
[PolicyDetails(PolicyType.SingleOrg, OrganizationUserType.Owner, isProvider: true)] PolicyDetails policy)
|
||||
{
|
||||
var sut = new TestPolicyRequirementFactory(
|
||||
[],
|
||||
[],
|
||||
false);
|
||||
|
||||
Assert.True(sut.Enforce(policy));
|
||||
}
|
||||
|
||||
private class TestPolicyRequirementFactory(
|
||||
IEnumerable<OrganizationUserType> exemptRoles,
|
||||
IEnumerable<OrganizationUserStatusType> exemptStatuses,
|
||||
bool exemptProviders
|
||||
) : BasePolicyRequirementFactory<TestPolicyRequirement>
|
||||
{
|
||||
public override PolicyType PolicyType => PolicyType.SingleOrg;
|
||||
protected override IEnumerable<OrganizationUserType> ExemptRoles => exemptRoles;
|
||||
protected override IEnumerable<OrganizationUserStatusType> ExemptStatuses => exemptStatuses;
|
||||
|
||||
protected override bool ExemptProviders => exemptProviders;
|
||||
|
||||
public override TestPolicyRequirement Create(IEnumerable<PolicyDetails> policyDetails)
|
||||
=> new() { Policies = policyDetails };
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,32 @@
|
||||
using Bit.Core.AdminConsole.Enums;
|
||||
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
|
||||
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
|
||||
using Bit.Core.Test.AdminConsole.AutoFixture;
|
||||
using Bit.Test.Common.AutoFixture;
|
||||
using Bit.Test.Common.AutoFixture.Attributes;
|
||||
using Xunit;
|
||||
|
||||
namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
|
||||
|
||||
[SutProviderCustomize]
|
||||
public class DisableSendPolicyRequirementFactoryTests
|
||||
{
|
||||
[Theory, BitAutoData]
|
||||
public void DisableSend_IsFalse_IfNoPolicies(SutProvider<DisableSendPolicyRequirementFactory> sutProvider)
|
||||
{
|
||||
var actual = sutProvider.Sut.Create([]);
|
||||
|
||||
Assert.False(actual.DisableSend);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public void DisableSend_IsTrue_IfAnyDisableSendPolicies(
|
||||
[PolicyDetails(PolicyType.DisableSend)] PolicyDetails[] policies,
|
||||
SutProvider<DisableSendPolicyRequirementFactory> sutProvider
|
||||
)
|
||||
{
|
||||
var actual = sutProvider.Sut.Create(policies);
|
||||
|
||||
Assert.True(actual.DisableSend);
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,49 @@
|
||||
using Bit.Core.AdminConsole.Enums;
|
||||
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
|
||||
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
|
||||
using Bit.Core.Test.AdminConsole.AutoFixture;
|
||||
using Bit.Test.Common.AutoFixture;
|
||||
using Bit.Test.Common.AutoFixture.Attributes;
|
||||
using Xunit;
|
||||
|
||||
namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
|
||||
|
||||
[SutProviderCustomize]
|
||||
public class SendOptionsPolicyRequirementFactoryTests
|
||||
{
|
||||
[Theory, BitAutoData]
|
||||
public void DisableHideEmail_IsFalse_IfNoPolicies(SutProvider<SendOptionsPolicyRequirementFactory> sutProvider)
|
||||
{
|
||||
var actual = sutProvider.Sut.Create([]);
|
||||
|
||||
Assert.False(actual.DisableHideEmail);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public void DisableHideEmail_IsFalse_IfNotConfigured(
|
||||
[PolicyDetails(PolicyType.SendOptions)] PolicyDetails[] policies,
|
||||
SutProvider<SendOptionsPolicyRequirementFactory> sutProvider
|
||||
)
|
||||
{
|
||||
policies[0].SetDataModel(new SendOptionsPolicyData { DisableHideEmail = false });
|
||||
policies[1].SetDataModel(new SendOptionsPolicyData { DisableHideEmail = false });
|
||||
|
||||
var actual = sutProvider.Sut.Create(policies);
|
||||
|
||||
Assert.False(actual.DisableHideEmail);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public void DisableHideEmail_IsTrue_IfAnyConfigured(
|
||||
[PolicyDetails(PolicyType.SendOptions)] PolicyDetails[] policies,
|
||||
SutProvider<SendOptionsPolicyRequirementFactory> sutProvider
|
||||
)
|
||||
{
|
||||
policies[0].SetDataModel(new SendOptionsPolicyData { DisableHideEmail = true });
|
||||
policies[1].SetDataModel(new SendOptionsPolicyData { DisableHideEmail = false });
|
||||
|
||||
var actual = sutProvider.Sut.Create(policies);
|
||||
|
||||
Assert.True(actual.DisableHideEmail);
|
||||
}
|
||||
}
|
||||
@ -1,138 +0,0 @@
|
||||
using AutoFixture.Xunit2;
|
||||
using Bit.Core.AdminConsole.Enums;
|
||||
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
|
||||
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
|
||||
using Bit.Core.Enums;
|
||||
using Bit.Core.Test.AdminConsole.AutoFixture;
|
||||
using Xunit;
|
||||
|
||||
namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
|
||||
|
||||
public class SendPolicyRequirementTests
|
||||
{
|
||||
[Theory, AutoData]
|
||||
public void DisableSend_IsFalse_IfNoDisableSendPolicies(
|
||||
[PolicyDetails(PolicyType.RequireSso)] PolicyDetails otherPolicy1,
|
||||
[PolicyDetails(PolicyType.SendOptions)] PolicyDetails otherPolicy2)
|
||||
{
|
||||
EnableDisableHideEmail(otherPolicy2);
|
||||
|
||||
var actual = SendPolicyRequirement.Create([otherPolicy1, otherPolicy2]);
|
||||
|
||||
Assert.False(actual.DisableSend);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineAutoData(OrganizationUserType.Owner, false)]
|
||||
[InlineAutoData(OrganizationUserType.Admin, false)]
|
||||
[InlineAutoData(OrganizationUserType.User, true)]
|
||||
[InlineAutoData(OrganizationUserType.Custom, true)]
|
||||
public void DisableSend_TestRoles(
|
||||
OrganizationUserType userType,
|
||||
bool shouldBeEnforced,
|
||||
[PolicyDetails(PolicyType.DisableSend)] PolicyDetails policyDetails)
|
||||
{
|
||||
policyDetails.OrganizationUserType = userType;
|
||||
|
||||
var actual = SendPolicyRequirement.Create([policyDetails]);
|
||||
|
||||
Assert.Equal(shouldBeEnforced, actual.DisableSend);
|
||||
}
|
||||
|
||||
[Theory, AutoData]
|
||||
public void DisableSend_Not_EnforcedAgainstProviders(
|
||||
[PolicyDetails(PolicyType.DisableSend, isProvider: true)] PolicyDetails policyDetails)
|
||||
{
|
||||
var actual = SendPolicyRequirement.Create([policyDetails]);
|
||||
|
||||
Assert.False(actual.DisableSend);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineAutoData(OrganizationUserStatusType.Confirmed, true)]
|
||||
[InlineAutoData(OrganizationUserStatusType.Accepted, true)]
|
||||
[InlineAutoData(OrganizationUserStatusType.Invited, false)]
|
||||
[InlineAutoData(OrganizationUserStatusType.Revoked, false)]
|
||||
public void DisableSend_TestStatuses(
|
||||
OrganizationUserStatusType userStatus,
|
||||
bool shouldBeEnforced,
|
||||
[PolicyDetails(PolicyType.DisableSend)] PolicyDetails policyDetails)
|
||||
{
|
||||
policyDetails.OrganizationUserStatus = userStatus;
|
||||
|
||||
var actual = SendPolicyRequirement.Create([policyDetails]);
|
||||
|
||||
Assert.Equal(shouldBeEnforced, actual.DisableSend);
|
||||
}
|
||||
|
||||
[Theory, AutoData]
|
||||
public void DisableHideEmail_IsFalse_IfNoSendOptionsPolicies(
|
||||
[PolicyDetails(PolicyType.RequireSso)] PolicyDetails otherPolicy1,
|
||||
[PolicyDetails(PolicyType.DisableSend)] PolicyDetails otherPolicy2)
|
||||
{
|
||||
var actual = SendPolicyRequirement.Create([otherPolicy1, otherPolicy2]);
|
||||
|
||||
Assert.False(actual.DisableHideEmail);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineAutoData(OrganizationUserType.Owner, false)]
|
||||
[InlineAutoData(OrganizationUserType.Admin, false)]
|
||||
[InlineAutoData(OrganizationUserType.User, true)]
|
||||
[InlineAutoData(OrganizationUserType.Custom, true)]
|
||||
public void DisableHideEmail_TestRoles(
|
||||
OrganizationUserType userType,
|
||||
bool shouldBeEnforced,
|
||||
[PolicyDetails(PolicyType.SendOptions)] PolicyDetails policyDetails)
|
||||
{
|
||||
EnableDisableHideEmail(policyDetails);
|
||||
policyDetails.OrganizationUserType = userType;
|
||||
|
||||
var actual = SendPolicyRequirement.Create([policyDetails]);
|
||||
|
||||
Assert.Equal(shouldBeEnforced, actual.DisableHideEmail);
|
||||
}
|
||||
|
||||
[Theory, AutoData]
|
||||
public void DisableHideEmail_Not_EnforcedAgainstProviders(
|
||||
[PolicyDetails(PolicyType.SendOptions, isProvider: true)] PolicyDetails policyDetails)
|
||||
{
|
||||
EnableDisableHideEmail(policyDetails);
|
||||
|
||||
var actual = SendPolicyRequirement.Create([policyDetails]);
|
||||
|
||||
Assert.False(actual.DisableHideEmail);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineAutoData(OrganizationUserStatusType.Confirmed, true)]
|
||||
[InlineAutoData(OrganizationUserStatusType.Accepted, true)]
|
||||
[InlineAutoData(OrganizationUserStatusType.Invited, false)]
|
||||
[InlineAutoData(OrganizationUserStatusType.Revoked, false)]
|
||||
public void DisableHideEmail_TestStatuses(
|
||||
OrganizationUserStatusType userStatus,
|
||||
bool shouldBeEnforced,
|
||||
[PolicyDetails(PolicyType.SendOptions)] PolicyDetails policyDetails)
|
||||
{
|
||||
EnableDisableHideEmail(policyDetails);
|
||||
policyDetails.OrganizationUserStatus = userStatus;
|
||||
|
||||
var actual = SendPolicyRequirement.Create([policyDetails]);
|
||||
|
||||
Assert.Equal(shouldBeEnforced, actual.DisableHideEmail);
|
||||
}
|
||||
|
||||
[Theory, AutoData]
|
||||
public void DisableHideEmail_HandlesNullData(
|
||||
[PolicyDetails(PolicyType.SendOptions)] PolicyDetails policyDetails)
|
||||
{
|
||||
policyDetails.PolicyData = null;
|
||||
|
||||
var actual = SendPolicyRequirement.Create([policyDetails]);
|
||||
|
||||
Assert.False(actual.DisableHideEmail);
|
||||
}
|
||||
|
||||
private static void EnableDisableHideEmail(PolicyDetails policyDetails)
|
||||
=> policyDetails.SetDataModel(new SendOptionsPolicyData { DisableHideEmail = true });
|
||||
}
|
||||
Reference in New Issue
Block a user