Merge pull request #5541 from NikCharlebois/FIXES-#5524

Fixes #5524
microsoft · Dec 12, 2024 · 0983c9c · 0983c9c
2 parents 4152ed1 + 458b726
commit 0983c9c
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,8 @@
 * AADApplication
   * Changed logic to remove all permissions when an empty array is specified.
     FIXES [#5534](https://github.com/microsoft/Microsoft365DSC/issues/5534)
+  * Changed logic to update AppRoles by first disabling the entry.
+    FIXES [#5524](https://github.com/microsoft/Microsoft365DSC/issues/5524)
 * AADFeatureRolloutPolicy
   * Fixed policy retrieval
     FIXES [#5521](https://github.com/microsoft/Microsoft365DSC/issues/5521)

diff --git a/Modules/Microsoft365DSC/DSCResources/MSFT_AADApplication/MSFT_AADApplication.psm1 b/Modules/Microsoft365DSC/DSCResources/MSFT_AADApplication/MSFT_AADApplication.psm1
@@ -888,6 +888,8 @@ function Set-TargetResource
         }
 
         $currentParameters.Add('ApplicationId', $AppIdValue)
+        $currentParameters.Remove('AppRoles') | Out-Null
+
         Write-Verbose -Message "Updating existing AzureAD Application {$DisplayName} with values:`r`n$($currentParameters | Out-String)"
         Update-MgApplication @currentParameters
 
@@ -898,6 +900,62 @@ function Set-TargetResource
         $needToUpdatePermissions = $true
         $needToUpdateAuthenticationBehaviors = $true
         $needToUpdateKeyCredentials = $true
+
+        # Update AppRoles
+        if ($null -ne $AppRoles)
+        {
+            Write-Verbose -Message "AppRoles were specified."
+
+            # Find roles to Remove
+            $fixedRoles = @()
+            $rolesToRemove = @()
+            foreach ($currentRole in $currentAADApp.AppRoles)
+            {
+                $associatedDesiredRoleEntry = $AppRoles | Where-Object -FilterScript {$_.DisplayName -eq $currentRole.DisplayName}
+                if ($null -eq $associatedDesiredRoleEntry)
+                {
+                    Write-Verbose -Message "Could not find matching AppRole entry in Desired values for {$($currentRole.DisplayName)}. Will remove role."
+                    $fixedRole = $currentRole
+                    $fixedRole.IsEnabled = $false
+                    $fixedRoles += $fixedRole
+                    $rolesToRemove += $currentRole.DisplayName
+                }
+                else
+                {
+                    Write-Verbose -Message "Found matching AppRole entry in Desired values for {$($currentRole.DisplayName)}. Keeping same value as current, but setting to disable."
+                    $entry = @{
+                        AllowedMemberTypes = $currentRole.AllowedMemberTypes
+                        Id                 = $currentRole.Id
+                        IsEnabled          = $false
+                        Origin             = $currentRole.Origin
+                        Value              = $currentRole.Value
+                        DisplayName        = $currentRole.DisplayName
+                        Description        = $currentRole.Description
+                    }
+                    $fixedRoles += $entry
+                }
+            }
+
+            Write-Verbose -Message "Updating AppRoles with the disabled roles to remove: {$($rolesToRemove -join ',')}"
+            Update-MgApplication -ApplicationId $currentAADApp.ObjectId -AppRoles $fixedRoles
+
+            Write-Verbose -Message "Updating the app a second time, this time removing the app roles {$($rolesToRemove -join ',')} and updating the others."
+            $resultingAppRoles = @()
+            foreach ($currentAppRole in $AppRoles)
+            {
+                $entry = @{
+                    AllowedMemberTypes = $currentAppRole.AllowedMemberTypes
+                    Id                 = $currentAppRole.Id
+                    IsEnabled          = $currentAppRole.IsEnabled
+                    Origin             = $currentAppRole.Origin
+                    Value              = $currentAppRole.Value
+                    DisplayName        = $currentAppRole.DisplayName
+                    Description        = $currentAppRole.Description
+                }
+                $resultingAppRoles += $entry
+            }
+            Update-MgApplication -ApplicationId $currentAADApp.ObjectId -AppRoles $resultingAppRoles
+        }
     }
     # App exists but should not
     elseif ($Ensure -eq 'Absent' -and $currentAADApp.Ensure -eq 'Present')
@@ -1039,6 +1097,10 @@ function Set-TargetResource
                         {
                             $roleId = $role.Id
                         }
+                        if ([System.String]::IsNullOrEmpty($roleId))
+                        {
+                            throw "Could not find associated role {$($permission.Name)} for API {$($sourceAPI)}"
+                        }
                         $appPermission = @{
                             Id   = $roleId
                             Type = 'Role'
@@ -1054,6 +1116,7 @@ function Set-TargetResource
         }
 
         Write-Verbose -Message "Updating permissions for Azure AD Application {$($currentAADApp.DisplayName)} with RequiredResourceAccess:`r`n$($allRequiredAccess | Out-String)"
+        Write-Verbose -Message "ResourceAccess:`r`n$($allRequiredAccess.ResourceAccess | Out-String)"
         Write-Verbose -Message "Current App Id: $($currentAADApp.AppId)"
 
         # Even if the property is named ApplicationId, we need to pass in the ObjectId
@@ -1072,7 +1135,8 @@ function Set-TargetResource
             requireClientServicePrincipal = $AuthenticationBehaviors.requireClientServicePrincipal
         }
 
