Improve CA policy location and service exception handling

JohnDuprey · JohnDuprey · commit f86aad31dad2 · 2025-12-18T18:23:55.000-05:00
Refactored location processing in New-CIPPCATemplate to reduce redundant API calls and ensure unique location info. Enhanced Set-CIPPCAPolicyServiceException with clearer logic, improved string handling, and added more informative logging. Minor bug fixes and code style improvements in Invoke-ExecCAServiceExclusion and New-CIPPCAPolicy for robustness.
diff --git a/Modules/CIPPCore/Public/Entrypoints/HTTP Functions/Tenant/Conditional/Invoke-ExecCAServiceExclusion.ps1 b/Modules/CIPPCore/Public/Entrypoints/HTTP Functions/Tenant/Conditional/Invoke-ExecCAServiceExclusion.ps1
@@ -1,4 +1,4 @@
-Function Invoke-ExecCAServiceExclusion {
+function Invoke-ExecCAServiceExclusion {
     <#
     .FUNCTIONALITY
         Entrypoint
@@ -17,15 +17,15 @@ Function Invoke-ExecCAServiceExclusion {
     try {
         $result = Set-CIPPCAPolicyServiceException -TenantFilter $TenantFilter -PolicyId $ID
         $Body = @{ Results = $result }
-        Write-LogMessage -headers $Headers -API 'Set-CIPPCAPolicyServiceException' -message $Message -Sev 'Info' -tenant $TenantFilter
+        Write-LogMessage -headers $Headers -API 'Set-CIPPCAPolicyServiceException' -message $result -Sev 'Info' -tenant $TenantFilter
     } catch {
         $ErrorMessage = Get-CippException -Exception $_
         $Body = @{ Results = "Failed to add service provider exception to policy $($ID): $($ErrorMessage.NormalizedError)" }
         Write-LogMessage -headers $Headers -API 'Set-CIPPCAPolicyServiceException' -message "Failed to update policy $($PolicyId) with service provider exception for tenant $($CSPtenantId): $($_.Exception.Message)" -Sev 'Error' -tenant $TenantFilter -LogData (Get-CippException -Exception $_)
     }
 
     return ([HttpResponseContext]@{
-        StatusCode = [HttpStatusCode]::OK
-        Body       = $Body
-    })
+            StatusCode = [HttpStatusCode]::OK
+            Body       = $Body
+        })
 }
