Skip to content
Open
Changes from all commits
Commits
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,258 @@
# IPolicyUpdateEvent

This is the policy update pattern that we want our system’s end state to follow.
This directory contains the interfaces and infrastructure for the policy save workflow used by `IVNextSavePolicyCommand`.
Currently, we’re using `IVNextSavePolicyCommand` to transition from the old `IPolicyValidator` pattern.
Comment on lines +3 to +5
Copy link
Member

@eliykat eliykat Mar 7, 2026

Choose a reason for hiding this comment

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

You have a separate section at the end to discuss the old pattern. I suggest this intro paragraph can just be focused on the new pattern.


---

## Overview

When an organization policy is created or updated, the save workflow runs a series of ordered steps. Each step acts like a hook that a handler may listen to by implementing the particular policy event interface.

Note: If you don’t want to hook into these events, you don’t need to create a handler, and your policy will simply upsert to the database with log events.
Comment on lines +11 to +13
Copy link
Member

Choose a reason for hiding this comment

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

I think the terminology could be clearer here: hook, handler, event, etc.

I suggest:

  • the policy getting saved is the event
  • the class that does something is a handler

Therefore: IEnforceDependentPoliciesHandler, IPolicyValidationHandler, etc.

The current wording makes it sound like the class is the event. But I think the class is the thing that handles the event (which is also how you describe it here).


```
SaveAsync()
β”œβ”€ 1. Validate org can use policies
Copy link
Member

Choose a reason for hiding this comment

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

Avoid contractions:

Suggested change
β”œβ”€ 1. Validate org can use policies
β”œβ”€ 1. Validate organization can use policies

β”œβ”€ 2. Validate policy dependencies ← IEnforceDependentPoliciesEvent
β”œβ”€ 3. Run policy-specific validation ← IPolicyValidationEvent
β”œβ”€ 4. Execute pre-save side effects ← IOnPolicyPreUpdateEvent
β”œβ”€ 5. Upsert policy + log event
└─ 6. Execute post-save side effects ← IOnPolicyPostUpdateEvent
```

The `PolicyEventHandlerHandlerFactory` resolves the correct handler for a given `PolicyType` and interface at each step. A handler is matched by its `IPolicyUpdateEvent.Type` property. At most one handler of each interface type is permitted per `PolicyType`.

---

## Limitations

1. We don't have a way to keep this whole process idempotent, so if there is an exception at any point that is not being handled, the state will stay where the process failed.



Comment on lines +29 to +34
Copy link
Member

Choose a reason for hiding this comment

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

Section appears to be unfinished? Also, this is a good callout, but I think it's more about it not being atomic than idempotent. A failure does not roll back any changes already made.

---

## Interfaces

### `IPolicyUpdateEvent`

The base interface that all policy event handlers must implement.

```csharp
public interface IPolicyUpdateEvent
{
PolicyType Type { get; }
}
```

Every handler declares which `PolicyType` it handles via `Type`. All other event interfaces extend this one.
Comment on lines +39 to +50
Copy link
Member

Choose a reason for hiding this comment

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

IΒ suggest this can be omitted given that no other team will ever have to implement this directly.


---

### `IEnforceDependentPoliciesEvent`

Declares prerequisite policies that must be enabled before this policy can be enabled. Also prevents a required policy from being disabled while a dependent policy is active.

```csharp
public interface IEnforceDependentPoliciesEvent : IPolicyUpdateEvent
{
IEnumerable<PolicyType> RequiredPolicies { get; }
}
```

- **Enabling**: Each `PolicyType` in `RequiredPolicies` must already be enabled, otherwise a `BadRequestException` is thrown.
- **Disabling a required policy**: If any other policy has this policy listed as a requirement and is currently enabled, the disable action is blocked.
Comment on lines +65 to +66
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: these lines are not required, you've already explained it above.


---

### `IPolicyValidationEvent`

Runs custom validation logic before the policy is saved.

```csharp
public interface IPolicyValidationEvent : IPolicyUpdateEvent
{
Task<string> ValidateAsync(SavePolicyModel policyRequest, Policy? currentPolicy);
}
```

Return an empty string to pass validation. Return a non-empty error message to throw a `BadRequestException` and abort the save.

---

### `IOnPolicyPreUpdateEvent`

Executes side effects **before** the policy is upserted to the database.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Executes side effects **before** the policy is upserted to the database.
Executes side effects **before** the policy is upserted to the database. If an exception is thrown, the policy will not be saved.


```csharp
public interface IOnPolicyPreUpdateEvent : IPolicyUpdateEvent
{
Task ExecutePreUpsertSideEffectAsync(SavePolicyModel policyRequest, Policy? currentPolicy);
}
```

