Inefficient PowerShell function for checking group membership

Question

I use the following function in a PowerShell script to check and - depending on the value of the $Action - either add or remove an Active Directory User Object from a Security Group.

Function Update-Group-Membership ($Action, $ADUser_Properties, $GroupName, $LogPathFileName) {

    $CurrentGroupMembers = Get-ADGroupMember -Identity:$GroupName | ? { $_.objectClass -ieq 'user' } | Select-Object -ExpandProperty "samAccountName"
    $ExistingMember = [bool]($ADUser_Properties | ? { $CurrentGroupMembers -contains $_.samAccountName })

    If ($Action -eq "ADD") {
        If ($ExistingMember) {
            "[" + (Get-Date -format "dd/MM/yyyy HH:mm:ss") + "]  ... Update NOT Required: Existing Member of $GroupName" | Out-File $LogPathFileName -Append
            Write-Host "  ... Update NOT Required: Existing Member of $GroupName" -ForegroundColor DarkGray      
        } Else {
            # Add to the group
            $PSCommand = $ADUser_Properties | Add-ADPrincipalGroupMembership -MemberOf:$GroupName
            "[" + (Get-Date -format "dd/MM/yyyy HH:mm:ss") + "]  ... Updated: Added to $GroupName" | Out-File $LogPathFileName -Append
            Write-Host "  ... Updated: Added to $GroupName" -ForegroundColor Yellow              
        }
    }

    If ($Action -eq "REMOVE") {
        If ($ExistingMember) {
            # Remove to the group
            $PSCommand = $ADUser_Properties | Remove-ADPrincipalGroupMembership -MemberOf:$GroupName -Confirm $False
            "[" + (Get-Date -format "dd/MM/yyyy HH:mm:ss") + "]  ... Updated: Removed from $GroupName" | Out-File $LogPathFileName -Append
            Write-Host "  ... Updated: Removed from $GroupName" -ForegroundColor Yellow
        }
}   
}

The function is called as follows:

Update-Group-Membership "ADD" $ADUser_Properties "SECURITY_GROUP_NAME" $LogPathFileName

$ADUser_Properties contains the output of the Get-ADUser command.

Whilst this function performs as expected, it is not particularly efficient and takes over a second to check a single user which consequently means processing several hundred users takes several hours.

Does anyone have any suggestions on how to improve or fine-tune it?

Answer 1

This is completely 100% NOT TESTED because I don't have the means to test it at this time however, it should work the way you want and a bit faster than before because it will do alot less queries. It should in theory only query each group once throughout the entire script which if your running even a single group against hundreds of users will save hundreds of queries. Next step I would suggest that you move to a more different loggin function such as streamwriter, is literally over 100x faster at writing logs.

Function Update-GroupMembership {
    Param (
        [Parameter(Mandatory=$True)]
        [ValidateSet('Add','Remove')]
        [String]$Action,
        [Parameter(Mandatory=$True)]
        $ADUser_Properties,
        [Parameter(Mandatory=$True)]
        [String]$ADGroup,
        [Parameter(Mandatory=$True)]
        [String]$LogPathFileName
    )

    IF (-NOT (Test-Path Variable:/Function)) {
        New-Variable -Force -ErrorAction Stop -Scope Script -Name UGM -Value @{}
        $Script:UGM.Users = @{}
        $Script:UGM.Groups = @{}
        if (-not (Get-Module -Name "ActiveDirectory")) {
            Try {
                Import-Module ActiveDirectory
                $Script:UGM.ActiveDirectory = $True
            } Catch {
                $Script:UGM.ActiveDirectory = $False
            }
        }
    }

    #Skip script is active directory not loaded.
    If (!$Script:UGM.ActiveDirectory) {
        Throw "Active Directory module not loaded."
    }


    IF (-NOT ($Script:UGM.ContainsKey($ADGroup))) {
        $Script:UGM.$ADGroupName = New-Object -Type 'System.Collections.ArrayList'
        Get-ADGroupMember -Identity $ADGroup |Select -ExpandProperty SamAccountName |ForEach {$Script:UGM.$ADGroupName.Add($_)}
    }

    $UGM.InGroup = $Script:UGM.$ADGroup.Contains($ADUser_Properties.SamAccountName)
    $Date = ([datetime]::Now).ToString('dd/MM/yyyy HH:mm:ss')
    Switch ($Action) {
        'Add' {
            If ($UGM.InGroup) {
                Write-Host "$($ADUser_Properties.Name) Already in $ADGroup"
            } Else {
                Try {
                    Add-ADPrincipalGroupMembership -MemberOf:$ADGroup -Identity:$ADUser_Properties |Out-Null
                    $Script:UGM.$ADGroup.Add($ADUser_Properties.SamAccountName)
                    Write-Host "  ... Updated: Added to $ADGroup" -ForegroundColor Yellow
                    "[$Date]  ... Updated: Added to $ADGroup" | Out-File $LogPathFileName -Append
                } Catch {
                    Throw "[$Date]  ... Failed: Unable to add $($ADUser_Properties.Name) to $ADGroup"
                }
            }
        }
        'Remove' {
            If ($UGM.InGroup) {
                Write-Host "$($ADUser_Properties.Name) not in $ADGroup"
            } Else {
                Try {
                    Remove-ADPrincipalGroupMembership -MemberOf:$ADGroup -Identity:$ADUser_Properties |Out-Null
                    $Script:UGM.$ADGroup.Remove($ADUser_Properties.SamAccountName)
                    Write-Host "  ... Updated: Removed from $ADGroup" -ForegroundColor Yellow
                    "[$Date]  ... Updated: Removed from $ADGroup" | Out-File $LogPathFileName -Append
                } Catch {
                    Throw "[$Date]  ... Failed: Unable to remove $($ADUser_Properties.Name) from $ADGroup"
                }
            }
        }
        Default {}
    }
}