1

I'm learning Powershell scripting on a Windows Server. I'm adding users by importing their data from a csv file. The users & their info get added correctly.

The problem is my powershell script has to auto-add new users to groups based on their Departments. The if-else statements I use are in a for-loop. With my current code, all new users get added to the IT group only, even if they're not in the IT Department. They don't get added to any other group. Any advice?

Import-Module ActiveDirectory
$Users = Import-CSV -Path "C:\Users\Administrator\Desktop\Users.csv"

foreach ($User in $Users){
    $uname = $User.Username
    $first = $User.Firstname
    $last = $User.Lastname

    if ( Get-ADUser -Filter {SamAccountName -eq $uname})
    {
        Write-Warning "The user $uname already exists."
    }
    else
    {
        New-ADUser `
        -Name "$first $last" `
        -SamAccountName $uname `
        -Path $User.OUPath `
        -Office $User.Office `
        -Department $User.Department `
        -Enabled $true

        if (Get-ADUser -Filter {Department -eq "IT"})
        {
                Add-ADGroupMember -Identity IT -Members $uname
        }
        elseif (Get-ADUser -Filter {Department -eq "Marketing"})
        {
                Add-ADGroupMember -Identity Marketing -Members $uname
        }
        elseif (Get-ADUser -Filter {Department -eq "Sales"})
        {
                Add-ADGroupMember -Identity Sales -Members $uname
        }
        else
        {
               Add-ADGroupMember -Identity HR -Members $uname
        }
    }  #close of first else loop
} #close of foreach loop```


  • As an aside: While seductively convenient, the [use of script blocks (`{ ... }`) as `-Filter` arguments](https://stackoverflow.com/a/44184818/45375) is conceptually problematic and can lead to misconceptions. – mklement0 Jul 27 '21 at 18:05
  • You don't need `if()`'s at all.just do `Add-ADGroupMember -Identity $user.Department -Members $uname` since the department names seem to match the group names. – Theo Jul 27 '21 at 18:09

2 Answers2

1

In your if-statements, you actually do not check if the department of a user from your CSV file matches a certain string. Instead, you fetch all users from your AD whose Department attribute matches a certain string. As the result is probably not empty, your first if-statement always evaluates to true.

Change this:

if (Get-ADUser -Filter {Department -eq "IT"})
{
    Add-ADGroupMember -Identity IT -Members $uname
}

to this:

if ($User.Department -eq "IT")
{
    Add-ADGroupMember -Identity IT -Members $uname
}

and repeat this pattern for all other if-statements, too.

stackprotector
  • 10,498
  • 4
  • 35
  • 64
1

The issue is how you are approaching department detection, and the fact that you never specify the user in your Get-ADUser calls. The script is hitting:

if (Get-ADUser -Filter {Department -eq "IT"})

Where it gets users where Department has a value of "IT". All users where Department equals "IT", not just the one you're concerned with. That returns data, so the If statement returns as $true, and it never makes it to the other departments since the first part evaluated as $true. What I would do here is use the $User.Department value you already have to add the user to the right group. Skip the entire If/ElseIf bit and just do:

Add-ADGroupMember  -Identity $(if ($User.Department -in 'IT', 'Marketing', 'Sales') { $User.Department } else { 'HR' }) -Members $uname
TheMadTechnician
  • 34,906
  • 3
  • 42
  • 56
  • 1
    Nice, though note the `else` part, which would require the `-Identity` argument to be: `$(if ($User.Department -in 'IT', 'Marketing', 'Sales') { $User.Department } else { 'HR' })` or, in PowerShell 7+, `($User.Department -in 'IT', 'Marketing', 'Sales' ? $User.Department : 'HR')` – mklement0 Jul 27 '21 at 18:09