Skip to content

Commit 6639901

Browse files
pull[bot]pull[bot]
authored
Merge pull request #1054 from KelvinTegelaar/dev
[pull] dev from KelvinTegelaar:dev
2 parents 5bbf74a + d30b2f4 commit 6639901

5 files changed

Lines changed: 158 additions & 344 deletions

File tree

‎Modules/CIPPCore/Public/Entrypoints/Timer Functions/Start-UpdateTokensTimer.ps1‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ function Start-UpdateTokensTimer {
4242
$AppRegistration = New-GraphGetRequest -uri "https://graph.microsoft.com/v1.0/applications(appId='$AppId')?`$select=id,passwordCredentials,servicePrincipalLockConfiguration" -NoAuthCheck $true -AsApp $true -ErrorAction Stop
4343
# sort by latest expiration date and get the first one
4444
$LastPasswordCredential = $AppRegistration.passwordCredentials | Sort-Object -Property endDateTime -Descending | Select-Object -First 1
45+
$PasswordCredentials = $AppRegistration.passwordCredentials | Sort-Object -Property endDateTime -Descending
4546

4647
try {
4748
$AppPolicyStatus = Update-AppManagementPolicy
@@ -60,24 +61,39 @@ function Start-UpdateTokensTimer {
6061
}
6162

6263
if ($AppSecret) {
63-
if ($env:AzureWebJobsStorage -eq 'UseDevelopmentStorage=true' -or $env:NonLocalHostAzurite -eq 'true') {
64-
$Table = Get-CIPPTable -tablename 'DevSecrets'
65-
$Secret = Get-CIPPAzDataTableEntity @Table -Filter "PartitionKey eq 'Secret' and RowKey eq 'Secret'"
66-
$Secret.ApplicationSecret = $AppSecret.secretText
67-
Add-AzDataTableEntity @Table -Entity $Secret -Force
68-
} else {
69-
Set-CippKeyVaultSecret -VaultName $KV -Name 'ApplicationSecret' -SecretValue (ConvertTo-SecureString -String $AppSecret.secretText -AsPlainText -Force)
64+
try {
65+
if ($env:AzureWebJobsStorage -eq 'UseDevelopmentStorage=true' -or $env:NonLocalHostAzurite -eq 'true') {
66+
$Table = Get-CIPPTable -tablename 'DevSecrets'
67+
$Secret = Get-CIPPAzDataTableEntity @Table -Filter "PartitionKey eq 'Secret' and RowKey eq 'Secret'"
68+
$Secret.ApplicationSecret = $AppSecret.secretText
69+
Add-AzDataTableEntity @Table -Entity $Secret -Force
70+
} else {
71+
Set-CippKeyVaultSecret -VaultName $KV -Name 'ApplicationSecret' -SecretValue (ConvertTo-SecureString -String $AppSecret.secretText -AsPlainText -Force)
72+
}
73+
Write-LogMessage -API 'Update Tokens' -message "New application secret generated for $AppId. Expiration date: $($AppSecret.endDateTime)." -sev 'INFO'
74+
} catch {
75+
# Storing the new secret failed. It exists on the app registration but not where CIPP reads
76+
# it, and as the newest credential it would suppress regeneration on the next run - leaving
77+
# the stored secret permanently stale. Roll the new secret back off the app registration so
78+
# state stays consistent and regeneration is retried on the next run.
79+
Write-LogMessage -API 'Update Tokens' -message "Failed to store new application secret for $AppId. Rolling back the generated secret, see Log Data for details. Will try again in 7 days." -sev 'CRITICAL' -LogData (Get-CippException -Exception $_)
80+
try {
81+
New-GraphPostRequest -uri "https://graph.microsoft.com/v1.0/applications/$($AppRegistration.id)/removePassword" -Body "{`"keyId`":`"$($AppSecret.keyId)`"}" -NoAuthCheck $true -AsApp $true -ErrorAction Stop
82+
Write-Information "Rolled back unstored application secret with keyId $($AppSecret.keyId)."
83+
} catch {
84+
Write-LogMessage -API 'Update Tokens' -message "Failed to roll back unstored application secret with keyId $($AppSecret.keyId) for $AppId, see Log Data for details." -sev 'CRITICAL' -LogData (Get-CippException -Exception $_)
85+
}
86+
$AppSecret = $null
7087
}
71-
Write-LogMessage -API 'Update Tokens' -message "New application secret generated for $AppId. Expiration date: $($AppSecret.endDateTime)." -sev 'INFO'
7288
}
7389

7490
# Clean up expired application secrets
75-
$ExpiredSecrets = $PasswordCredentials.passwordCredentials | Where-Object { $_.endDateTime -lt (Get-Date).ToUniversalTime() }
91+
$ExpiredSecrets = $PasswordCredentials | Where-Object { $_.endDateTime -lt (Get-Date).ToUniversalTime() }
7692
if ($ExpiredSecrets.Count -gt 0) {
7793
Write-Information "Found $($ExpiredSecrets.Count) expired application secrets for $AppId. Removing them."
7894
foreach ($Secret in $ExpiredSecrets) {
7995
try {
80-
New-GraphPostRequest -uri "https://graph.microsoft.com/v1.0/applications/$($PasswordCredentials.id)/removePassword" -Body "{`"keyId`":`"$($Secret.keyId)`"}" -NoAuthCheck $true -AsApp $true -ErrorAction Stop
96+
New-GraphPostRequest -uri "https://graph.microsoft.com/v1.0/applications/$($AppRegistration.id)/removePassword" -Body "{`"keyId`":`"$($Secret.keyId)`"}" -NoAuthCheck $true -AsApp $true -ErrorAction Stop
8197
Write-Information "Removed expired application secret with keyId $($Secret.keyId)."
8298
} catch {
8399
Write-LogMessage -API 'Update Tokens' -message "Error removing expired application secret with keyId $($Secret.keyId), see Log Data for details." -sev 'CRITICAL' -LogData (Get-CippException -Exception $_)

‎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

0 commit comments

Comments
 (0)