0

I have written a script to disable and move users listed in a text file, and as part of this script, I would like to remove them from groups that grant them licenses to certain software.

My script removes the users, but gets stuck in a loop telling me that the group has been removed, and I can't figure out why.

I'm not sure what I'm missing here, help?!?

Thank you!

This is the block that's looping:

foreach ($group in $groups){
#Write-Host $group
foreach ($user in Get-ADGroupMember -Identity $group){
    If ((Get-ADUser $user.SamAccountName -Properties MemberOf).MemberOf -Contains $group){
        Write-Host "$term is a member of $group"
        Remove-ADGroupMember -Identity $group -Member $user -Confirm:$false
        Write-Host "$term membership of $group removed."
        }
    else{
        Write-Host "$term is not a member of any groups"
        }
}

}

This is the whole script if needed (sanitized):

    #Import the Active Directory Module for Powershell
import-module activedirectory

#Get List of Terms
$terms = Get-Content "Terms.txt"

#Your name
$admin = Read-Host -Prompt "Please enter your name "

#foreach loop
foreach($term in $terms){

$user = Get-ADUser -Filter {displayName -like $term} -Properties CanonicalName

#Get location of User
$split = $user.DistinguishedName.Split(',')
$path = "$($split[-4])"
$location = $path
$ou = 'OU=Disabled,OU=People,'
$dn = ',DC=some,DC=domain,DC=tld'
$base = $ou + $location + $dn

# disable user
Disable-ADAccount -identity $user

#Add Description
$day = Get-Date -Format g
Set-ADUser $user -Description "Disabled by $admin $day"

Write-Host "$term Disabled by $admin on $day"

#Groups to remove user from on termination
$groups = @('CN=Group,OU=SomeOU,OU=Groups,OU=OU2,DC=some,DC=domain,DC=tld', 'CN=Group,OU=SomeOU,OU=Groups,OU=OU2,DC=some,DC=domain,DC=tld', 'sec_software_license_group1', 'sec_software_license_group2', 'sec_software_license_group3')

foreach ($group in $groups){
    #Write-Host $group
    foreach ($user in Get-ADGroupMember -Identity $group){
        If ((Get-ADUser $user.SamAccountName -Properties MemberOf).MemberOf -Contains $group){
            Write-Host "$term is a member of $group"
            Remove-ADGroupMember -Identity $group -Member $user -Confirm:$false
            Write-Host "$term membership of $group removed."
            }
        else{
            Write-Host "$term is not a member of any groups"
            }
    }
}

Write-Host 'Disabling and moving '$term

# move user
move-adobject -Identity $user -targetpath $base

write-host $term' is moved to Disabled'
}
1

So, my first thought is that there is a typo or transposing error that you ran into when sanitizing your script. So, the following is based strictly upon how the post was originally written...

Well, I can't be sure but it looks like this bit needs to go:

foreach ($user in Get-ADGroupMember -Identity $group){

It looks as if you're grabbing all users in every group, and then for every user you find, you remove them from the group - which might be fine - except you're not just doing this for termed users - you're doing this for every user in every group you've specified.

As best as I can tell, if you ran this against an account that was in the Domain Admin's group - you would effectively wipe the Domain Admin's security group.


I think the source of your problem may stem from the fact that the $user you define in line 13 is not the same as the $user that you're using in line 37.


I would suggest you replace line 37-46 with this single line:

Remove-ADGroupMember -Identity $Group -Member $User -Confirm:$False -ErrorAction:SilentlyContinue

It addresses a couple of concerns I have with the selection of code it replaces:

  1. It's efficient, and
  2. It gets rid of the unnecessary "Write-Host" lines ("-Verbose" exists for a reason), and
  3. It prevents any blood on the screen from appearing if the user isn't a member of any of the special groups.
  • I'm not sure what you mean about touching all groups. The groups variable is an array of specified groups. When running this against a test account it removed only that account from the specified groups, it simply looped indefinitely in the Write-Host user has been removed line. – Stryke_the_Orc May 3 '18 at 21:58
  • I do agree that verbose exists for a reason, but it's an over abundance of information scrolling across the screen for the folks that will be running this repeatedly. The output is simply to verify that user has been moved as quickly as possible. – Stryke_the_Orc May 3 '18 at 22:00
  • 3
    I agree with @semicolon; it's not stuck in a loop it's working through the loop busily removing every user from every group in your list, and it's happening because you are using the variable name $user twice for two different things, and you'd see it if you changed Write-Host "$term membership of $group removed." to Write-Host "$user membership of $group removed." to show you which user has just been removed, I think the names would be many, and changing. – TessellatingHeckler May 4 '18 at 1:03
  • 1
    It's very odd that the script I had only removed the one user, but you and @TessellatingHeckler were correct, it was listing every member of every group. I've made the suggested edit, thank you. – Stryke_the_Orc May 25 '18 at 20:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.