How to pass -Identity from Get-ADUser to a variable? (For copying user groups to new user)-CodePudding

Basically the idea of this script is to create a new user in AD but to also copy groups from another user in AD from a search with user input.

For example copy sales groups from a current team member to the newly created member. The error I'm getting is that my $ID variable is always empty and -Identity cant use it. If I hardcode the user I want to copy from this code works.

I can just ask for user input and have them put in the identity / username / samaccountname to copy groups from but they're not going to know that off the top of their head as the naming convention in AD includes employee numbers. They'd have to navigate AD to find that and this avoids the point of the script.

I want this script to be able to lookup a user based on just name for ease of use. This is why it uses -filter. If you have suggestions on how to handle potential duplicates of users with same name during this search I'm all ears for that too.

After it finds the user to copy from it copies the groups from the searched user to the newly created user.

Thanks for any help!

Do {

$Given = Read-Host -Prompt "Input new user first name"
$Surname = Read-Host -Prompt "Input new user last name"
$PW = Read-Host -Prompt "Input new user password"
$Phone = Read-Host -Prompt "Input new user phone number"
$NewSam = Read-Host -Prompt "Input preferred new user ID"
$User = "$Given $Surname"

$Confirmation = Read-Host "You input '$User' , '$NewSam' , '$PW' , and '$Phone' is this correct (y/n)?" 
}
while ($confirmation -ne "y")


New-ADUser -Name $User -GivenName $Given -Surname $Surname -SamAccountName $NewSam -AccountPassword (ConvertTo-SecureString -AsPlaintext "$PW" -Force) -Enabled $True `
-OfficePhone $Phone -ChangePasswordAtLogon $true

Do {

$clone = Read-Host -Prompt "Who are we copying groups from?"
$Confirmation2 = Read-Host "You input '$clone' is this correct (y/n)?" 
}
while ($confirmation2 -ne "y")

$ID = Get-ADUser -Filter 'Name -eq "$clone"'| Select-Object -ExpandProperty SamAccountName
$GetUserGroups = Get-ADUser -Identity "$ID" -Properties memberof | Select-Object -ExpandProperty memberof
$GetUserGroups | Add-ADGroupMember -Members $NewSam -Verbose

CodePudding user response：

You script starts to go sideways here:

$ID = Get-ADUser -Filter 'Name -eq "$clone"'| Select-Object -ExpandProperty SamAccountName
$GetUserGroups = Get-ADUser -Identity "$ID" -Properties memberof | Select-Object -ExpandProperty memberof

And you're so close, what's needed is:

$ID = Get-ADUser -Filter "Name -eq '$clone'"|
    Select-Object -ExpandProperty SamAccountName

The Filter requires single quotes around the name. The documentation on this is horrible and, for the Filter parameter, uses ScriptBlocks (code inside curly braces) while the actual type is [string]. I learned to stick with strings after fixing problems that were obscured by using ScriptBlocks.

You wouldn't even run into this problem if you simplified to:

$ID = Get-ADUser -Identity $clone | 
    Select-Object -ExpandProperty SamAccountName

As long as we're simplifying, you only need one line:

$GetUserGroups = Get-ADUser -Identity $clone -Properties memberof |
    Select-Object -ExpandProperty memberof

One more thing to consider. While piping to Select-Object is the PowerShell way and is the style I tend to use from the command line, in scripts I personally prefer:

$GetUserGroups = (Get-ADUser -Identity $clone -Properties memberof).memberof

But this is a matter of taste (while also being faster (which only matters in long running scripts)).

CodePudding user response：

While asking for user input via Read-Host is always tricky (a user can type in any bogus text he/she wants), I would at least give that user the opportunity to quit the loop by adding the q option in there as well.

Then you really should first do a check if the user perhaps already exists or not before creating with New-ADUser.

Finally, $GetUserGroups | Add-ADGroupMember -Members $NewSam -Verbose will not work as you expect, because the -Identity parameter for Add-ADGroup only takes one single group id at a time, so you need to loop over the groups there.

Try

do {
    $Given   = Read-Host -Prompt "Input new user first name"
    $Surname = Read-Host -Prompt "Input new user last name"
    $PW      = Read-Host -Prompt "Input new user password"
    $Phone   = Read-Host -Prompt "Input new user phone number"
    $NewSam  = Read-Host -Prompt "Input preferred new user ID (SamAccountName)"
    $User    = "$Given $Surname"

    $Confirmation = Read-Host "You input '$User' , '$NewSam' , '$PW' , and '$Phone' is this correct (y/n/q)?" 
    switch -Wildcard ($confirmation) {
        'q*' {
            # user wants to quit
            exit
        }
        'y*' {
            # here first check if that user already exists or not
            $existingUser = Get-ADUser -Filter "SamAccountName -eq '$NewSam'"
            if ($existingUser) {
                Write-Warning "A user with SamAccountName '$NewSam' already exists"
                $Confirmation = 'n'  # rerun the loop
            }
        }
    }
} while ($confirmation -notlike "y*")

# now proceed creating the new AD user
# because New-ADUser can take a lot of parameters, the cvleanest way is to use splatting
$userProps = @{
    Name                  = $User
    GivenName             = $Given
    Surname               = $Surname
    SamAccountName        = $NewSam
    AccountPassword       = ConvertTo-SecureString -AsPlaintext $PW -Force
    Enabled               = $True
    OfficePhone           = $Phone
    ChangePasswordAtLogon = $true
    # add switch parameter PassThru, so the cmdlet returns the new user object
    PassThru              = $true
}
$newUser = New-ADUser @userProps

do {
    $clone = Read-Host -Prompt "Who are we copying groups from? (SamAccountName)"
    $Confirmation = Read-Host "You input '$clone' is this correct (y/n/q)?" 
    switch -Wildcard ($Confirmation) {
        'q*' {
            # user wants to quit
            Write-Host "New user '$($newUser.Name)' has been created but not added to any groups.."
            exit
        }
        'y*' {
            # here first check if that user already exists or not
            $cloneUser = Get-ADUser -Filter "SamAccountName -eq '$clone'" -Properties MemberOf
            if (!$cloneUser) {
                Write-Warning "A user with SamAccountName '$clone' does not exist"
                $Confirmation = 'n'  # rerun the loop
            }
            else {
                # get the MemberOf properties from the second user
                # and add the new user to these groups
                $cloneUser.MemberOf | ForEach-Object {
                    $_ | Add-ADGroupMember -Members $newUser -Verbose
                }
            }
        }
    }
}
while ($confirmation -notlike "y*")

P.S. I'm using wildcard comparisons ('y*') on the confirmation input because otherwise if a user types 'yes' the loop will not see that as a YES