Skip to content

Commit 3227cac

Browse files
BTrestonjaasen-livefront
BTreston
authored and
jaasen-livefront
committed
Implement master password policy requirement (#7537)
* implement master password policy requirement * revert unrelated chagned files * fix aggregate pattern in factory, move away from ctor * remove admin/owner exemption * revert mp exemption in policyService
1 parent 769d20f commit 3227cac

8 files changed

Lines changed: 289 additions & 184 deletions

File tree

‎src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/MasterPasswordPolicyRequirement.cs‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
#nullable enable
2+
3+
using Bit.Core.AdminConsole.Enums;
4+
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
5+
using Bit.Core.Enums;
6+
7+
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
8+
9+
/// <summary>
10+
/// Policy requirements for the Master Password policy.
11+
/// </summary>
12+
public class MasterPasswordPolicyRequirement : IPolicyRequirement
13+
{
14+
/// <summary>
15+
/// The combined Master Password policy options enforced against the user, or null if no policy applies.
16+
/// </summary>
17+
public MasterPasswordPolicyData? EnforcedOptions { get; init; }
18+
}
19+
20+
/// <summary>
21+
/// Factory for <see cref="MasterPasswordPolicyRequirement"/>.
22+
/// Invited and Revoked users are exempt (inherited default from <see cref="BasePolicyRequirementFactory{T}"/>),
23+
/// which is intentional: master password requirements are enforced at login/unlock for active members only.
24+
/// </summary>
25+
public class MasterPasswordPolicyRequirementFactory : BasePolicyRequirementFactory<MasterPasswordPolicyRequirement>
26+
{
27+
public override PolicyType PolicyType => PolicyType.MasterPassword;
28+
29+
/// <summary>
30+
/// No roles are exempt from the master password policy.
31+
/// </summary>
32+
protected override IEnumerable<OrganizationUserType> ExemptRoles { get; } = [];
33+
34+
public override MasterPasswordPolicyRequirement Create(IEnumerable<PolicyDetails> policyDetails)
35+
{
36+
var combined = policyDetails
37+
.Select(p => p.GetDataModel<MasterPasswordPolicyData>())
38+
.Aggregate(new MasterPasswordPolicyData(), (result, data) =>
39+
{
40+
result.CombineWith(data);
41+
return result;
42+
});
43+
44+
// Only set EnforcedOptions if at least one field has a meaningful value.
45+
// A policy saved with no options set produces an all-null MasterPasswordPolicyData,
46+
// and callers rely on EnforcedOptions == null to mean "no policy enforced".
47+
var hasAnyOption = combined.MinComplexity.HasValue || combined.MinLength.HasValue ||
48+
(combined.RequireLower ?? false) || (combined.RequireUpper ?? false) ||
49+
(combined.RequireNumbers ?? false) || (combined.RequireSpecial ?? false) ||
50+
(combined.EnforceOnLogin ?? false);
51+
52+
return new MasterPasswordPolicyRequirement
53+
{
54+
EnforcedOptions = hasAnyOption ? combined : null
55+
};
56+
}
57+
}

‎src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyServiceCollectionExtensions.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,5 +56,6 @@ private static void AddPolicyRequirements(this IServiceCollection services)
5656
services.AddScoped<IPolicyRequirementFactory<IPolicyRequirement>, RequireTwoFactorPolicyRequirementFactory>();
5757
services.AddScoped<IPolicyRequirementFactory<IPolicyRequirement>, SingleOrganizationPolicyRequirementFactory>();
5858
services.AddScoped<IPolicyRequirementFactory<IPolicyRequirement>, AutomaticUserConfirmationPolicyRequirementFactory>();
59+
services.AddScoped<IPolicyRequirementFactory<IPolicyRequirement>, MasterPasswordPolicyRequirementFactory>();
5960
}
6061
}

‎src/Core/AdminConsole/Services/Implementations/PolicyService.cs‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33

