Pulling data from a CSV and populate AD

Question

I've been piecing this together little by little for a little over a week. I'm curious if there's a cleaner way to accomplish what this is doing. Basically, it's pulling data from a CSV, creating some variables and generating a user in AD and adding the user to various groups. The script works: the data is scraped from the CSV and the user is created and added to various groups. The logging works to a point but I can't seem to replicate an outright failure. If the user already exists, there's not much you can do - in my testing.

The logging, in my opinion, makes things quite ugly/cumbersome but in some respects, I'd like to have it more verbose and just give me details - pass or fail. Furthermore, I feel like the adding of users to the groups could be handled better.

# CSV file being imported.
$CsvFile = "$env:USERPROFILE\Downloads\SampleData.csv"

# Import the contents of the CSV file.
$Users = Import-Csv -Path "$CsvFile"

# Logs will be dumped here.
$LogFolder = "C:\Temp"

# Setting up an array for holding results.
$UserCreationSuccess = @()
$UserCreationFailure  = @()
$UsersAlreadyExist = @()
#$GroupJoinSuccess = @()
$VerbosePreference = "Continue"

# Loop through each line of the CSV, creating variables for each field.
ForEach ($User in $Users) {
    # Creating the basic variables.
    $FirstName = $User.'Student First Name'
    $MiddleInitial = $User.'I'
    $LastName = $User.'Student Last Name'
    $ADUserName = $User.'Stu Access Login'
    $StudentID = $User.'Other ID'
    $GradYear = $User.'Grad Year'
    $CapFInitial = $FirstName.substring(0,1).ToUpper()
    $MInitial = $MiddleInitial.substring(0,1).ToLower()
    $LInitial = $LastName.substring(0,1).ToLower()
    $Password = "$CapFInitial$MInitial$LInitial" + "#" + "$StudentID"
    # The folowing couple variables are created via Switch statements.
    $SchoolCode = Switch ($User.'School')
    {
        20 { "Exeter Township Senior High" }
        30 { "Exeter Township Junior High" }
        40 { "Lorane Elementary School" }
        50 { "Jacksonwald ES" }
        70 { "Reiffton School" }
        90 { "Owatin Creek Elementary School" }
    }

    $ADGroups = Switch ($User.'School')
    {
        20 { "Secondary Students", "Students" }
        30 { "Secondary Students", "Students" }
        40 { "K4 Students", "Students" }
        50 { "K4 Students", "Students" }
        70 { "Secondary Students", "Students" }
        90 { "K4 Students", "Students" }
    }

    # Headers for the CSV exported later.
    $ExportCsvProperties = @{
        FirstName = $FirstName;
        LastName = $LastName;
        UserName = $ADUserName;    
        Error = $Null;
        Date = (Get-Date)
    }

    If (-Not(Get-ADUser -Filter {SamAccountName -eq $ADUserName})) {
        Try {
            # Create user.
            New-ADUser `
                -Name "$FirstName $LastName" `
                -SamAccountName "$ADUserName" `
                -GivenName "$FirstName" `
                -Initials "$MiddleInitial" `
                -Surname "$LastName" `
                -DisplayName "$FirstName $MiddleInitial. $LastName" `
                -UserPrincipalName "[email protected]" `
                -EmailAddress "[email protected]" `
                -AccountPassword (ConvertTo-SecureString $Password -AsPlainText -Force) `
                -Enabled $false `
                -PasswordNeverExpires $true `
                -CannotChangePassword $true `
                -Path "OU=$GradYear,OU=Students,OU=$SchoolCode,OU=accounts,DC=academic,DC=mydomain,DC=com"

            # If you've gotten this far, the user has been created; output to screen.
            Write-Verbose "[PASS] Created [$($ADUserName)]."

            # Add SUCCESS data to the array.
            $UserCreationSuccess += New-Object -TypeName PSCUSTOMOBJECT -Property $ExportCsvProperties

            # Add user to group(s).
            ForEach ($ADGroup in $ADGroups) {
                # Add user to group.
                Get-ADUser -Identity $ADUserName | Add-ADPrincipalGroupMembership -MemberOf $ADGroup

                # Output to the screen
                Write-Verbose "[PASS] Added [$($ADUserName)] to [$($ADGroup)]."

                # Add SUCCESS data to the array for successful join.
                #$GroupJoinSuccess += New-Object -TypeName PSCUSTOMOBJECT -Property $ExportCsvProperties

            } # End ForEach

        } # End Try

        Catch {
            #Write-Error "[ERROR] Can't create user [$($ADUserName)] : $_"
            Write-Error "[ERROR] Can't create user [$($ADUserName)] : $($_.Exception.Message)"

            # Collect FAILURE data for the array.
            $ExportCsvProperties.error = $($_.Exception.Message)

            # Add FAILURE data to the array.
            $UserCreationFailure += New-Object -TypeName PSCUSTOMOBJECT -Property $ExportCsvProperties

        } # End Catch

    } # End IF

    Else {
        Write-Warning "The account [$($ADUserName)] wasn't created, it already exists."
        $UsersAlreadyExist += New-Object -TypeName PSCUSTOMOBJECT -Property $ExportCsvProperties

    } # End Else

} # End ForEach


If (-Not(Test-Path $LogFolder)) {
    Write-Verbose "Folder [$($LogFolder)] does not exist, creating"
    New-Item $LogFolder -Force
}

# Export data in the array to CSV.
$UserCreationSuccess | Export-Csv -Path "$LogFolder\UserCreationSuccess.csv" -NoTypeInformation -Append
$UserCreationFailure | Export-Csv -Path "$LogFolder\UserCreationFailure.csv" -NoTypeInformation -Append
$UsersAlreadyExist | Export-Csv -Path "$LogFolder\UsersAlreadyExist.csv" -NoTypeInformation -Append
#$GroupJoinSuccess | Export-Csv -Path "$LogFolder\GroupJoinSuccess.csv" -NoTypeInformation -Append

Answer 1

Separate data and code

As a general rule, you should separate your data and your code. You said in a comment that you were unhappy with the switch statement, so I think you already intuit this point.

What we can do is put the data in a hashtable at the top of the code:

$SchoolCodes = @{
        20 = "Exeter Township Senior High"
        30 = "Exeter Township Junior High"
        40 = "Lorane Elementary School"
        50 = "Jacksonwald ES"
        70 = "Reiffton School"
        90 = "Owatin Creek Elementary School"
    }

And then when you want to look up a school name, instead of the switch statement, you could have this:

$SchoolCode = $SchoolCodes[$User.School]

(Is $SchoolCode correctly named, by the way? I would have thought $SchoolName would be a more accurate name. Another important principle of programming is to name things accurately. Otherwise you will quickly get confused.)

There are numerous benefits to separating data and code. You code becomes more compact and easier to read. It's easier to update the data. You can put the data in a file if you want.

Logging

When logging, it's best to log as you go rather than writing all the entries all at once at the end. If you log at the end, then all that info will get lost if the script stops for whatever reason. Also if you log as you go, then you can watch what is happening in a log viewer.

I would create a logging function (or functions) and just call that. For example:

function log($FileName, $FirstName, $LastName, $Error)
{
    # Sorry, I don't have time to write the body of this function.
}

You might call it like this:

log "UserCreationSuccess.csv" $FirstName $LastName ""

That's just a sketch. The point is to use a function. Whenever you have repeated code, you should wrap it up in some way such as in a function.

Answer 2

Logging

For logging, what I do is create a log function at the beginning of the script:

function Write-Log {
    [CmdletBinding()]
    Param (
        [Parameter(Mandatory=$true,ValueFromPipeline=$true,Position=0)]
        [ValidateNotNullOrEmpty()]
        [System.String]$Message,

        [Parameter(Mandatory=$true)]
        [System.String]$Path
    )

    Add-Content -Path $Path -Value ("{0}`t{1}" -f ([System.DateTime]::Now.ToString('yyyy-MM-dd HH:mm:ss')),$Message) -Encoding UTF8;
}

Then, I define a log file name:

$LogFile = Join-Path -Path $PSScriptRoot -ChildPath ('{0} {1:yyyy-MM-dd HH_mm_ss}.log' -f ($MyInvocation.MyCommand.Name.SubString(0,$MyInvocation.MyCommand.Name.LastIndexOf('.'))), [System.DateTime]::Now);

Although I do typically have a parameter on the script that lets me override the log file path if I want.

And finally, I set a default value for the Path parameter of the function:

$PSDefaultParameterValues = @{"Write-Log:Path"=$LogFile};

This is great because this is a script-wide variable, so no matter where I call Write-Log, the -Path parameter is going to be correct.

My try/catch blocks in scripts that I want a lot of detail in tend to look like this:

try {
    Write-Log "Doing thing";
    Write-Verbose "Doing thing";
    if ($Item.IsUnusual) {
        Write-Log "Warning: $($Item.Name) is unusual.";
        Write-Warning "$($Item.Name) is unusual.";
    }
    Do-Thing $Item;
}
catch {
    Write-Log "ERROR: Error doing thing to item $($Item.Name).";
    Write-Log $($_.Exception | Format-List | Out-String);
    throw $_;
}

---

Appending to Arrays

Generally speaking, you want to avoid this pattern as much as possible:

$Report = @()
foreach ($item in $set) {
    $Record = Do-Stuff -To $item
    $Report += $Record
}
$Report | Export-Csv

PowerShell arrays are fixed in size. When you append an item to an existing array, PowerShell will create a new array, copy all the items over with the new item, and then delete the old array. This will work fine up to a few hundred small items, but it becomes a noticeable problem as the size of $set increases. It gets exponentially worse as set size increases.

What you want to do instead is something like:

$Report = foreach ($item in $set) {
    Do-Stuff -To $item
}
$Report | Export-Csv

Or, if that doesn't work:

$Report = New-Object -TypeName System.Collections.ArrayList
foreach ($item in $set) {
    $Record = Do-Stuff -To $item
    $Report.Add($Record)
}
$Report | Export-Csv

You can see the difference pretty easily by comparing:

$Set = 1..10000

# Slow
Measure-Command { $x = @(); foreach ($item in $set) { $x += $item } } | Select-Object -ExpandProperty TotalMilliseconds

# Fast
Measure-Command { $x = New-Object -TypeName System.Collections.ArrayList; foreach ($item in $set) { $x.Add($item) } } | Select-Object -ExpandProperty TotalMilliseconds

# Fastest 
Measure-Command { $x = foreach ($item in $set) { $item } } | Select-Object -ExpandProperty TotalMilliseconds

I see a difference of about 2 orders of magnitude.

---

Querying Active Directory

This is also a problem:

ForEach ($User in $Users) {
    [...]
    If (-Not(Get-ADUser -Filter {SamAccountName -eq $ADUserName})) {
        New-ADUser [...]
        [...]
        ForEach ($ADGroup in $ADGroups) {
            Get-ADUser -Identity $ADUserName | Add-ADPrincipalGroupMembership -MemberOf $ADGroup
        }
    }
}

Your poor domain controller. If you're creating users with 2 groups per user, this is going to cause 6 individual queries to Active Directory for every new user and 1 query for every existing user.

First, fetch your list of users first and save them to a hashtable for quick lookup:

$UserListing = @{};
Get-ADUser -Filter * | ForEach-Object {
    $UserListing.Add($_.samAccountName,$_.DistinguishedName);
}

This takes a few seconds to pull the data, but it's much faster for your script and much easier for your domain controller.

Now when you test, you can just look in the $UserListing to see if it's there instead of asking AD:

ForEach ($User in $Users) {
    [...]
    If (!$UserListing.ContainsKey($ADUserName)) {

And otherwise it provides an easy way to tell exactly which object it's colliding with:

    Else {
        Write-Warning "The account [$($ADUserName)] wasn't created, it already exists at [$($UserListing[$ADUserName])]."
        [...]
    }

Next, you can use the -PassThru option with New-ADUser, which will make the command return the user object back so you don't have to ask for it later:

        $NewUser = New-ADUser -PassThru [...]

Finally, when you call Add-ADPrincipalGroupMembership, you're allowed to specify a list of groups with the -MemberOf parameter. You can just do this:

        Add-ADPrincipalGroupMembership -Identity $NewUser -MemberOf $AdGroups

You may have noticed that there may be problems if your CSV file happens to have duplicate usernames since $UserListing isn't being updated. You should be able to avoid that pretty easily with either this:

        $UserListing.Add($NewUser.samAccountName, $NewUser.distinguishedName)

Or this:

        $UserListing.Add($ADUserName, "CN=$ADUserName,OU=$GradYear,OU=Students,OU=$SchoolCode,OU=accounts,DC=academic,DC=mydomain,DC=com")

After you create the user. Which you use will depend on exactly what properties New-ADUser -PassThru actually returns. Most commands return both samAccountName and distinguishedName, however.