Skip to content
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
0a34040
Add interfaces and factory
JimmyVo16 Sep 14, 2025
f822b48
Remove noops handler
JimmyVo16 Sep 14, 2025
c8d4d01
Add V3SaveAsync
JimmyVo16 Sep 15, 2025
914461b
Merge branch 'main' into policy-pattern
JimmyVo16 Sep 15, 2025
0ccd35c
undo
JimmyVo16 Sep 15, 2025
6118b76
wip
JimmyVo16 Sep 15, 2025
71eda10
wip
JimmyVo16 Sep 15, 2025
d68695a
wip
JimmyVo16 Sep 15, 2025
53beacc
wip
JimmyVo16 Sep 15, 2025
f6b2070
wip
JimmyVo16 Sep 16, 2025
d4d8897
Merge branch 'main' into PM-24069-policy-pattern
JimmyVo16 Sep 24, 2025
c4efbed
[ERROR: BRANCH NAME DOES NOT MATCH THE EXPECTED FORMAT.] Move interfa…
JimmyVo16 Sep 24, 2025
81508c9
[ERROR: BRANCH NAME DOES NOT MATCH THE EXPECTED FORMAT.] Fix factory
JimmyVo16 Sep 24, 2025
f594f3e
[ERROR: BRANCH NAME DOES NOT MATCH THE EXPECTED FORMAT.] Fix factory
JimmyVo16 Sep 24, 2025
ccae2a4
[PM-25823] undo
JimmyVo16 Sep 24, 2025
ff64c2a
[PM-25823] Create vnext save policy command
JimmyVo16 Sep 24, 2025
f9d62d6
[PM-25823] wip
JimmyVo16 Sep 26, 2025
7a7af87
[PM-25823] fix merge conflicts
JimmyVo16 Oct 7, 2025
7ed773b
[PM-25823] undo
JimmyVo16 Oct 7, 2025
6fc635b
[PM-25823] undo
JimmyVo16 Oct 7, 2025
c428e0a
[PM-25823] undo
JimmyVo16 Oct 7, 2025
f3c29af
[PM-25823] undo
JimmyVo16 Oct 7, 2025
b55cb05
[PM-25823] wip
JimmyVo16 Oct 7, 2025
6a682c6
[PM-25823] wip
JimmyVo16 Oct 7, 2025
d8d0dc7
[PM-25823] wip
JimmyVo16 Oct 8, 2025
f1323a5
[PM-25823] wip
JimmyVo16 Oct 8, 2025
26c23d5
[PM-25823] wip
JimmyVo16 Oct 8, 2025
a183266
[PM-25823] wip
JimmyVo16 Oct 8, 2025
f5690cb
Merge branch 'main' into ac/pm-25823/vnext-policy-upsert-pattern
JimmyVo16 Oct 8, 2025
50d22fc
[PM-25823] undo
JimmyVo16 Oct 8, 2025
26db238
[PM-25823] fix build issues
JimmyVo16 Oct 8, 2025
86cdc11
[PM-25823] Add tests for factory
JimmyVo16 Oct 8, 2025
e4145b6
[PM-25823] fix build issue
JimmyVo16 Oct 8, 2025
1a8b8b8
Merge branch 'main' into ac/pm-25823/vnext-policy-upsert-pattern
JimmyVo16 Oct 8, 2025
5ecd6f2
[PM-25823] code review
JimmyVo16 Oct 9, 2025
2b1b80c
Merge branch 'main' into ac/pm-25823/vnext-policy-upsert-pattern
JimmyVo16 Oct 9, 2025
c7b3b42
Update src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyUpda…
JimmyVo16 Oct 10, 2025
201762f
Update src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyUpda…
JimmyVo16 Oct 10, 2025
3ca28b0
Update src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyUpda…
JimmyVo16 Oct 10, 2025
4a6a401
Merge branch 'main' into ac/pm-25823/vnext-policy-upsert-pattern
JimmyVo16 Oct 10, 2025
d098767
[PM-25823] fix lint issues
JimmyVo16 Oct 10, 2025
feba8ec
Update src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyUpda…
JimmyVo16 Oct 10, 2025
be80a93
[PM-25823] fix lint issues
JimmyVo16 Oct 10, 2025
7b027c0
[PM-25823] fix build issues
JimmyVo16 Oct 10, 2025
4408040
[PM-25823] fix build issues
JimmyVo16 Oct 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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>();
Copy link
Contributor Author

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.

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)
{
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);

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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we always expect the currentPolicy to not be null? What happens when we're setting up a policy the first time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that should have used the type from the request object.

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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currentPolicy is not used

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
Expand Up @@ -16,6 +16,8 @@ public record PolicyUpdate
public PolicyType Type { get; set; }
public string? Data { get; set; }
public bool Enabled { get; set; }

[Obsolete("Please use SavePolicyModel.PerformedBy instead.")]
public IActingUser? PerformedBy { get; set; }

public T GetDataModel<T>() where T : IPolicyDataModel, new()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Implementations;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyValidators;
using Bit.Core.AdminConsole.Services;
using Bit.Core.AdminConsole.Services.Implementations;
Expand All @@ -13,7 +15,9 @@ public static void AddPolicyServices(this IServiceCollection services)
{
services.AddScoped<IPolicyService, PolicyService>();
services.AddScoped<ISavePolicyCommand, SavePolicyCommand>();
services.AddScoped<IVNextSavePolicyCommand, VNextSavePolicyCommand>();
services.AddScoped<IPolicyRequirementQuery, PolicyRequirementQuery>();
services.AddScoped<IPolicyEventHandlerFactory, PolicyEventHandlerHandlerFactory>();

services.AddPolicyValidators();
services.AddPolicyRequirements();
Expand Down
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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

add xmldoc to every new interface clarifying what it represents and when its called

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;

public interface IPolicyEventHandlerFactory
{
OneOf<T, None> GetHandler<T>(PolicyType policyType) where T : IPolicyUpdateEvent;
}
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;

public interface IVNextSavePolicyCommand
{
Task<Policy> SaveAsync(SavePolicyModel policyRequest);
}
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;
}
}
Loading
Loading