Skip to content

Commit 06e2cd8

Browse files
fix(auth): clear request context per request
Add `Initialize-CippRequestContext` and call it at the start of `New-CippCoreRequest` so AsyncLocal-backed tenant/group scope and role cache are always reset on reused workers. Also ensure unrestricted paths explicitly clear scope slots instead of leaving stale restricted values behind. Add `Get-CippRequestContext` for safe cross-module diagnostics, then update `Invoke-ListApiTest` to read context through CIPPCore rather than CIPPHTTP module scope. Update access-role in-memory caching to include a 15-minute TTL aligned with table cache expiration so role changes are picked up without waiting for process recycle. Synced from CyberDrain/CIPP@f1caebb
1 parent 10563f6 commit 06e2cd8

5 files changed

Lines changed: 141 additions & 27 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
function Get-CippRequestContext {
2+
<#
3+
.SYNOPSIS
4+
Return the per-invocation access context for the current request.
5+
6+
.DESCRIPTION
7+
Accessor for the context slots managed by Initialize-CippRequestContext.
8+
9+
These slots are held in CIPPCore module scope. `$script:` is per-module, so a function in
10+
another module - CIPPHTTP, for example - that reads $script:CippAllowedTenantsStorage
11+
directly gets that module's own, permanently empty, variable rather than this one.
12+
Diagnostics outside CIPPCore must go through this function or they will report that no
13+
scoping is applied no matter what is actually in force.
14+
15+
.EXAMPLE
16+
$Context = Get-CippRequestContext
17+
$Context.AllowedTenants
18+
19+
.FUNCTIONALITY
20+
Internal
21+
#>
22+
[CmdletBinding()]
23+
param()
24+
25+
$InvocationId = if ($script:CippInvocationIdStorage) { $script:CippInvocationIdStorage.Value } else { $null }
26+
$AllowedTenants = if ($script:CippAllowedTenantsStorage) { $script:CippAllowedTenantsStorage.Value } else { $null }
27+
$AllowedGroups = if ($script:CippAllowedGroupsStorage) { $script:CippAllowedGroupsStorage.Value } else { $null }
28+
29+
# Count only. The keys are user principal names and the diagnostic endpoint that surfaces
30+
# this is gated on CIPP.Core.Read, which is not a high enough bar to hand out a list of who
31+
# else has been using this worker.
32+
$CachedUserRoleCount = if ($script:CippUserRolesStorage -and $script:CippUserRolesStorage.Value) {
33+
$script:CippUserRolesStorage.Value.Count
34+
} else {
35+
0
36+
}
37+
38+
return [PSCustomObject]@{
39+
InvocationId = $InvocationId
40+
AllowedTenants = $AllowedTenants
41+
AllowedGroups = $AllowedGroups
42+
CachedUserRoleCount = $CachedUserRoleCount
43+
}
44+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
function Initialize-CippRequestContext {
2+
<#
3+
.SYNOPSIS
4+
Reset the per-invocation context slots at the start of a request.
5+
6+
.DESCRIPTION
7+
The tenant scope, group scope, invocation id and user role cache live in module scoped
8+
AsyncLocal slots so deep helpers (Get-Tenants, Get-TenantGroups) can read the current
9+
request's access scope without every call site threading it through as a parameter.
10+
11+
AsyncLocal does not give per-request isolation in this host. Both Craft and the Azure
12+
Functions PowerShell worker reuse runspaces and execution contexts between invocations,
13+
so a value written by one request is still visible to the next request that lands on the
14+
same worker. Whatever is left behind is inherited, and a request whose access is
15+
unrestricted has no reason to overwrite it - which is how a restricted user's tenant
16+
scope ends up silently filtering an unrestricted user's results.
17+
18+
Every slot is therefore reset here, unconditionally, before any of them is read.
19+
20+
.EXAMPLE
21+
Initialize-CippRequestContext
22+
23+
.FUNCTIONALITY
24+
Internal
25+
#>
26+
[CmdletBinding()]
27+
param()
28+
29+
if (-not $script:CippInvocationIdStorage) {
30+
$script:CippInvocationIdStorage = [System.Threading.AsyncLocal[string]]::new()
31+
}
32+
if (-not $script:CippAllowedTenantsStorage) {
33+
$script:CippAllowedTenantsStorage = [System.Threading.AsyncLocal[object]]::new()
34+
}
35+
if (-not $script:CippAllowedGroupsStorage) {
36+
$script:CippAllowedGroupsStorage = [System.Threading.AsyncLocal[object]]::new()
37+
}
38+
if (-not $script:CippUserRolesStorage) {
39+
$script:CippUserRolesStorage = [System.Threading.AsyncLocal[hashtable]]::new()
40+
}
41+
42+
# Clear anything inherited from an earlier invocation on this worker. Consumers treat a null
43+
# scope as 'unrestricted', so a stale value can only ever hide data that the caller is
44+
# entitled to see - it never grants access. That makes the symptom silent rather than loud,
45+
# which is exactly why it has to be cleared here rather than relied on to be overwritten.
46+
$script:CippInvocationIdStorage.Value = $null
47+
$script:CippAllowedTenantsStorage.Value = $null
48+
$script:CippAllowedGroupsStorage.Value = $null
49+
$script:CippUserRolesStorage.Value = @{}
50+
}

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

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,30 @@ function Test-CIPPAccessUserRole {
2424
$UserRoleTotalSw = [System.Diagnostics.Stopwatch]::StartNew()
2525
$Roles = @()
2626

27-
# Check AsyncLocal cache first (per-request cache)
27+
# TTL for the in-memory tier, deliberately identical to the cacheAccessUserRoles window used
28+
# below. That tier fronts the table cache, so leaving it without an expiry lets it outlive
29+
# the cache it is caching: a role change, or a user being removed from an access group,
30+
# would not take effect on a warm worker until the process recycled.
31+
$RoleCacheMinutes = 15
32+
33+
# Check the in-memory cache first, discarding the entry once it is past its TTL
34+
$CachedEntry = $null
2835
if ($script:CippUserRolesStorage -and $script:CippUserRolesStorage.Value -and $script:CippUserRolesStorage.Value.ContainsKey($User.userDetails)) {
29-
$Roles = $script:CippUserRolesStorage.Value[$User.userDetails]
36+
$CachedEntry = $script:CippUserRolesStorage.Value[$User.userDetails]
37+
if ($null -eq $CachedEntry.Expires -or $CachedEntry.Expires -le (Get-Date).ToUniversalTime()) {
38+
$script:CippUserRolesStorage.Value.Remove($User.userDetails)
39+
$CachedEntry = $null
40+
}
41+
}
42+
43+
if ($CachedEntry) {
44+
$Roles = $CachedEntry.Roles
3045
} else {
3146
# Check table storage cache (persistent cache)
3247
try {
3348
$swTableLookup = [System.Diagnostics.Stopwatch]::StartNew()
3449
$Table = Get-CippTable -TableName cacheAccessUserRoles
35-
$Filter = "PartitionKey eq 'AccessUser' and RowKey eq '$($User.userDetails)' and Timestamp ge datetime'$((Get-Date).AddMinutes(-15).ToUniversalTime().ToString('yyyy-MM-ddTHH:mm:ss.fffZ'))'"
50+
$Filter = "PartitionKey eq 'AccessUser' and RowKey eq '$($User.userDetails)' and Timestamp ge datetime'$((Get-Date).AddMinutes(-$RoleCacheMinutes).ToUniversalTime().ToString('yyyy-MM-ddTHH:mm:ss.fffZ'))'"
3651
$UserRole = Get-CIPPAzDataTableEntity @Table -Filter $Filter
3752
$swTableLookup.Stop()
3853
$UserRoleTimings['TableLookup'] = $swTableLookup.Elapsed.TotalMilliseconds
@@ -44,9 +59,12 @@ function Test-CIPPAccessUserRole {
4459
Write-Information "Found cached user role for $($User.userDetails)"
4560
$Roles = $UserRole.Role | ConvertFrom-Json
4661

47-
# Store in AsyncLocal cache for this request
62+
# Store in the in-memory cache, expiring in step with the table cache
4863
if ($script:CippUserRolesStorage -and $script:CippUserRolesStorage.Value) {
49-
$script:CippUserRolesStorage.Value[$User.userDetails] = $Roles
64+
$script:CippUserRolesStorage.Value[$User.userDetails] = @{
65+
Roles = $Roles
66+
Expires = (Get-Date).ToUniversalTime().AddMinutes($RoleCacheMinutes)
67+
}
5068
}
5169
} else {
5270
try {
@@ -114,9 +132,12 @@ function Test-CIPPAccessUserRole {
114132
}
115133
}
116134

117-
# Store in AsyncLocal cache for this request
135+
# Store in the in-memory cache, expiring in step with the table cache
118136
if ($script:CippUserRolesStorage -and $script:CippUserRolesStorage.Value) {
119-
$script:CippUserRolesStorage.Value[$User.userDetails] = $Roles
137+
$script:CippUserRolesStorage.Value[$User.userDetails] = @{
138+
Roles = $Roles
139+
Expires = (Get-Date).ToUniversalTime().AddMinutes($RoleCacheMinutes)
140+
}
120141
}
121142
}
122143
}

Modules/CIPPCore/Public/Entrypoints/HTTP Functions/New-CippCoreRequest.ps1

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,24 +16,10 @@ function New-CippCoreRequest {
1616
$HttpTimings = @{}
1717
$HttpTotalStopwatch = [System.Diagnostics.Stopwatch]::StartNew()
1818

19-
# Initialize AsyncLocal storage for thread-safe per-invocation context
20-
if (-not $script:CippInvocationIdStorage) {
21-
$script:CippInvocationIdStorage = [System.Threading.AsyncLocal[string]]::new()
22-
}
23-
if (-not $script:CippAllowedTenantsStorage) {
24-
$script:CippAllowedTenantsStorage = [System.Threading.AsyncLocal[object]]::new()
25-
}
26-
if (-not $script:CippAllowedGroupsStorage) {
27-
$script:CippAllowedGroupsStorage = [System.Threading.AsyncLocal[object]]::new()
28-
}
29-
if (-not $script:CippUserRolesStorage) {
30-
$script:CippUserRolesStorage = [System.Threading.AsyncLocal[hashtable]]::new()
31-
}
32-
33-
# Initialize user roles cache for this request
34-
if (-not $script:CippUserRolesStorage.Value) {
35-
$script:CippUserRolesStorage.Value = @{}
36-
}
19+
# Initialize and reset the per-invocation context slots. This has to happen before anything
20+
# reads them: workers are reused between requests, so whatever the previous invocation left
21+
# behind is still in scope until it is explicitly cleared.
22+
Initialize-CippRequestContext
3723

3824
# Set InvocationId in AsyncLocal storage for console logging correlation
3925
if ($global:TelemetryClient -and $TriggerMetadata.InvocationId) {
@@ -158,13 +144,21 @@ function New-CippCoreRequest {
158144
$swGroups.Stop()
159145
$HttpTimings['AllowedGroups'] = $swGroups.Elapsed.TotalMilliseconds
160146

147+
# Assign on every path, including the unrestricted one. Only writing the slot when
148+
# access is limited is what allowed a restricted user's scope to survive into the
149+
# next request handled by the same worker, silently filtering results for someone
150+
# entitled to see everything.
161151
if ($AllowedTenants -notcontains 'AllTenants') {
162152
Write-Warning 'Limiting tenant access'
163153
$script:CippAllowedTenantsStorage.Value = $AllowedTenants
154+
} else {
155+
$script:CippAllowedTenantsStorage.Value = $null
164156
}
165157
if ($AllowedGroups -notcontains 'AllGroups') {
166158
Write-Warning 'Limiting group access'
167159
$script:CippAllowedGroupsStorage.Value = $AllowedGroups
160+
} else {
161+
$script:CippAllowedGroupsStorage.Value = $null
168162
}
169163

170164
try {

Modules/CIPPHTTP/Public/Entrypoints/HTTP Functions/CIPP/Core/Invoke-ListApiTest.ps1

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,13 @@ function Invoke-ListApiTest {
2727
$Request = New-CIPPAzRestRequest -Method POST -Resource 'https://management.azure.com/' -Uri 'https://management.azure.com/providers/Microsoft.ResourceGraph/resources?api-version=2022-10-01' -Body $Json
2828
$Response.ResourceGraphTest = $Request
2929
}
30-
$Response.AllowedTenants = $script:CippAllowedTenantsStorage.Value
31-
$Response.AllowedGroups = $script:CippAllowedGroupsStorage.Value
30+
# Has to come from CIPPCore. These slots are module scoped, so reading $script:Cipp*Storage
31+
# from here would return CIPPHTTP's own - always empty - variables and report that no tenant
32+
# scoping is applied regardless of what is actually in force.
33+
$RequestContext = Get-CippRequestContext
34+
$Response.AllowedTenants = $RequestContext.AllowedTenants
35+
$Response.AllowedGroups = $RequestContext.AllowedGroups
36+
$Response.RequestContext = $RequestContext
3237

3338
return ([HttpResponseContext]@{
3439
StatusCode = [HttpStatusCode]::OK

0 commit comments

Comments
 (0)