Typical uses: revoking non-compliant users, removing emergency access grants.
Copy link
Member

Choose a reason for hiding this comment

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

Removing EA is not a typical use case and is probably not a very clear example.

Suggested change
Typical uses: revoking non-compliant users, removing emergency access grants.
Typical uses: revoking non-compliant users.


---

### `IOnPolicyPostUpdateEvent`

Executes side effects **after** the policy has been upserted to the database.

```csharp
public interface IOnPolicyPostUpdateEvent : IPolicyUpdateEvent
{
Task ExecutePostUpsertSideEffectAsync(
SavePolicyModel policyRequest,
Policy postUpsertedPolicyState,
Policy? previousPolicyState);
}
```

Typical uses: creating collections, sending notifications that depend on the new policy state.

Note: This is more useful for enabling a policy than for disabling a policy, since when the policy is disabled, there is no easy way to find the users the policy should be enforced on.
Copy link
Member

Choose a reason for hiding this comment

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

I think this note is confusing/unnecessary. You wouldn't be enforcing a policy that has been disabled.


---

### `IPolicyEventHandlerFactory`

Resolves the correct handler for a given `PolicyType` and event interface type.

```csharp
OneOf<T, None> GetHandler<T>(PolicyType policyType) where T : IPolicyUpdateEvent;
```

Returns the matching handler, or `None` if the policy type does not implement the requested interface. Throws `InvalidOperationException` if more than one handler is registered for the same `PolicyType` and interface.
Comment on lines +120 to +128
Copy link
Member

Choose a reason for hiding this comment

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

Probably not required here as it's internal.


---

## Adding a New Policy Handler

1. Create a class in `PolicyValidators/` implementing `IPolicyUpdateEvent` and any combination of the event interfaces above.
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to implement IPolicyUpdateEvent because it's implemented by one of those other interfaces.

Suggested change
1. Create a class in `PolicyValidators/` implementing `IPolicyUpdateEvent` and any combination of the event interfaces above.
1. Create a class in `PolicyValidators/` implementing any combination of the event interfaces above.

2. Set `Type` to the appropriate `PolicyType`.
3. Register the class as `IPolicyUpdateEvent` (and the legacy interfaces if needed) in `PolicyServiceCollectionExtensions.AddPolicyUpdateEvents()`.
Copy link
Member

Choose a reason for hiding this comment

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

Are we still using the legacy interfaces - is there any need to register them?


Note: No changes to `VNextSavePolicyCommand` or `PolicyEventHandlerHandlerFactory` are required.

### Example

`AutomaticUserConfirmationPolicyEventHandler` is a good reference. It requires `SingleOrg`, validates org compliance before enabling, and removes emergency access grants as a pre-save side effect.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`AutomaticUserConfirmationPolicyEventHandler` is a good reference. It requires `SingleOrg`, validates org compliance before enabling, and removes emergency access grants as a pre-save side effect.
`AutomaticUserConfirmationPolicyEventHandler` is a good reference. It requires `SingleOrg`, validates organization compliance before enabling, and removes emergency access grants as a pre-save side effect.


**Step 1: Create the handler** (`PolicyValidators/AutomaticUserConfirmationPolicyEventHandler.cs`):

```csharp
public class AutomaticUserConfirmationPolicyEventHandler(
IAutomaticUserConfirmationOrganizationPolicyComplianceValidator validator,
IOrganizationUserRepository organizationUserRepository,
IDeleteEmergencyAccessCommand deleteEmergencyAccessCommand)
: IPolicyValidationEvent, IEnforceDependentPoliciesEvent, IOnPolicyPreUpdateEvent
{
public PolicyType Type => PolicyType.AutomaticUserConfirmation;

// IEnforceDependentPoliciesEvent: SingleOrg must be enabled before this policy can be enabled
public IEnumerable<PolicyType> RequiredPolicies => [PolicyType.SingleOrg];

// IPolicyValidationEvent: Validates org compliance
public async Task<string> ValidateAsync(SavePolicyModel savePolicyModel, Policy? currentPolicy)
{
var policyUpdate = savePolicyModel.PolicyUpdate
var isNotEnablingPolicy = policyUpdate is not { Enabled: true };
var policyAlreadyEnabled = currentPolicy is { Enabled: true };
if (isNotEnablingPolicy || policyAlreadyEnabled)
{
return string.Empty;
}

return (await validator.IsOrganizationCompliantAsync(
new AutomaticUserConfirmationOrganizationPolicyComplianceValidatorRequest(policyUpdate.OrganizationId)))
.Match(
error => error.Message,
_ => string.Empty);
}

// IOnPolicyPreUpdateEvent: Revokes non-compliant users, removes emergency access grants before enabling
public async Task ExecutePreUpsertSideEffectAsync(SavePolicyModel policyRequest, Policy? currentPolicy)
{
var isNotEnablingPolicy = policyRequest.PolicyUpdate is not { Enabled: true };
var policyAlreadyEnabled = currentPolicy is { Enabled: true };
if (isNotEnablingPolicy || policyAlreadyEnabled)
{
return;
}

var orgUsers = await organizationUserRepository.GetManyByOrganizationAsync(policyRequest.PolicyUpdate.OrganizationId, null);
var orgUserIds = orgUsers.Where(w => w.UserId != null).Select(s => s.UserId!.Value).ToList();

await deleteEmergencyAccessCommand.DeleteAllByUserIdsAsync(orgUserIds);
}

// IOnPolicyPostUpdateEvent: No implementation is needed since this handler doesn’t require it.
}
```

