Skip to content

Commit 4c8654c

Browse files
committed
Made changes as per Alek's suggestions
1 parent 0e92659 commit 4c8654c

2 files changed

Lines changed: 41 additions & 18 deletions

File tree

src/powershell/private/tests-shared/Get-ZtEmergencyAccessAccounts.ps1

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,12 @@ function Get-ZtEmergencyAccessAccounts {
7575
$sql = "SELECT id, userPrincipalName, displayName FROM User WHERE LOWER(userPrincipalName) = '$escapedUpn'"
7676
}
7777
elseif ($id) {
78-
$escapedId = $id -replace "'", "''"
78+
$guidRef = [System.Guid]::Empty
79+
if (-not [System.Guid]::TryParse($id, [ref]$guidRef)) {
80+
Write-PSFMessage "Skipping invalid user entry: Id '$id' is not a valid GUID" -Level Warning
81+
continue
82+
}
83+
$escapedId = $guidRef.ToString()
7984
$sql = "SELECT id, userPrincipalName, displayName FROM User WHERE id = '$escapedId'"
8085
}
8186
else {
@@ -104,27 +109,45 @@ function Get-ZtEmergencyAccessAccounts {
104109
continue
105110
}
106111

112+
$guidRef = [System.Guid]::Empty
113+
if (-not [System.Guid]::TryParse($id, [ref]$guidRef)) {
114+
Write-PSFMessage "Skipping invalid group entry: Id '$id' is not a valid GUID" -Level Warning
115+
continue
116+
}
117+
107118
# Resolve group members via Microsoft Graph API (GroupMember table not available in DB)
108119
try {
109120
Write-PSFMessage "Resolving emergency access group members via Graph API: Id=$id" -Level Verbose
110-
$membersResponse = Get-ZtGroupMember -GroupId $id -ErrorAction Stop
121+
$membersResponse = Get-ZtGroupMember -GroupId $id -Recurse -ErrorAction Stop
111122
$members = @($membersResponse | Where-Object { $_.'@odata.type' -eq '#microsoft.graph.user' })
112123

113124
if ($members.Count -gt 0) {
114-
# Batch all member ids into a single SQL lookup to avoid N+1 queries
115-
$escapedIds = $members | ForEach-Object { "'" + (($_.id) -replace "'", "''") + "'" }
116-
$idList = $escapedIds -join ','
117-
$memberSql = "SELECT id, userPrincipalName, displayName FROM User WHERE id IN ($idList)"
118-
$userDetailsList = @(Invoke-DatabaseQuery -Database $Database -Sql $memberSql)
119-
120-
foreach ($userDetails in $userDetailsList) {
121-
$emergencyAccessAccounts += [PSCustomObject]@{
122-
Id = $userDetails.id
123-
UserPrincipalName = $userDetails.userPrincipalName
124-
DisplayName = $userDetails.displayName
125-
Type = 'GroupMember'
125+
# Batch all member IDs into a single SQL lookup to avoid N+1 queries;
126+
# member IDs come from Graph API responses which are always valid GUIDs.
127+
$escapedIds = $members | ForEach-Object {
128+
$memberGuid = [System.Guid]::Empty
129+
if ([System.Guid]::TryParse($_.id, [ref]$memberGuid)) {
130+
"'" + $memberGuid.ToString() + "'"
131+
}
132+
} | Where-Object { $_ }
133+
134+
if (-not $escapedIds) {
135+
Write-PSFMessage "Emergency access group members had no valid GUIDs: Id=$id" -Level Warning
136+
}
137+
else {
138+
$idList = $escapedIds -join ','
139+
$memberSql = "SELECT id, userPrincipalName, displayName FROM User WHERE id IN ($idList)"
140+
$userDetailsList = @(Invoke-DatabaseQuery -Database $Database -Sql $memberSql)
141+
142+
foreach ($userDetails in $userDetailsList) {
143+
$emergencyAccessAccounts += [PSCustomObject]@{
144+
Id = $userDetails.id
145+
UserPrincipalName = $userDetails.userPrincipalName
146+
DisplayName = $userDetails.displayName
147+
Type = 'GroupMember'
148+
}
149+
Write-PSFMessage "Emergency access group member found: $($userDetails.userPrincipalName)" -Level Verbose
126150
}
127-
Write-PSFMessage "Emergency access group member found: $($userDetails.userPrincipalName)" -Level Verbose
128151
}
129152
}
130153
else {

src/powershell/tests/Test-Assessment.21815.ps1

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ from vwRole
4646
$emergencyAccountIds = @($emergencyAccounts | Select-Object -ExpandProperty Id)
4747

4848
# Filter out emergency access accounts from the results
49-
$results = @($permanentPrivileged | Where-Object { $emergencyAccountIds -notcontains $_.principalId })
50-
$excludedEmergencyAccounts = @($permanentPrivileged | Where-Object { $emergencyAccountIds -contains $_.principalId })
49+
$results = @($permanentPrivileged | Where-Object { $_.principalId -notin $emergencyAccountIds })
50+
$excludedEmergencyAccounts = @($permanentPrivileged | Where-Object { $_.principalId -in $emergencyAccountIds })
5151

5252
# Count of *distinct* excluded emergency accounts (one user can have multiple permanent role assignments)
5353
$excludedAccountCount = @($excludedEmergencyAccounts | Select-Object -ExpandProperty principalId -Unique).Count
@@ -113,7 +113,7 @@ The following emergency access accounts were excluded from this check as they ar
113113
'@
114114
foreach ($emergency in $excludedEmergencyAccounts) {
115115
$portalLink = 'https://entra.microsoft.com/#view/Microsoft_AAD_UsersAndTenants/UserProfileMenuBlade/~/AdministrativeRole/userId/{0}/hidePreviewBanner~/true' -f $emergency.principalId
116-
$emergencySection += "| [$(Get-SafeMarkdown($emergency.principalDisplayName))]($portalLink) | $(Get-SafeMarkdown($emergency.userPrincipalName)) | $(Get-SafeMarkdown($emergency.roleDisplayName)) |`n"
116+
$emergencySection += "| [$(Get-SafeMarkdown($emergency.principalDisplayName))]($portalLink) | $($emergency.userPrincipalName) | $($emergency.roleDisplayName) |`n"
117117
}
118118
}
119119

0 commit comments

Comments
 (0)