44
using Bit.Core.AdminConsole.Enums;
55
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
6-
using Bit.Core.AdminConsole.Repositories;
6+
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
7+
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
78
using Bit.Core.Entities;
89
using Bit.Core.Enums;
910
using Bit.Core.Models.Data.Organizations;
@@ -18,41 +19,25 @@ public class PolicyService : IPolicyService
1819
{
1920
private readonly IApplicationCacheService _applicationCacheService;
2021
private readonly IOrganizationUserRepository _organizationUserRepository;
21-
private readonly IPolicyRepository _policyRepository;
22+
private readonly IPolicyRequirementQuery _policyRequirementQuery;
2223
private readonly GlobalSettings _globalSettings;
2324

2425
public PolicyService(
2526
IApplicationCacheService applicationCacheService,
2627
IOrganizationUserRepository organizationUserRepository,
27-
IPolicyRepository policyRepository,
28+
IPolicyRequirementQuery policyRequirementQuery,
2829
GlobalSettings globalSettings)
2930
{
3031
_applicationCacheService = applicationCacheService;
3132
_organizationUserRepository = organizationUserRepository;
32-
_policyRepository = policyRepository;
33+
_policyRequirementQuery = policyRequirementQuery;
3334
_globalSettings = globalSettings;
3435
}
3536

3637
public async Task<MasterPasswordPolicyData> GetMasterPasswordPolicyForUserAsync(User user)
3738
{
38-
var policies = (await _policyRepository.GetManyByUserIdAsync(user.Id))
39-
.Where(p => p.Type == PolicyType.MasterPassword && p.Enabled)
40-
.ToList();
41-
42-
if (!policies.Any())
43-
{
44-
return null;
45-
}
46-
47-
var enforcedOptions = new MasterPasswordPolicyData();
48-
49-
foreach (var policy in policies)
50-
{
51-
enforcedOptions.CombineWith(policy.GetDataModel<MasterPasswordPolicyData>());
52-
}
53-
54-
return enforcedOptions;
55-
39+
var requirement = await _policyRequirementQuery.GetAsyncVNext<MasterPasswordPolicyRequirement>(user.Id);
40+
return requirement.EnforcedOptions;
5641
}
5742

5843
public async Task<ICollection<OrganizationUserPolicyDetails>> GetPoliciesApplicableToUserAsync(Guid userId, PolicyType policyType, OrganizationUserStatusType minStatus = OrganizationUserStatusType.Accepted)

‎test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/MasterPasswordPolicyRequirementFactoryTests.cs‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
using Bit.Core.AdminConsole.Enums;
2+
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
3+
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
4+
using Bit.Core.Enums;
5+
using Bit.Core.Test.AdminConsole.AutoFixture;
6+
using Bit.Test.Common.AutoFixture;
7+
using Bit.Test.Common.AutoFixture.Attributes;
8+
using Xunit;
9+
10+
namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
11+
12+
[SutProviderCustomize]
13+
public class MasterPasswordPolicyRequirementFactoryTests
14+
{
15+
[Theory]
16+
[BitAutoData(OrganizationUserType.Owner)]
17+
[BitAutoData(OrganizationUserType.Admin)]
18+
[BitAutoData(OrganizationUserType.User)]
19+
[BitAutoData(OrganizationUserType.Custom)]
20+
public void Enforce_EnforcesAgainstAllRoles(
21+
OrganizationUserType userType,
22+
SutProvider<MasterPasswordPolicyRequirementFactory> sutProvider)
23+
{
24+
var policy = new PolicyDetails
25+
{
26+
PolicyType = PolicyType.MasterPassword,
27+
OrganizationUserType = userType,
28+
OrganizationUserStatus = OrganizationUserStatusType.Confirmed
29+
};
30+
31+
Assert.True(sutProvider.Sut.Enforce(policy));
32+
}
33+
34+
35+
[Theory, BitAutoData]
36+
public void Create_WithNoPolicies_EnforcedOptionsIsNull(SutProvider<MasterPasswordPolicyRequirementFactory> sutProvider)
37+
{
38+
var actual = sutProvider.Sut.Create([]);
39+
40+
Assert.Null(actual.EnforcedOptions);
41+
}
42+
43+
[Theory, BitAutoData]
44+
public void Create_WithAllNullOptions_EnforcedOptionsIsNull(
45+
[PolicyDetails(PolicyType.MasterPassword)] PolicyDetails[] policies,
46+
SutProvider<MasterPasswordPolicyRequirementFactory> sutProvider)
47+
{
48+
foreach (var policy in policies)
49+
{
50+
policy.SetDataModel(new MasterPasswordPolicyData());
51+
}
52+
53+
var actual = sutProvider.Sut.Create(policies);
54+
55+
Assert.Null(actual.EnforcedOptions);
56+
}
57+
58+
[Theory, BitAutoData]
59+
public void Create_WithSinglePolicy_ReturnsEnforcedOptions(
60+
[PolicyDetails(PolicyType.MasterPassword)] PolicyDetails[] policies,
61+
SutProvider<MasterPasswordPolicyRequirementFactory> sutProvider)
62+
{
63+
policies[0].SetDataModel(new MasterPasswordPolicyData { MinLength = 12, RequireUpper = true });
64+
65+
var actual = sutProvider.Sut.Create([policies[0]]);
66+
67+
Assert.NotNull(actual.EnforcedOptions);
68+
Assert.Equal(12, actual.EnforcedOptions.MinLength);
69+
Assert.True(actual.EnforcedOptions.RequireUpper);
70+
}
71+
72+
[Theory, BitAutoData]
73+
public void Create_WithMultiplePolicies_TakesMaxMinComplexity(
74+
[PolicyDetails(PolicyType.MasterPassword)] PolicyDetails[] policies,
75+
SutProvider<MasterPasswordPolicyRequirementFactory> sutProvider)
76+
{
77+
policies[0].SetDataModel(new MasterPasswordPolicyData { MinComplexity = 2 });
78+
policies[1].SetDataModel(new MasterPasswordPolicyData { MinComplexity = 4 });
79+
80+
var actual = sutProvider.Sut.Create([policies[0], policies[1]]);
81+
82+
Assert.NotNull(actual.EnforcedOptions);
83+
Assert.Equal(4, actual.EnforcedOptions.MinComplexity);
84+
}
85+
86+
[Theory, BitAutoData]
87+
public void Create_WithMultiplePolicies_TakesMaxMinLength(
88+
[PolicyDetails(PolicyType.MasterPassword)] PolicyDetails[] policies,
89+
SutProvider<MasterPasswordPolicyRequirementFactory> sutProvider)
90+
{
91+
policies[0].SetDataModel(new MasterPasswordPolicyData { MinLength = 12 });
92+
policies[1].SetDataModel(new MasterPasswordPolicyData { MinLength = 20 });
93+
94+
var actual = sutProvider.Sut.Create([policies[0], policies[1]]);
95+
96+
Assert.NotNull(actual.EnforcedOptions);
97+
Assert.Equal(20, actual.EnforcedOptions.MinLength);
98+
}
99+
100+
[Theory, BitAutoData]
101+
public void Create_WithMultiplePolicies_OrsAllBooleanFlags(
102+
[PolicyDetails(PolicyType.MasterPassword)] PolicyDetails[] policies,
103+
SutProvider<MasterPasswordPolicyRequirementFactory> sutProvider)
104+
{
105+
policies[0].SetDataModel(new MasterPasswordPolicyData { RequireUpper = true });
106+
policies[1].SetDataModel(new MasterPasswordPolicyData { RequireSpecial = true });
107+
108+
var actual = sutProvider.Sut.Create([policies[0], policies[1]]);
109+
110+
Assert.NotNull(actual.EnforcedOptions);
111+
Assert.True(actual.EnforcedOptions.RequireUpper);
112+
Assert.True(actual.EnforcedOptions.RequireSpecial);
113+
}
114+
}

‎test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs‎

Lines changed: 16 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
using Bit.Core.AdminConsole.Entities;
2-
using Bit.Core.AdminConsole.Enums;
1+
using Bit.Core.AdminConsole.Enums;
32
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
4-
using Bit.Core.AdminConsole.Repositories;
3+
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
4+
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
55
using Bit.Core.AdminConsole.Services.Implementations;
66
using Bit.Core.Entities;
77
using Bit.Core.Enums;
@@ -125,123 +125,31 @@ public async Task AnyPoliciesApplicableToUserAsync_WithDisableSendTypeFilter_Wit
125125
[Theory, BitAutoData]
126126
public async Task GetMasterPasswordPolicyForUserAsync_ReturnsEnforcedOptions(User user, SutProvider<PolicyService> sutProvider)
127127
{
128-
// Arrange: Create three policies with different requirements to test combining behavior
129-
var policy1 = new Policy
130-
{
131-
Id = Guid.NewGuid(),
132-
OrganizationId = Guid.NewGuid(),
133-
Type = PolicyType.MasterPassword,
134-
Enabled = true
135-
};
136-
policy1.SetDataModel(new MasterPasswordPolicyData
137-
{
138-
MinComplexity = 3,
139-
MinLength = 12,
140-
RequireLower = true,
141-
RequireUpper = false,
142-
RequireNumbers = true,
143-
RequireSpecial = false,
144-
EnforceOnLogin = true
145-
});
146-
147-
var policy2 = new Policy
148-
{
149-
Id = Guid.NewGuid(),
150-
OrganizationId = Guid.NewGuid(),
151-
Type = PolicyType.MasterPassword,
152-
Enabled = true
153-
};
154-
policy2.SetDataModel(new MasterPasswordPolicyData
155-
{
156-
MinComplexity = 4,
157-
MinLength = 10,
158-
RequireLower = false,
159-
RequireUpper = true,
160-
RequireNumbers = false,
161-
RequireSpecial = true,
162-
EnforceOnLogin = false
163-
});
164-
165-
var policy3 = new Policy
166-
{
167-
Id = Guid.NewGuid(),
168-
OrganizationId = Guid.NewGuid(),
169-
Type = PolicyType.MasterPassword,
170-
Enabled = true
171-
};
172-
policy3.SetDataModel(new MasterPasswordPolicyData
173-
{
174-
MinComplexity = 2,
175-
MinLength = 15,
176-
RequireLower = false,
177-
RequireUpper = false,
178-
RequireNumbers = false,
179-
RequireSpecial = false,
180-
EnforceOnLogin = false
181-
});
182-
183-
sutProvider.GetDependency<IPolicyRepository>()
184-
.GetManyByUserIdAsync(user.Id)
185-
.Returns([policy1, policy2, policy3]);
186-
187-
// Act
188-
var result = await sutProvider.Sut.GetMasterPasswordPolicyForUserAsync(user);
128+
var enforcedOptions = new MasterPasswordPolicyData { MinLength = 12, RequireUpper = true };
129+
var requirement = new MasterPasswordPolicyRequirement { EnforcedOptions = enforcedOptions };
189130

190-
// Assert: Verify that policies were combined correctly
191-
Assert.NotNull(result);
131+
sutProvider.GetDependency<IPolicyRequirementQuery>()
132+
.GetAsyncVNext<MasterPasswordPolicyRequirement>(user.Id)
133+
.Returns(requirement);
192134

193-
// MinComplexity and MinLength should take the highest values
194-
Assert.Equal(4, result.MinComplexity); // highest from policy2
195-
Assert.Equal(15, result.MinLength); // highest from policy3
135+
var result = await sutProvider.Sut.GetMasterPasswordPolicyForUserAsync(user);
196136

197-
// Boolean flags should use OR logic (true if any policy has true)
198-
Assert.True(result.RequireLower); // true from policy1
199-
Assert.True(result.RequireUpper); // true from policy2
200-
Assert.True(result.RequireNumbers); // true from policy1
201-
Assert.True(result.RequireSpecial); // true from policy2
202-
Assert.True(result.EnforceOnLogin); // true from policy1
137+
Assert.NotNull(result);
138+
Assert.Equal(12, result.MinLength);
139+
Assert.True(result.RequireUpper);
203140
}
204141

205142
[Theory, BitAutoData]
206143
public async Task GetMasterPasswordPolicyForUserAsync_WithNoPolicies_ReturnsNull(User user, SutProvider<PolicyService> sutProvider)
207144
{
208-
// Arrange: No enabled policies
209-
sutProvider.GetDependency<IPolicyRepository>()
210-
.GetManyByUserIdAsync(user.Id)
211-
.Returns(new List<Policy>());
145+
var requirement = new MasterPasswordPolicyRequirement();
212146

213-
// Act
214-
var result = await sutProvider.Sut.GetMasterPasswordPolicyForUserAsync(user);
147+
sutProvider.GetDependency<IPolicyRequirementQuery>()
148+
.GetAsyncVNext<MasterPasswordPolicyRequirement>(user.Id)
149+
.Returns(requirement);
215150

216-
// Assert
217-
Assert.Null(result);
218-
}
219-
220-
[Theory, BitAutoData]
221-
public async Task GetMasterPasswordPolicyForUserAsync_WithDisabledPolicies_ReturnsNull(User user, SutProvider<PolicyService> sutProvider)
222-
{
223-
// Arrange: Policies exist but are disabled
224-
var disabledPolicy = new Policy
225-
{
226-
Id = Guid.NewGuid(),
227-
OrganizationId = Guid.NewGuid(),
228-
Type = PolicyType.MasterPassword,
229-
Enabled = false
230-
};
231-
disabledPolicy.SetDataModel(new MasterPasswordPolicyData
232-
{
233-
MinComplexity = 3,
234-
MinLength = 12
235-
});
236-
237-
sutProvider.GetDependency<IPolicyRepository>()
238-
.GetManyByUserIdAsync(user.Id)
239-
.Returns(new List<Policy> { disabledPolicy });
240-
241-
// Act
242151
var result = await sutProvider.Sut.GetMasterPasswordPolicyForUserAsync(user);
243152

244-
// Assert
245153
Assert.Null(result);
246154
}
247155

@@ -277,13 +185,6 @@ await sutProvider.GetDependency<IApplicationCacheService>()
277185
!ids.Contains(excludedOrgId)));
278186
}
279187

280-
private static void SetupOrg(SutProvider<PolicyService> sutProvider, Guid organizationId, Organization organization)
281-
{
282-
sutProvider.GetDependency<IOrganizationRepository>()
283-
.GetByIdAsync(organizationId)
284-
.Returns(Task.FromResult(organization));
285-
}
286-
287188
private static void SetupUserPolicies(Guid userId, SutProvider<PolicyService> sutProvider)
288189
{
289190
sutProvider.GetDependency<IOrganizationUserRepository>()

0 commit comments

Comments
 (0)