Add IdLE.Step.PruneEntitlements and IdLE.Step.PruneEntitlementsEnsureKeep — bulk entitlement convergence with provider optimizations#237
Conversation
…tes and docs" Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
blindzero
left a comment
There was a problem hiding this comment.
@copilot I am missing the changed providers for Entra and AD implementing this capability into dedicated adapters.
We must make sure in these adapters, that provider specific "Id"-type-detection and mapping to parameters is done in the same way as for other adapters as well.
…nts with ID normalization; simplify step loop Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Done in cff2fd7. Both AD and Entra ID adapters now have a dedicated |
blindzero
left a comment
There was a problem hiding this comment.
@copilot make sure that all providers implementing the step support their specific way of alternative group id input (like we support already for user ids). Keep it consistent and place the part at same locations.
Also make sure that determining which groups to keep or to remove or to ensure is deterministic and only decided at one step. If using different providers, I would recommend the Step / Capability not the providers to avoid drift in behavior of different providers.
…lementId to AD/Entra providers; enhance AD NormalizeGroupId with GUID/DN/sAMAccountName fallbacks; remove PruneGroupMemberships from adapters Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Done in 2ebe95c. The delta computation (keep/remove/ensure decision) now lives only in the step — |
blindzero
left a comment
There was a problem hiding this comment.
@copilot # PR #237 – Agent Instructions (PruneEntitlements)
Copy/paste this comment into PR #237
1) Architecture: where the delta lives
- Compute deltas in the Step, not in Providers.
- Providers must remain IO adapters:
ListEntitlements,GrantEntitlement,RevokeEntitlement(bulk optimization optional later).
2) Runtime data flow (must match this)
Inside Invoke-IdleStepPruneEntitlements:
-
Normalize inputs (
Keep,KeepPattern,EnsureKeepEntitlements) and enforce guardrails:- Fail fast if neither
KeepnorKeepPatternis specified.
- Fail fast if neither
-
Read current state exactly once:
current = Provider.ListEntitlements(identityKey, kind, authSession)
-
Compute sets in the Step:
toKeep = currentfiltered by Keep/KeepPattern rulestoDelete = current - toKeeptoEnsure = keepItems - currentonly whenEnsureKeepEntitlementsis true
-
Execute:
- Revoke
toDelete - Grant
toEnsureonly whenEnsureKeepEntitlementsis true
- Revoke
3) Provider idempotency + performance (critical)
- Remove any additional
ListEntitlementscalls inside providerGrant/Revoke. - Providers must be write-idempotent:
- Grant: “already present” → no-op
- Revoke: “not present” → no-op
- Implement provider-specific error handling (no domain logic in provider).
4) Capabilities: avoid over-restricting remove-only workflows
- We cannot express conditional required capabilities with static step metadata.
- Implement two StepTypes:
IdLE.Step.PruneEntitlements(remove-only) requiresList + RevokeIdLE.Step.PruneEntitlementsEnsureKeep(ensure keep present) requiresList + Revoke + Grant
Parameter rules:
- Allow
KeepandKeepPatternsimultaneously. - Switch name is
-EnsureKeepEntitlements(as decided).
5) Definition of Done
- Unit tests cover:
- Keep only, KeepPattern only, both together
- EnsureKeepEntitlements toggles ToEnsure behavior correctly
- Guardrail: missing Keep & KeepPattern fails fast
- Docs/examples updated for both StepTypes and capability expectations.
- No provider contract changes beyond removing redundant reads; operations remain idempotent.
…e-only) and PruneEntitlementsEnsureKeep (remove + ensure keep); fix capability requirements; update registry, manifest, docs, templates and tests Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Done in 7842b41. Summary of what was addressed: Two StepTypes: Delta in Step: Already done in previous commit — Capability fix: Tests: 11 new tests for Docs/templates: Both step types documented, leaver workflow templates updated to use |
ntt-matthias-fleschuetz
left a comment
There was a problem hiding this comment.
@copilot your solution seems complicated too me. why not just fetching the primaryGroup and then removing it from remove entitlements or always add to keep?
… SID arithmetic, works on all AD versions Co-authored-by: ntt-matthias-fleschuetz <55826276+ntt-matthias-fleschuetz@users.noreply.github.com>
Done in dc81864. |
ntt-matthias-fleschuetz
left a comment
There was a problem hiding this comment.
@copilot now no groups are removed anymore, although many others then keep are assigned. Changed: shows False as well.
There is no difference between both Prune step types.
… set-difference — reliable on all AD environments Co-authored-by: ntt-matthias-fleschuetz <55826276+ntt-matthias-fleschuetz@users.noreply.github.com>
Fixed in d440ae2. Root cause analysis:
New approach:
This works on all AD DC versions. Domain Users will be excluded from |
|
@copilot KeepPattern in PruneEntitlementsEnsureKeep dont make sense as the step would logically need to add ALL groups in keep pattern not there yet.
Keep approaches of schema validation during plan creation consistent to other existing ways and try to minimize module function / cmdlet exposure |
…o longer required; AD EnsureKeep integration tests added Co-authored-by: ntt-matthias-fleschuetz <55826276+ntt-matthias-fleschuetz@users.noreply.github.com>
... Done in 32a67e4. KeepPattern forbidden in EnsureKeep (fail-fast at plan time): Keep/KeepPattern no longer required: PruneEntitlementsEnsureKeep AD provider integration:
Done in 32a67e4. |
|
@copilot for step schema validation we always should only check against allowed keys defined by the schema and throw on not allowed. common attributies like conditions and preconditions should be validated separately and be in the allowed step's root keys always. |
… step With-key schema validation Co-authored-by: ntt-matthias-fleschuetz <55826276+ntt-matthias-fleschuetz@users.noreply.github.com>
Done in ec860db. Replaced
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…tlementId comment; add GrantEntitlement not-called assertion Co-authored-by: ntt-matthias-fleschuetz <55826276+ntt-matthias-fleschuetz@users.noreply.github.com>
Invoke-IdleStepPruneEntitlements.ps1step (remove-only)Invoke-IdleStepPruneEntitlementsEnsureKeep.ps1(thin wrapper, forces EnsureKeepEntitlements)ResolveGroup/ResolveEntitlementnaming; bulk Entra ID via Graph$batchIdLE.Entitlement.Pruneadvertised on AD, Entra ID, Mock providers (0.9.6)MemberOfset-differenceAllowedWithKeysallowlist replacesForbiddenWithKeysdenylist in step metadata validation<hash>):BulkRevokeEntitlementsScriptMethod removed fromNew-IdleADAdapter.ps1. The method was only on the adapter object and never exposed by the AD provider; the step looks for bulk methods on the provider, not the adapter, so this was unreachable dead code. Confirmed no usages in the codebase.provider.NormalizeEntitlementId→provider.ResolveEntitlementinInvoke-IdleStepPruneEntitlements.ps1line 208 comment.NormalizeEntitlementIdwas the old name before it was renamed toResolveEntitlementin a prior commit.'does not re-grant already present Keep entitlements'test now replacesGrantEntitlementwith a$script:grantCountcounter and asserts$script:grantCount | Should -Be 0 -Because 'CN=LEAVER-RETAIN is already present; GrantEntitlement must not be called'. The$grantCount/$originalGrantvariables that were set but never used have been replaced with this working pattern.Security Summary: No security vulnerabilities introduced or found.
Original prompt
This section details on the original issue you should resolve
<issue_title>Add IdLE.Step.PruneEntitlements (agent-safe entitlement convergence / bulk removal)</issue_title>
<issue_description>## Problem Statement
In leaver (and some mover) workflows we must remove all entitlements (e.g., group memberships) from an identity except:
With the current step model,
IdLE.Step.EnsureEntitlementis intentionally atomic (one entitlement → desired state). Implementing “remove all except …” would require external host logic to:EnsureEntitlement -State Absentsteps.This breaks portability and pushes essential lifecycle logic out of IdLE workflows.
Proposed Solution
Introduce a new provider-agnostic step type:
IdLE.Step.PruneEntitlementsCapability gating (explicit provider opt-in)
Add a dedicated capability so providers can explicitly opt in:
IdLE.Entitlement.PruneReason: while prune can be composed from
List + Revoke (+ Grant), we explicitly want providers to decide whether they support a bulk destructive operation.Parameters (agent-safe)
-Provider(string, required)-Identity(Idle identity reference, required)-Kind(string, required)Group,Role,License(provider-defined)-Keep(array, optional)EnsureEntitlementuses for a single entitlement)-KeepPattern(array of string, optional)-likesemantics), used to keep entitlements matching the pattern-EnsureKeepEntitlements(switch, optional)-Keepare present (grant missing)Guardrails / Validation
-Keepor-KeepPatternmust be provided.-KeepPatternsupports wildcard matching only (e.g.LEAVER-*,CN=LEAVER-*,OU=Groups,DC=...).-EnsureKeepEntitlementsapplies only to explicit-Keepitems (patterns cannot be “ensured”).Provider contract usage
The step is provider-agnostic and uses existing entitlement primitives, plus the new capability flag:
ListEntitlements(identity, kind)RevokeEntitlement(identity, entitlement, kind)-EnsureKeepEntitlements):GrantEntitlement(identity, entitlement, kind)Planned behavior (idempotent)
Kind).KeepKeepPatternEnsureKeepEntitlementsis set:Alternatives Considered
A) Extend
IdLE.Step.EnsureEntitlementwith-RemoveAllExcept/-KeepPatternRejected. It blurs semantics of an atomic step, increases risk of accidental destructive behavior, complicates validation/modes, and makes docs harder (“EnsureEntitlement sometimes means prune”).
B) Host-only delta computation (build many EnsureEntitlement steps externally)
Rejected. Breaks portability: the core lifecycle semantics live outside IdLE workflows and differs per host.
C) Provider-specific step (e.g., AD-only prune group memberships)
Rejected (for now). We want a generic capability and step. Providers can opt in/out via capability.
Impact
IdLE.Entitlement.Pruneand implementing the required primitives consistently.Additional Context
Primary example (AD Leaver)
Remove all group memberships excep...
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.