-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PM-14439] Add PolicyRequirements for enforcement logic #5336
base: main
Are you sure you want to change the base?
Changes from all commits
8701e20
d0bcb2b
010d6b8
9fa1096
b3977d4
a6d4680
983eb70
6f5df04
205dde2
e335e30
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
๏ปฟusing Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies; | ||
|
||
public interface IPolicyRequirementQuery | ||
{ | ||
Task<T> GetAsync<T>(Guid userId) where T : IPolicyRequirement; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
๏ปฟusing Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; | ||
using Bit.Core.AdminConsole.Repositories; | ||
using Bit.Core.Models.Data.Organizations.OrganizationUsers; | ||
using Bit.Core.Settings; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.Implementations; | ||
|
||
public class PolicyRequirementQuery : IPolicyRequirementQuery | ||
{ | ||
private readonly IPolicyRepository _policyRepository; | ||
private readonly PolicyRequirementRegistry _policyRequirements = new(); | ||
|
||
public PolicyRequirementQuery(IGlobalSettings globalSettings, IPolicyRepository policyRepository) | ||
{ | ||
_policyRepository = policyRepository; | ||
|
||
// Register Policy Requirement factory functions below | ||
_policyRequirements.Add(SendPolicyRequirement.Create); | ||
_policyRequirements.Add(up | ||
=> SsoPolicyRequirement.Create(up, globalSettings.Sso)); | ||
Comment on lines
+18
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Example of how you can just register the factory function directly for simple requirements (which will be most of them), or inject additional values if needed (settings, feature flags, etc). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just an idea but maybe we could use Reflection here to find all classes that inherit |
||
} | ||
|
||
public async Task<T> GetAsync<T>(Guid userId) where T : IPolicyRequirement | ||
=> _policyRequirements.Get<T>()(await GetPolicyDetails(userId)); | ||
|
||
private Task<IEnumerable<OrganizationUserPolicyDetails>> GetPolicyDetails(Guid userId) => | ||
_policyRepository.GetPolicyDetailsByUserId(userId); | ||
|
||
/// <summary> | ||
/// Helper class used to register and retrieve Policy Requirement factories by type. | ||
/// </summary> | ||
private class PolicyRequirementRegistry | ||
{ | ||
private readonly Dictionary<Type, CreateRequirement<IPolicyRequirement>> _registry = new(); | ||
|
||
public void Add<T>(CreateRequirement<T> factory) where T : IPolicyRequirement | ||
{ | ||
// Explicitly convert T to an IPolicyRequirement (C# doesn't do this automatically). | ||
IPolicyRequirement Converted(IEnumerable<OrganizationUserPolicyDetails> up) => factory(up); | ||
_registry.Add(typeof(T), Converted); | ||
} | ||
|
||
public CreateRequirement<T> Get<T>() where T : IPolicyRequirement | ||
{ | ||
if (!_registry.TryGetValue(typeof(T), out var factory)) | ||
{ | ||
throw new NotImplementedException("No Policy Requirement found for " + typeof(T)); | ||
} | ||
|
||
// Explicitly convert IPolicyRequirement back to T (C# doesn't do this automatically). | ||
// The cast here relies on the Register method correctly associating the type and factory function. | ||
T Converted(IEnumerable<OrganizationUserPolicyDetails> up) => (T)factory(up); | ||
return Converted; | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
๏ปฟusing Bit.Core.Models.Data.Organizations.OrganizationUsers; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; | ||
|
||
public interface IPolicyRequirement; | ||
|
||
public delegate T CreateRequirement<T>(IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails) | ||
where T : IPolicyRequirement; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
๏ปฟusing Bit.Core.AdminConsole.Enums; | ||
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; | ||
using Bit.Core.Models.Data.Organizations.OrganizationUsers; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; | ||
|
||
public class MasterPasswordPolicyRequirement : MasterPasswordPolicyData, IPolicyRequirement | ||
{ | ||
public static MasterPasswordPolicyRequirement Create(IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails) => | ||
userPolicyDetails | ||
.GetPolicyType(PolicyType.MasterPassword) | ||
.ExcludeProviders() | ||
.ExcludeRevokedAndInvitedUsers() | ||
.Select(up => up.GetDataModel<MasterPasswordPolicyData>()) | ||
.Aggregate( | ||
new MasterPasswordPolicyRequirement(), | ||
(result, current) => | ||
{ | ||
result.CombineWith(current); | ||
return result; | ||
} | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
๏ปฟusing Bit.Core.AdminConsole.Enums; | ||
using Bit.Core.Models.Data.Organizations.OrganizationUsers; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; | ||
|
||
public class PersonalOwnershipPolicyRequirement : IPolicyRequirement | ||
{ | ||
public bool DisablePersonalOwnership { get; init; } | ||
|
||
public static PersonalOwnershipPolicyRequirement Create(IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails) | ||
=> new() | ||
{ | ||
DisablePersonalOwnership = userPolicyDetails | ||
.GetPolicyType(PolicyType.PersonalOwnership) | ||
.ExcludeOwnersAndAdmins() | ||
.ExcludeProviders() | ||
.ExcludeRevokedAndInvitedUsers() | ||
.Any() | ||
}; | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is great! |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,29 @@ | ||||||
๏ปฟusing Bit.Core.AdminConsole.Enums; | ||||||
using Bit.Core.Enums; | ||||||
using Bit.Core.Models.Data.Organizations.OrganizationUsers; | ||||||
|
||||||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; | ||||||
|
||||||
public static class PolicyRequirementHelpers | ||||||
{ | ||||||
public static IEnumerable<OrganizationUserPolicyDetails> GetPolicyType( | ||||||
this IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails, | ||||||
PolicyType type) => | ||||||
userPolicyDetails.Where(x => x.PolicyType == type); | ||||||
|
||||||
public static IEnumerable<OrganizationUserPolicyDetails> ExcludeOwnersAndAdmins( | ||||||
this IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails) => | ||||||
userPolicyDetails.Where(x => x.OrganizationUserType != OrganizationUserType.Owner); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
public static IEnumerable<OrganizationUserPolicyDetails> ExcludeProviders( | ||||||
this IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails) => | ||||||
userPolicyDetails.Where(x => !x.IsProvider); | ||||||
|
||||||
public static IEnumerable<OrganizationUserPolicyDetails> ExcludeRevokedAndInvitedUsers( | ||||||
this IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails) => | ||||||
userPolicyDetails.Where(x => x.OrganizationUserStatus >= OrganizationUserStatusType.Accepted); | ||||||
|
||||||
public static IEnumerable<OrganizationUserPolicyDetails> ExcludeRevokedUsers( | ||||||
this IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails) => | ||||||
userPolicyDetails.Where(x => x.OrganizationUserStatus >= OrganizationUserStatusType.Invited); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a huge fan of using > and < on enum values. Maybe its better to add an explicit list of allowed types for each and just do a contains? ๐คท non-blocking. just a thought. |
||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,31 @@ | ||||||
๏ปฟusing Bit.Core.AdminConsole.Enums; | ||||||
using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; | ||||||
using Bit.Core.Models.Data.Organizations.OrganizationUsers; | ||||||
|
||||||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; | ||||||
|
||||||
public class SendPolicyRequirement : IPolicyRequirement | ||||||
{ | ||||||
public bool DisableSend { get; init; } | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
public bool DisableHideEmail { get; init; } | ||||||
|
||||||
public static SendPolicyRequirement Create(IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails) | ||||||
{ | ||||||
var filteredPolicies = userPolicyDetails | ||||||
.ExcludeOwnersAndAdmins() | ||||||
.ExcludeRevokedAndInvitedUsers() | ||||||
.ToList(); | ||||||
|
||||||
return new SendPolicyRequirement | ||||||
{ | ||||||
DisableSend = filteredPolicies | ||||||
.GetPolicyType(PolicyType.DisableSend) | ||||||
.Any(), | ||||||
|
||||||
DisableHideEmail = filteredPolicies | ||||||
.GetPolicyType(PolicyType.SendOptions) | ||||||
.Select(up => up.GetDataModel<SendOptionsPolicyData>()) | ||||||
.Any(d => d.DisableHideEmail) | ||||||
}; | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
๏ปฟusing Bit.Core.AdminConsole.Enums; | ||
using Bit.Core.Models.Data.Organizations.OrganizationUsers; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; | ||
|
||
public enum SingleOrganizationRequirementResult | ||
{ | ||
Ok = 1, | ||
RequiredByThisOrganization = 2, | ||
RequiredByOtherOrganization = 3 | ||
} | ||
|
||
public class SingleOrganizationPolicyRequirement : IPolicyRequirement | ||
{ | ||
private IEnumerable<OrganizationUserPolicyDetails> PolicyDetails { get; } | ||
|
||
public SingleOrganizationPolicyRequirement(IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need a Create method here? |
||
{ | ||
PolicyDetails = userPolicyDetails | ||
.GetPolicyType(PolicyType.SingleOrg) | ||
.ExcludeOwnersAndAdmins() | ||
.ExcludeProviders() | ||
.ToList(); | ||
} | ||
|
||
public SingleOrganizationRequirementResult CanJoinOrganization(Guid organizationId) | ||
{ | ||
// Check for the org the user is trying to join | ||
if (PolicyDetails.Any(x => x.OrganizationId == organizationId)) | ||
{ | ||
return SingleOrganizationRequirementResult.RequiredByThisOrganization; | ||
} | ||
|
||
// Check for other orgs the user might already be a member of (accepted or confirmed status only) | ||
if (PolicyDetails.ExcludeRevokedAndInvitedUsers().Any()) | ||
{ | ||
return SingleOrganizationRequirementResult.RequiredByOtherOrganization; | ||
} | ||
|
||
return SingleOrganizationRequirementResult.Ok; | ||
} | ||
|
||
public SingleOrganizationRequirementResult CanBeRestoredToOrganization(Guid organizationId) => | ||
CanJoinOrganization(organizationId); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
๏ปฟusing Bit.Core.AdminConsole.Enums; | ||
using Bit.Core.Enums; | ||
using Bit.Core.Models.Data.Organizations.OrganizationUsers; | ||
using Bit.Core.Settings; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; | ||
|
||
public class SsoPolicyRequirement : IPolicyRequirement | ||
{ | ||
public bool RequireSso { get; init; } | ||
|
||
public static SsoPolicyRequirement Create( | ||
IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails, | ||
ISsoSettings ssoSettings) | ||
=> new() | ||
{ | ||
RequireSso = userPolicyDetails | ||
.GetPolicyType(PolicyType.RequireSso) | ||
.ExcludeProviders() | ||
// TODO: confirm minStatus - maybe confirmed? | ||
.ExcludeRevokedAndInvitedUsers() | ||
.Any(up => | ||
up.OrganizationUserType is not OrganizationUserType.Owner and not OrganizationUserType.Admin || | ||
ssoSettings.EnforceSsoPolicyForAllUsers) | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
๏ปฟusing Bit.Core.AdminConsole.Enums; | ||
using Bit.Core.Models.Data.Organizations.OrganizationUsers; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; | ||
|
||
public class TwoFactorAuthenticationPolicyRequirement : IPolicyRequirement | ||
{ | ||
private IEnumerable<OrganizationUserPolicyDetails> PolicyDetails { get; } | ||
|
||
public TwoFactorAuthenticationPolicyRequirement(IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need a create method here? |
||
{ | ||
PolicyDetails = userPolicyDetails | ||
.GetPolicyType(PolicyType.TwoFactorAuthentication) | ||
.ExcludeOwnersAndAdmins() | ||
.ExcludeProviders() | ||
.ToList(); | ||
} | ||
|
||
public bool RequiredToJoinOrganization(Guid organizationId) | ||
=> PolicyDetails.Any(x => x.OrganizationId == organizationId); | ||
|
||
public IEnumerable<Guid> OrganizationsRequiringTwoFactor | ||
=> PolicyDetails | ||
.ExcludeRevokedAndInvitedUsers() | ||
.Select(x => x.OrganizationId); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an intentional split of responsibilities here: all business logic is in the policy requirements, which are written in a functional style. The query is agnostic about what policies are being handled - its only job is to connect policy requirements to dependencies.