Skip to content

Commit

Permalink
fixed flaw in how Recursioncount was incremented and added more unit …
Browse files Browse the repository at this point in the history 
…tests
  • Loading branch information
@tkol2022 @mitchelbaker-cisa
tkol2022 authored and mitchelbaker-cisa committed Sep 24, 2024
1 parent 182a0e5 commit 851c7ee
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 11 deletions.
20 changes: 12 additions & 8 deletions PowerShell/ScubaGear/Modules/Providers/ExportAADProvider.psm1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,14 +292,14 @@ function Get-PrivilegedUser {
$Objecttype = $User.AdditionalProperties."@odata.type" -replace "#microsoft.graph."

if ($Objecttype -eq "user") {
LoadObjectDataIntoPrivilegedUserHashtable -RoleName $Role.DisplayName -PrivilegedUsers $PrivilegedUsers -ObjectId $User.Id -TenantHasPremiumLicense $TenantHasPremiumLicense -Objecttype "user"
LoadObjectDataIntoPrivilegedUserHashtable -RoleName $Role.DisplayName -PrivilegedUsers $PrivilegedUsers -ObjectId $User.Id -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment -Objecttype "user"
}
elseif ($Objecttype -eq "group") {
# In this context $User.Id is a group identifier
$GroupId = $User.Id

# Process all of the group members that are transitively assigned to the current role as Active via group membership
LoadObjectDataIntoPrivilegedUserHashtable -RoleName $Role.DisplayName -PrivilegedUsers $PrivilegedUsers -ObjectId $GroupId -TenantHasPremiumLicense $TenantHasPremiumLicense -Objecttype "group"
LoadObjectDataIntoPrivilegedUserHashtable -RoleName $Role.DisplayName -PrivilegedUsers $PrivilegedUsers -ObjectId $GroupId -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment -Objecttype "group"
}
}
}
Expand All @@ -320,7 +320,7 @@ function Get-PrivilegedUser {

foreach ($PIMRoleAssignment in $PIMRoleAssignments) {
$UserObjectId = $PIMRoleAssignment.PrincipalId
LoadObjectDataIntoPrivilegedUserHashtable -RoleName $Role.DisplayName -PrivilegedUsers $PrivilegedUsers -ObjectId $UserObjectId -TenantHasPremiumLicense $TenantHasPremiumLicense
LoadObjectDataIntoPrivilegedUserHashtable -RoleName $Role.DisplayName -PrivilegedUsers $PrivilegedUsers -ObjectId $UserObjectId -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment
}
}
}
Expand Down Expand Up @@ -352,6 +352,10 @@ function LoadObjectDataIntoPrivilegedUserHashtable {
[ValidateNotNullOrEmpty()]
[bool]$TenantHasPremiumLicense,

[Parameter(Mandatory=$true)]
[ValidateNotNullOrEmpty()]
[string]$M365Environment,

# This describes the type of Entra Id object that the parameter ObjectId is referencing.
# Valid values are "user", "group". If this is not passed, the function will call Graph to dynamically determine the object type.
[Parameter()]
Expand All @@ -360,10 +364,10 @@ function LoadObjectDataIntoPrivilegedUserHashtable {
[Parameter()]
[int]$Recursioncount = 0
)

# We support group nesting up to 2 levels deep.
Write-Warning "Recursion level: $recursioncount"
# We support group nesting up to 2 levels deep (allows levels 0 and 1).
# Safeguard: Also protects against infinite loop attacks if there is a circular group assignment in PIM.
if ($recursioncount -gt 2) {
if ($recursioncount -ge 2) {
return
}

Expand Down Expand Up @@ -435,8 +439,8 @@ Write-Warning "Processing role: $($RoleName) PIM group Eligible member: $($Group
if ($GroupMember.AccessId -ne "member") { continue }
$PIMEligibleUserId = $GroupMember.PrincipalId

$Recursioncount++
LoadObjectDataIntoPrivilegedUserHashtable -RoleName $RoleName -PrivilegedUsers $PrivilegedUsers -ObjectId $PIMEligibleUserId -TenantHasPremiumLicense $TenantHasPremiumLicense -Recursioncount $Recursioncount
$LoopIterationRecursioncount = $Recursioncount + 1
LoadObjectDataIntoPrivilegedUserHashtable -RoleName $RoleName -PrivilegedUsers $PrivilegedUsers -ObjectId $PIMEligibleUserId -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment -Recursioncount $LoopIterationRecursioncount
}
}
}
Expand Down
159 changes: 156 additions & 3 deletions ...Unit/PowerShell/Providers/AADProvider/LoadObjectDataIntoPrivilegedUserHashtable.Tests.ps1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ InModuleScope ExportAADProvider {
Describe -Tag 'LoadObjectDataIntoPrivilegedUserHashtable' -Name 'Not Found' {
BeforeAll {
}
It 'Deleted user Request_ResourceNotFound' {

It 'Deleted user triggers Request_ResourceNotFound exception' {
# Set up the parameters for the test
$Role = [PSCustomObject]@{ DisplayName = "Global Administrator" } # Mock role with DisplayName
$RoleName = "Global Administrator" # Mock role
$PrivilegedUsers = @{} # Empty hashtable for privileged users
$ObjectId = [Guid]::NewGuid().Guid # Random GUID for ObjectId
$TenantHasPremiumLicense = $true
$M365Environment = "commercial"

# Simulate the "Request_ResourceNotFound" exception
Mock Get-MgBetaDirectoryObject {
Expand All @@ -22,14 +24,165 @@ InModuleScope ExportAADProvider {
Mock Write-Warning

# Call the function under test
LoadObjectDataIntoPrivilegedUserHashtable -Role $Role -PrivilegedUsers $PrivilegedUsers -ObjectId $ObjectId -TenantHasPremiumLicense $TenantHasPremiumLicense
LoadObjectDataIntoPrivilegedUserHashtable -RoleName $RoleName -PrivilegedUsers $PrivilegedUsers -ObjectId $ObjectId -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment

# Ensure the Write-Warning was called because Get-MgBetaDirectoryObject throws an exception
Should -Invoke -CommandName Write-Warning -Times 1

# Check that the function returned early and did not add anything to $PrivilegedUsers
$PrivilegedUsers.Count | Should -Be 0
}

It 'Objecttype is is a user' {
# Set up the parameters for the test
$RoleName = "Global Administrator" # Mock role
$PrivilegedUsers = @{} # Empty hashtable for privileged users
$ObjectId = [Guid]::NewGuid().Guid # Random GUID for ObjectId
$TenantHasPremiumLicense = $true
$M365Environment = "commercial"

# Mock Get-MgBetaDirectoryObject to return a user-type object
Mock Get-MgBetaDirectoryObject {
[PSCustomObject]@{
AdditionalProperties = @{
"@odata.type" = "#microsoft.graph.user" # Simulates a user type
}
}
}

# Mock Get-MgBetaUser to return a user with DisplayName and OnPremisesImmutableId
Mock Get-MgBetaUser {
[PSCustomObject]@{
DisplayName = "John Doe"
OnPremisesImmutableId = "ABC123"
}
}

# Test 1 - Do NOT pass ObjectType
LoadObjectDataIntoPrivilegedUserHashtable -RoleName $RoleName -PrivilegedUsers $PrivilegedUsers -ObjectId $ObjectId -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment
# Assertions to ensure the user was processed correctly
$PrivilegedUsers[$ObjectId].DisplayName | Should -Be "John Doe"
$PrivilegedUsers[$ObjectId].OnPremisesImmutableId | Should -Be "ABC123"
$PrivilegedUsers[$ObjectId].roles | Should -Contain $RoleName

# Test 2 - Pass ObjectType
$PrivilegedUsers = @{}
$Objecttype = "user"
LoadObjectDataIntoPrivilegedUserHashtable -Objecttype $Objecttype -RoleName $RoleName -PrivilegedUsers $PrivilegedUsers -ObjectId $ObjectId -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment

# Assertions to ensure the user was processed correctly
$PrivilegedUsers[$ObjectId].DisplayName | Should -Be "John Doe"
$PrivilegedUsers[$ObjectId].OnPremisesImmutableId | Should -Be "ABC123"
$PrivilegedUsers[$ObjectId].roles | Should -Contain $RoleName
}


It 'Objecttype is a group' {
# Set up the parameters for the test
$RoleName = "Global Administrator" # Mock role
$PrivilegedUsers = @{} # Empty hashtable for privileged users
$ObjectId = [Guid]::NewGuid().Guid # Random GUID for ObjectId, simulating a group ID
$TenantHasPremiumLicense = $true
$M365Environment = "commercial"

# Mock Get-MgBetaDirectoryObject to return a group-type object
Mock Get-MgBetaDirectoryObject {
[PSCustomObject]@{
AdditionalProperties = @{
"@odata.type" = "#microsoft.graph.group" # Simulates a group type
}
}
}

# Mock Get-MgBetaGroupMember to return two group members (users)
Mock Get-MgBetaGroupMember {
@(
[PSCustomObject]@{
Id = [Guid]::NewGuid().Guid
AdditionalProperties = @{
"@odata.type" = "#microsoft.graph.user" # First user in the group
}
},
[PSCustomObject]@{
Id = [Guid]::NewGuid().Guid
AdditionalProperties = @{
"@odata.type" = "#microsoft.graph.user" # Second user in the group
}
}
)
}

# Mock Get-MgBetaUser to return a user object with DisplayName and OnPremisesImmutableId for both users
Mock Get-MgBetaUser {
param ($UserId)
[PSCustomObject]@{
DisplayName = "User $UserId"
OnPremisesImmutableId = "ImmutableId-$UserId"
}
}

# Mock Invoke-GraphDirectly to return no PIM eligible members
Mock Invoke-GraphDirectly {
@() # Returns an empty array
}

########## Test 1 - Do NOT pass ObjectType
LoadObjectDataIntoPrivilegedUserHashtable -RoleName $RoleName -PrivilegedUsers $PrivilegedUsers -ObjectId $ObjectId -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment

# Assertions to ensure the group members were processed correctly
$PrivilegedUsers.Count | Should -Be 2 # Two users should have been added

# Ensure both users have their properties set correctly
$PrivilegedUsers.Values | ForEach-Object {
$_.roles | Should -Contain $RoleName
$_.DisplayName | Should -Match "User"
$_.OnPremisesImmutableId | Should -Match "ImmutableId"
}

########## Test 2 - Pass ObjectType
$PrivilegedUsers = @{}
$Objecttype = "group"
LoadObjectDataIntoPrivilegedUserHashtable -Objecttype $Objecttype -RoleName $RoleName -PrivilegedUsers $PrivilegedUsers -ObjectId $ObjectId -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment

# Assertions to ensure the group members were processed correctly
$PrivilegedUsers.Count | Should -Be 2 # Two users should have been added

# Ensure both users have their properties set correctly
$PrivilegedUsers.Values | ForEach-Object {
$_.roles | Should -Contain $RoleName
$_.DisplayName | Should -Match "User"
$_.OnPremisesImmutableId | Should -Match "ImmutableId"
}

########## Test 3 - Trigger recursion by mocking Invoke-GraphDirectly to return some users
$PrivilegedUsers = @{}
$Objecttype = "group"
# Mock Invoke-GraphDirectly to return two PIM eligible users (simulating a recursion case)
Mock Invoke-GraphDirectly {
@(
[PSCustomObject]@{
PrincipalId = [Guid]::NewGuid().Guid # First PIM eligible user
AccessId = "member" # Simulates eligible PIM member
},
[PSCustomObject]@{
PrincipalId = [Guid]::NewGuid().Guid # Second PIM eligible user
AccessId = "member" # Simulates eligible PIM member
}
)
}

LoadObjectDataIntoPrivilegedUserHashtable -Objecttype $Objecttype -RoleName $RoleName -PrivilegedUsers $PrivilegedUsers -ObjectId $ObjectId -TenantHasPremiumLicense $TenantHasPremiumLicense -M365Environment $M365Environment

# Two group members that each trigger the recursion 2 levels deep = 2 + 2 + 2 = 6
$PrivilegedUsers.Count | Should -Be 6

# Ensure all users have their properties set correctly
$PrivilegedUsers.Values | ForEach-Object {
$_.roles | Should -Contain $RoleName
$_.DisplayName | Should -Match "User"
$_.OnPremisesImmutableId | Should -Match "ImmutableId"
}
}
}
}

Expand Down

0 comments on commit 851c7ee

Please sign in to comment.