-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Ac/pm 25823/vnext policy upsert pattern #6426
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
Changes from 34 commits
0a34040
f822b48
c8d4d01
914461b
0ccd35c
6118b76
71eda10
d68695a
53beacc
f6b2070
d4d8897
c4efbed
81508c9
f594f3e
ccae2a4
ff64c2a
f9d62d6
7a7af87
7ed773b
6fc635b
c428e0a
f3c29af
b55cb05
6a682c6
d8d0dc7
f1323a5
26c23d5
a183266
f5690cb
50d22fc
26db238
86cdc11
e4145b6
1a8b8b8
5ecd6f2
2b1b80c
c7b3b42
201762f
3ca28b0
4a6a401
d098767
feba8ec
be80a93
7b027c0
4408040
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,209 @@ | ||
| using Bit.Core.AdminConsole.Entities; | ||
| using Bit.Core.AdminConsole.Enums; | ||
| using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; | ||
| using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces; | ||
| using Bit.Core.AdminConsole.Repositories; | ||
| using Bit.Core.Enums; | ||
| using Bit.Core.Exceptions; | ||
| using Bit.Core.Services; | ||
|
|
||
| namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.Implementations; | ||
|
|
||
| public class VNextSavePolicyCommand( | ||
| IApplicationCacheService applicationCacheService, | ||
| IEventService eventService, | ||
| IPolicyRepository policyRepository, | ||
| IEnumerable<IEnforceDependentPoliciesEvent> policyValidationEventHandlers, | ||
| TimeProvider timeProvider, | ||
| IPolicyEventHandlerFactory policyEventHandlerFactory) | ||
| : IVNextSavePolicyCommand | ||
| { | ||
| private readonly IReadOnlyDictionary<PolicyType, IEnforceDependentPoliciesEvent> _policyValidationEvents = MapToDictionary(policyValidationEventHandlers); | ||
|
|
||
| private static Dictionary<PolicyType, IEnforceDependentPoliciesEvent> MapToDictionary(IEnumerable<IEnforceDependentPoliciesEvent> policyValidationEventHandlers) | ||
| { | ||
| var policyValidationEventsDict = new Dictionary<PolicyType, IEnforceDependentPoliciesEvent>(); | ||
| foreach (var policyValidationEvent in policyValidationEventHandlers) | ||
| { | ||
| if (!policyValidationEventsDict.TryAdd(policyValidationEvent.Type, policyValidationEvent)) | ||
| { | ||
| throw new Exception($"Duplicate PolicyValidationEvent for {policyValidationEvent.Type} policy."); | ||
| } | ||
| } | ||
| return policyValidationEventsDict; | ||
| } | ||
|
|
||
| public async Task<Policy> SaveAsync(SavePolicyModel policyRequest) | ||
JimmyVo16 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| var policyUpdateRequest = policyRequest.PolicyUpdate; | ||
| var organizationId = policyUpdateRequest.OrganizationId; | ||
|
|
||
| await EnsureOrganizationCanUsePolicyAsync(organizationId); | ||
|
|
||
| var savedPoliciesDict = await GetCurrentPolicyStateAsync(organizationId); | ||
|
|
||
| var currentPolicy = savedPoliciesDict.GetValueOrDefault(policyUpdateRequest.Type); | ||
|
|
||
| ValidatePolicyDependencies(policyUpdateRequest, currentPolicy, savedPoliciesDict); | ||
|
|
||
| await ValidateTargetedPolicyAsync(policyRequest, currentPolicy); | ||
|
|
||
| await ExecutePreUpsertSideEffectAsync(policyRequest, currentPolicy); | ||
|
|
||
| var upsertedPolicy = await UpsertPolicyAsync(policyUpdateRequest); | ||
|
|
||
| await eventService.LogPolicyEventAsync(upsertedPolicy, EventType.Policy_Updated); | ||
r-tome marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| await ExecutePostUpsertSideEffectAsync(policyRequest, upsertedPolicy, currentPolicy); | ||
|
|
||
| return upsertedPolicy; | ||
| } | ||
|
|
||
| private async Task EnsureOrganizationCanUsePolicyAsync(Guid organizationId) | ||
| { | ||
| var org = await applicationCacheService.GetOrganizationAbilityAsync(organizationId); | ||
| if (org == null) | ||
| { | ||
| throw new BadRequestException("Organization not found"); | ||
| } | ||
|
|
||
| if (!org.UsePolicies) | ||
| { | ||
| throw new BadRequestException("This organization cannot use policies."); | ||
| } | ||
| } | ||
|
|
||
| private async Task<Policy> UpsertPolicyAsync(PolicyUpdate policyUpdateRequest) | ||
| { | ||
| var policy = await policyRepository.GetByOrganizationIdTypeAsync(policyUpdateRequest.OrganizationId, policyUpdateRequest.Type) | ||
| ?? new Policy | ||
| { | ||
| OrganizationId = policyUpdateRequest.OrganizationId, | ||
| Type = policyUpdateRequest.Type, | ||
| CreationDate = timeProvider.GetUtcNow().UtcDateTime | ||
| }; | ||
|
|
||
| policy.Enabled = policyUpdateRequest.Enabled; | ||
| policy.Data = policyUpdateRequest.Data; | ||
| policy.RevisionDate = timeProvider.GetUtcNow().UtcDateTime; | ||
|
|
||
| await policyRepository.UpsertAsync(policy); | ||
|
|
||
| return policy; | ||
| } | ||
|
|
||
| private async Task ValidateTargetedPolicyAsync(SavePolicyModel policyRequest, | ||
| Policy? currentPolicy) | ||
| { | ||
| await ExecutePolicyEventAsync<IPolicyValidationEvent>( | ||
| currentPolicy!.Type, | ||
|
||
| async validator => | ||
| { | ||
| var validationError = await validator.ValidateAsync(policyRequest, currentPolicy); | ||
| if (!string.IsNullOrEmpty(validationError)) | ||
| { | ||
| throw new BadRequestException(validationError); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| private void ValidatePolicyDependencies( | ||
| PolicyUpdate policyUpdateRequest, | ||
| Policy? currentPolicy, | ||
| Dictionary<PolicyType, Policy> savedPoliciesDict) | ||
| { | ||
| var result = policyEventHandlerFactory.GetHandler<IEnforceDependentPoliciesEvent>(currentPolicy!.Type); | ||
|
|
||
| result.Switch( | ||
| validator => | ||
| { | ||
| var isCurrentlyEnabled = currentPolicy?.Enabled == true; | ||
|
|
||
| switch (policyUpdateRequest.Enabled) | ||
| { | ||
| case true when !isCurrentlyEnabled: | ||
| ValidateEnablingRequirements(validator, currentPolicy, savedPoliciesDict); | ||
| return; | ||
| case false when isCurrentlyEnabled: | ||
| ValidateDisablingRequirements(validator, policyUpdateRequest.Type, savedPoliciesDict); | ||
| break; | ||
| } | ||
| }, | ||
| _ => { }); | ||
| } | ||
|
|
||
| private void ValidateDisablingRequirements( | ||
| IEnforceDependentPoliciesEvent validator, | ||
| PolicyType policyType, | ||
| Dictionary<PolicyType, Policy> savedPoliciesDict) | ||
| { | ||
| var dependentPolicyTypes = _policyValidationEvents.Values | ||
| .Where(otherValidator => otherValidator.RequiredPolicies.Contains(policyType)) | ||
| .Select(otherValidator => otherValidator.Type) | ||
| .Where(otherPolicyType => savedPoliciesDict.TryGetValue(otherPolicyType, out var savedPolicy) && | ||
| savedPolicy.Enabled) | ||
| .ToList(); | ||
|
|
||
| switch (dependentPolicyTypes) | ||
| { | ||
| case { Count: 1 }: | ||
| throw new BadRequestException($"Turn off the {dependentPolicyTypes.First().GetName()} policy because it requires the {validator.Type.GetName()} policy."); | ||
| case { Count: > 1 }: | ||
| throw new BadRequestException($"Turn off all of the policies that require the {validator.Type.GetName()} policy."); | ||
| } | ||
| } | ||
|
|
||
| private static void ValidateEnablingRequirements( | ||
| IEnforceDependentPoliciesEvent validator, | ||
| Policy? currentPolicy, | ||
|
||
| Dictionary<PolicyType, Policy> savedPoliciesDict) | ||
| { | ||
| var missingRequiredPolicyTypes = validator.RequiredPolicies | ||
| .Where(requiredPolicyType => savedPoliciesDict.GetValueOrDefault(requiredPolicyType) is not { Enabled: true }) | ||
| .ToList(); | ||
|
|
||
| if (missingRequiredPolicyTypes.Count != 0) | ||
| { | ||
| throw new BadRequestException($"Turn on the {missingRequiredPolicyTypes.First().GetName()} policy because it is required for the {validator.Type.GetName()} policy."); | ||
| } | ||
| } | ||
|
|
||
| private async Task ExecutePreUpsertSideEffectAsync( | ||
| SavePolicyModel policyRequest, | ||
| Policy? currentPolicy) | ||
| { | ||
| await ExecutePolicyEventAsync<IOnPolicyPreUpdateEvent>( | ||
| policyRequest.PolicyUpdate.Type, | ||
| handler => handler.ExecutePreUpsertSideEffectAsync(policyRequest, currentPolicy)); | ||
| } | ||
| private async Task ExecutePostUpsertSideEffectAsync( | ||
| SavePolicyModel policyRequest, | ||
| Policy postUpsertedPolicyState, | ||
| Policy? previousPolicyState) | ||
| { | ||
| await ExecutePolicyEventAsync<IOnPolicyPostUpdateEvent>( | ||
| policyRequest.PolicyUpdate.Type, | ||
| handler => handler.ExecutePostUpsertSideEffectAsync( | ||
| policyRequest, | ||
| postUpsertedPolicyState, | ||
| previousPolicyState)); | ||
| } | ||
|
|
||
| private async Task ExecutePolicyEventAsync<T>(PolicyType type, Func<T, Task> func) where T : IPolicyUpdateEvent | ||
| { | ||
| var handler = policyEventHandlerFactory.GetHandler<T>(type); | ||
|
|
||
| await handler.Match( | ||
| async h => await func(h), | ||
| _ => Task.CompletedTask | ||
| ); | ||
| } | ||
|
|
||
| private async Task<Dictionary<PolicyType, Policy>> GetCurrentPolicyStateAsync(Guid organizationId) | ||
| { | ||
| var savedPolicies = await policyRepository.GetManyByOrganizationIdAsync(organizationId); | ||
| // Note: policies may be missing from this dict if they have never been enabled | ||
| var savedPoliciesDict = savedPolicies.ToDictionary(p => p.Type); | ||
| return savedPoliciesDict; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| using Bit.Core.AdminConsole.Enums; | ||
|
|
||
| namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces; | ||
|
|
||
| public interface IEnforceDependentPoliciesEvent : IPolicyUpdateEvent | ||
| { | ||
| /// <summary> | ||
| /// PolicyTypes that must be enabled before this policy can be enabled, if any. | ||
| /// These dependencies will be checked when this policy is enabled and when any required policy is disabled. | ||
| /// </summary> | ||
| public IEnumerable<PolicyType> RequiredPolicies { get; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| using Bit.Core.AdminConsole.Entities; | ||
| using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; | ||
|
|
||
| namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces; | ||
| public interface IOnPolicyPostUpdateEvent : IPolicyUpdateEvent | ||
| { | ||
| /// <summary> | ||
| /// Performs side effects after a policy has been upserted. | ||
| /// For example, this can be used for cleanup tasks or notifications. | ||
| /// </summary> | ||
| /// <param name="policyRequest">The policy save request</param> | ||
| /// <param name="postUpsertedPolicyState">The policy after it was upserted</param> | ||
| /// <param name="previousPolicyState">The policy state before it was updated, if any</param> | ||
| public Task ExecutePostUpsertSideEffectAsync( | ||
| SavePolicyModel policyRequest, | ||
| Policy postUpsertedPolicyState, | ||
| Policy? previousPolicyState); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| using Bit.Core.AdminConsole.Entities; | ||
| using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; | ||
|
|
||
| namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces; | ||
|
|
||
| public interface IOnPolicyPreUpdateEvent : IPolicyUpdateEvent | ||
| { | ||
| /// <summary> | ||
| /// Performs side effects before a policy is upserted. | ||
|
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. Hey, I’m addressing your comments here so we can have a thread on it. Original comment
I added documentation to the method in the interface, would that be sufficient? I’m happy to adjust it if needed, but I’d prefer to avoid duplicating the same text in two places. |
||
| /// For example, this can be used to remove non-compliant users from the organization. | ||
| /// </summary> | ||
| /// <param name="policyRequest">The policy save request containing the policy update and metadata</param> | ||
| /// <param name="currentPolicy">The current policy, if any</param> | ||
| public Task ExecutePreUpsertSideEffectAsync( | ||
| SavePolicyModel policyRequest, | ||
| Policy? currentPolicy); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| #nullable enable | ||
|
|
||
| using Bit.Core.AdminConsole.Enums; | ||
| using OneOf; | ||
| using OneOf.Types; | ||
|
|
||
| namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces; | ||
|
|
||
JimmyVo16 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| public interface IPolicyEventHandlerFactory | ||
| { | ||
| OneOf<T, None> GetHandler<T>(PolicyType policyType) where T : IPolicyUpdateEvent; | ||
JimmyVo16 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| using Bit.Core.AdminConsole.Enums; | ||
|
|
||
| namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces; | ||
|
|
||
| public interface IPolicyUpdateEvent | ||
| { | ||
| /// <summary> | ||
| /// The policy type that the associated handler will handle. | ||
| /// </summary> | ||
| public PolicyType Type { get; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| using Bit.Core.AdminConsole.Entities; | ||
| using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; | ||
|
|
||
| namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces; | ||
|
|
||
| public interface IPolicyValidationEvent : IPolicyUpdateEvent | ||
| { | ||
| /// <summary> | ||
| /// Performs side effects after a policy is validated but before it is saved. | ||
| /// For example, this can be used to remove non-compliant users from the organization. | ||
| /// Implementation is optional; by default, it will not perform any side effects. | ||
| /// </summary> | ||
| /// <param name="policyRequest">The policy save request containing the policy update and metadata</param> | ||
| /// <param name="currentPolicy">The current policy, if any</param> | ||
| public Task<string> ValidateAsync( | ||
| SavePolicyModel policyRequest, | ||
| Policy? currentPolicy); | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| #nullable enable | ||
| using Bit.Core.AdminConsole.Entities; | ||
| using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; | ||
|
|
||
| namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces; | ||
|
|
||
JimmyVo16 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| public interface IVNextSavePolicyCommand | ||
| { | ||
| Task<Policy> SaveAsync(SavePolicyModel policyRequest); | ||
JimmyVo16 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| | ||
| using Bit.Core.AdminConsole.Enums; | ||
| using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces; | ||
| using OneOf; | ||
| using OneOf.Types; | ||
|
|
||
| namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents; | ||
|
|
||
| public class PolicyEventHandlerHandlerFactory( | ||
| IEnumerable<IPolicyUpdateEvent> allEventHandlers) : IPolicyEventHandlerFactory | ||
| { | ||
| public OneOf<T, None> GetHandler<T>(PolicyType policyType) where T : IPolicyUpdateEvent | ||
| { | ||
| var tEventHandlers = allEventHandlers.OfType<T>().ToList(); | ||
|
|
||
| var matchingHandlers = tEventHandlers.Where(h => h.Type == policyType).ToList(); | ||
|
|
||
| if (matchingHandlers.Count > 1) | ||
| { | ||
| throw new InvalidOperationException( | ||
| $"Multiple {nameof(IPolicyUpdateEvent)} handlers of type {typeof(T).Name} found for {nameof(PolicyType)} {policyType}. " + | ||
| $"Expected one {typeof(T).Name} handler per {nameof(PolicyType)}."); | ||
| } | ||
|
|
||
| var policyTEventHandler = matchingHandlers.SingleOrDefault(); | ||
| if (policyTEventHandler is null) | ||
| { | ||
| return new None(); | ||
| } | ||
|
|
||
| return policyTEventHandler; | ||
| } | ||
| } |
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.
I’m not sure if this is needed since it’s from the old pattern. I think it was meant to prevent duplicates, but that was when everything was in one interface. Now we have four interfaces, so I’m not sure it makes sense anymore. I’m happy to remove it.