-        Update-MgBetaApplication -ApplicationId $currentAADApp.Id -AuthenticationBehaviors $IAuthenticationBehaviors | Out-Null
+        Update-MgBetaApplication -ApplicationId $currentAADApp.Id `
+                                 -AuthenticationBehaviors $IAuthenticationBehaviors | Out-Null
     }
 
     if ($needToUpdateKeyCredentials -and $KeyCredentials)

diff --git a/...oft365DSC/DSCResources/MSFT_FabricAdminTenantSettings/MSFT_FabricAdminTenantSettings.psm1 b/...oft365DSC/DSCResources/MSFT_FabricAdminTenantSettings/MSFT_FabricAdminTenantSettings.psm1
@@ -574,7 +574,7 @@ function Get-TargetResource
         }
         else
         {
-            $uri = (Get-MSCloudLoginConnectionProfile -Workload Fabric).HostUrl + '/v1/admin/tenantsettings'
+            $uri = (Get-MSCloudLoginConnectionProfile -Workload 'Fabric').HostUrl + '/v1/admin/tenantsettings'
             $instance = Invoke-M365DSCFabricWebRequest -Uri $uri -Method 'GET'
         }
         if ($null -eq $instance)
@@ -1941,7 +1941,7 @@ function Export-TargetResource
     try
     {
         $Script:ExportMode = $true
-        $uri = (Get-MSCloudLoginConnectionProfile -Workload Fabric).HostUrl + '/v1/admin/tenantsettings'
+        $uri = (Get-MSCloudLoginConnectionProfile -Workload 'Fabric').HostUrl + '/v1/admin/tenantsettings'
         [array] $Script:exportedInstances = Invoke-M365DSCFabricWebRequest -Uri $uri -Method 'GET'
 
         if ($null -ne $Global:M365DSCExportResourceInstancesCount)

diff --git a/Modules/Microsoft365DSC/Modules/WorkloadHelpers/M365DSCFabricHelper.psm1 b/Modules/Microsoft365DSC/Modules/WorkloadHelpers/M365DSCFabricHelper.psm1
@@ -17,7 +17,7 @@ function Invoke-M365DSCFabricWebRequest
     )
 
     $headers = @{
-        Authorization = (Get-MSCloudLoginConnectionProfile -Workload Fabric).AccessToken
+        Authorization = (Get-MSCloudLoginConnectionProfile -Workload 'Fabric').AccessToken
     }
 
     $response = Invoke-WebRequest -Method $Method -Uri $Uri -Headers $headers -Body $Body

diff --git a/Tests/Unit/Microsoft365DSC/Microsoft365DSC.AADApplication.Tests.ps1 b/Tests/Unit/Microsoft365DSC/Microsoft365DSC.AADApplication.Tests.ps1
@@ -57,7 +57,8 @@ Describe -Name $Global:DscHelper.DescribeHeader -Fixture {
             Mock -CommandName Get-MgServicePrincipal -MockWith {
                 $servicePrincipal = New-Object PSCustomObject
                 $servicePrincipal | Add-Member -MemberType NoteProperty -Name DisplayName -Value 'Microsoft Graph'
-                $servicePrincipal | Add-Member -MemberType NoteProperty -Name ObjectID -Value '12345-12345-12345-12345-12345'
+                $servicePrincipal | Add-Member -MemberType NoteProperty -Name ObjectID -Value '12345-12345-12345-12345-12345'                
+                $servicePrincipal | Add-Member -MemberType NoteProperty -Name AppRoles -Value @(@{Value = "User.Read.All";Id="123"})
                 return $servicePrincipal
             }