Merge pull request #1097 from KelvinTegelaar/dev

[pull] dev from KelvinTegelaar:dev

Author: pull[bot]

Config/SAMManifest.json

@@ -582,6 +582,34 @@
         {
           "id": "b7887744-6746-4312-813d-72daeaee7e2d",
           "type": "Scope"
+        },
+        {
+          "id": "dd199f4a-f148-40a4-a2ec-f0069cc799ec",
+          "type": "Role"
+        },
+        {
+          "id": "8c026be3-8e26-4774-9372-8d5d6f21daff",
+          "type": "Scope"
+        },
+        {
+          "id": "fee28b28-e1f3-4841-818e-2704dc62245f",
+          "type": "Role"
+        },
+        {
+          "id": "62ade113-f8e0-4bf9-a6ba-5acb31db32fd",
+          "type": "Scope"
+        },
+        {
+          "id": "9e3f62cf-ca93-4989-b6ce-bf83c28f9fe8",
+          "type": "Role"
+        },
+        {
+          "id": "31e08e0a-d3f7-4ca2-ac39-7343fb83e8ad",
+          "type": "Role"
+        },
+        {
+          "id": "1ff1be21-34eb-448c-9ac9-ce1f506b2a68",
+          "type": "Scope"
         }
       ]
     },
@@ -689,4 +717,4 @@
     "isEnabled": true,
     "allProperties": true
   }
-}
+}

Modules/CIPPCore/Public/Get-CIPPTestData.ps1

