From f1528cd309fc32fb592fcaf381383ed797779040 Mon Sep 17 00:00:00 2001 From: Devesh Agarwal Date: Thu, 30 Jan 2025 12:22:03 -0500 Subject: [PATCH] addressing PR comments, adding more error handling --- .../Modules/CreateReport/CreateReport.psm1 | 34 +++-- .../Modules/Providers/ExportAADProvider.psm1 | 135 +++--------------- .../ProviderHelpers/CommandTracker.psm1 | 2 +- 3 files changed, 39 insertions(+), 132 deletions(-) diff --git a/PowerShell/ScubaGear/Modules/CreateReport/CreateReport.psm1 b/PowerShell/ScubaGear/Modules/CreateReport/CreateReport.psm1 index 274bf4b03..1666cadc2 100644 --- a/PowerShell/ScubaGear/Modules/CreateReport/CreateReport.psm1 +++ b/PowerShell/ScubaGear/Modules/CreateReport/CreateReport.psm1 @@ -270,27 +270,33 @@ function New-Report { # Create a section header for the licensing information $LicensingHTML = "

Tenant Licensing Information

" + $LicenseTable - # Create a section for privileged service principals - $privilegedServicePrincipalsTable = $SettingsExport.privileged_service_principals.psobject.properties | ForEach-Object { - $principal = $_.Value - [pscustomobject]@{ - "Display Name" = $principal.DisplayName - "Service Principal ID" = $principal.ServicePrincipalId - "Roles" = ($principal.roles -join ", ") - "App ID" = $principal.AppId + if ($null -ne $SettingsExport -and $null -ne $SettingsExport.privileged_service_principals) { - } - } | ConvertTo-Html -Fragment + # Create a section for privileged service principals + $privilegedServicePrincipalsTable = $SettingsExport.privileged_service_principals.psobject.properties | ForEach-Object { + $principal = $_.Value + [pscustomobject]@{ + "Display Name" = $principal.DisplayName + "Service Principal ID" = $principal.ServicePrincipalId + "Roles" = ($principal.roles -join ", ") + "App ID" = $principal.AppId + + } + } | ConvertTo-Html -Fragment - $privilegedServicePrincipalsTable = $privilegedServicePrincipalsTable -replace '^(.*?)', '
' + $privilegedServicePrincipalsTable = $privilegedServicePrincipalsTable -replace '^(.*?)
', '
' - # Create a section header for the service principal information - $privilegedServicePrincipalsTableHTML = "

Privileged Service Principal Table

" + $privilegedServicePrincipalsTable + # Create a section header for the service principal information + $privilegedServicePrincipalsTableHTML = "

Privileged Service Principal Table

" + $privilegedServicePrincipalsTable + $ReportHTML = $ReportHTML.Replace("{SERVICE_PRINCIPAL}", $privilegedServicePrincipalsTableHTML) + } + else{ + $ReportHTML = $ReportHTML.Replace("{SERVICE_PRINCIPAL}", "") + } $ReportHTML = $ReportHTML.Replace("{AADWARNING}", $AADWarning) $ReportHTML = $ReportHTML.Replace("{LICENSING_INFO}", $LicensingHTML) - $ReportHTML = $ReportHTML.Replace("{SERVICE_PRINCIPAL}", $privilegedServicePrincipalsTableHTML) $CapJson = ConvertTo-Json $SettingsExport.cap_table_data } else { diff --git a/PowerShell/ScubaGear/Modules/Providers/ExportAADProvider.psm1 b/PowerShell/ScubaGear/Modules/Providers/ExportAADProvider.psm1 index 1ba2ba533..3b3fe3757 100644 --- a/PowerShell/ScubaGear/Modules/Providers/ExportAADProvider.psm1 +++ b/PowerShell/ScubaGear/Modules/Providers/ExportAADProvider.psm1 @@ -123,26 +123,28 @@ function Export-AADProvider { $PrivilegedUsers = @{} $PrivilegedServicePrincipals = @{} - #PrivilegedObjects is an array because of the tracker.trycommand, and so the first index is the hashtable - foreach ($key in $PrivilegedObjects[0].Keys) { + if ($PrivilegedObjects.Count -gt 0 -and $null -ne $PrivilegedObjects[0].Keys) { - # Check if it has ServicePrincipalId property instead of AppId - if ($null -ne $PrivilegedObjects[0][$key].ServicePrincipalId) { - $PrivilegedServicePrincipals[$key] = $PrivilegedObjects[0][$key] - } - else { - $PrivilegedUsers[$key] = $PrivilegedObjects[0][$key] - } - } + #PrivilegedObjects is an array because of the tracker.trycommand, and so the first index is the hashtable + foreach ($key in $PrivilegedObjects[0].Keys) { - $PrivilegedUsers = ConvertTo-Json $PrivilegedUsers - $PrivilegedServicePrincipals = ConvertTo-Json $PrivilegedServicePrincipals + # Check if it has ServicePrincipalId property instead of AppId + if ($null -ne $PrivilegedObjects[0][$key].ServicePrincipalId) { + $PrivilegedServicePrincipals[$key] = $PrivilegedObjects[0][$key] + } + else { + $PrivilegedUsers[$key] = $PrivilegedObjects[0][$key] + } + } - # While ConvertTo-Json won't mess up a dict as described in the above comment, - # on error, $TryCommand returns an empty list, not a dictionary. - $PrivilegedUsers = if ($null -eq $PrivilegedUsers) {"{}"} else {$PrivilegedUsers} - $PrivilegedServicePrincipals = if ($null -eq $PrivilegedServicePrincipals) {"{}"} else {$PrivilegedServicePrincipals} + $PrivilegedUsers = ConvertTo-Json $PrivilegedUsers + $PrivilegedServicePrincipals = ConvertTo-Json $PrivilegedServicePrincipals + # While ConvertTo-Json won't mess up a dict as described in the above comment, + # on error, $TryCommand returns an empty list, not a dictionary. + $PrivilegedUsers = if ($null -eq $PrivilegedUsers) {"{}"} else {$PrivilegedUsers} + $PrivilegedServicePrincipals = if ($null -eq $PrivilegedServicePrincipals) {"{}"} else {$PrivilegedServicePrincipals} + } # Get-PrivilegedRole provides a list of security configurations for each privileged role and information about Active user assignments if ($RequiredServicePlan){ # If the tenant has the premium license then we also include calls to PIM APIs @@ -490,107 +492,6 @@ function LoadObjectDataIntoPrivilegedUserHashtable { } } -# function LoadObjectDataIntoPrivilegedUserHashtable { -# param ( -# [Parameter(Mandatory=$true)] -# [ValidateNotNullOrEmpty()] -# [string]$RoleName, - -# [Parameter(Mandatory=$true)] -# [hashtable]$PrivilegedUsers, - -# [Parameter(Mandatory=$true)] -# [ValidateNotNullOrEmpty()] -# [string]$ObjectId, - -# [Parameter(Mandatory=$true)] -# [ValidateNotNullOrEmpty()] -# [bool]$TenantHasPremiumLicense, - -# [Parameter(Mandatory=$true)] -# [ValidateNotNullOrEmpty()] -# [string]$M365Environment, - -# [Parameter()] -# [string]$Objecttype = "", - -# [Parameter()] -# [int]$Recursioncount = 0 -# ) - -# if ($recursioncount -ge 2) { -# return -# } - -# if ($Objecttype -eq "") { -# try { -# $DirectoryObject = Get-MgBetaDirectoryObject -ErrorAction Stop -DirectoryObjectId $ObjectId -# } catch { -# if ($_.Exception.Message -match "Request_ResourceNotFound") { -# Write-Warning "Processing privileged users. Resource $ObjectId may have been recently deleted from the directory because it was not found." -# return -# } -# else { -# throw $_ -# } -# } -# $Objecttype = $DirectoryObject.AdditionalProperties."@odata.type" -replace "#microsoft.graph." -# } - -# if ($Objecttype -eq "user") { -# if (-Not $PrivilegedUsers.ContainsKey($ObjectId)) { -# $AADUser = Get-MgBetaUser -ErrorAction Stop -UserId $ObjectId -# $PrivilegedUsers[$ObjectId] = @{"DisplayName"=$AADUser.DisplayName; "OnPremisesImmutableId"=$AADUser.OnPremisesImmutableId; "roles"=@()} -# } -# if ($PrivilegedUsers[$ObjectId].roles -notcontains $RoleName) { -# $PrivilegedUsers[$ObjectId].roles += $RoleName -# } -# } -# elseif ($Objecttype -eq "servicePrincipal") { -# if (-Not $PrivilegedUsers.ContainsKey($ObjectId)) { -# $AADServicePrincipal = Get-MgBetaServicePrincipal -ServicePrincipalId $ObjectId -ErrorAction Stop -# $PrivilegedUsers[$ObjectId] = @{ -# "DisplayName" = $AADServicePrincipal.DisplayName -# "ServicePrincipalId" = $AADServicePrincipal.Id -# "AppId" = $AADServicePrincipal.AppId -# "roles" = @() -# } -# } -# if ($PrivilegedUsers[$ObjectId].roles -notcontains $RoleName) { -# $PrivilegedUsers[$ObjectId].roles += $RoleName -# } -# } -# elseif ($Objecttype -eq "group") { -# $GroupId = $ObjectId -# $GroupMembers = Get-MgBetaGroupMember -All -ErrorAction Stop -GroupId $GroupId - -# foreach ($GroupMember in $GroupMembers) { -# $Membertype = $GroupMember.AdditionalProperties."@odata.type" -replace "#microsoft.graph." -# if ($Membertype -eq "user" -or $Membertype -eq "servicePrincipal") { -# if (-Not $PrivilegedUsers.ContainsKey($GroupMember.Id)) { -# LoadObjectDataIntoPrivilegedUserHashtable -RoleName $RoleName -PrivilegedUsers $PrivilegedUsers -ObjectId $GroupMember.Id -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment -Objecttype $Membertype -# } -# if ($PrivilegedUsers[$GroupMember.Id].roles -notcontains $RoleName) { -# $PrivilegedUsers[$GroupMember.Id].roles += $RoleName -# } -# } -# } - -# if ($TenantHasPremiumLicense) { -# $graphArgs = @{ -# "commandlet" = "Get-MgBetaIdentityGovernancePrivilegedAccessGroupEligibilityScheduleInstance" -# "queryParams" = @{'$filter' = "groupId eq '$GroupId'"} -# "M365Environment" = $M365Environment } -# $PIMGroupMembers = Invoke-GraphDirectly @graphArgs -# foreach ($GroupMember in $PIMGroupMembers) { -# if ($GroupMember.AccessId -ne "member") { continue } -# $PIMEligibleUserId = $GroupMember.PrincipalId -# $LoopIterationRecursioncount = $Recursioncount + 1 -# LoadObjectDataIntoPrivilegedUserHashtable -RoleName $RoleName -PrivilegedUsers $PrivilegedUsers -ObjectId $PIMEligibleUserId -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment -Recursioncount $LoopIterationRecursioncount -# } -# } -# } -# } function AddRuleSource{ <# .NOTES diff --git a/PowerShell/ScubaGear/Modules/Providers/ProviderHelpers/CommandTracker.psm1 b/PowerShell/ScubaGear/Modules/Providers/ProviderHelpers/CommandTracker.psm1 index d8c91fc32..a365903be 100644 --- a/PowerShell/ScubaGear/Modules/Providers/ProviderHelpers/CommandTracker.psm1 +++ b/PowerShell/ScubaGear/Modules/Providers/ProviderHelpers/CommandTracker.psm1 @@ -1,5 +1,5 @@ Import-Module -Name $PSScriptRoot/../ExportEXOProvider.psm1 -Function Get-ScubaSpfRecord, Get-ScubaDkimRecord, Get-ScubaDmarcRecord -Import-Module -Name $PSScriptRoot/../ExportAADProvider.psm1 -Function Get-PrivilegedRole, Get-PrivilegedUser, Get-PrivilegedServicePrincipal +Import-Module -Name $PSScriptRoot/../ExportAADProvider.psm1 -Function Get-PrivilegedRole, Get-PrivilegedUser class CommandTracker { [string[]]$SuccessfulCommands = @()