Powershell code for removing disabled AD logins doesn't work (doesn't remove) - what is wrong with the code?

Question

I'm a beginner in Powershell.

I'm trying to write a code using DbaTools module that gets AD logins from SQL Server, checks them against AD and removes logins for disabled AD users. The script seems to be running without errors, but on the following executions the same logins to be removed appear again - meaning they weren't removed.

Thanks!

This is the code I tried:

cls

#get all Windows logins, strip domain and store them
$users = Get-DbaLogin -SqlInstance SQL-DWH-DEV -Type Windows | Where-Object {($_.LoginType -eq 'WindowsUser') -and ($_.Name -like 'CONTOSO-AD\*') -and ($_.Name -notlike '*$*')} | Select-Object Name
$logins = @()

#for each login check against AD if the user is disabled
#if disabled - store and add back domain name 
foreach ($user in $users)
{
    
    #$user
    $SamAccountName = ($user.Name -split '\\')[1]
    $login = Get-ADUser -Identity $SamAccountName | Where-Object {($_.Enabled -eq $false)} | Select-Object @{name="Login"; expression={"CONTOSO-AD\"+$_.SamAccountName}}
    $logins += $login
}

#convert array to comma separated list
$remove = $logins.Login.ForEach{"$_"} -join ','

#remove disabled logins
Remove-DbaLogin -SqlInstance sql-dwh-dev -Login $remove

Answer 1

There are a couple of issues with your code, and you do to much string manipulation.
That can influence what gets to the Remove-DbaLogin Cmdlet.

You can accomplish the same with:

# You need to either Select-Object -ExpandProperty or ().Property to get a list of user names (strings).
$users = (Get-DbaLogin -SqlInstance 'SQL-DWH-DEV' -Type 'Windows' | Where-Object { ($_.LoginType -eq 'WindowsUser') -and ($_.Name -like 'CONTOSO-AD*') -and ($_.Name -notlike '$') }).Name
[System.Collections.ArrayList]$disabledUsers = @()

foreach ($user in $users) {

    # -split uses regex. You can use .NET .Split().
    $samAccountName = $user.Name.Split('\')[1]
    
    # Get-ADUser have a -Filter property. Where-Object is much more expensive.
    $adUser = Get-ADUser -Identity $SamAccountName -Filter { Enabled -eq $false }
    
    # There is no need to split the user name, than join it again. If the disabled user exists, you add it to the list.
    if ($adUser) {
        $disabledUsers.Add($user)
    }
}

# Remove-DbaLogin parameter -Login accepts a [string[]], we don't need to join, just convert our ArrayList to a string[]
Remove-DbaLogin -SqlInstance sql-dwh-dev -Login $disabledUsers.ToArray()

Take a look at the comments.
I didn't tested this, so if you get parsing errors on the Get-ADUser -Filter parameter, you either have to fix, or go back to Where-Object.

Hope it helps.
Happy scripting!