diff --git a/Modules/CIPPCore/Public/New-CIPPCAPolicy.ps1 b/Modules/CIPPCore/Public/New-CIPPCAPolicy.ps1
@@ -204,6 +204,7 @@ function New-CIPPCAPolicy {
     Write-Information ($LocationLookupTable | ConvertTo-Json -Depth 10)
 
     foreach ($location in $JSONobj.conditions.locations.includeLocations) {
+        if ($null -eq $location) { continue }
         $lookup = $LocationLookupTable | Where-Object { $_.name -eq $location -or $_.displayName -eq $location -or $_.templateId -eq $location }
         if (!$lookup) { continue }
         Write-Information "Replacing named location - $location"
@@ -212,6 +213,7 @@ function New-CIPPCAPolicy {
     }
 
     foreach ($location in $JSONobj.conditions.locations.excludeLocations) {
+        if ($null -eq $location) { continue }
         $lookup = $LocationLookupTable | Where-Object { $_.name -eq $location -or $_.displayName -eq $location -or $_.templateId -eq $location }
         if (!$lookup) { continue }
         Write-Information "Replacing named location - $location"
diff --git a/Modules/CIPPCore/Public/New-CIPPCATemplate.ps1 b/Modules/CIPPCore/Public/New-CIPPCATemplate.ps1
@@ -13,6 +13,15 @@ function New-CIPPCATemplate {
         $NonEmptyProperties = $_.psobject.Properties | Where-Object { $null -ne $_.Value } | Select-Object -ExpandProperty Name
         $_ | Select-Object -Property $NonEmptyProperties
     }
+
+    Write-Information "Processing CA Template for tenant $TenantFilter"
+    Write-Information ($JSON | ConvertTo-Json -Depth 10)
+
+    # Function to check if a string is a GUID
+    function Test-IsGuid($string) {
+        return [guid]::tryparse($string, [ref][guid]::Empty)
+    }
+
     if ($preloadedUsers) {
         $users = $preloadedUsers
     } else {
@@ -23,18 +32,25 @@ function New-CIPPCATemplate {
     } else {
         $groups = (New-GraphGetRequest -uri "https://graph.microsoft.com/beta/groups?`$top=999&`$select=displayName,id" -tenantid $TenantFilter)
     }
-    $includelocations = New-Object System.Collections.ArrayList
+
+    $namedLocations = $null
+    if ($JSON.conditions.locations.includeLocations -or $JSON.conditions.locations.excludeLocations) {
+        $namedLocations = New-GraphGetRequest -uri 'https://graph.microsoft.com/beta/identity/conditionalAccess/namedLocations' -tenantid $TenantFilter
+    }
+
+    $AllLocations = [system.collections.generic.list[object]]::new()
+
+    $includelocations = [system.collections.generic.list[object]]::new()
     $IncludeJSON = foreach ($Location in $JSON.conditions.locations.includeLocations) {
-        $locationinfo = New-GraphGetRequest -uri 'https://graph.microsoft.com/beta/identity/conditionalAccess/namedLocations' -tenantid $TenantFilter | Where-Object -Property id -EQ $location | Select-Object * -ExcludeProperty id, *time*
+        $locationinfo = $namedLocations | Where-Object -Property id -EQ $location | Select-Object * -ExcludeProperty id, *time*
         $null = if ($locationinfo) { $includelocations.add($locationinfo.displayName) } else { $includelocations.add($location) }
         $locationinfo
     }
     if ($includelocations) { $JSON.conditions.locations.includeLocations = $includelocations }
 
-
-    $excludelocations = New-Object System.Collections.ArrayList
+    $excludelocations = [system.collections.generic.list[object]]::new()
     $ExcludeJSON = foreach ($Location in $JSON.conditions.locations.excludeLocations) {
-        $locationinfo = New-GraphGetRequest -uri 'https://graph.microsoft.com/beta/identity/conditionalAccess/namedLocations' -tenantid $TenantFilter | Where-Object -Property id -EQ $location | Select-Object * -ExcludeProperty id, *time*
+        $locationinfo = $namedLocations | Where-Object -Property id -EQ $location | Select-Object * -ExcludeProperty id, *time*
         $null = if ($locationinfo) { $excludelocations.add($locationinfo.displayName) } else { $excludelocations.add($location) }
         $locationinfo
     }
@@ -44,59 +60,45 @@ function New-CIPPCATemplate {
         $JSON.conditions.users.includeUsers = @($JSON.conditions.users.includeUsers | ForEach-Object {
                 $originalID = $_
                 if ($_ -in 'All', 'None', 'GuestOrExternalUsers') { return $_ }
-                try {
-                    ($users | Where-Object { $_.id -eq $_ }).displayName
-                } catch {
-                    return $originalID
-                }
+                $match = $users | Where-Object { $_.id -eq $originalID }
+                if ($match) { $match.displayName } else { $originalID }
             })
     }
 
     if ($JSON.conditions.users.excludeUsers) {
         $JSON.conditions.users.excludeUsers = @($JSON.conditions.users.excludeUsers | ForEach-Object {
                 if ($_ -in 'All', 'None', 'GuestOrExternalUsers') { return $_ }
                 $originalID = $_
-
-                try {
-                    ($users | Where-Object { $_.id -eq $_ }).displayName
-                } catch {
-                    return $originalID
-                }
+                $match = $users | Where-Object { $_.id -eq $originalID }
+                if ($match) { $match.displayName } else { $originalID }
             })
     }
 
-    # Function to check if a string is a GUID
-    function Test-IsGuid($string) {
-        return [guid]::tryparse($string, [ref][guid]::Empty)
-    }
-
     if ($JSON.conditions.users.includeGroups) {
         $JSON.conditions.users.includeGroups = @($JSON.conditions.users.includeGroups | ForEach-Object {
                 $originalID = $_
                 if ($_ -in 'All', 'None', 'GuestOrExternalUsers' -or -not (Test-IsGuid $_)) { return $_ }
-                try {
-                    ($groups | Where-Object { $_.id -eq $_ }).displayName
-                } catch {
-                    return $originalID
-                }
+                $match = $groups | Where-Object { $_.id -eq $originalID }
+                if ($match) { $match.displayName } else { $originalID }
             })
     }
     if ($JSON.conditions.users.excludeGroups) {
         $JSON.conditions.users.excludeGroups = @($JSON.conditions.users.excludeGroups | ForEach-Object {
                 $originalID = $_
-
                 if ($_ -in 'All', 'None', 'GuestOrExternalUsers' -or -not (Test-IsGuid $_)) { return $_ }
-                try {
-                    ($groups | Where-Object { $_.id -eq $_ }).displayName
-                } catch {
-                    return $originalID
-
-                }
+                $match = $groups | Where-Object { $_.id -eq $originalID }
+                if ($match) { $match.displayName } else { $originalID }
             })
     }
 
-    $JSON | Add-Member -NotePropertyName 'LocationInfo' -NotePropertyValue @($IncludeJSON, $ExcludeJSON)
+    foreach ($Location in $IncludeJSON) {
+        $AllLocations.Add($Location)
+    }
+    foreach ($Location in $ExcludeJSON) {
+        $AllLocations.Add($Location)
+    }
 
+    $JSON | Add-Member -NotePropertyName 'LocationInfo' -NotePropertyValue @($AllLocations | Select-Object -Unique) -Force
     $JSON = (ConvertTo-Json -Compress -Depth 100 -InputObject $JSON)
     return $JSON
 }
diff --git a/Modules/CIPPCore/Public/Set-CIPPCAPolicyServiceException.ps1 b/Modules/CIPPCore/Public/Set-CIPPCAPolicyServiceException.ps1
@@ -11,20 +11,20 @@ function Set-CIPPCAPolicyServiceException {
     $policy = New-GraphGETRequest -uri "https://graph.microsoft.com/beta/identity/conditionalAccess/policies/$($PolicyId)" -tenantid $TenantFilter -AsApp $true
 
     # If the policy is set to affect either all or all guests/external users
-    if ($policy.conditions.users.includeUsers -eq "All" -OR $policy.conditions.users.includeGuestsOrExternalUsers.externalTenants.membershipKind -eq "all") {
+    if ($policy.conditions.users.includeUsers -eq 'All' -or $policy.conditions.users.includeGuestsOrExternalUsers.externalTenants.membershipKind -eq 'all') {
 
         # Check if the policy already has the correct service provider exception
         if ($policy.conditions.users.excludeGuestsOrExternalUsers) {
             $excludeConfig = $policy.conditions.users.excludeGuestsOrExternalUsers
 
             # Check if serviceProvider is already in guestOrExternalUserTypes
-            $hasServiceProvider = $excludeConfig.guestOrExternalUserTypes -match "serviceProvider"
+            $hasServiceProvider = $excludeConfig.guestOrExternalUserTypes -match 'serviceProvider'
 
             # Check if externalTenants is properly configured
             if ($excludeConfig.externalTenants) {
                 $externalTenants = $excludeConfig.externalTenants
-                $hasCorrectExternalTenants = ($externalTenants.membershipKind -eq "enumerated" -and
-                                           $externalTenants.members -contains $CSPtenantId)
+                $hasCorrectExternalTenants = ($externalTenants.membershipKind -eq 'enumerated' -and
+                    $externalTenants.members -contains $CSPtenantId)
 
                 # If already configured, exit without making changes
                 if ($hasServiceProvider -and $hasCorrectExternalTenants) {
@@ -38,11 +38,11 @@ function Set-CIPPCAPolicyServiceException {
 
             # Define data
             $excludeServiceProviderData = [pscustomobject]@{
-                guestOrExternalUserTypes = "serviceProvider"
-                externalTenants = [pscustomobject]@{
-                    '@odata.type' = "#microsoft.graph.conditionalAccessEnumeratedExternalTenants"
-                    membershipKind = "enumerated"
-                    members = @(
+                guestOrExternalUserTypes = 'serviceProvider'
+                externalTenants          = [pscustomobject]@{
+                    '@odata.type'  = '#microsoft.graph.conditionalAccessEnumeratedExternalTenants'
+                    membershipKind = 'enumerated'
+                    members        = @(
                         $CSPtenantId
                     )
                 }
@@ -56,27 +56,31 @@ function Set-CIPPCAPolicyServiceException {
         if ($policy.conditions.users.excludeGuestsOrExternalUsers) {
 
             # If guestOrExternalUserTypes doesn't include type serviceProvider add it
-            if ($policy.conditions.users.excludeGuestsOrExternalUsers.guestOrExternalUserTypes -notmatch "serviceProvider") {
-                $policy.conditions.users.excludeGuestsOrExternalUsers.guestOrExternalUserTypes += ",serviceProvider"
+            if ($policy.conditions.users.excludeGuestsOrExternalUsers.guestOrExternalUserTypes -notmatch 'serviceProvider') {
+                $policy.conditions.users.excludeGuestsOrExternalUsers.guestOrExternalUserTypes += ',serviceProvider'
             }
 
             # If guestOrExternalUserTypes includes type serviceProvider and membershipKind is not all tenants
-            if ($policy.conditions.users.excludeGuestsOrExternalUsers.guestOrExternalUserTypes -match "serviceProvider" -AND $policy.conditions.users.excludeGuestsOrExternalUsers.externalTenants.membershipKind -ne "all") {
+            if ($policy.conditions.users.excludeGuestsOrExternalUsers.guestOrExternalUserTypes -match 'serviceProvider' -and $policy.conditions.users.excludeGuestsOrExternalUsers.externalTenants.membershipKind -ne 'all') {
 
                 # If membershipKind is enumerated and members does not include our tenant add it
-                if ($policy.conditions.users.excludeGuestsOrExternalUsers.externalTenants.membershipKind -eq "enumerated" -AND $policy.conditions.users.excludeGuestsOrExternalUsers.externalTenants.members -notmatch $CSPtenantId) {
+                if ($policy.conditions.users.excludeGuestsOrExternalUsers.externalTenants.membershipKind -eq 'enumerated' -and $policy.conditions.users.excludeGuestsOrExternalUsers.externalTenants.members -notmatch $CSPtenantId) {
                     $policy.conditions.users.excludeGuestsOrExternalUsers.externalTenants.members += $($CSPtenantId)
                 }
             }
         }
+        $Json = $policy | Select-Object * -ExcludeProperty TemplateId, createdDateTime, modifiedDateTime | ConvertTo-Json -Depth 20
 
-    }
+        Write-Information 'Updated policy JSON:'
+        Write-Information $Json
 
-    # Patch policy with updated data.
-    # TemplateId,createdDateTime,modifiedDateTime can't be written back so exclude them using -ExcludeProperty
-    if ($PSCmdlet.ShouldProcess($PolicyId, "Update policy with service provider exception")) {
-        $patch = New-GraphPOSTRequest -uri "https://graph.microsoft.com/beta/identity/conditionalAccess/policies/$($policy.id)" -tenantid $TenantFilter -type PATCH -body ($policy | Select-Object * -ExcludeProperty TemplateId,createdDateTime,modifiedDateTime | ConvertTo-Json -Depth 20) -AsApp $true
-        return "Successfully added service provider to policy $PolicyId"
+        # Patch policy with updated data.
+        # TemplateId,createdDateTime,modifiedDateTime can't be written back so exclude them using -ExcludeProperty
+        if ($PSCmdlet.ShouldProcess($PolicyId, 'Update policy with service provider exception')) {
+            $patch = New-GraphPOSTRequest -uri "https://graph.microsoft.com/beta/identity/conditionalAccess/policies/$($policy.id)" -tenantid $TenantFilter -type PATCH -body ($policy | Select-Object * -ExcludeProperty TemplateId, createdDateTime, modifiedDateTime | ConvertTo-Json -Depth 20) -AsApp $true
+            return "Successfully added service provider to policy $PolicyId"
+        }
+    } else {
+        return "Policy $PolicyId does not target all users or all guest/external users. No changes made."
     }
-
 }
