From 571111e8974b26e770efe0dd40ddfa8c20ee2ecf Mon Sep 17 00:00:00 2001 From: Brandon Treston Date: Fri, 25 Jul 2025 10:14:16 -0400 Subject: [PATCH] [PM-18239] Master password policy requirement (#5936) * wip * initial implementation * add tests * more tests, fix policy Enabled * remove exempt statuses * test EnforcedOptions is populated * clean up, add test * fix test, add json attributes for deserialization * fix attribute casing * fix test --------- Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> --- .../Policies/MasterPasswordPolicyData.cs | 12 ++- .../MasterPasswordPolicyRequirement.cs | 55 ++++++++++++++ .../Services/Implementations/PolicyService.cs | 23 +++++- .../Controllers/PoliciesControllerTests.cs | 14 ++-- .../MasterPasswordPolicyRequirementTests.cs | 75 +++++++++++++++++++ .../Services/PolicyServiceTests.cs | 38 ++++++++++ 6 files changed, 207 insertions(+), 10 deletions(-) create mode 100644 src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/MasterPasswordPolicyRequirement.cs create mode 100644 test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/MasterPasswordPolicyRequirementTests.cs diff --git a/src/Core/AdminConsole/Models/Data/Organizations/Policies/MasterPasswordPolicyData.cs b/src/Core/AdminConsole/Models/Data/Organizations/Policies/MasterPasswordPolicyData.cs index f2f275b708..b66244ba5f 100644 --- a/src/Core/AdminConsole/Models/Data/Organizations/Policies/MasterPasswordPolicyData.cs +++ b/src/Core/AdminConsole/Models/Data/Organizations/Policies/MasterPasswordPolicyData.cs @@ -1,20 +1,28 @@ -namespace Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using System.Text.Json.Serialization; +namespace Bit.Core.AdminConsole.Models.Data.Organizations.Policies; public class MasterPasswordPolicyData : IPolicyDataModel { + [JsonPropertyName("minComplexity")] public int? MinComplexity { get; set; } + [JsonPropertyName("minLength")] public int? MinLength { get; set; } + [JsonPropertyName("requireLower")] public bool? RequireLower { get; set; } + [JsonPropertyName("requireUpper")] public bool? RequireUpper { get; set; } + [JsonPropertyName("requireNumbers")] public bool? RequireNumbers { get; set; } + [JsonPropertyName("requireSpecial")] public bool? RequireSpecial { get; set; } + [JsonPropertyName("enforceOnLogin")] public bool? EnforceOnLogin { get; set; } /// /// Combine the other policy data with this instance, taking the most secure options /// /// The other policy instance to combine with this - public void CombineWith(MasterPasswordPolicyData other) + public void CombineWith(MasterPasswordPolicyData? other) { if (other == null) { diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/MasterPasswordPolicyRequirement.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/MasterPasswordPolicyRequirement.cs new file mode 100644 index 0000000000..9e8154db53 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/MasterPasswordPolicyRequirement.cs @@ -0,0 +1,55 @@ +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.Enums; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; + +/// +/// Policy requirements for the Master Password Requirements policy. +/// +public class MasterPasswordPolicyRequirement : IPolicyRequirement +{ + /// + /// Indicates whether MasterPassword requirements are enabled for the user. + /// + public bool Enabled { get; init; } + + /// + /// Master Password Policy data model associated with this Policy + /// + public MasterPasswordPolicyData? EnforcedOptions { get; init; } +} + +public class MasterPasswordPolicyRequirementFactory : BasePolicyRequirementFactory +{ + public override PolicyType PolicyType => PolicyType.MasterPassword; + + protected override bool ExemptProviders => false; + + protected override IEnumerable ExemptRoles => []; + + protected override IEnumerable ExemptStatuses => + [OrganizationUserStatusType.Accepted, + OrganizationUserStatusType.Invited, + OrganizationUserStatusType.Revoked, + ]; + + public override MasterPasswordPolicyRequirement Create(IEnumerable policyDetails) + { + var result = policyDetails + .Select(p => p.GetDataModel()) + .Aggregate( + new MasterPasswordPolicyRequirement(), + (result, data) => + { + data.CombineWith(result.EnforcedOptions); + return new MasterPasswordPolicyRequirement + { + Enabled = true, + EnforcedOptions = data + }; + }); + + return result; + } +} diff --git a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs index 5ba39e8054..a83eccc301 100644 --- a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs +++ b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs @@ -3,6 +3,8 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Entities; using Bit.Core.Enums; @@ -19,21 +21,39 @@ public class PolicyService : IPolicyService private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IPolicyRepository _policyRepository; private readonly GlobalSettings _globalSettings; + private readonly IFeatureService _featureService; + private readonly IPolicyRequirementQuery _policyRequirementQuery; public PolicyService( IApplicationCacheService applicationCacheService, IOrganizationUserRepository organizationUserRepository, IPolicyRepository policyRepository, - GlobalSettings globalSettings) + GlobalSettings globalSettings, + IFeatureService featureService, + IPolicyRequirementQuery policyRequirementQuery) { _applicationCacheService = applicationCacheService; _organizationUserRepository = organizationUserRepository; _policyRepository = policyRepository; _globalSettings = globalSettings; + _featureService = featureService; + _policyRequirementQuery = policyRequirementQuery; } public async Task GetMasterPasswordPolicyForUserAsync(User user) { + if (_featureService.IsEnabled(FeatureFlagKeys.PolicyRequirements)) + { + var masterPaswordPolicy = (await _policyRequirementQuery.GetAsync(user.Id)); + + if (!masterPaswordPolicy.Enabled) + { + return null; + } + + return masterPaswordPolicy.EnforcedOptions; + } + var policies = (await _policyRepository.GetManyByUserIdAsync(user.Id)) .Where(p => p.Type == PolicyType.MasterPassword && p.Enabled) .ToList(); @@ -51,6 +71,7 @@ public class PolicyService : IPolicyService } return enforcedOptions; + } public async Task> GetPoliciesApplicableToUserAsync(Guid userId, PolicyType policyType, OrganizationUserStatusType minStatus = OrganizationUserStatusType.Accepted) diff --git a/test/Api.Test/Controllers/PoliciesControllerTests.cs b/test/Api.Test/Controllers/PoliciesControllerTests.cs index 1f652c80f5..f5f3eddd3b 100644 --- a/test/Api.Test/Controllers/PoliciesControllerTests.cs +++ b/test/Api.Test/Controllers/PoliciesControllerTests.cs @@ -73,13 +73,13 @@ public class PoliciesControllerTests // Assert that the data is deserialized correctly into a Dictionary // for all MasterPasswordPolicyData properties - Assert.Equal(mpPolicyData.MinComplexity, ((JsonElement)result.Data["MinComplexity"]).GetInt32()); - Assert.Equal(mpPolicyData.MinLength, ((JsonElement)result.Data["MinLength"]).GetInt32()); - Assert.Equal(mpPolicyData.RequireLower, ((JsonElement)result.Data["RequireLower"]).GetBoolean()); - Assert.Equal(mpPolicyData.RequireUpper, ((JsonElement)result.Data["RequireUpper"]).GetBoolean()); - Assert.Equal(mpPolicyData.RequireNumbers, ((JsonElement)result.Data["RequireNumbers"]).GetBoolean()); - Assert.Equal(mpPolicyData.RequireSpecial, ((JsonElement)result.Data["RequireSpecial"]).GetBoolean()); - Assert.Equal(mpPolicyData.EnforceOnLogin, ((JsonElement)result.Data["EnforceOnLogin"]).GetBoolean()); + Assert.Equal(mpPolicyData.MinComplexity, ((JsonElement)result.Data["minComplexity"]).GetInt32()); + Assert.Equal(mpPolicyData.MinLength, ((JsonElement)result.Data["minLength"]).GetInt32()); + Assert.Equal(mpPolicyData.RequireLower, ((JsonElement)result.Data["requireLower"]).GetBoolean()); + Assert.Equal(mpPolicyData.RequireUpper, ((JsonElement)result.Data["requireUpper"]).GetBoolean()); + Assert.Equal(mpPolicyData.RequireNumbers, ((JsonElement)result.Data["requireNumbers"]).GetBoolean()); + Assert.Equal(mpPolicyData.RequireSpecial, ((JsonElement)result.Data["requireSpecial"]).GetBoolean()); + Assert.Equal(mpPolicyData.EnforceOnLogin, ((JsonElement)result.Data["enforceOnLogin"]).GetBoolean()); } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/MasterPasswordPolicyRequirementTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/MasterPasswordPolicyRequirementTests.cs new file mode 100644 index 0000000000..d3991bcde7 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/MasterPasswordPolicyRequirementTests.cs @@ -0,0 +1,75 @@ +using System.Text.Json; +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 MasterPasswordPolicyRequirementFactoryTests +{ + [Theory, BitAutoData] + public void MasterPasswordPolicyData_CombineWith_Joins_Policy_Options(SutProvider sutProvider) + { + var mpd1 = JsonSerializer.Serialize(new MasterPasswordPolicyData { MinLength = 20, RequireLower = false, RequireSpecial = false }); + var mpd2 = JsonSerializer.Serialize(new MasterPasswordPolicyData { RequireLower = true }); + var mpd3 = JsonSerializer.Serialize(new MasterPasswordPolicyData { RequireSpecial = true }); + + var policyDetails1 = new PolicyDetails + { + PolicyType = PolicyType.MasterPassword, + PolicyData = mpd1 + }; + + var policyDetails2 = new PolicyDetails + { + PolicyType = PolicyType.MasterPassword, + PolicyData = mpd2 + }; + var policyDetails3 = new PolicyDetails + { + PolicyType = PolicyType.MasterPassword, + PolicyData = mpd3 + }; + + + var actual = sutProvider.Sut.Create([policyDetails1, policyDetails2, policyDetails3]); + + Assert.NotNull(actual); + Assert.True(actual.Enabled); + Assert.True(actual.EnforcedOptions.RequireLower); + Assert.True(actual.EnforcedOptions.RequireSpecial); + Assert.Equal(20, actual.EnforcedOptions.MinLength); + } + + [Theory, BitAutoData] + public void MasterPassword_IsFalse_IfNoPolicies(SutProvider sutProvider) + { + var actual = sutProvider.Sut.Create([]); + + Assert.False(actual.Enabled); + Assert.Null(actual.EnforcedOptions); + } + + [Theory, BitAutoData] + public void MasterPassword_IsTrue_IfAnyDisableSendPolicies( + [PolicyDetails(PolicyType.MasterPassword)] PolicyDetails[] policies, + SutProvider sutProvider) + { + var actual = sutProvider.Sut.Create(policies); + + Assert.True(actual.Enabled); + Assert.NotNull(actual.EnforcedOptions); + Assert.NotNull(actual.EnforcedOptions.EnforceOnLogin); + Assert.NotNull(actual.EnforcedOptions.RequireLower); + Assert.NotNull(actual.EnforcedOptions.RequireNumbers); + Assert.NotNull(actual.EnforcedOptions.RequireSpecial); + Assert.NotNull(actual.EnforcedOptions.RequireUpper); + Assert.Null(actual.EnforcedOptions.MinComplexity); + Assert.Null(actual.EnforcedOptions.MinLength); + } +} diff --git a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs index 62ab584c4b..0af9eef12e 100644 --- a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs @@ -1,9 +1,15 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; +using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services.Implementations; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; +using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; @@ -117,6 +123,38 @@ public class PolicyServiceTests Assert.True(result); } + [Theory, BitAutoData] + public async Task GetMasterPasswordPolicyForUserAsync_WithFeatureFlagEnabled_EvaluatesPolicyRequirement(User user, SutProvider sutProvider) + { + SetupUserPolicies(user.Id, sutProvider); + var policyRequirement = new MasterPasswordPolicyRequirement + { + Enabled = true, + EnforcedOptions = new MasterPasswordPolicyData() + }; + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(true); + sutProvider.GetDependency().GetAsync(user.Id).Returns(policyRequirement); + + var result = await sutProvider.Sut.GetMasterPasswordPolicyForUserAsync(user); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.PolicyRequirements); + await sutProvider.GetDependency().DidNotReceive().GetManyByUserIdAsync(user.Id); + await sutProvider.GetDependency().Received(1).GetAsync(user.Id); + } + + [Theory, BitAutoData] + public async Task GetMasterPasswordPolicyForUserAsync_WithFeatureFlagDisabled_EvaluatesPolicyDetails(User user, SutProvider sutProvider) + { + SetupUserPolicies(user.Id, sutProvider); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.PolicyRequirements).Returns(false); + + var result = await sutProvider.Sut.GetMasterPasswordPolicyForUserAsync(user); + + sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.PolicyRequirements); + await sutProvider.GetDependency().Received(1).GetManyByUserIdAsync(user.Id); + await sutProvider.GetDependency().DidNotReceive().GetAsync(user.Id); + } + private static void SetupOrg(SutProvider sutProvider, Guid organizationId, Organization organization) { sutProvider.GetDependency()