What is wrong with my "if" statement

PowerShell Help
1

So I have a powershell script which is meant to disable active directory users using a specific [stupid freaking, needlessly overcomplicated] process designated by one of my companies clients, which, given an Active Directory user object Display name, will determine all of the groups to which the user is a member and remove the user from all Groups with the exception of the “domain users” group and a custom security group with the phrase “xenoport” in the SamAccountName attribute. But hey, I’m just a lowly system engineer, so the complications involved in administrating an active directory are way above my bachelors in IT and experience my own personal domain at home DURRRP (Snide sarcasm). Now that I have gotten my personal frustration out of the way, I am running into an issue where, when running what I think to be the correct code, my script keeps ignoring the if statement which is supposed to prevent the foreach-object loop from operating on groups that meet the aforementioned criteria and proceeds to attempt to remove the user from all groups to which it is a member. I know the code is right up to the if statement, as I tried replacing the code within the foreach-object script block to write the SamAccountName for each of the objects which it obtains from the pipeline, and it all looks correct, so I can only assume there is something wrong with the nested if statement. Any ideas? Oh also, how can I suppress the prompt which keeps asking me “are you sure” when removing the user from each group? Super annoying microsoft… That is what “-whatif” is for. Also, if you see any other issues, informing me of them would be much appreciated.

$user = [Microsoft.VisualBasic.Interaction]::InputBox("Enter user's first and last name", "Disable AD user", "")
$sam = Get-ADUser -Filter {DisplayName -like $user} -Properties *
Get-ADUser -Filter {DisplayName -like $user} -Properties SamAccountName | Get-ADPrincipalGroupMembership | ForEach-Object {
If (($_.SamAccountName -ne 'Domain Users') -or ($_.SamAccountName -notlike 'Xenoport'))
{
Remove-ADGroupMember -Identity $_ -Members $sam
}
}
$newdi = "zz$($sam.DisplayName)"
$samacc = $sam.SamAccountName
$desti = $sam.DistinguishedName
$type = $sam.office
Write-Host "$newdi`n$samacc"
Set-ADUser $sam -DisplayName $newdi -Add @{msExchHideFromAddressLists="TRUE";mailNickname="$samacc"}
If ($type -eq 'Field'){
Move-ADObject -Identity $desti -TargetPath "OU=Disabled Users DO-NOT-DELETE,OU=Arbor FIELD,DC=ArborPharma,DC=Local"}
Else{
Move-ADObject -Identity $desti -TargetPath "OU=Disabled Users DO-NOT-DELETE,OU=Arbor ATL,DC=ArborPharma,DC=Local"}
Disable-ADAccount $sam
2

Could you perhaps restate the question a bit more succinctly? I’m having trouble parsing all of that to figure out what I’m supposed to be looking at…

3

The script looks ok from a quick glance. Are you saying the move doesn’t happen from either the if or else? Also, you have some debug info in there, I’d add $type to make sure its populating.

4

The comparison operator you are using is like but you are not specifying a wildcard, so it should be -like ‘something’. To stop confirmation popups, you need to do -Confirm:$false to force no confirmation. I’ve added some other general suggestions to your code, but you should be using -WhatIf to test if everything is working and there is no error handling in this script, so you could get some unexpected results:

$user = [Microsoft.VisualBasic.Interaction]::InputBox("Enter user's first and last name", "Disable AD user", "")
#You are getting a user and ALL properties, so there is no reason to do another Get-ADUser since
#this is being set to a variable.  This will be faster if you only return the properties that you are going to use below.
#DistinguishedName and SamAccountName are part of the default resultset, so you don't even need to specify -Properties
$sam = Get-ADUser -Filter {DisplayName -like $user} -Properties *

#Rather than looping through all memberships and then seeing if a condition is met, filter
#the object and use the pipeline to pass what you want to remove
$sam | 
Get-ADPrincipalGroupMembership | 
Where { ($_.SamAccountName -ne 'Domain Users') -or ($_.SamAccountName -notlike '*Xenoport*') } |
Remove-ADGroupMember -Members $sam

#Setting variables with other variables is confusing IMHO. When I read this script,
#I have to go find $type and see what that is versus just using the actual property.
#newdi is a new variable, but personally I would do $newDisplayName so that it's obvious
#what you are trying to do
$newdi = "zz$($sam.DisplayName)"

Write-Host "$newdi`n$sam.SamAccountName"

Set-ADUser $sam -DisplayName $newdi -Add @{msExchHideFromAddressLists="TRUE";mailNickname=$sam.SamAccountName} -Confirm:$false

#splatting:
#use hash tables to pass parameters, it's clean and you can dynamically, add, remove or update params
$moveParams = @{
    Identity   = $sam.DistinguishedName 
    TargetPath = "OU=Disabled Users DO-NOT-DELETE,OU=Arbor ATL,DC=ArborPharma,DC=Local" 
    Confirm    = $false
}

#You are passing the same params to Move-ADObject, so we can define them once and then
#just update the TargetPath if the criteria is met
if ( $sam.office -eq 'Field' ){
    $moveParams.Set_Item("TargetPath", "OU=Disabled Users DO-NOT-DELETE,OU=Arbor FIELD,DC=ArborPharma,DC=Local")
}

Move-ADObject @moveParams

Disable-ADAccount -Identity $sam -Confirm:$false