diff --git a/CHANGELOG.md b/CHANGELOG.md index 4484a2b..ba5d279 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ release notes. ### Fixed +- **`Test-IntuneDeployedScript` decided whether a group holds devices from its first 20 members.** A mixed group + whose first page was all devices passed as a device group, and a user-context app assigned to it was flagged + as never installing. The check now counts the group's members and the devices among them with two `$count` + queries, so the whole group decides. - **`Repair-IntuneScript` analyzed every script under the inferred context and architecture**, whatever the caller deployed to, because it had no `-Context`, `-Architecture` or `-EnforceSignatureCheck` to pass on. It takes the three now and hands them to both analyses, so its findings, fixes and `Remaining` count are @@ -71,6 +75,10 @@ release notes. The filters now run on the raw record before an entry is built, which was most of an entry's cost; the 44 event patterns are one expression matched once per message instead of 44 statements; a rolled-over file last written before `-After` is not read at all. Results are unchanged. +- Every Graph request the tenant commands make is sent again when Graph throttles it (429) or is briefly + unavailable (503, 504): after the `Retry-After` seconds when the header is there, after 2, 4 and 8 seconds + when it is not, three times at most before the failure is thrown as it came. A pre-flight over a few hundred + policies makes two requests per policy and meets the throttle. ## [0.27.0] - 2026-10-05 Fixes to the runtime harness and to four rules, each rule change backed by a tenth validation diff --git a/Private/Invoke-IslGraphRequest.ps1 b/Private/Invoke-IslGraphRequest.ps1 index c57102c..31762a0 100644 --- a/Private/Invoke-IslGraphRequest.ps1 +++ b/Private/Invoke-IslGraphRequest.ps1 @@ -5,11 +5,19 @@ function Invoke-IslGraphRequest { .DESCRIPTION A thin wrapper over Invoke-MgGraphRequest: one place that checks the Graph module is loaded - and connected, follows @odata.nextLink when -All is given, and is the single seam the unit - tests mock. The module does not depend on Microsoft.Graph.Authentication; the caller - installs it and runs Connect-MgGraph with DeviceManagementConfiguration.Read.All, - DeviceManagementApps.Read.All and DeviceManagementScripts.Read.All (plus GroupMember.Read.All - when assignments are inspected) before Test-IntuneDeployedScript is used. + and connected, follows @odata.nextLink when -All is given, retries a throttled or + unavailable answer, and is the single seam the unit tests mock. The module does not depend + on Microsoft.Graph.Authentication; the caller installs it and runs Connect-MgGraph with + DeviceManagementConfiguration.Read.All, DeviceManagementApps.Read.All and + DeviceManagementScripts.Read.All (plus GroupMember.Read.All when assignments are inspected) + before Test-IntuneDeployedScript is used. + + Graph answers 429 when a client is throttled and 503 or 504 when a service is briefly + unavailable, with a Retry-After header on the first. A request that gets one of those is + sent again up to three times, after the Retry-After seconds when the header is there and + after 2, 4 and 8 seconds when it is not; the fourth failure is thrown as it came. A tenant + command makes two requests per policy, so a pre-flight over a few hundred policies meets + the throttle without this. .PARAMETER Uri The request URI, absolute or relative to the Graph root, e.g. @@ -21,6 +29,9 @@ function Invoke-IslGraphRequest { .PARAMETER Body The request body for a POST. + .PARAMETER Headers + Request headers, e.g. @{ ConsistencyLevel = 'eventual' } for a $count query. + .PARAMETER All Follow @odata.nextLink and return every item of the collection instead of the first page. @@ -28,6 +39,12 @@ function Invoke-IslGraphRequest { Invoke-IslGraphRequest -Uri '/beta/deviceManagement/deviceHealthScripts?$select=id,displayName' -All Every remediation in the tenant, id and name only. + + .EXAMPLE + $countSplat = @{ Headers = @{ ConsistencyLevel = 'eventual' } } + Invoke-IslGraphRequest -Uri '/v1.0/groups/{id}/members/microsoft.graph.device/$count' @countSplat + + The number of devices among the group's members, as text. #> [CmdletBinding()] param( @@ -39,6 +56,8 @@ function Invoke-IslGraphRequest { [hashtable]$Body, + [hashtable]$Headers, + [switch]$All ) @@ -50,14 +69,59 @@ function Invoke-IslGraphRequest { } $requestSplat = @{ Method = $Method; Uri = $Uri; ErrorAction = 'Stop' } if ($Body) { $requestSplat.Body = $Body } + if ($Headers) { $requestSplat.Headers = $Headers } + + # How long to wait before sending a failed request again, or nothing when the failure is not + # one that passes. Invoke-MgGraphRequest's exception carries the status code and the response; + # both are read by name so a stand-in exception with the same members serves in the tests + function Get-RetryDelay { + param($ErrorRecord, [int]$Attempt) + $exception = $ErrorRecord.Exception + $status = 0 + if ($exception.PSObject.Properties['StatusCode'] -and $exception.StatusCode) { + $status = [int]$exception.StatusCode + } + $response = if ($exception.PSObject.Properties['Response']) { $exception.Response } else { $null } + if (-not $status -and $response) { $status = [int]$response.StatusCode } + if ($status -notin 429, 503, 504) { return $null } + + $delay = 0 + $retryAfter = if ($response) { $response.Headers.RetryAfter } else { $null } + if ($retryAfter) { + if ($null -ne $retryAfter.Delta) { $delay = [Math]::Ceiling($retryAfter.Delta.TotalSeconds) } + elseif ($null -ne $retryAfter.Date) { + $delay = [Math]::Ceiling(($retryAfter.Date.UtcDateTime - [datetime]::UtcNow).TotalSeconds) + } + } + if ($delay -lt 1) { $delay = [Math]::Pow(2, $Attempt) } + [int][Math]::Min($delay, 60) + } + + function Invoke-Once { + param([hashtable]$Splat) + $attempt = 0 + while ($true) { + try { + return Invoke-MgGraphRequest @Splat + } + catch { + $attempt++ + $delay = if ($attempt -le 3) { Get-RetryDelay -ErrorRecord $_ -Attempt $attempt } else { $null } + if ($null -eq $delay) { throw } + Write-Verbose ("Graph answered $($_.Exception.Message); retry $attempt of 3 in $delay s: " + + $Splat.Uri) + Start-Sleep -Seconds $delay + } + } + } - if (-not $All) { return Invoke-MgGraphRequest @requestSplat } + if (-not $All) { return Invoke-Once -Splat $requestSplat } $items = [System.Collections.Generic.List[object]]::new() $next = $Uri while ($next) { $requestSplat.Uri = $next - $page = Invoke-MgGraphRequest @requestSplat + $page = Invoke-Once -Splat $requestSplat if ($page.value) { $items.AddRange([object[]]$page.value) } $next = $page.'@odata.nextLink' } diff --git a/Public/Test-IntuneDeployedScript.ps1 b/Public/Test-IntuneDeployedScript.ps1 index 8c64e73..a4351cf 100644 --- a/Public/Test-IntuneDeployedScript.ps1 +++ b/Public/Test-IntuneDeployedScript.ps1 @@ -129,10 +129,15 @@ function Test-IntuneDeployedScript { $isDeviceGroup = $false if (-not $SkipGroupLookup -and -not $groupState.Blocked) { try { - $members = Invoke-IslGraphRequest -Uri "/v1.0/groups/$GroupId/members?`$select=id&`$top=20" - $types = @($members.value | ForEach-Object { "$($_.'@odata.type')" }) - $others = @($types | Where-Object { $_ -ne '#microsoft.graph.device' }) - $isDeviceGroup = $types.Count -gt 0 -and $others.Count -eq 0 + # Two counts, every member and the devices among them, rather than a page of + # members: a page of 20 decided for the whole group, and a mixed group whose first + # 20 members were devices passed as a device group. $count needs the eventual + # consistency header and answers with the number as text (dev tenant, 2026-10-06) + $countSplat = @{ Headers = @{ ConsistencyLevel = 'eventual' } } + $total = [int](Invoke-IslGraphRequest -Uri "/v1.0/groups/$GroupId/members/`$count" @countSplat) + $deviceUri = "/v1.0/groups/$GroupId/members/microsoft.graph.device/`$count" + $devices = [int](Invoke-IslGraphRequest -Uri $deviceUri @countSplat) + $isDeviceGroup = $total -gt 0 -and $devices -eq $total } catch { $groupState.Blocked = $true diff --git a/Tests/Unit/Private/Invoke-IslGraphRequest.Tests.ps1 b/Tests/Unit/Private/Invoke-IslGraphRequest.Tests.ps1 index 9455ad9..e66d283 100644 --- a/Tests/Unit/Private/Invoke-IslGraphRequest.Tests.ps1 +++ b/Tests/Unit/Private/Invoke-IslGraphRequest.Tests.ps1 @@ -12,11 +12,34 @@ BeforeAll { if (-not (Get-Command Invoke-MgGraphRequest -ErrorAction SilentlyContinue)) { # The parameters exist so the mocks' parameter filters can bind to them function global:Invoke-MgGraphRequest { - param($Method, $Uri, $Body, $ErrorAction) - $null = $Method, $Uri, $Body, $ErrorAction + param($Method, $Uri, $Body, $Headers, $ErrorAction) + $null = $Method, $Uri, $Body, $Headers, $ErrorAction } $script:StubCreated = $true } + # What Invoke-MgGraphRequest throws, as far as the seam reads it: a StatusCode and a Response + # whose headers may carry Retry-After. Windows PowerShell 5.1 loads System.Net.Http on demand + Add-Type -AssemblyName System.Net.Http -ErrorAction SilentlyContinue + function script:New-GraphFailure { + param([int]$StatusCode, [int]$RetryAfterSeconds) + # .NET Framework's HttpStatusCode has no name for 429; the enum takes the number all the same + $code = [System.Enum]::ToObject([System.Net.HttpStatusCode], $StatusCode) + $response = [System.Net.Http.HttpResponseMessage]::new($code) + if ($RetryAfterSeconds) { + $delta = [timespan]::FromSeconds($RetryAfterSeconds) + $response.Headers.RetryAfter = [System.Net.Http.Headers.RetryConditionHeaderValue]::new($delta) + } + $failure = [FakeGraphException]::new("Response status code does not indicate success: $StatusCode") + $failure.StatusCode = $code + $failure.Response = $response + $failure + } +} + +class FakeGraphException : System.Exception { + $StatusCode + $Response + FakeGraphException([string]$message) : base($message) { } } AfterAll { @@ -58,4 +81,83 @@ Describe 'Invoke-IslGraphRequest' -Tag 'Unit', 'Private' { Mock Invoke-MgGraphRequest -ModuleName IntuneScriptLab { @{ value = @() } } @(InModuleScope IntuneScriptLab { Invoke-IslGraphRequest -Uri '/beta/things' -All }).Count | Should-Be 0 } + + It 'passes request headers through' { + Mock Invoke-MgGraphRequest -ModuleName IntuneScriptLab { '17' } + $reply = InModuleScope IntuneScriptLab { + Invoke-IslGraphRequest -Uri '/v1.0/groups/g/members/$count' -Headers @{ ConsistencyLevel = 'eventual' } + } + $reply | Should-Be '17' + Should-Invoke Invoke-MgGraphRequest -ModuleName IntuneScriptLab -Times 1 -Exactly -ParameterFilter { + $Headers.ConsistencyLevel -eq 'eventual' + } + } + + Context 'Retry' { + BeforeEach { + Mock Start-Sleep -ModuleName IntuneScriptLab { } + $script:Failures = [System.Collections.Generic.List[object]]::new() + # Throws the next queued failure, or returns the answer given + function script:Invoke-NextFailure { + param($Answer) + if ($script:Failures.Count) { + $next = $script:Failures[0] + $script:Failures.RemoveAt(0) + throw $next + } + $Answer + } + } + + It 'sends a throttled request again after the Retry-After seconds, then returns the answer' { + $script:Failures.Add((New-GraphFailure -StatusCode 429 -RetryAfterSeconds 7)) + $script:Failures.Add((New-GraphFailure -StatusCode 429 -RetryAfterSeconds 3)) + Mock Invoke-MgGraphRequest -ModuleName IntuneScriptLab { Invoke-NextFailure -Answer @{ id = 'x' } } + $reply = InModuleScope IntuneScriptLab { Invoke-IslGraphRequest -Uri '/beta/things/x' } + $reply.id | Should-Be 'x' + Should-Invoke Invoke-MgGraphRequest -ModuleName IntuneScriptLab -Times 3 -Exactly + $sleepSplat = @{ ModuleName = 'IntuneScriptLab'; Times = 1; Exactly = $true } + Should-Invoke Start-Sleep @sleepSplat -ParameterFilter { $Seconds -eq 7 } + Should-Invoke Start-Sleep @sleepSplat -ParameterFilter { $Seconds -eq 3 } + } + + It 'backs off 2, 4 and 8 seconds for without Retry-After, then throws the fourth' -ForEach @( + @{ Status = 503 } + @{ Status = 504 } + ) { + 1..4 | ForEach-Object { $script:Failures.Add((New-GraphFailure -StatusCode $Status)) } + Mock Invoke-MgGraphRequest -ModuleName IntuneScriptLab { Invoke-NextFailure } + { InModuleScope IntuneScriptLab { Invoke-IslGraphRequest -Uri '/beta/things' } } | + Should-Throw -ExceptionMessage "*$Status*" + Should-Invoke Invoke-MgGraphRequest -ModuleName IntuneScriptLab -Times 4 -Exactly + foreach ($seconds in 2, 4, 8) { + $expected = $seconds + Should-Invoke Start-Sleep -ModuleName IntuneScriptLab -Times 1 -Exactly -ParameterFilter { + $Seconds -eq $expected + } + } + } + + It 'throws any other failure at once' { + $script:Failures.Add((New-GraphFailure -StatusCode 404)) + Mock Invoke-MgGraphRequest -ModuleName IntuneScriptLab { Invoke-NextFailure } + { InModuleScope IntuneScriptLab { Invoke-IslGraphRequest -Uri '/beta/things/missing' } } | + Should-Throw -ExceptionMessage '*404*' + Should-Invoke Invoke-MgGraphRequest -ModuleName IntuneScriptLab -Times 1 -Exactly + Should-Invoke Start-Sleep -ModuleName IntuneScriptLab -Times 0 -Exactly + } + + It 'retries a page in the middle of -All and keeps every item' { + $script:Failures.Add((New-GraphFailure -StatusCode 429 -RetryAfterSeconds 1)) + Mock Invoke-MgGraphRequest -ModuleName IntuneScriptLab { + if ($Uri -like '*skiptoken*') { Invoke-NextFailure -Answer @{ value = @(@{ id = 3 }) } } + else { @{ value = @(@{ id = 1 }, @{ id = 2 }); '@odata.nextLink' = "$Uri&`$skiptoken=abc" } } + } + $items = @(InModuleScope IntuneScriptLab { + Invoke-IslGraphRequest -Uri '/beta/things?$select=id' -All + }) + $items.id | Should-BeCollection @(1, 2, 3) + Should-Invoke Invoke-MgGraphRequest -ModuleName IntuneScriptLab -Times 3 -Exactly + } + } } diff --git a/Tests/Unit/Public/Test-IntuneDeployedScript.Tests.ps1 b/Tests/Unit/Public/Test-IntuneDeployedScript.Tests.ps1 index a3fc4bf..841516a 100644 --- a/Tests/Unit/Public/Test-IntuneDeployedScript.Tests.ps1 +++ b/Tests/Unit/Public/Test-IntuneDeployedScript.Tests.ps1 @@ -1,4 +1,4 @@ -#Requires -Modules @{ ModuleName = 'Pester'; ModuleVersion = '6.2.0' } +#Requires -Modules @{ ModuleName = 'Pester'; ModuleVersion = '6.2.0' } <# The Graph pre-flight against a fake tenant: the Graph seam (Invoke-IslGraphRequest) is mocked @@ -143,9 +143,13 @@ Describe 'Test-IntuneDeployedScript' -Tag 'Unit', 'Public' { } '^/beta/deviceAppManagement/mobileApps\?' { & $summary $tenant.apps } '^/beta/deviceAppManagement/mobileApps/([^?]+)' { $tenant.apps | Where-Object id -eq $Matches[1] } - '^/v1.0/groups/([^/]+)/members' { + '^/v1.0/groups/([^/]+)/members(/microsoft\.graph\.device)?/\$count$' { + # $count answers with the number as text; the device cast counts devices only if (-not $tenant.groups.ContainsKey($Matches[1])) { throw 'Insufficient privileges' } - $tenant.groups[$Matches[1]] + if ($Headers.ConsistencyLevel -ne 'eventual') { throw 'ConsistencyLevel header is required' } + $members = @($tenant.groups[$Matches[1]].value) + $devices = @($members | Where-Object { $_.'@odata.type' -eq '#microsoft.graph.device' }) + if ($Matches[2]) { "$($devices.Count)" } else { "$($members.Count)" } } default { throw "unexpected uri $Uri" } } @@ -206,9 +210,22 @@ Describe 'Test-IntuneDeployedScript' -Tag 'Unit', 'Public' { $findings[0].Severity | Should-Be 'Warning' $findings[0].Message | Should-BeLikeString '*assigned to devices (*grp-devices*' $findings[0].Message | Should-BeLikeString '*all devices*' + # Two counts per group, members and devices, once per group however often it is assigned Should-Invoke Invoke-IslGraphRequest -ModuleName IntuneScriptLab -ParameterFilter { $Uri -like '/v1.0/groups/*' - } -Times 1 -Exactly + } -Times 2 -Exactly + } + + It 'does not take a mixed group for a device group, however many devices lead its members' { + # The check used to read the first page of 20 members: a group of 20 devices and 5 users + # passed as a device group. The counts say 25 members, 20 devices + Mock Invoke-IslGraphRequest -ModuleName IntuneScriptLab -ParameterFilter { + $Uri -like '/v1.0/groups/grp-devices/members/*$count' + } { if ($Uri -like '*microsoft.graph.device*') { '20' } else { '25' } } + $findings = @(Test-IntuneDeployedScript -Kind Win32App -IncludeRule IslAssignmentIssue) + $findings.Count | Should-Be 1 + $findings[0].Message | Should-BeLikeString '*assigned to devices (all devices)*' + $findings[0].Message | Should-NotBeLikeString '*grp-devices*' } It 'notes a remediation without a remediation script (REM-DETECTONLY)' {