**Step 2: Register the handler** in `PolicyServiceCollectionExtensions.AddPolicyUpdateEvents()`:

```csharp
services.AddScoped<IPolicyUpdateEvent, AutomaticUserConfirmationPolicyEventHandler>();
```

---

## Adding a New Event Interface

Use this when the existing interfaces don't cover your use case and you need a new hook in the save workflow.

### Step 1: Define the interface in `PolicyUpdateEvents/Interfaces/`:

```csharp
public interface IMyNewEvent : IPolicyUpdateEvent
{
Task ExecuteMyNewEventAsync(SavePolicyModel policyRequest, Policy? currentPolicy);
}
```

It must extend `IPolicyUpdateEvent`.

### Step 2: Add a step to `SavePolicyCommand.SaveAsync()` or `VNextSavePolicyCommand.SaveAsync()` during transition

1. Call your method at the appropriate position in the workflow
2. You can use the existing `ExecutePolicyEventAsync<T>` helper or have your method use `policyEventHandlerFactory` directly to retrieve the handlers.
3. **Note on cross-policy logic:** `IEnforceDependentPoliciesEvent` is a special case. It scans *all* registered handlers (not just the targeted policy's handler) to find dependents when disabling a policy. If your new interface requires similar cross-policy scanning, you will need to add that logic directly to `SavePolicyCommand` or `VNextSavePolicyCommand.SaveAsync()` during transition rather than using `ExecutePolicyEventAsync<T>`.

### Step 3: Document the interface in the [Interfaces](#interfaces) section of this README and add it to the workflow diagram.
Comment on lines +204 to +225
Copy link
Member

Choose a reason for hiding this comment

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

I don't expect this to happen often if ever - the current interfaces are fairly comprehensive. I think we could omit this just so the documentation stays focused on adding new policy types.


---

# IPolicyValidator (Legacy)
Copy link
Member

Choose a reason for hiding this comment

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

This has been deprecated for a while. When can we get rid of it?


`IPolicyValidator` is the **old pattern** and is being phased out. It is consumed by `ISavePolicyCommand` / `PolicyService.SavePolicyAsync`.

```csharp
public interface IPolicyValidator
{
PolicyType Type { get; }
IEnumerable<PolicyType> RequiredPolicies { get; }
Task<string> ValidateAsync(PolicyUpdate policyUpdate, Policy? currentPolicy);
Task OnSaveSideEffectsAsync(PolicyUpdate policyUpdate, Policy? currentPolicy);
}
```

---

## Reason for transition

1. `IPolicyValidator` combines dependency enforcement (`RequiredPolicies`), validation (`ValidateAsync`), and pre-save side effects (`OnSaveSideEffectsAsync`) into a single flat interface. This makes it awkward to add a post-save side effect, since that must be executed after the policy is saved, which lives in a different abstraction.
2. The request body has also expanded, and we need to support metadata that the server needs to perform operations, but that data is not intended to be saved with the policy.
3. By breaking each event hook into a separate interface, it reduces boilerplate, and new hooks can be added without affecting existing services.

---

## During the transition

1. New policies should implement **both** `IPolicyValidator` and the appropriate `IPolicyUpdateEvent` sub-interfaces so they work correctly regardless of which save path is called. Once `ISavePolicyCommand` is fully replaced by `IVNextSavePolicyCommand` and removed, the `IPolicyValidator` implementation can be dropped.
2. Previous implementations of `IPolicyValidator` classes have a postfix of `Validator`, but once we move to `IPolicyUpdateEvent`, they should be renamed to `Handler`. This will reduce confusion since validation normally implies there are no write operations, but there are in this context.

---
Loading