Skip to content

Commit 417fc28

Browse files
feat(auth): add cached access scope rules for custom roles
Introduces a per-worker cache for access scope rules derived from custom roles and tenant group membership, keyed on a shared version stamp stored in Azure Table. - Add Get-CippAccessScopeRule to cache the rule (allow/block lists) rather than resolved tenant ids, keeping correctness as tenants change - Add Get-CippAccessScopeVersion to memo the shared stamp with a short TTL - Add Clear-CippAccessScopeCache to bump the stamp and drop local memos; called from ExecCustomRole, ExecTenantGroup, Set-CIPPAccessRole, and Update-CIPPDynamicTenantGroups - Add Expand-CippScopeTenantItem to resolve tenant group references in allowed/blocked lists - Refactor Test-CIPPAccess to use the cache for TenantList and GroupList requests, avoiding a full tenant load on every warm request - Extract Resolve-CippRoleTenantEntry and fix silent drop of unresolvable tenant ids in ListCustomRole, which previously caused a blocked tenant to render as unrestricted Synced from CyberDrain/CIPP@aa01643
1 parent 06e2cd8 commit 417fc28

11 files changed

Lines changed: 433 additions & 119 deletions
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
function Clear-CippAccessScopeCache {
2+
<#
3+
.SYNOPSIS
4+
Invalidate cached access scope rules across every worker.
5+
6+
.DESCRIPTION
7+
Bumps the shared version stamp that Get-CippAccessScopeRule keys on, so every worker misses
8+
and recomputes from source, and drops this worker's caches immediately.
9+
10+
Propagation is not instant on the other workers. They each memo the stamp for a short
11+
window (see Get-CippAccessScopeVersion), so a change lands there within that window rather
12+
than on the very next request. Verified behaviour, not an assumption.
13+
14+
Call this from anything that changes what a role is allowed to see: custom role
15+
definitions, access role group mappings, and tenant group membership. Missing a call site
16+
does not cause indefinite staleness - the rule cache TTL still expires entries - but it
17+
does mean an operator can save a role change and not see it take effect straight away.
18+
19+
A failure is logged rather than thrown. Failing the role save the operator just made
20+
because a cache table hiccuped would be the worse outcome, and the TTL bounds the damage.
21+
22+
.EXAMPLE
23+
Clear-CippAccessScopeCache
24+
25+
.FUNCTIONALITY
26+
Internal
27+
#>
28+
[CmdletBinding(SupportsShouldProcess = $true)]
29+
param()
30+
31+
if (-not $PSCmdlet.ShouldProcess('access scope cache', 'Invalidate')) {
32+
return
33+
}
34+
35+
$script:CippAccessScopeRuleCache = @{}
36+
$script:CippAccessScopeVersionMemo = $null
37+
38+
try {
39+
$Table = Get-CIPPTable -tablename 'CacheVersions'
40+
$Entity = @{
41+
PartitionKey = 'CacheVersion'
42+
RowKey = 'AccessScope'
43+
Version = [string][guid]::NewGuid()
44+
}
45+
Add-CIPPAzDataTableEntity @Table -Entity $Entity -Force | Out-Null
46+
} catch {
47+
Write-LogMessage -API 'AccessScopeCache' -message "Failed to invalidate the access scope cache. Other workers will keep their current rules until the cache expires. $($_.Exception.Message)" -Sev 'Error'
48+
}
49+
}
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
function Expand-CippScopeTenantItem {
2+
<#
3+
.SYNOPSIS
4+
Expand a role's tenant entries, resolving tenant groups to their member tenant ids.
5+
6+
.DESCRIPTION
7+
Entries stored against a role are either literal tenant ids, the 'AllTenants' sentinel, or
8+
a tenant group reference that has to be resolved to the ids currently in that group.
9+
10+
A group that cannot be expanded warns and contributes nothing, which is deliberate: the
11+
alternative - treating an unresolvable group as 'everything' - would widen access on the
12+
back of a lookup failure.
13+
14+
Used by Get-CippAccessScopeRule for both the allowed and blocked lists.
15+
16+
.PARAMETER Items
17+
The stored AllowedTenants or BlockedTenants entries.
18+
19+
.PARAMETER Kind
20+
'allowed' or 'blocked', used only to make the warning readable.
21+
22+
.EXAMPLE
23+
Expand-CippScopeTenantItem -Items $Permission.BlockedTenants -Kind 'blocked'
24+
25+
.FUNCTIONALITY
26+
Internal
27+
#>
28+
[CmdletBinding()]
29+
param(
30+
$Items,
31+
[string]$Kind
32+
)
33+
34+
foreach ($Item in $Items) {
35+
if ($Item -is [PSCustomObject] -and $Item.type -eq 'Group') {
36+
try {
37+
$GroupMembers = Expand-CIPPTenantGroups -TenantFilter @($Item)
38+
$GroupMembers | ForEach-Object { $_.addedFields.customerId }
39+
} catch {
40+
Write-Warning "Failed to expand $Kind tenant group '$($Item.label)': $($_.Exception.Message)"
41+
}
42+
} else {
43+
$Item
44+
}
45+
}
46+
}
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
function Get-CippAccessScopeRule {
2+
<#
3+
.SYNOPSIS
4+
The tenant and group scope rule a custom role expresses.
5+
6+
.DESCRIPTION
7+
Returns the rule itself - allow everything, or an explicit allow list minus a block list -
8+
rather than the set of tenant ids it currently resolves to.
9+
10+
That distinction is the whole point of the cache. Caching the resolved id list would bake
11+
in a snapshot of the tenant table, so a tenant onboarded afterwards would silently stay
12+
invisible to the role until the entry expired: the same silent-omission failure that made
13+
the request scope leak so hard to spot. A rule depends only on the role definition and
14+
tenant group membership, both covered by the version stamp, so it stays correct however
15+
many tenants come and go. Callers expand it against the live tenant list at the point of
16+
use, which is an in-memory set operation over data they already hold.
17+
18+
Cached per worker, keyed by the shared version stamp so a role change invalidates every
19+
worker at once, with a TTL as a backstop in case an invalidation call site is missed.
20+
21+
.PARAMETER Role
22+
Name of the custom role.
23+
24+
.EXAMPLE
25+
$Rule = Get-CippAccessScopeRule -Role 'helpdesk'
26+
if (-not $Rule.Unrestricted) { $Rule.BlockedTenants }
27+
28+
.FUNCTIONALITY
29+
Internal
30+
#>
31+
[CmdletBinding()]
32+
param(
33+
[Parameter(Mandatory = $true)]
34+
[string]$Role
35+
)
36+
37+
$TtlMinutes = 5
38+
$Now = (Get-Date).ToUniversalTime()
39+
$Version = Get-CippAccessScopeVersion
40+
$CacheKey = '{0}|{1}' -f $Version, $Role
41+
42+
if (-not $script:CippAccessScopeRuleCache) {
43+
$script:CippAccessScopeRuleCache = @{}
44+
}
45+
46+
$Cached = $script:CippAccessScopeRuleCache[$CacheKey]
47+
if ($Cached -and $Cached.Expires -gt $Now) {
48+
return $Cached.Rule
49+
}
50+
51+
# Throws when the role no longer exists, which callers already handle as 'this role
52+
# contributes no scope'
53+
$Permission = Get-CIPPRolePermissions -Role $Role
54+
55+
$Unrestricted = ((($Permission.AllowedTenants | Measure-Object).Count -eq 0 -or
56+
$Permission.AllowedTenants -contains 'AllTenants') -and
57+
($Permission.BlockedTenants | Measure-Object).Count -eq 0)
58+
59+
$AllowAllTenants = $false
60+
$AllowedTenants = @()
61+
$BlockedTenants = @()
62+
$AllowedGroups = @()
63+
64+
if (-not $Unrestricted) {
65+
$Expanded = @(Expand-CippScopeTenantItem -Items $Permission.AllowedTenants -Kind 'allowed')
66+
67+
# 'AllTenants' alongside a block list means 'everything except', and the caller substitutes
68+
# the live tenant list. Held as a flag rather than a resolved list precisely so that
69+
# substitution happens at read time.
70+
$AllowAllTenants = $Expanded -contains 'AllTenants'
71+
$AllowedTenants = @($Expanded | Where-Object { $_ -ne 'AllTenants' })
72+
$BlockedTenants = @(Expand-CippScopeTenantItem -Items $Permission.BlockedTenants -Kind 'blocked')
73+
$AllowedGroups = @(
74+
foreach ($Item in $Permission.AllowedTenants) {
75+
if ($Item -is [PSCustomObject] -and $Item.type -eq 'Group') { $Item.value }
76+
}
77+
)
78+
}
79+
80+
$Rule = [PSCustomObject]@{
81+
Role = $Role
82+
Unrestricted = $Unrestricted
83+
AllowAllTenants = $AllowAllTenants
84+
AllowedTenants = $AllowedTenants
85+
BlockedTenants = $BlockedTenants
86+
AllowedGroups = $AllowedGroups
87+
}
88+
89+
# Entries keyed on a superseded version are dead weight; drop the lot rather than track ages
90+
if ($script:CippAccessScopeRuleCache.Count -gt 200) {
91+
$script:CippAccessScopeRuleCache = @{}
92+
}
93+
$script:CippAccessScopeRuleCache[$CacheKey] = @{
94+
Rule = $Rule
95+
Expires = $Now.AddMinutes($TtlMinutes)
96+
}
97+
98+
return $Rule
99+
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
function Get-CippAccessScopeVersion {
2+
<#
3+
.SYNOPSIS
4+
Current version stamp for cached access scope rules.
5+
6+
.DESCRIPTION
7+
Access scope rules are derived from CustomRoles, AccessRoleGroups and tenant group
8+
membership, and are cached per worker. Those inputs are shared, and one worker has no way
9+
to reach into another's memory, so invalidation goes through this stamp instead: every
10+
worker keys its cache on the current value, and Clear-CippAccessScopeCache bumps it
11+
whenever the underlying definitions change.
12+
13+
The stamp is itself memoised for a short window so a warm request costs no storage read at
14+
all. That window is the upper bound on how long a role change takes to reach a worker that
15+
is already warm.
16+
17+
A read failure returns a fresh value rather than the last known one. That forces a miss and
18+
a recompute from source, so a storage blip costs latency instead of serving a scope that
19+
may no longer be correct.
20+
21+
.EXAMPLE
22+
Get-CippAccessScopeVersion
23+
24+
.FUNCTIONALITY
25+
Internal
26+
#>
27+
[CmdletBinding()]
28+
param()
29+
30+
# How long a worker trusts its copy of the stamp, and therefore the worst case delay before a
31+
# role change reaches a worker that is already warm. The worker that made the change clears
32+
# its own memo, so this only applies to the others. Kept short deliberately: the cost is one
33+
# small table read per worker per window, which is nothing next to an operator changing a role
34+
# and being unable to tell whether it took effect.
35+
$MemoSeconds = 10
36+
$Now = (Get-Date).ToUniversalTime()
37+
38+
if ($script:CippAccessScopeVersionMemo -and $script:CippAccessScopeVersionMemo.Expires -gt $Now) {
39+
return $script:CippAccessScopeVersionMemo.Version
40+
}
41+
42+
try {
43+
$Table = Get-CIPPTable -tablename 'CacheVersions'
44+
$Entity = Get-CIPPAzDataTableEntity @Table -Filter "PartitionKey eq 'CacheVersion' and RowKey eq 'AccessScope'"
45+
$Version = if ($Entity.Version) { [string]$Entity.Version } else { 'initial' }
46+
} catch {
47+
Write-Warning "Could not read the access scope cache version, forcing a recompute. $($_.Exception.Message)"
48+
return [string][guid]::NewGuid()
49+
}
50+
51+
$script:CippAccessScopeVersionMemo = @{
52+
Version = $Version
53+
Expires = $Now.AddSeconds($MemoSeconds)
54+
}
55+
56+
return $Version
57+
}

Modules/CIPPCore/Public/Authentication/Set-CIPPAccessRole.ps1

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,5 +47,9 @@ function Set-CIPPAccessRole {
4747

4848
if ($PSCmdlet.ShouldProcess("Setting access role $Role for group $($Group.displayName)")) {
4949
Add-CIPPAzDataTableEntity -Table $Table -Entity $AccessGroup -Force
50+
51+
# Group to role mapping decides which roles a user resolves to, so the cached scope rules
52+
# have to be invalidated with it
53+
Clear-CippAccessScopeCache
5054
}
5155
}

Modules/CIPPCore/Public/Authentication/Test-CIPPAccess.ps1

Lines changed: 51 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,55 @@ function Test-CIPPAccess {
331331
if (@('admin', 'superadmin') -contains $BaseRole.Name) {
332332
return $true
333333
} else {
334+
# Scope-only requests resolve from the cached rules. On a warm cache this needs no
335+
# storage read at all, and it only needs the tenant table when a rule actually says
336+
# 'all tenants except', because that is the one case where the answer depends on
337+
# which tenants currently exist.
338+
if ($TenantList.IsPresent -or $GroupList.IsPresent) {
339+
$swScopeRules = [System.Diagnostics.Stopwatch]::StartNew()
340+
$ScopeRules = foreach ($CustomRole in $CustomRoles) {
341+
try {
342+
Get-CippAccessScopeRule -Role $CustomRole
343+
} catch {
344+
Write-Information $_.Exception.Message
345+
}
346+
}
347+
$swScopeRules.Stop()
348+
$AccessTimings['GetScopeRules'] = $swScopeRules.Elapsed.TotalMilliseconds
349+
350+
if (($ScopeRules | Measure-Object).Count -eq 0) {
351+
# No role produced a scope, so the caller is entitled to nothing
352+
return @()
353+
}
354+
355+
if ($TenantList.IsPresent) {
356+
$swTenantList = [System.Diagnostics.Stopwatch]::StartNew()
357+
$NeedsTenantList = @($ScopeRules | Where-Object { -not $_.Unrestricted -and $_.AllowAllTenants }).Count -gt 0
358+
$Tenants = if ($NeedsTenantList) { Get-Tenants -IncludeErrors } else { @() }
359+
360+
$LimitedTenantList = foreach ($Rule in $ScopeRules) {
361+
if ($Rule.Unrestricted) {
362+
@('AllTenants')
363+
} else {
364+
$AllowedForRule = if ($Rule.AllowAllTenants) { $Tenants.customerId } else { $Rule.AllowedTenants }
365+
$AllowedForRule | Where-Object { $Rule.BlockedTenants -notcontains $_ }
366+
}
367+
}
368+
$swTenantList.Stop()
369+
$AccessTimings['BuildTenantList'] = $swTenantList.Elapsed.TotalMilliseconds
370+
return @($LimitedTenantList | Sort-Object -Unique)
371+
}
372+
373+
Write-Information "Getting allowed groups for roles: $($CustomRoles -join ', ')"
374+
$swGroupList = [System.Diagnostics.Stopwatch]::StartNew()
375+
$LimitedGroupList = foreach ($Rule in $ScopeRules) {
376+
if ($Rule.Unrestricted) { @('AllGroups') } else { $Rule.AllowedGroups }
377+
}
378+
$swGroupList.Stop()
379+
$AccessTimings['BuildGroupList'] = $swGroupList.Elapsed.TotalMilliseconds
380+
return @($LimitedGroupList | Sort-Object -Unique)
381+
}
382+
334383
$swTenantsLoad = [System.Diagnostics.Stopwatch]::StartNew()
335384
$Tenants = Get-Tenants -IncludeErrors
336385
$swTenantsLoad.Stop()
@@ -349,69 +398,8 @@ function Test-CIPPAccess {
349398
$AccessTimings['GetRolePermissions'] = $swRolePerms.Elapsed.TotalMilliseconds
350399

351400
if ($PermissionsFound) {
352-
if ($TenantList.IsPresent) {
353-
$swTenantList = [System.Diagnostics.Stopwatch]::StartNew()
354-
$LimitedTenantList = foreach ($Permission in $PermissionSet) {
355-
if ((($Permission.AllowedTenants | Measure-Object).Count -eq 0 -or $Permission.AllowedTenants -contains 'AllTenants') -and (($Permission.BlockedTenants | Measure-Object).Count -eq 0)) {
356-
@('AllTenants')
357-
} else {
358-
# Expand tenant groups to individual tenant IDs
359-
$ExpandedAllowedTenants = foreach ($AllowedItem in $Permission.AllowedTenants) {
360-
if ($AllowedItem -is [PSCustomObject] -and $AllowedItem.type -eq 'Group') {
361-
try {
362-
$GroupMembers = Expand-CIPPTenantGroups -TenantFilter @($AllowedItem)
363-
$GroupMembers | ForEach-Object { $_.addedFields.customerId }
364-
} catch {
365-
Write-Warning "Failed to expand tenant group '$($AllowedItem.label)': $($_.Exception.Message)"
366-
@()
367-
}
368-
} else {
369-
$AllowedItem
370-
}
371-
}
372-
373-
$ExpandedBlockedTenants = foreach ($BlockedItem in $Permission.BlockedTenants) {
374-
if ($BlockedItem -is [PSCustomObject] -and $BlockedItem.type -eq 'Group') {
375-
try {
376-
$GroupMembers = Expand-CIPPTenantGroups -TenantFilter @($BlockedItem)
377-
$GroupMembers | ForEach-Object { $_.addedFields.customerId }
378-
} catch {
379-
Write-Warning "Failed to expand blocked tenant group '$($BlockedItem.label)': $($_.Exception.Message)"
380-
@()
381-
}
382-
} else {
383-
$BlockedItem
384-
}
385-
}
386-
387-
if ($ExpandedAllowedTenants -contains 'AllTenants') {
388-
$ExpandedAllowedTenants = $Tenants.customerId
389-
}
390-
$ExpandedAllowedTenants | Where-Object { $ExpandedBlockedTenants -notcontains $_ }
391-
}
392-
}
393-
$swTenantList.Stop()
394-
$AccessTimings['BuildTenantList'] = $swTenantList.Elapsed.TotalMilliseconds
395-
return @($LimitedTenantList | Sort-Object -Unique)
396-
} elseif ($GroupList.IsPresent) {
397-
$swGroupList = [System.Diagnostics.Stopwatch]::StartNew()
398-
Write-Information "Getting allowed groups for roles: $($CustomRoles -join ', ')"
399-
$LimitedGroupList = foreach ($Permission in $PermissionSet) {
400-
if ((($Permission.AllowedTenants | Measure-Object).Count -eq 0 -or $Permission.AllowedTenants -contains 'AllTenants') -and (($Permission.BlockedTenants | Measure-Object).Count -eq 0)) {
401-
@('AllGroups')
402-
} else {
403-
foreach ($AllowedItem in $Permission.AllowedTenants) {
404-
if ($AllowedItem -is [PSCustomObject] -and $AllowedItem.type -eq 'Group') {
405-
$AllowedItem.value
406-
}
407-
}
408-
}
409-
}
410-
$swGroupList.Stop()
411-
$AccessTimings['BuildGroupList'] = $swGroupList.Elapsed.TotalMilliseconds
412-
return @($LimitedGroupList | Sort-Object -Unique)
413-
}
414-
401+
# Tenant list and group list requests have already returned above, from the
402+
# cached scope rules. Everything from here is the per-endpoint access decision.
415403
$TenantAllowed = $false
416404
$APIAllowed = $false
417405
$swPermissionEval = [System.Diagnostics.Stopwatch]::StartNew()

0 commit comments

Comments
 (0)