Powershell script that updates Active Directory user information

Question

I wrote this Powershell script years ago for updating AD from a CSV file. I'd like to try to optimize to run quicker and be more efficient... but I'm not sure where to start / what could improve it. Any suggestions are welcome.

EDIT: I've updated my original script to this:

$users = Import-Csv C:\Scripts\Employees.csv

foreach ($user in $users) {

$employeeNumber = $User.employeeNumber
$Department     = $User.department
$Title          = $User.title
$Office         = $User.office
$Address        = $User.address
$City           = $User.city
$State          = $User.state
$PostalCode     = $User.postalCode
$Company        = $User.company
$Telephone      = $User.telephone
$Mobile         = $User.mobile
$Fax            = $User.fax
$Custom1        = $User.custom1
$Custom2        = $User.custom2
$Custom3        = $User.custom3
$Custom4        = $User.custom4

Get-ADUser -Filter "EmployeeID -eq '$($user.EmployeeID)'" `
    -SearchBase "OU=Logins,DC=domain,DC=com" |
    Set-ADUser -Replace @{ `
    employeeNumber=$EmployeeNumber;
    department=$Department;
    title=$Title;
    Office=$office;
    streetAddress=$Address;
    l=$City;
    st=$State;
    postalCode=$PostalCode;
    company=$Company;
    telephoneNumber=$telephone;
    mobile=$cell;
    facsimileTelephoneNumber=$Fax;
    ExtensionAttribute1=($user.custom1);
    ExtensionAttribute2=($user.custom2);
    ExtensionAttribute3=($user.custom3);
    ExtensionAttribute4=($user.custom4)
                    }
}

Answer 1

I agree with what @veefu suggests, and would like to build off that. To start with, you have different columns in your CSV as to what Set-ADUser is expecting. To work around this I would suggest creating a property map hashtable.

$PropMap = @{
    EmployeeNumber='EmployeeNumber'
    Department='Department'
    Title='Title'
    office='Office'
    Address='StreetAddress'
    City='City'
    State='State'
    PostalCode='PostalCode'
    Company='Company'
    telephone='OfficePhone'
    cell='Mobile'
    Fax='Fax'
    custom1='ExtensionAttribute1'
    custom2='ExtensionAttribute2'
    custom3='ExtensionAttribute3'
    custom4='ExtensionAttribute4'
}

Then you want to find out what the valid parameters for Set-ADUser are to determine if a given column should be applied as a parameter, or be fed to the -Replace parameter.

#Find the valid parameters for the Set-ADUser cmdlet (only strings, to avoid complex objects and switches, and no common parameters)
$ValidParams = Get-Command Set-ADUser -ShowCommandInfo |% ParameterSets |% Parameters |?{$_.ParameterType.FullName -match 'String' -and $_.Name -notin [System.Management.Automation.PSCmdlet]::CommonParameters} |Select -Expand Name -Unique

Then you import the CSV and loop through the users. For each user you find what properties they have filled in on the spreadsheet, see if it is a parameter for the Set-ADUser cmdlet, and build a hashtable to splat to the cmdlet.

#Import the list of users and updates
$Users = Import-Csv C:\Scripts\Employees.csv

#Loop through users
ForEach($User in $Users){
    #Set an empty hashtable for what this user needs to have updated
    $UserParams = @{}
    #Find the properties for this user that have values
    $UserProps = $User.PSObject.Properties.Name |?{![string]::IsNullOrWhiteSpace($User.$_)}
    #If it has a parameter in Set-ADUser add it to $UserParams
    $UserProps |?{($ParamName=If($_ -in $PropMap.Keys){$PropMap[$_]}Else{$_}) -in $ValidParams} |%{$UserParams.Add($ParamName,$User.$_)}
    #If it doesn't have a valid parameter build a hashtable for the -Replace parameter, and add that to $UserParams
    $UserProps |?{($ParamName=If($_ -in $PropMap.Keys){$PropMap[$_]}Else{$_}) -notin $ValidParams} |% -Begin {$ToReplace = $_} -Process {$ToReplace.Add($ParamName,$User.$_)} -End {$UserParams.Add('Replace',$ToReplace)}

Here I added a little Try/Catch to let you know if it failed to find or update any of the users, and if it failed to find them, or failed to update them, and gives the error. I used the shortened -ea instead of -ErrorAction out of habit.

    #Try to find the user, and apply the updates
    Try{
        Get-AdUser -Filter "EmployeeID -eq '$($user.EmployeeID)'" -SearchBase "ou=Logins,dc=domain,dc=com" -ea Stop | Set-ADUser @UserParams -ea Stop
    }Catch{
        #If the Get-ADUser or Set-ADUser cmdlets failed throw a warning with the user's EmployeeID for investigation

        #Determine if it was Get-ADUser or Set-ADUser that failed, to see if we couldn't find the user or couldn't update the user
        $WarningMessage = If($_.InvocationInfo.MyCommand -eq 'Get-ADUser'){'find'}else{'update'}
        Write-Warning "Unable to $WarningMessage user with EmployeeID (actual error follows): $($User.EmployeeID)"
        Write-Warning "$_`n"
    }
}

Answer 2

Here are a few comments:

# -3. No parameters to the script and hard-coded path to the input CSV
#     This makes the script harder for others to reuse
# -2. No error handling
#     if an employee in the CSV doesn't exist in AD, for instance,
#     you might want to know about it.
# -1. Rigid requirements for the CSV file
#     What happens if one of the columns is missing? 
#     Would it be worthwhile to add flexibility logic, so 
#     so the script could handle CSVs containing fewer columns of
#     modification?

$users = Import-Csv -Path C:\Scripts\Employees.csv
# 0. Style - use more vertical space in your code for better readability

foreach ($user in $users) {
# 1. Eliminate duplicate Get-ADUser calls
Get-ADUser -ErrorAction SilentlyContinue -Filter "EmployeeID -eq '$($user.EmployeeID)'" `
        -Properties * -SearchBase "ou=Logins,dc=domain,dc=com" |
# 2. Eliminate '-properties *': your script doesn't read 
#    current user property values, so why retrieve them?

# 3. Consider eliminating multiple calls to Set-ADUser
#    All modifications can probably be done in a single
#    Set-ADUser -Replace $Properties
#    call. You'd need to reorganize the modifications into
#    a single hashtable.

    Set-ADUser -EmployeeNumber $($user."EmployeeNumber") `
        -Department $($user."Department") `
        -Title $($user."Title") `
        -Office $($user."office") `
        -StreetAddress $($user."Address") `
        -City $($user."City") `
        -State $($user."State") `
        -PostalCode $($user."PostalCode") `
        -Company $($user."Company") `
        -OfficePhone $($user."telephone") `
        -Mobile $($user."cell") `
        -Fax $($user."Fax")

Get-ADUser -ErrorAction SilentlyContinue -Filter "EmployeeID -eq '$($user.EmployeeID)'" -Properties * -SearchBase "ou=Logins,dc=domain,dc=com" |
    Set-ADUser -Replace @{
            ExtensionAttribute1=($user.custom1) 
            ExtensionAttribute2=($user.custom2)
            ExtensionAttribute3=($user.custom3)
            ExtensionAttribute4=($user.custom4)
        }
}