From c30ef6ec24520ced1a5d4939235213b89bcf2ec7 Mon Sep 17 00:00:00 2001 From: Dave Date: Fri, 21 Aug 2026 00:50:56 -0400 Subject: [PATCH] push --- .../Invoke-PersonaGraphRequest.ps1 | 43 +++++++++++-------- src/Engine/Invoke-PersonaEngineRun.ps1 | 4 +- tests/Unit/ExitCodes.Tests.ps1 | 34 +++++++++++++-- 3 files changed, 57 insertions(+), 24 deletions(-) diff --git a/src/DataProviders/Invoke-PersonaGraphRequest.ps1 b/src/DataProviders/Invoke-PersonaGraphRequest.ps1 index 9fc8157..9419b9a 100644 --- a/src/DataProviders/Invoke-PersonaGraphRequest.ps1 +++ b/src/DataProviders/Invoke-PersonaGraphRequest.ps1 @@ -149,40 +149,47 @@ function Get-PersonaGraphStatusCode { return $null } -function Test-PersonaTargetAttributeUnavailable { +function Test-PersonaEnumerationRecoverable { <# .SYNOPSIS - True when a Graph 400 means the target attribute does not exist in this - tenant, rather than some other client error. + True when a Graph 400 during user enumeration is plausibly caused by the + target attribute not existing in this tenant, and safe to retry without it. .DESCRIPTION - A dev tenant with no app registration has no persona extension property. - Requesting it in $select then fails with 400 and a message naming the - property. This distinguishes that specific, recoverable case from every - other 400 (bad query syntax, an unrelated bad property, a permission denial - phrased as 400), which must still fail loudly rather than being swallowed. + A dev tenant with no app registration has no persona extension property, and + requesting it in $select then fails with 400. The obvious approach - reading + the offending property name out of the error - does not hold up in practice: + Graph often returns a 400 with an empty body, and Invoke-MgGraphRequest then + reports only "Response status code does not indicate success: BadRequest + (Bad Request)." with no property name and not even a numeric status in the + text (Get-PersonaGraphStatusCode still resolves it, from the structured + Response.StatusCode rather than the message). + + So this checks the only two facts actually available: the status was 400, + and the target attribute was one of the properties requested. That is not + proof the attribute is the cause, but confirming it is impossible from what + Graph sends back, and the retry this justifies is cheap, read-only, and + confined to preview mode - if the attribute was not the problem, the retry + fails too and the original error still surfaces untouched. .PARAMETER ErrorRecord - The error caught from Invoke-PersonaGraphRequest. + The error caught from Get-PersonaUsers. .PARAMETER TargetAttribute The configured target attribute name. + + .PARAMETER SelectProperties + The $select list the failing request used. #> [CmdletBinding()] [OutputType([bool])] param( [Parameter(Mandatory)] $ErrorRecord, - [Parameter(Mandatory)] [string] $TargetAttribute + [Parameter(Mandatory)] [string] $TargetAttribute, + [Parameter(Mandatory)] [AllowEmptyCollection()] [string[]] $SelectProperties ) - if ((Get-PersonaGraphStatusCode -ErrorRecord $ErrorRecord) -ne 400) { return $false } - - $text = @($ErrorRecord.Exception.Message, $ErrorRecord.ErrorDetails.Message) -join ' ' - if ([string]::IsNullOrWhiteSpace($text)) { return $false } - - if ($text -notmatch [regex]::Escape($TargetAttribute)) { return $false } - - [bool]($text -match '(?i)could not find a property|invalid property|is not a valid property|does not exist on type') + ((Get-PersonaGraphStatusCode -ErrorRecord $ErrorRecord) -eq 400) -and ($SelectProperties -ccontains $TargetAttribute) } function Get-PersonaRetryAfterMs { diff --git a/src/Engine/Invoke-PersonaEngineRun.ps1 b/src/Engine/Invoke-PersonaEngineRun.ps1 index d6ce285..5030240 100644 --- a/src/Engine/Invoke-PersonaEngineRun.ps1 +++ b/src/Engine/Invoke-PersonaEngineRun.ps1 @@ -98,7 +98,7 @@ function Invoke-PersonaEngineRun { : @(Get-PersonaUsers -SelectProperties $selectProperties) } catch { - if ((-not $IsEnforcing) -and (Test-PersonaTargetAttributeUnavailable -ErrorRecord $_ -TargetAttribute $TargetAttribute)) { + if ((-not $IsEnforcing) -and (Test-PersonaEnumerationRecoverable -ErrorRecord $_ -TargetAttribute $TargetAttribute -SelectProperties $selectProperties)) { # Dev/test tenants often have no app registration yet, so the persona # extension property was never created. Preview mode never writes # regardless of what StoredPersona holds, so treating the attribute as @@ -106,7 +106,7 @@ function Invoke-PersonaEngineRun { # Enforcement still fails loudly - IsEnforcing gates this precisely # because writing to an attribute that does not exist must never be # silently tolerated. - Write-Warning "Target attribute '$TargetAttribute' was not found in this tenant (no app registration / extension property?). Continuing in What-If mode with it treated as null for every account." + Write-Warning "Graph rejected the request with a 400 while '$TargetAttribute' was in the requested properties. Retrying without it and treating it as null for this What-If run - if the retry also fails, the underlying error will be reported." $fallbackProperties = @($selectProperties | Where-Object { $_ -cne $TargetAttribute }) diff --git a/tests/Unit/ExitCodes.Tests.ps1 b/tests/Unit/ExitCodes.Tests.ps1 index c6bf333..a1d0afd 100644 --- a/tests/Unit/ExitCodes.Tests.ps1 +++ b/tests/Unit/ExitCodes.Tests.ps1 @@ -145,7 +145,17 @@ Describe 'Exit codes produced by the run loop' { Describe 'Target attribute unavailable - dev tenants without an app registration' { BeforeAll { - $script:unavailableError = "Response status code does not indicate success: 400 (Bad Request): Could not find a property named '$($script:target)' on type 'microsoft.graph.user'." + # Mirrors what Graph actually sends for a bad $select property: a 400 with + # an empty body. Invoke-MgGraphRequest's message then names no property and + # carries no digits at all ("BadRequest", not "400") - the status is only + # recoverable from the structured Response.StatusCode, never from text. A + # fixture that embedded "400" or the property name in the message text would + # test a shape Graph does not actually produce. + function New-PersonaGraphBadRequestError { + $ex = [System.Exception]::new('Response status code does not indicate success: BadRequest (Bad Request).') + $ex | Add-Member -MemberType NoteProperty -Name Response -Value ([pscustomobject]@{ StatusCode = 400 }) -Force + [System.Management.Automation.ErrorRecord]::new($ex, 'GraphError', [System.Management.Automation.ErrorCategory]::InvalidResult, $null) + } } BeforeEach { @@ -157,7 +167,7 @@ Describe 'Target attribute unavailable - dev tenants without an app registration It 'continues in What-If mode, treating the attribute as null, when it is not registered' { Mock Get-PersonaUsers { param($SelectProperties, $UserObjectId, $PageSize) - if ($SelectProperties -ccontains $script:target) { throw $script:unavailableError } + if ($SelectProperties -ccontains $script:target) { throw (New-PersonaGraphBadRequestError) } @(New-TestPopulation -Count 5 -TargetAttribute $script:target) } @@ -173,7 +183,7 @@ Describe 'Target attribute unavailable - dev tenants without an app registration It 'requests everything except the target attribute on the retry' { Mock Get-PersonaUsers { param($SelectProperties, $UserObjectId, $PageSize) - if ($SelectProperties -ccontains $script:target) { throw $script:unavailableError } + if ($SelectProperties -ccontains $script:target) { throw (New-PersonaGraphBadRequestError) } @(New-TestPopulation -Count 5 -TargetAttribute $script:target) } @@ -183,8 +193,24 @@ Describe 'Target attribute unavailable - dev tenants without an app registration Should -Invoke Get-PersonaUsers -Times 1 -Exactly -ParameterFilter { $SelectProperties -cnotcontains $script:target } } + It 'also recovers for a single-user run (-UserObjectId)' { + Mock Get-PersonaUsers { + param($SelectProperties, $UserObjectId, $PageSize) + if ($SelectProperties -ccontains $script:target) { throw (New-PersonaGraphBadRequestError) } + @(New-TestPopulation -Count 1 -TargetAttribute $script:target) + } + + $outcome = Invoke-PersonaEngineRun -Configuration (New-TestRuntimeConfiguration -TargetAttribute $script:target) ` + -TargetAttribute $script:target -Context (New-TestAuditContext) -IsEnforcing:$false ` + -UserObjectId '00000000-0000-0000-0000-000000000001' + + $outcome.ExitCode | Should -Be 0 + $outcome.Counters.Processed | Should -Be 1 + Should -Invoke Get-PersonaUsers -Times 2 -Exactly -ParameterFilter { $UserObjectId -eq '00000000-0000-0000-0000-000000000001' } + } + It 'still fails enumeration in enforcement mode - the fallback never applies to a real write run' { - Mock Get-PersonaUsers { throw $script:unavailableError } + Mock Get-PersonaUsers { throw (New-PersonaGraphBadRequestError) } $outcome = Invoke-PersonaEngineRun -Configuration (New-TestRuntimeConfiguration -TargetAttribute $script:target) ` -TargetAttribute $script:target -Context (New-TestAuditContext -Mode 'Enforce') -IsEnforcing