Skip to content

Commit 9ac0173

Browse files
pull[bot]pull[bot]
authored
Merge pull request #1058 from KelvinTegelaar/master
[pull] master from KelvinTegelaar:master
2 parents f8e3bb5 + a9d5c4e commit 9ac0173

5 files changed

Lines changed: 135 additions & 101 deletions

File tree

‎Modules/CIPPCore/Public/GraphHelper/Get-CippSamPermissions.ps1‎

Lines changed: 54 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ function Get-CippSamPermissions {
1212
The effective set returned in .Permissions is therefore always manifest ∪ extras. Each permission
1313
is annotated with a 'required' boolean so the UI can lock the manifest-defined defaults.
1414
15-
Unless -NoDiff is used, the function also pulls the live CIPP-SAM application registration from the
16-
partner tenant and diffs its requiredResourceAccess against the effective set, surfacing
17-
permissions that need to be added to (MissingPermissions) and removed from (PartnerAppDiff) the app.
15+
Unless -NoDiff is used, the function also reads what is actually granted on the CIPP-SAM enterprise
16+
application (service principal) in the partner tenant - appRoleAssignments (application/Role) and
17+
oauth2PermissionGrants (delegated/Scope) - and diffs those grants against the effective set,
18+
surfacing permissions that need to be granted (MissingPermissions) and grants that are present but
19+
not in the effective set (PartnerAppDiff). The app registration's requiredResourceAccess is not used.
1820
1921
.EXAMPLE
2022
Get-CippSamPermissions
@@ -195,38 +197,68 @@ function Get-CippSamPermissions {
195197
}
196198
}
197199

