Skip to content

Commit 2ca150c

Browse files
authored
Merge pull request #48 from KelvinTegelaar/master
[pull] master from KelvinTegelaar:master
2 parents 4c42f72 + 0ec258d commit 2ca150c

5 files changed

Lines changed: 105 additions & 137 deletions

File tree

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

Lines changed: 29 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -197,88 +197,44 @@ function Get-CippSamPermissions {
197197
}
198198
}
199199

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.
200+
# Diff the manifest-required base against the saved AppPermissions table. The table records what has
201+
# been applied to the CIPP-SAM app - the repair/update flow persists it as manifest ∪ extras - so it
202+
# stands in for the "current" permission set and no partner-tenant Graph call is needed here.
203+
# MissingPermissions = manifest-required perms not yet present in the table (a Permissions repair is needed).
204+
# PartnerAppDiff mirrors MissingPermissions in the shape the SAM permissions page expects.
207205
$MissingPermissions = @{}
208206
$PartnerAppDiff = @{}
209207
if (!$NoDiff.IsPresent) {
210-
try {
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-
}
208+
foreach ($AppId in $AllAppIds) {
209+
$ManifestApp = $ManifestPermissions.$AppId
210+
$SavedApp = $SavedPermissions.$AppId
229211

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-
}
212+
$SavedAppIds = @($SavedApp.applicationPermissions.id)
213+
$SavedDelIds = @($SavedApp.delegatedPermissions.id)
238214

239-
foreach ($AppId in $AllAppIds) {
240-
$ServicePrincipal = $ServicePrincipals | Where-Object -Property appId -EQ $AppId
241-
$GrantedRoleIds = @($GrantedRoleIdsByApp[$AppId] | Where-Object { $_ })
242-
$GrantedScopeNames = @($GrantedScopesByApp[$AppId] | Where-Object { $_ })
243-
244-
# Application (Role) permissions compare by GUID against appRoleAssignments.
245-
$EffApp = @($EffectivePermissions.$AppId.applicationPermissions | 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)
249-
$EffAppIds = @($EffApp.id)
250-
$EffDelNames = @($EffDel.value)
251-
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)) {
255-
if ($EffAppIds -notcontains $Id) {
256-
[PSCustomObject]@{ id = $Id; value = (($ServicePrincipal.appRoles | Where-Object -Property id -EQ $Id).value) ?? $Id }
257-
}
258-
})
259-
$ExtraDel = @(foreach ($Name in ($GrantedScopeNames | Sort-Object -Unique)) {
260-
if ($EffDelNames -notcontains $Name) {
261-
[PSCustomObject]@{ id = $Name; value = $Name }
262-
}
263-
})
264-
265-
if ($MissingApp.Count -gt 0 -or $MissingDel.Count -gt 0) {
266-
$MissingPermissions.$AppId = @{
267-
applicationPermissions = $MissingApp
268-
delegatedPermissions = $MissingDel
215+
$MissingApp = @(foreach ($Permission in $ManifestApp.applicationPermissions) {
216+
if ($Permission.id -and $SavedAppIds -notcontains $Permission.id) {
217+
[PSCustomObject]@{ id = $Permission.id; value = $Permission.value }
269218
}
270-
}
271-
if ($MissingApp.Count -gt 0 -or $MissingDel.Count -gt 0 -or $ExtraApp.Count -gt 0 -or $ExtraDel.Count -gt 0) {
272-
$PartnerAppDiff.$AppId = @{
273-
missingApplicationPermissions = $MissingApp
274-
missingDelegatedPermissions = $MissingDel
275-
extraApplicationPermissions = $ExtraApp
276-
extraDelegatedPermissions = $ExtraDel
219+
})
220+
$MissingDel = @(foreach ($Permission in $ManifestApp.delegatedPermissions) {
221+
if ($Permission.id -and $SavedDelIds -notcontains $Permission.id) {
222+
[PSCustomObject]@{ id = $Permission.id; value = $Permission.value }
277223
}
224+
})
225+
226+
if ($MissingApp.Count -gt 0 -or $MissingDel.Count -gt 0) {
227+
$MissingPermissions.$AppId = @{
228+
applicationPermissions = $MissingApp
229+
delegatedPermissions = $MissingDel
230+
}
231+
$PartnerAppDiff.$AppId = @{
232+
missingApplicationPermissions = $MissingApp
233+
missingDelegatedPermissions = $MissingDel
234+
extraApplicationPermissions = @()
235+
extraDelegatedPermissions = @()
278236
}
279237
}
280-
} catch {
281-
Write-Information "Failed to retrieve partner enterprise app grants for permission diff: $($_.Exception.Message)"
282238
}
283239
}
284240

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

Lines changed: 41 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,18 @@
11
function Update-CippSamPermissions {
22
<#
33
.SYNOPSIS
4-
Reconciles the saved CIPP-SAM additional-permission set in the AppPermissions table.
4+
Reconciles the applied CIPP-SAM permission set in the AppPermissions table.
55
.DESCRIPTION
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".
6+
Writes the full applied permission set - the SAM manifest base PLUS any admin-configured extra
7+
permissions - into the AppPermissions table, so the table always reflects everything the
8+
CIPP-SAM app is expected to have. Get-CippSamPermissions diffs the manifest against this table
9+
to decide when a Permissions repair is needed, so persisting the manifest here is what lets that
10+
check clear after a repair.
1111
1212
It deliberately does NOT write the partner CIPP-SAM app registration's requiredResourceAccess.
1313
Permissions reach the CIPP-SAM service principal(s) - partner and clients - through the grant
1414
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).
15+
through the app registration.
1716
.PARAMETER UpdatedBy
1817
The user or system that is performing the update. Defaults to 'CIPP-API'.
1918
.OUTPUTS
@@ -26,69 +25,68 @@ function Update-CippSamPermissions {
2625
)
2726

2827
try {
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.
28+
# Manifest base - the always-required permissions.
3129
$ManifestPermissions = (Get-CippSamPermissions -ManifestOnly).Permissions
3230

3331
$Table = Get-CIPPTable -TableName 'AppPermissions'
3432
$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.'
37-
}
38-
39-
try {
40-
$Saved = $SavedRow.Permissions | ConvertFrom-Json -ErrorAction Stop
41-
} catch {
42-
return 'Saved additional permissions could not be parsed; nothing to reconcile.'
33+
$Saved = $null
34+
if ($SavedRow.Permissions) {
35+
try {
36+
$Saved = $SavedRow.Permissions | ConvertFrom-Json -ErrorAction Stop
37+
} catch {
38+
$Saved = $null
39+
}
4340
}
4441

45-
# Keep only the entries the manifest does NOT already cover.
46-
$Extras = @{}
47-
$RemovedCount = 0
48-
foreach ($AppId in $Saved.PSObject.Properties.Name) {
42+
# Build the full applied set = manifest base ∪ admin extras, keyed by resource appId.
43+
$Applied = @{}
44+
$AppIds = @(@($ManifestPermissions.PSObject.Properties.Name) + @($Saved.PSObject.Properties.Name)) | Where-Object { $_ } | Sort-Object -Unique
45+
foreach ($AppId in $AppIds) {
4946
$ManifestApp = $ManifestPermissions.$AppId
47+
$SavedApp = $Saved.$AppId
5048
$ManifestAppIds = @($ManifestApp.applicationPermissions.id)
5149
$ManifestDelIds = @($ManifestApp.delegatedPermissions.id)
5250

53-
$ExtraApp = [System.Collections.Generic.List[object]]::new()
54-
foreach ($Permission in $Saved.$AppId.applicationPermissions) {
51+
$AppPerms = [System.Collections.Generic.List[object]]::new()
52+
$DelPerms = [System.Collections.Generic.List[object]]::new()
53+
54+
# Manifest base (always applied).
55+
foreach ($Permission in $ManifestApp.applicationPermissions) {
56+
$AppPerms.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
57+
}
58+
foreach ($Permission in $ManifestApp.delegatedPermissions) {
59+
$DelPerms.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
60+
}
61+
# Admin extras (anything the manifest does not already cover).
62+
foreach ($Permission in $SavedApp.applicationPermissions) {
5563
if ($Permission.id -and $ManifestAppIds -notcontains $Permission.id) {
56-
$ExtraApp.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
57-
} else {
58-
$RemovedCount++
64+
$AppPerms.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
5965
}
6066
}
61-
$ExtraDel = [System.Collections.Generic.List[object]]::new()
62-
foreach ($Permission in $Saved.$AppId.delegatedPermissions) {
67+
foreach ($Permission in $SavedApp.delegatedPermissions) {
6368
if ($Permission.id -and $ManifestDelIds -notcontains $Permission.id) {
64-
$ExtraDel.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
65-
} else {
66-
$RemovedCount++
69+
$DelPerms.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
6770
}
6871
}
6972

70-
if ($ExtraApp.Count -gt 0 -or $ExtraDel.Count -gt 0) {
71-
$Extras.$AppId = @{
72-
applicationPermissions = @($ExtraApp)
73-
delegatedPermissions = @($ExtraDel)
73+
if ($AppPerms.Count -gt 0 -or $DelPerms.Count -gt 0) {
74+
$Applied.$AppId = @{
75+
applicationPermissions = @($AppPerms)
76+
delegatedPermissions = @($DelPerms)
7477
}
7578
}
7679
}
7780

78-
if ($RemovedCount -eq 0) {
79-
return 'Saved additional permissions already reconciled; no manifest-covered entries to remove.'
80-
}
81-
8281
$Entity = @{
8382
'PartitionKey' = 'CIPP-SAM'
8483
'RowKey' = 'CIPP-SAM'
85-
'Permissions' = [string]([PSCustomObject]$Extras | ConvertTo-Json -Depth 10 -Compress)
84+
'Permissions' = [string]([PSCustomObject]$Applied | ConvertTo-Json -Depth 10 -Compress)
8685
'UpdatedBy' = $UpdatedBy
8786
}
8887
$null = Add-CIPPAzDataTableEntity @Table -Entity $Entity -Force
8988

90-
$Plural = if ($RemovedCount -eq 1) { 'entry' } else { 'entries' }
91-
return "Reconciled saved additional permissions: removed $RemovedCount $Plural now covered by the CIPP manifest."
89+
return 'CIPP-SAM permissions reconciled: the applied permission table now contains the CIPP manifest permissions plus any additional permissions.'
9290
} catch {
9391
throw "Failed to reconcile permissions: $($_.Exception.Message)"
9492
}

Modules/CIPPHTTP/Public/Entrypoints/HTTP Functions/CIPP/Settings/Invoke-ExecSAMAppPermissions.ps1

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,43 +16,57 @@ function Invoke-ExecSAMAppPermissions {
1616
$Submitted = $Request.Body.Permissions
1717
$ManifestPermissions = (Get-CippSamPermissions -ManifestOnly).Permissions
1818

19-
$Extras = @{}
20-
foreach ($AppId in $Submitted.PSObject.Properties.Name) {
19+
# Persist the full applied set = manifest base ∪ submitted extras, so the AppPermissions
20+
# table always reflects everything the CIPP-SAM app should have (the manifest is always
21+
# applied and cannot be removed). Get-CippSamPermissions diffs the manifest against this
22+
# table to decide when a Permissions repair is needed.
23+
$Applied = @{}
24+
$AppIds = @(@($ManifestPermissions.PSObject.Properties.Name) + @($Submitted.PSObject.Properties.Name)) | Where-Object { $_ } | Sort-Object -Unique
25+
foreach ($AppId in $AppIds) {
2126
$ManifestApp = $ManifestPermissions.$AppId
2227
$ManifestAppIds = @($ManifestApp.applicationPermissions.id)
2328
$ManifestDelIds = @($ManifestApp.delegatedPermissions.id)
2429

25-
$ExtraApp = @(foreach ($Permission in $Submitted.$AppId.applicationPermissions) {
26-
if ($Permission.id -and $ManifestAppIds -notcontains $Permission.id) {
27-
[PSCustomObject]@{ id = $Permission.id; value = $Permission.value }
28-
}
29-
})
30-
$ExtraDel = @(foreach ($Permission in $Submitted.$AppId.delegatedPermissions) {
31-
if ($Permission.id -and $ManifestDelIds -notcontains $Permission.id) {
32-
[PSCustomObject]@{ id = $Permission.id; value = $Permission.value }
33-
}
34-
})
30+
$AppPerms = [System.Collections.Generic.List[object]]::new()
31+
$DelPerms = [System.Collections.Generic.List[object]]::new()
3532

36-
if ($ExtraApp.Count -gt 0 -or $ExtraDel.Count -gt 0) {
37-
$Extras.$AppId = @{
38-
applicationPermissions = $ExtraApp
39-
delegatedPermissions = $ExtraDel
33+
foreach ($Permission in $ManifestApp.applicationPermissions) {
34+
$AppPerms.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
35+
}
36+
foreach ($Permission in $ManifestApp.delegatedPermissions) {
37+
$DelPerms.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
38+
}
39+
foreach ($Permission in $Submitted.$AppId.applicationPermissions) {
40+
if ($Permission.id -and $ManifestAppIds -notcontains $Permission.id) {
41+
$AppPerms.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
42+
}
43+
}
44+
foreach ($Permission in $Submitted.$AppId.delegatedPermissions) {
45+
if ($Permission.id -and $ManifestDelIds -notcontains $Permission.id) {
46+
$DelPerms.Add([PSCustomObject]@{ id = $Permission.id; value = $Permission.value })
47+
}
48+
}
49+
50+
if ($AppPerms.Count -gt 0 -or $DelPerms.Count -gt 0) {
51+
$Applied.$AppId = @{
52+
applicationPermissions = @($AppPerms)
53+
delegatedPermissions = @($DelPerms)
4054
}
4155
}
4256
}
4357

4458
$Entity = @{
4559
'PartitionKey' = 'CIPP-SAM'
4660
'RowKey' = 'CIPP-SAM'
47-
'Permissions' = [string]([PSCustomObject]$Extras | ConvertTo-Json -Depth 10 -Compress)
61+
'Permissions' = [string]([PSCustomObject]$Applied | ConvertTo-Json -Depth 10 -Compress)
4862
'UpdatedBy' = $User.UserDetails ?? 'CIPP-API'
4963
}
5064
$Table = Get-CIPPTable -TableName 'AppPermissions'
5165
$null = Add-CIPPAzDataTableEntity @Table -Entity $Entity -Force
5266
$Body = @{
53-
'Results' = 'Additional permissions updated. Default CIPP permissions are always applied and cannot be removed. Please run a Permissions check and CPV refresh to finalise the changes.'
67+
'Results' = 'Permissions updated. Default CIPP permissions are always applied and cannot be removed. Please run a Permissions check and CPV refresh to finalise the changes.'
5468
}
55-
Write-LogMessage -headers $Request.Headers -API 'ExecSAMAppPermissions' -message 'CIPP-SAM additional permissions updated' -Sev 'Info' -LogData $Extras
69+
Write-LogMessage -headers $Request.Headers -API 'ExecSAMAppPermissions' -message 'CIPP-SAM permissions updated' -Sev 'Info' -LogData $Applied
5670
} catch {
5771
$Body = @{
5872
'Results' = $_.Exception.Message

host.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
"distributedTracingEnabled": false,
1717
"version": "None"
1818
},
19-
"defaultVersion": "10.5.7",
19+
"defaultVersion": "10.5.8",
2020
"versionMatchStrategy": "Strict",
2121
"versionFailureStrategy": "Fail"
2222
}

0 commit comments

Comments
 (0)