@@ -4,23 +4,80 @@ function Get-CIPPTestData {
         Cached wrapper around New-CIPPDbRequest for test functions
 
     .DESCRIPTION
-        Returns cached tenant data during test suite execution. The cache is
-        backed by CIPP.TestDataCache (static ConcurrentDictionary in C#) so
-        it is shared across all PowerShell runspaces within the worker process.
+        Returns cached tenant data during test suite execution. The cache is backed by
+        CIPP.TestDataCache (static ConcurrentDictionary in C#) so it is shared across all
+        PowerShell runspaces within the worker process.
+
+        RECORDS ARE FIELD-PROJECTED. Only the fields listed for $Type in
+        Get-CippTestDataFieldManifest are materialized. Reading an unlisted field returns $null
+        with no error, so the test emits a wrong verdict silently. If you add a field read to a
+        test, add it to the manifest first. A type absent from the manifest is fetched whole,
+        which is the safe default.
+
+        Projection is record-level: a kept field keeps its entire subtree, so
+        `$policy.conditions.users.includeRoles` only requires 'conditions'.
+
+        Records are PSCustomObject and must stay so. Hashtable output changes scalar semantics
+        ($x[0] becomes a key lookup returning $null; $x.Count returns the record's field count
+        instead of 1) and only when a pipeline yields exactly one record — data-dependent, and
+        easily missed in testing.
 
     .PARAMETER TenantFilter
         The tenant domain or GUID to filter by
 
     .PARAMETER Type
         The data type to retrieve (e.g., Users, Groups, ConditionalAccessPolicies)
+
+    .PARAMETER Fields
+        Optional. Override the manifest for this call: keep only these top-level fields. Prefer
+        the manifest; this is for one-off and diagnostic callers, not tests.
+
+        The field set is part of the cache key — it must be, or callers asking for the same
+        tenant+type with different lists would receive each other's projection. Distinct lists
+        therefore fragment the cache: the same rows parsed once per list, all alive together for
+        the TTL. Per-call-site lists measured several times worse than one shared per-type entry.
+        Fields belong to the type — see Get-CippTestDataFieldManifest.
+
+    .PARAMETER NoProjection
+        Return every field, ignoring the manifest.
+
+        Required for the custom-script sandbox (Get-CippSandboxData): those scripts are
+        customer-authored, so the fields they read are unknowable and a manifest built from our
+        own tests would silently hand them incomplete records. Any caller fetching data on behalf
+        of code we did not write must pass this.
+
+    .EXAMPLE
+        Get-CIPPTestData -TenantFilter $Tenant -Type 'Users'
+        Normal use: the manifest decides the fields. This is what tests should do.
+
+    .EXAMPLE
+        Get-CIPPTestData -TenantFilter $Tenant -Type 'Users' -NoProjection
+        Every field. For callers serving code we did not author.
+
+    .NOTES
+        Verifying a change: "0 errored" proves nothing, because the failure modes here are silent
+        — a broken test still reports success. Diff test verdicts against a baseline, across more
+        than one tenant: a bug that needs exactly one matching record will not show on a tenant
+        that has two.
+
+        Useful endpoints: ExecTestRun then ListTests for verdicts; ListDBCache to confirm a field
+        exists and its real casing before adding it to the manifest (type=_availableTypes lists
+        the types); ListWorkerHealth?Action=CacheDiag for entries, hit rate and a per-type
+        breakdown whose keys carry the projected field list.
     #>
     [CmdletBinding()]
     param(
         [Parameter(Mandatory = $false)]
         [string]$TenantFilter,
 
         [Parameter(Mandatory = $false)]
-        [string]$Type
+        [string]$Type,
+
+        [Parameter(Mandatory = $false)]
+        [string[]]$Fields,
+
+        [Parameter(Mandatory = $false)]
+        [switch]$NoProjection
     )
 
     # Enforce tenant lock when running inside custom script execution
@@ -32,14 +89,36 @@ function Get-CIPPTestData {
         throw 'TenantFilter is required.'
     }
 
-    $CacheKey = '{0}|{1}' -f $TenantFilter, $Type
+    # Resolve the field set: an explicit -Fields wins, otherwise consult the per-type manifest.
+    # -NoProjection suppresses both (the sandbox path must never receive projected records).
+    $EffectiveFields = if ($NoProjection) {
+        $null
+    } elseif ($Fields) {
+        $Fields
+    } else {
+        Get-CippTestDataFieldManifest -Type $Type
+    }
+
+    # Normalize the field set (sorted + de-duplicated) so that callers asking for the same
+    # fields in a different order share one cache entry instead of parsing twice.
+    $NormalizedFields = if ($EffectiveFields) { @($EffectiveFields | Sort-Object -Unique) } else { $null }
+
+    $CacheKey = if ($NormalizedFields) {
+        '{0}|{1}|{2}' -f $TenantFilter, $Type, ($NormalizedFields -join ',')
+    } else {
+        '{0}|{1}' -f $TenantFilter, $Type
+    }
 
     $CachedValue = $null
     if ([CIPP.TestDataCache]::TryGet($CacheKey, [ref]$CachedValue)) {
         return $CachedValue
     }
 
-    $Data = New-CIPPDbRequest -TenantFilter $TenantFilter -Type $Type
+    $Data = if ($NormalizedFields) {
+        New-CIPPDbRequest -TenantFilter $TenantFilter -Type $Type -Fields $NormalizedFields
+    } else {
+        New-CIPPDbRequest -TenantFilter $TenantFilter -Type $Type
+    }
 
     [CIPP.TestDataCache]::Set($CacheKey, $Data)
 

Modules/CIPPCore/Public/Get-CippDbRoleMembers.ps1

@@ -1,4 +1,47 @@
 function Get-CippDbRoleMembers {
+    <#
+    .SYNOPSIS
+        Resolve the members of a directory role from cached PIM + directoryRole data.
+
+    .DESCRIPTION
+        Merges three sources into one member list, de-duplicated by principal id:
+          Active   - PIM roleAssignmentScheduleInstances with assignmentType 'Assigned'
+          Eligible - PIM roleEligibilitySchedules (can activate, not currently active)
+          Direct   - directoryRole membership assigned outside PIM
+
+        A principal may be a user, a servicePrincipal, or a group — use '@odata.type' to tell
+        them apart. userPrincipalName is null for anything that isn't a user, and appId is
+        populated only for servicePrincipals.
+
+    .PARAMETER TenantFilter
+        The tenant to resolve members for.
+
+    .PARAMETER RoleTemplateId
+        The role's TEMPLATE id (e.g. Global Administrator = 62e90394-69f5-4237-9190-012177145e10).
+
+        NOT the directoryRole instance id. PIM's roleDefinitionId carries template ids, so passing
+        an instance id (Roles.id) matches nothing and silently returns an empty list. When starting
+        from a Get-CippDbRole record, pass $Role.roleTemplateId — never $Role.id.
+
+    .OUTPUTS
+        PSCustomObject per member:
+          id                - principal object id
+          displayName       - principal display name
+          userPrincipalName - users only; null for servicePrincipals and groups
+          appId             - servicePrincipals only; null otherwise
+          '@odata.type'     - '#microsoft.graph.user' | '...servicePrincipal' | '...group'
+          AssignmentType    - 'Active' | 'Eligible' | 'Direct'
+          EndDateTime       - when the assignment expires; null when it does not
+          IsPermanent       - $true when the assignment has no expiry
+
+    .NOTES
+        Depends on the cached records carrying an expanded 'principal' object — the Graph APIs
+        return only principalId by default, so the collectors request $expand=principal. Without
+        it every displayName/userPrincipalName here is null.
+
+    .FUNCTIONALITY
+        Internal
+    #>
     [CmdletBinding()]
     param (
         [Parameter(Mandatory = $true)]
@@ -27,8 +70,12 @@ function Get-CippDbRoleMembers {
             id                = $member.principalId
             displayName       = $member.principal.displayName
             userPrincipalName = $member.principal.userPrincipalName
+            appId             = $member.principal.appId
             '@odata.type'     = $member.principal.'@odata.type'
             AssignmentType    = 'Active'
+            EndDateTime       = $member.endDateTime
+            # A PIM assignment with no endDateTime never expires.
+            IsPermanent       = ($null -eq $member.endDateTime)
         }
         $AllMembers.Add($memberObj)
     }
@@ -39,8 +86,12 @@ function Get-CippDbRoleMembers {
                 id                = $member.principalId
                 displayName       = $member.principal.displayName
                 userPrincipalName = $member.principal.userPrincipalName
+                appId             = $member.principal.appId
                 '@odata.type'     = $member.principal.'@odata.type'
                 AssignmentType    = 'Eligible'
+                # Eligibilities carry their expiry under scheduleInfo, not endDateTime.
+                EndDateTime       = $member.scheduleInfo.expiration.endDateTime
+                IsPermanent       = ($member.scheduleInfo.expiration.type -eq 'noExpiration')
             }
             $AllMembers.Add($memberObj)
         }
@@ -52,8 +103,12 @@ function Get-CippDbRoleMembers {
                 id                = $member.id
                 displayName       = $member.displayName
                 userPrincipalName = $member.userPrincipalName
+                appId             = $member.appId
                 '@odata.type'     = $member.'@odata.type'
                 AssignmentType    = 'Direct'
+                # directoryRole membership assigned outside PIM has no expiry.
+                EndDateTime       = $null
+                IsPermanent       = $true
             }
             $AllMembers.Add($memberObj)
         }

Modules/CIPPCore/Public/Get-CippSandboxData.ps1

@@ -70,7 +70,7 @@ function Get-CippSandboxData {
 
         $Key = if ($Type) { $Type } else { '' }
         if (-not $Data.ContainsKey($Key)) {
-            $Data[$Key] = @(Get-CIPPTestData -TenantFilter $TenantFilter -Type $Type)
+            $Data[$Key] = @(Get-CIPPTestData -TenantFilter $TenantFilter -Type $Type -NoProjection)
         }
     }