198-
# Diff the effective set against the live CIPP-SAM application registration in the partner tenant.
199-
# MissingPermissions = effective perms not yet on the app (need to be added).
200-
# PartnerAppDiff also surfaces extra perms on the app that are not in the effective set (need to be removed).
200+
# Diff the effective set against what is actually GRANTED on the partner CIPP-SAM enterprise
201+
# application (service principal): appRoleAssignments for application (Role) permissions and
202+
# oauth2PermissionGrants for delegated (Scope) permissions. The app registration's
203+
# requiredResourceAccess is intentionally NOT used - permissions are applied as SP grants, so the
204+
# grants are the real source of truth for what the app can do.
205+
# MissingPermissions = effective perms not yet granted on the SP (need to be added).
206+
# PartnerAppDiff also surfaces extra grants on the SP that are not in the effective set.
201207
$MissingPermissions = @{}
202208
$PartnerAppDiff = @{}
203209
if (!$NoDiff.IsPresent) {
204210
try {
205-
$PartnerApp = New-GraphGETRequest -uri "https://graph.microsoft.com/beta/applications(appId='$($env:ApplicationID)')?`$select=requiredResourceAccess" -tenantid $env:TenantID -NoAuthCheck $true
211+
$PartnerSP = New-GraphGETRequest -uri "https://graph.microsoft.com/beta/servicePrincipals(appId='$($env:ApplicationID)')?`$select=id" -tenantid $env:TenantID -NoAuthCheck $true
212+
$AppRoleAssignments = New-GraphGETRequest -uri "https://graph.microsoft.com/beta/servicePrincipals/$($PartnerSP.id)/appRoleAssignments?`$top=999" -tenantid $env:TenantID -NoAuthCheck $true
213+
$OAuthGrants = New-GraphGETRequest -uri "https://graph.microsoft.com/beta/servicePrincipals/$($PartnerSP.id)/oauth2PermissionGrants?`$top=999" -tenantid $env:TenantID -NoAuthCheck $true
214+
215+
# Grants reference the resource SP's object id; map it back to the resource appId the
216+
# effective set is keyed on. Use $UsedServicePrincipals - it carries both id and appId
217+
# ($ServicePrincipals is selected without id, so its .id is null).
218+
$ResourceIdToAppId = @{}
219+
foreach ($SP in $UsedServicePrincipals) { if ($SP.id) { $ResourceIdToAppId[$SP.id] = $SP.appId } }
220+
221+
# Granted application roles (GUIDs) per resource appId.
222+
$GrantedRoleIdsByApp = @{}
223+
foreach ($Assignment in $AppRoleAssignments) {
224+
$ResAppId = $ResourceIdToAppId[$Assignment.resourceId]
225+
if (!$ResAppId -or !$Assignment.appRoleId) { continue }
226+
if (-not $GrantedRoleIdsByApp.ContainsKey($ResAppId)) { $GrantedRoleIdsByApp[$ResAppId] = [System.Collections.Generic.List[string]]::new() }
227+
$GrantedRoleIdsByApp[$ResAppId].Add([string]$Assignment.appRoleId)
228+
}
229+
230+
# Granted delegated scope NAMES per resource appId (oauth2 grants store space-delimited names).
231+
$GrantedScopesByApp = @{}
232+
foreach ($Grant in $OAuthGrants) {
233+
$ResAppId = $ResourceIdToAppId[$Grant.resourceId]
234+
if (!$ResAppId) { continue }
235+
if (-not $GrantedScopesByApp.ContainsKey($ResAppId)) { $GrantedScopesByApp[$ResAppId] = [System.Collections.Generic.List[string]]::new() }
236+
foreach ($ScopeName in @(($Grant.scope -split ' ') | Where-Object { $_ })) { $GrantedScopesByApp[$ResAppId].Add($ScopeName) }
237+
}
238+
206239
foreach ($AppId in $AllAppIds) {
207240
$ServicePrincipal = $ServicePrincipals | Where-Object -Property appId -EQ $AppId
208-
$AppRegResource = $PartnerApp.requiredResourceAccess | Where-Object -Property resourceAppId -EQ $AppId
209-
$AppRegRoleIds = @(($AppRegResource.resourceAccess | Where-Object { $_.type -eq 'Role' }).id)
210-
$AppRegScopeIds = @(($AppRegResource.resourceAccess | Where-Object { $_.type -eq 'Scope' }).id)
241+
$GrantedRoleIds = @($GrantedRoleIdsByApp[$AppId] | Where-Object { $_ })
242+
$GrantedScopeNames = @($GrantedScopesByApp[$AppId] | Where-Object { $_ })
211243

212-
# Only GUID-based permissions live in the app registration's requiredResourceAccess.
213-
# String-named scopes (e.g. the .Sdp AdditionalPermissions) are applied as direct grants,
214-
# so excluding them here avoids permanent false-positive "missing" entries.
244+
# Application (Role) permissions compare by GUID against appRoleAssignments.
215245
$EffApp = @($EffectivePermissions.$AppId.applicationPermissions | Where-Object { $_.id -match $GuidRegex })
216-
$EffDel = @($EffectivePermissions.$AppId.delegatedPermissions | Where-Object { $_.id -match $GuidRegex })
246+
# Delegated (Scope) permissions compare by NAME (value) against oauth2 grant scopes -
247+
# this covers both GUID-resolved scopes and the string-named AdditionalPermissions.
248+
$EffDel = @($EffectivePermissions.$AppId.delegatedPermissions)
217249
$EffAppIds = @($EffApp.id)
218-
$EffDelIds = @($EffDel.id)
250+
$EffDelNames = @($EffDel.value)
219251

220-
$MissingApp = @(foreach ($Permission in $EffApp) { if ($AppRegRoleIds -notcontains $Permission.id) { $Permission } })
221-
$MissingDel = @(foreach ($Permission in $EffDel) { if ($AppRegScopeIds -notcontains $Permission.id) { $Permission } })
222-
$ExtraApp = @(foreach ($Id in $AppRegRoleIds) {
252+
$MissingApp = @(foreach ($Permission in $EffApp) { if ($GrantedRoleIds -notcontains $Permission.id) { $Permission } })
253+
$MissingDel = @(foreach ($Permission in $EffDel) { if ($Permission.value -and $GrantedScopeNames -notcontains $Permission.value) { $Permission } })
254+
$ExtraApp = @(foreach ($Id in ($GrantedRoleIds | Sort-Object -Unique)) {
223255
if ($EffAppIds -notcontains $Id) {
224256
[PSCustomObject]@{ id = $Id; value = (($ServicePrincipal.appRoles | Where-Object -Property id -EQ $Id).value) ?? $Id }
225257
}
226258
})
227-
$ExtraDel = @(foreach ($Id in $AppRegScopeIds) {
228-
if ($EffDelIds -notcontains $Id) {
229-
[PSCustomObject]@{ id = $Id; value = (($ServicePrincipal.publishedPermissionScopes | Where-Object -Property id -EQ $Id).value) ?? $Id }
259+
$ExtraDel = @(foreach ($Name in ($GrantedScopeNames | Sort-Object -Unique)) {
260+
if ($EffDelNames -notcontains $Name) {
261+
[PSCustomObject]@{ id = $Name; value = $Name }
230262
}
231263
})
232264

@@ -246,7 +278,7 @@ function Get-CippSamPermissions {
246278
}
247279
}
248280
} catch {
249-
Write-Information "Failed to retrieve partner app registration for permission diff: $($_.Exception.Message)"
281+
Write-Information "Failed to retrieve partner enterprise app grants for permission diff: $($_.Exception.Message)"
250282
}
251283
}
252284

‎Modules/CIPPCore/Public/SAMManifest/Update-CippSamPermissions.ps1‎

Lines changed: 59 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
function Update-CippSamPermissions {
22
<#
33
.SYNOPSIS
4-
Repairs the CIPP-SAM app registration permissions in the partner tenant.
4+
Reconciles the saved CIPP-SAM additional-permission set in the AppPermissions table.
55
.DESCRIPTION
6-
Diffs the effective CIPP-SAM permission set (manifest defaults + saved extras) against the live
7-
CIPP-SAM application registration in the partner tenant and ADDS any missing permissions to the
8-
app registration's requiredResourceAccess. This is additive only: it never removes permissions,
9-
so it cannot strip a legitimately-configured entry. Extra permissions found on the app that are
10-
not part of the effective set are reported back so an admin can review/remove them manually.
6+
The SAM manifest is the immutable permission base and is always layered in at read time by
7+
Get-CippSamPermissions, so the AppPermissions table only ever needs to hold the EXTRA
8+
permissions an admin layered on top. This function keeps that row clean: it drops any saved
9+
entries the manifest now covers (e.g. legacy rows that stored the full manifest+extras set)
10+
so the table stays "extras only".
1111
12-
Pushing these permissions out to customer tenants is handled separately by the CPV refresh.
12+
It deliberately does NOT write the partner CIPP-SAM app registration's requiredResourceAccess.
13+
Permissions reach the CIPP-SAM service principal(s) - partner and clients - through the grant
14+
flow (Add-CIPPApplicationPermission / Add-CIPPDelegatedPermission, which read this table), not
15+
through the app registration. Refreshing those grants is handled by the caller
16+
(Invoke-ExecPermissionRepair for the partner, the per-tenant permission refresh for clients).
1317
.PARAMETER UpdatedBy
1418
The user or system that is performing the update. Defaults to 'CIPP-API'.
1519
.OUTPUTS
@@ -22,87 +26,70 @@ function Update-CippSamPermissions {
2226
)
2327

2428
try {
25-
$CurrentPermissions = Get-CippSamPermissions
26-
$PartnerAppDiff = $CurrentPermissions.PartnerAppDiff
27-
$MissingPermissions = $CurrentPermissions.MissingPermissions
29+
# Manifest base - always-required permissions that are layered in at read time, so they never
30+
# need to live in the saved extras row.
31+
$ManifestPermissions = (Get-CippSamPermissions -ManifestOnly).Permissions
2832

29-
$MissingAppIds = @($MissingPermissions.PSObject.Properties.Name)
30-
$ExtraAppIds = @($PartnerAppDiff.PSObject.Properties.Name | Where-Object {
31-
($PartnerAppDiff.$_.extraApplicationPermissions | Measure-Object).Count -gt 0 -or
32-
($PartnerAppDiff.$_.extraDelegatedPermissions | Measure-Object).Count -gt 0
33-
})
34-
35-
if ($MissingAppIds.Count -eq 0) {
36-
if ($ExtraAppIds.Count -gt 0) {
37-
$ExtraSummary = foreach ($AppId in $ExtraAppIds) {
38-
$Names = @($PartnerAppDiff.$AppId.extraApplicationPermissions.value) + @($PartnerAppDiff.$AppId.extraDelegatedPermissions.value)
39-
"$AppId ($($Names -join ', '))"
40-
}
41-
return "No missing permissions to add. The following extra permissions are present on the app and should be reviewed/removed manually: $($ExtraSummary -join '; ')"
42-
}
43-
return 'No permissions to update'
33+
$Table = Get-CIPPTable -TableName 'AppPermissions'
34+
$SavedRow = Get-CippAzDataTableEntity @Table -Filter "PartitionKey eq 'CIPP-SAM' and RowKey eq 'CIPP-SAM'"
35+
if (-not $SavedRow.Permissions) {
36+
return 'No additional permissions saved. CIPP default (manifest) permissions are always applied.'
4437
}
4538

46-
# Retrieve the live CIPP-SAM application registration in the partner tenant.
47-
$PartnerApp = New-GraphGETRequest -uri "https://graph.microsoft.com/beta/applications(appId='$($env:ApplicationID)')?`$select=id,requiredResourceAccess" -tenantid $env:TenantID -NoAuthCheck $true
48-
49-
$RequiredResourceAccess = [System.Collections.Generic.List[object]]::new()
50-
foreach ($Resource in $PartnerApp.requiredResourceAccess) {
51-
$ResourceAccess = [System.Collections.Generic.List[object]]::new()
52-
foreach ($Access in $Resource.resourceAccess) {
53-
$ResourceAccess.Add(@{ id = $Access.id; type = $Access.type })
54-
}
55-
$RequiredResourceAccess.Add([PSCustomObject]@{
56-
resourceAppId = $Resource.resourceAppId
57-
resourceAccess = $ResourceAccess
58-
})
39+
try {
40+
$Saved = $SavedRow.Permissions | ConvertFrom-Json -ErrorAction Stop
41+
} catch {
42+
return 'Saved additional permissions could not be parsed; nothing to reconcile.'
5943
}
6044

61-
$AddedPermissions = [System.Collections.Generic.List[string]]::new()
62-
foreach ($AppId in $MissingAppIds) {
63-
$Resource = $RequiredResourceAccess | Where-Object -Property resourceAppId -EQ $AppId | Select-Object -First 1
64-
if (!$Resource) {
65-
$Resource = [PSCustomObject]@{
66-
resourceAppId = $AppId
67-
resourceAccess = [System.Collections.Generic.List[object]]::new()
45+
# Keep only the entries the manifest does NOT already cover.
46+
$Extras = @{}
47+
$RemovedCount = 0
48+
foreach ($AppId in $Saved.PSObject.Properties.Name) {
49+
$ManifestApp = $ManifestPermissions.$AppId
50+
$ManifestAppIds = @($ManifestApp.applicationPermissions.id)
51+
$ManifestDelIds = @($ManifestApp.delegatedPermissions.id)
52+
53+
$ExtraApp = [System.Collections.Generic.List[object]]::new()
54+
foreach ($Permission in $Saved.$AppId.applicationPermissions) {
55+
if ($Permission.id -and $ManifestAppIds -notcontains $Permission.id) {
56+
$ExtraApp.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
57+
} else {
58+
$RemovedCount++
6859
}
69-
$RequiredResourceAccess.Add($Resource)
7060
}
71-
$ExistingIds = @($Resource.resourceAccess.id)
72-
73-
foreach ($Permission in $MissingPermissions.$AppId.applicationPermissions) {
74-
if ($Permission.id -and $ExistingIds -notcontains $Permission.id) {
75-
$Resource.resourceAccess.Add(@{ id = $Permission.id; type = 'Role' })
76-
$AddedPermissions.Add("$($Permission.value) (Application)")
61+
$ExtraDel = [System.Collections.Generic.List[object]]::new()
62+
foreach ($Permission in $Saved.$AppId.delegatedPermissions) {
63+
if ($Permission.id -and $ManifestDelIds -notcontains $Permission.id) {
64+
$ExtraDel.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
65+
} else {
66+
$RemovedCount++
7767
}
7868
}
79-
foreach ($Permission in $MissingPermissions.$AppId.delegatedPermissions) {
80-
if ($Permission.id -and $ExistingIds -notcontains $Permission.id) {
81-
$Resource.resourceAccess.Add(@{ id = $Permission.id; type = 'Scope' })
82-
$AddedPermissions.Add("$($Permission.value) (Delegated)")
69+
70+
if ($ExtraApp.Count -gt 0 -or $ExtraDel.Count -gt 0) {
71+
$Extras.$AppId = @{
72+
applicationPermissions = @($ExtraApp)
73+
delegatedPermissions = @($ExtraDel)
8374
}
8475
}
8576
}
8677

87-
if ($AddedPermissions.Count -eq 0) {
88-
return 'No permissions to update'
78+
if ($RemovedCount -eq 0) {
79+
return 'Saved additional permissions already reconciled; no manifest-covered entries to remove.'
8980
}
9081

91-
$PatchBody = @{ requiredResourceAccess = @($RequiredResourceAccess) } | ConvertTo-Json -Depth 10 -Compress
92-
$null = New-GraphPOSTRequest -uri "https://graph.microsoft.com/beta/applications/$($PartnerApp.id)" -tenantid $env:TenantID -body $PatchBody -type PATCH -NoAuthCheck $true
93-
94-
Write-LogMessage -API 'UpdateCippSamPermissions' -message "CIPP-SAM app registration permissions repaired by $UpdatedBy" -Sev 'Info' -LogData @{ Added = $AddedPermissions }
95-
96-
$Result = "Added $($AddedPermissions.Count) missing permission(s) to the CIPP-SAM app registration: $($AddedPermissions -join ', '). Run a CPV refresh to apply these to customer tenants."
97-
if ($ExtraAppIds.Count -gt 0) {
98-
$ExtraSummary = foreach ($AppId in $ExtraAppIds) {
99-
$Names = @($PartnerAppDiff.$AppId.extraApplicationPermissions.value) + @($PartnerAppDiff.$AppId.extraDelegatedPermissions.value)
100-
"$AppId ($($Names -join ', '))"
101-
}
102-
$Result += " Extra permissions present on the app that should be reviewed/removed manually: $($ExtraSummary -join '; ')."
82+
$Entity = @{
83+
'PartitionKey' = 'CIPP-SAM'
84+
'RowKey' = 'CIPP-SAM'
85+
'Permissions' = [string]([PSCustomObject]$Extras | ConvertTo-Json -Depth 10 -Compress)
86+
'UpdatedBy' = $UpdatedBy
10387
}
104-
return $Result
88+
$null = Add-CIPPAzDataTableEntity @Table -Entity $Entity -Force
89+
90+
$Plural = if ($RemovedCount -eq 1) { 'entry' } else { 'entries' }
91+
return "Reconciled saved additional permissions: removed $RemovedCount $Plural now covered by the CIPP manifest."
10592
} catch {
106-
throw "Failed to update permissions: $($_.Exception.Message)"
93+
throw "Failed to reconcile permissions: $($_.Exception.Message)"
10794
}
10895
}

0 commit comments

Comments
 (0)