-
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
9fc08b4
3f7de0f
2049cc8
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,55 @@ | ||
๏ปฟ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(IPolicyRepository policyRepository) | ||
{ | ||
_policyRepository = policyRepository; | ||
|
||
// Register Policy Requirement factory functions below | ||
_policyRequirements.Add(SingleOrganizationPolicyRequirement.Create); | ||
} | ||
|
||
public async Task<T> GetAsync<T>(Guid userId) where T : IPolicyRequirement | ||
=> _policyRequirements.Get<T>()(await GetPolicyDetails(userId)); | ||
Check warning on line 22 in src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/PolicyRequirementQuery.cs
|
||
|
||
private Task<IEnumerable<OrganizationUserPolicyDetails>> GetPolicyDetails(Guid userId) => | ||
_policyRepository.GetPolicyDetailsByUserId(userId); | ||
Check warning on line 25 in src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/PolicyRequirementQuery.cs
|
||
|
||
/// <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); | ||
Check warning on line 37 in src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/PolicyRequirementQuery.cs
|
||
_registry.Add(typeof(T), Converted); | ||
} | ||
|
||
public CreateRequirement<T> Get<T>() where T : IPolicyRequirement | ||
{ | ||
Check warning on line 42 in src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/PolicyRequirementQuery.cs
|
||
if (!_registry.TryGetValue(typeof(T), out var factory)) | ||
{ | ||
Check warning on line 44 in src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/PolicyRequirementQuery.cs
|
||
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; | ||
} | ||
Check warning on line 52 in src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/PolicyRequirementQuery.cs
|
||
} | ||
} | ||
|
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; |
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); | ||||||
Check warning on line 28 in src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/PolicyRequirementHelpers.cs
|
||||||
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,51 @@ | ||
๏ปฟusing Bit.Core.AdminConsole.Enums; | ||
using Bit.Core.Enums; | ||
using Bit.Core.Models.Data.Organizations.OrganizationUsers; | ||
|
||
namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; | ||
|
||
public enum SingleOrganizationRequirementResult | ||
{ | ||
Ok = 1, | ||
BlockedByThisOrganization = 2, | ||
BlockedByOtherOrganization = 3 | ||
} | ||
|
||
public class SingleOrganizationPolicyRequirement : IPolicyRequirement | ||
{ | ||
/// <summary> | ||
/// Single organization policy details filtered by user role but not status. | ||
/// This lets us check whether a user is compliant before being accepted/restored. | ||
/// </summary> | ||
private IEnumerable<OrganizationUserPolicyDetails> PolicyDetails { get; init; } | ||
|
||
public static SingleOrganizationPolicyRequirement Create(IEnumerable<OrganizationUserPolicyDetails> userPolicyDetails) | ||
=> new() | ||
{ | ||
PolicyDetails = userPolicyDetails | ||
.GetPolicyType(PolicyType.SingleOrg) | ||
.ExcludeOwnersAndAdmins() | ||
.ExcludeProviders() | ||
.ToList() | ||
}; | ||
|
||
public SingleOrganizationRequirementResult CanJoinOrganization(Guid organizationId) | ||
{ | ||
// Check for the org the user is trying to join; status doesn't matter | ||
if (PolicyDetails.Any(x => x.OrganizationId == organizationId)) | ||
{ | ||
return SingleOrganizationRequirementResult.BlockedByThisOrganization; | ||
} | ||
|
||
// Check for other orgs the user might already be a member of (accepted or confirmed status only) | ||
if (PolicyDetails.ExcludeRevokedAndInvitedUsers().Any()) | ||
{ | ||
return SingleOrganizationRequirementResult.BlockedByOtherOrganization; | ||
} | ||
|
||
return SingleOrganizationRequirementResult.Ok; | ||
} | ||
|
||
public SingleOrganizationRequirementResult CanBeRestoredToOrganization(Guid organizationId) => | ||
CanJoinOrganization(organizationId); | ||
Check warning on line 50 in src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyRequirements/SingleOrganizationPolicyRequirement.cs
|
||
} |
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.