1

I'm trying to write a script that removes users from a security group if they aren't in a specific OU.

I'm having trouble comparing my array of users from the OU, to the array of users from the security group.

To test I looped through the content in $testGroup and $userList. Both look similar to me but it's clear they don't compare as just outputting $userList -contains $user gives me a bunch of false results even though it should be true.

$userList = @()
$testGroup = @()

#Get current members of group. Using this instead of get-adgroupmember due to speed
$testGroup = Get-AdGroup "testGroup" -properties member | select-object -ExpandProperty member | get-aduser 

#Define OUs that we want to get members from
$OUlist = "OU1","OU2"

#Populate $userList with members of each OU
$OUlist | foreach {
    $userList += get-aduser -filter {Enabled -eq $True} -SearchBase "OU=$_,DC=dc,DC=dc2,DC=dc3"

}

#Check the group for anyone no longer in one of the approved OUs
$testGroup | foreach {

    if($userList -notcontains $user){
        #remove the user from $testGroup
    }

}
henrycarteruk
  • 12,708
  • 2
  • 36
  • 40
Andres M.
  • 13
  • 4
  • 2
    Not sure if you're aware, but when you use $x | foreach, you need to start working with the pipe. So in this case, the individual users are $_ (current item). I am not sure what $user is here, but you may need to replace it with $_. – Jacob Colvin Apr 18 '17 at 14:57

3 Answers3

2

Consider using Compare-Object with the property value set to compare by distinguished name; i.e.

compare-object -ReferenceObject $OUList -DifferenceObject $userList -Property 'DistinguishedName' | 
    ?{$_.SideIndicator -eq '=>'} | 
    select -expand InputObject

Full Code:

(untested)

$userList = @()
$testGroup = @()

$groupName = 'testGroup'

#Get current members of group. Using this instead of get-adgroupmember due to speed
$testGroup = Get-AdGroup $groupName -properties member | select-object -ExpandProperty member | get-aduser 

#Define OUs that we want to get members from
$OUlist = "OU1","OU2"

#Populate $userList with members of each OU
$OUlist | foreach {
    $userList += get-aduser -filter {Enabled -eq $True} -SearchBase "OU=$_,DC=dc,DC=dc2,DC=dc3" | Get-AdUser

}

#Check the group for anyone no longer in one of the approved OUs & remove group group
Remove-ADGroupMember -Identity $groupName -Members (compare-object -ReferenceObject $OUList -DifferenceObject $userList -Property 'DistinguishedName' | ?{$_.SideIndicator -eq '=>'} | select -ExpandProperty InputObject)
JohnLBevan
  • 22,735
  • 13
  • 96
  • 178
1

Here an example of comparing arrays:

 $a1=@(1,2,3,4,5)
 $b1=@(1,2,3,4,5,6)

 $result = Compare-Object -ReferenceObject ($a1) -DifferenceObject ($b1) -PassThru
 write-host $result

take also a look at this post compare arrays

Community
  • 1
  • 1
guiwhatsthat
  • 2,349
  • 1
  • 12
  • 24
1

There are a handful of issues... Using $Variable instead of $_ in $Variable | Foreach like MacroPower mentioned is one of them.

You can condense the whole thing like this:

# Get-ADGroupMember is easier than Get-ADGroup | Get-ADUser. 
# You also only need the SamAccountName.
# $TestGroup will be an array automatially... No need to $TestGroup = @()
$TestGroup = (Get-ADGroupMember 'TestGroup').SamAccountName

#Define OUs using their full paths.
$OUList = @(
    'OU=Whatever,DC=example,DC=com',
    'OU=Something,DC=example,DC=com'
)

# Easily call the OU's from $OUList using $_.
# Again, we only need SamAccountName
# Again, $UserList will automaticall be an array no '= @()' needed.
$OUList | ForEach-Object {
    $UserList += (Get-ADUser -Filter * -SearchBase $_).SamAccountName
}


# A proper foreach construct will let you work with $User instead of $_
foreach ($User in $TestGroup) 
{
    if ($User -notin $UserList)
    {
        # Put your action here.
    }
}

A final note, you switch between camelCase, PascalCase, and lowercase all over the place. While there is no official standard for PowerShell consistency makes code easier to read. PascalCase also tends to be the recommended due to the .NET style guide.

Also, if you wanted to use a compare instead of the foreach ($User in $TestGroup):

$Compare = Compare-Object -ReferenceObject ($UserList | Select -Unique) -DifferenceObject $TestGroup

$Compare | ForEach-Object {
    if ($_.sideindicator -eq '=>')
    {
        # Action here.
    }
}
omniomi
  • 130
  • 1
  • 5