Requesting review and suggestions: AD account creation script

PowerShell Help
1

Hello again,

I am grateful for all of the help I have received in the short time I have been apart of this community.

Today I am hoping that I can once again get assistance from the community to review my first attempt at an automation script for work.

I am hoping to get feedback on styling, logic, and additional features that would make sense.

One idea that I am thinking to implement when I get back to work on Monday is to check if the SamAccountName is already taken and if it is adding an additional letter from their first name until the name is available.

Any feedback is greatly appreciated. Thanks again!

function New-Employee {
    [CmdletBinding()]
    Param (
        [Parameter(Mandatory)]
        [String]$FullName,
        [Parameter(Mandatory)]
        [ValidateSet("Company1", "Company2", "Company3", "Company4", "Company5")]
        [String]$Company,
        [Parameter(Mandatory)]
        [String]$JobTitle,
        [Parameter(Mandatory)]
        [ValidateSet("City1", "City2", "City3", "City4", "City5", "City6", "City7", "City8", "City9", "City10")]
        [String]$LocationCity,
        [Parameter(Mandatory)]
        [String]$Supervisor
    )
    # Format and separate user information based on name and company
    $FirstName = $FullName.Split(" ")[0]
    $LastName = $FullName.Split(" ")[1]
    $FirstInitalLastName = "$($FirstName[0])$($LastName)"
    $UserName = $FirstInitalLastName
    $Password = ConvertTo-SecureString -String "P4sSw0Rd" -AsPlainText -Force
    if (($Company -eq "Company1") -or ($Company -eq "Company2")) {
        $EmailAddress = "$($UserName)@$($Company)llc.com"
    } else {
        $EmailAddress = "$($UserName)@$($Company).com"
    }

    # Determine correct OU based on city and company
    if ($LocationCity -in ("City1", "City2", "City3", "City4", "City5", "City6")) {
        $Path = "OU=Users,OU=$($Company) - $($LocationCity),OU=$($LocationCity),OU=Western State,OU=State,DC=Company,DC=local"
    } elseif ($LocationCity -in ("City7", "City8", "City9", "City10")) {
        $Path = "OU=Users,OU=$($Company) - $($LocationCity),OU=$($LocationCity),OU=Eastern State,OU=State,DC=Company,DC=local"
    }
    # Find a user with the same position to copy permissions from
    $SimilarUserSearch = Get-ADUser -Filter * -SearchBase $Path -Properties Description
    foreach ($SimilarUser in $SimilarUserSearch) {
        if ($SimilarUser.Description -eq $JobTitle) {
            $Source = Get-ADUser -Identity $SimilarUser -Properties MemberOf
            $SourceGroupsList = $Source.MemberOf
        }
    }

    $NewUserParameterSet = @{
        Name                  = $FullName
        DisplayName           = $FullName
        GivenName             = $FirstName
        Surname               = $LastName
        SamAccountName        = $UserName
        UserPrincipalName     = $UserName
        AccountPassword       = $Password
        EmailAddress          = $EmailAddress
        Enabled               = $True
        ChangePasswordAtLogon = $True
        Company               = $Company
        Description           = $JobTitle
        Path                  = $Path
        Manager               = $Supervisor
    }
    New-ADUser @NewUserParameterSet

    # Copy permissions from similar user and enable dial-in
    $User = Get-ADUser -Identity $UserName
    $User | Set-ADUser -Replace @{msNPAllowDialIn=$true}
    foreach ($Group in $SourceGroupsList) {
        $ThisGroup = $Group.split(",")[0].split("=")[1]
        Add-ADGroupMember -Identity $ThisGroup -Members $User
    }

    Sart-AdSyncSyncCycle -PolicyType Delta
    $attempts = 0
    do {
        try {
            $MsolAccount = Get-MsolUser -UserPrincipalName $EmailAddress -ErrorAction:Stop 
            $attempts++
        } catch {
            Start-Sleep -Seconds 300
        }
    } while ($null -eq $MsolAccount -and $attempts -le 6)

    # Enable MFA
    $StrongAuthenticationRequirement = New-Object -TypeName Microsoft.Online.Administration.StrongAuthenticationRequirement
    $StrongAuthenticationRequirement.RelyingParty = "*"
    $StrongAuthenticationRequirement.State = "Enabled"

    Set-MsolUser -UserPrincipalName $EmailAddress -StrongAuthenticationRequirements @($StrongAuthenticationRequirement) -UsageLocation "US"

    # Assign Office 365 license
    $E1Licenses = Get-MsolAccountSku | where {$_.AccountSkuID -eq "contoso:STANDARDPACK"}
    $E1LicensesRemaining = $E1Licenses.ActiveUnits - $E1Licenses.ConsumedUnits
    if ($E1LicensesRemaining -gt 0) { 
        Set-MsolUserLicense -UserPrincipalName $EmailAddress -AddLicenses "contoso:STANDARDPACK" -UsageLocation "US"
    } else {
        Throw "No available licenses remaining, purchase another license then assign it to this account manually."
    }
}
2

Some thoughts from a quick review:

Generate a random password for each user, don’t store the password in your script.

Consider replacing your if/else statements with a switch block.

Getting an existing user’s groups could result in your new user getting inappropriate access. Consider using some form of template as a baseline for your new users instead.

Add error handling, particularly where you get, create, or change something.

Set-MsolUserLicense is deprecated, and the MSOL and Azure AD modules are scheduled for retirement. You should use the Microsoft Graph PowerShell module instead.

Consider separating your formatting, user creation, syncing, and license assignments into separate functions, or even separate scripts within a module.

3 Likes
3

I agree with matt-bloomfield in just about everything, but most especially in the templating bit. It’s not uncommon for a user to be assigned access beyond their regular job description. So trusting a script to grab a random user based on an attribute such as title or description is risky. It’s better to have a couple of templates in the script and then choose one with a switch block.

Separating the script into discrete functions would also make it easier to read and maintain in the future.

As an aside, you are missing a “t” in the Sart-AdSyncSyncCycle -PolicyType Delta line.

2 Likes
4

This are all great suggestions that I am eager to implement when I can find the time. When I do I’ll upload the script with the added changes.

The one I am intimidated by is adding templates for permissions. There is a pretty wide range of different job titles, should I create a template for each?

5

I am most excited about implementing this part. After breaking the script into manageable pieces I think it will be a lot easier to refine.

Can’t be that big of a deal :wink:

6

That is one way of doing it. Another way of going about it is to create a base or default user template containing the permissions and settings which are common across the organization, this template will be applied to every user created by the script.
Then you create smaller templates for each role with only the permissions and settings which are unique for the role and apply that once the user role has been chosen.
Depending on the organizational structure of your company, you may even be able to create a type of heredity or branching that way. Say you have the base user template and you need to add a new manager to Marketing, instead of a “Marketing_manager”-template you may be able to get away with creating templates for “Manager” and “Marketing”. That may sound like more work up front, but if you need to add a Manager to sales you would just reuse the “Manager”-template with a “Sales”-template, and equally if a new agent starts in Marketing you reuse the “Marketing”-template with an “Agent”-template.
I feel like I may be overexplaining, but I hope you get what I mean?

However, as mentioned by both Matt and I, it’s not at all uncommon that user’s are given individual access to projects and folders etc. so don’t overburden yourself with creating too many and detailed templates. Better to start with a few clearly defined roles and then you can expand as needed. That’s where the modularity and maintainability comes in.

3 Likes
7

Would please help me understand how I would turn the following into a switch block? I am having a hard time understanding how one might go about doing this.

if (($Company -eq "Company1") -or ($Company -eq "Company2")) {
        $EmailAddress = "$($UserName)@$($Company)llc.com"
    } else {
        $EmailAddress = "$($UserName)@$($Company).com"
    }

    if ($LocationCity -in ("city1", "city2", "city3", "city4", "city5", "city6")) {
        $Path = "OU=Users,OU=$($Company) - $($LocationCity),OU=$($LocationCity),OU=Western State,OU=State,DC=Company,DC=local"
    } elseif ($LocationCity -in ("city7", "city8", "city9", "city10")) {
        $Path = "OU=Users,OU=$($Company) - $($LocationCity),OU=$($LocationCity),OU=Eastern State,OU=State,DC=Company,DC=local"
    }
8

The basic replacement would be something like this:

$CompanyList = @('Company1','Company2')

$WesternCities = @(
    'City1'
    'City2'
    'City3'
    'City4'
    'City5'
    'City6'
)

$EasternCities = @(
    'City7'
    'City8'
    'City9'
    'City10'
)

switch ($Company) {
    {$_ -in $CompanyList} {
        $EmailAddress = "$($UserName)@$($Company)llc.com"
    }
    default {
        $EmailAddress = "$($UserName)@$($Company).com"
    }
}

switch ($LocationCity) {
    {$_ -in $WesternCities} {
        $Path = "OU=Users,OU=$($Company) - $($LocationCity),OU=$($LocationCity),OU=Western State,OU=State,DC=Company,DC=local"
    }
    {$_ -in $EasternCities} {
        $Path = "OU=Users,OU=$($Company) - $($LocationCity),OU=$($LocationCity),OU=Eastern State,OU=State,DC=Company,DC=local"
    }
}
2 Likes
9

I like the way this looks. Thank you for the demonstration!

I have started separating the script into multiple functions and playing with Microsoft Graph. I’m hoping to post an updated script sometime this week.

Thank you for all of the directions, your pointers are going to be extremely helpful in me increasing the quality of my scripts!

10

The script is far from complete, but I wanted to post an update to thank for the help and showoff what the script looks like with most of the suggestions at least partially updated.


$EmailAddress = " "
function New-Employee {
    [CmdletBinding()]
    Param (
        [Parameter(Mandatory)]
        [String]$FullName,
        [Parameter(Mandatory)]
        [String]$Company,
        [Parameter(Mandatory)]
        [String]$JobTitle,
        [Parameter(Mandatory)]
        [String]$LocationCity,
        [Parameter(Mandatory)]
        [String]$Supervisor
    )

    $DomainAccountParameterSet = @{
        FullName     = $FullName
        Company      = $Company
        JobTitle     = $JobTitle
        LocationCity = $LocationCity
        Supervisor   = $Supervisor
        Session      = $Session
    }

    New-DomainAccount @DomainAccountParameterSet
    Invoke-Command -Session $Session -ScriptBlock { Start-AdSyncSyncCycle -PolicyType Delta }
    $attempts = 1
    do {
        try {
            "Attempting to find O365 account. Attempt: $attempts"
            $MsolAccount = Get-MgUser -UserId $EmailAddress -ErrorAction Stop 
            $attempts++
        }catch {
            "$($_.Exception.Message)"
            Start-Sleep -Seconds 300
        }
    } while ($null -eq $MsolAccount -and $attempts -le 6)

    Enable-Office365MFA -EmailAddress $EmailAddress
    Add-Office365License -EmailAddress $EmailAddress
}

 
function New-DomainAccount {
    [CmdletBinding()]
    Param (
        [Parameter(Mandatory)]
        [String]$FullName,
        [Parameter(Mandatory)]
        [String]$Company,
        [Parameter(Mandatory)]
        [String]$JobTitle,
        [Parameter(Mandatory)]
        [String]$LocationCity,
        [Parameter(Mandatory)]
        [String]$Supervisor,
        [System.Management.Automation.Runspaces.PSSession]$session
    )

    $FirstName = $FullName.Split(" ")[0]
    $LastName = $FullName.Split(" ")[1]
    $FirstInitalLastName = "$($FirstName[0])$($LastName)"
    $UserName = $FirstInitalLastName
    $Password = ConvertTo-SecureString New-StringPassword(14) -AsPlainText -Force

    $CompanyList = @('Company1', 'Company2')

    $WesternCities = @(
        'City1'
        'City2' 
        'City3' 
        'City4'
        'City5'
        'City6'
    )
    $EasternCities = @(
        'City7'
        'City8'
        'City9'
        'City10'
    )

    switch ($Company) {
        { $_ -in $CompanyList } {
            $EmailAddress = "$($UserName)@$($Company)llc.com"
        }
        default {
            $EmailAddress = "$($UserName)@$($Company).com"

            if ($Company -eq 'CompanyA') {
                $Company = 'CompanyB'
            }
        }
    }

    $global:EmailAddress = $EmailAddress

    switch ($LocationCity) {
        { $_ -in $WesternCities } {
            $Path = "OU=Users,OU=$($Company) - $($LocationCity),OU=$($LocationCity),OU=Western State,OU=State,DC=Domain,DC=local"
        }
        { $_ -in $EasternCities } {
            $Path = "OU=Users,OU=$($Company) - $($LocationCity),OU=$($LocationCity),OU=Eastern Washington,OU=Washington,DC=Domain,DC=local"
        }
    }

    $Domain = "domain.local"
    $NewUserParameterSet = @{
        Name                  = $FullName
        DisplayName           = $FullName
        GivenName             = $FirstName
        Surname               = $LastName
        SamAccountName        = $UserName
        UserPrincipalName     = "$UserName@$Domain"
        AccountPassword       = $Password
        EmailAddress          = $EmailAddress
        Enabled               = $True
        ChangePasswordAtLogon = $True
        Company               = $Company
        Description           = $JobTitle
        Path                  = $Path
    }
    $NewUserParameterSet

    Invoke-Command -Session $session -ScriptBlock {
        New-ADUser @Using:NewUserParameterSet
        $User = Get-ADUser -Identity $Using:UserName
        $User | Set-ADUser -Replace @{msNPAllowDialIn = $true }

        $SourceUser = Get-ADUser -Identity "TemplateUser" -Properties MemberOf
        $SourceGroupsList = $SourceUser.MemberOf
        foreach ($Group in $SourceGroupsList) {
            $ThisGroup = $Group.Split(",")[0].Split("=")[1]
            Add-ADGroupMember -Identity $ThisGroup -Members $Using:UserName
        }
    }
}
function Enable-Office365MFA {
    [CmdletBinding()]
    Param (
        [Parameter(Mandatory)]
        [string]$EmailAddress
    )

    $StrongAuthenticationRequirement = New-Object -TypeName Microsoft.Online.Administration.StrongAuthenticationRequirement
    $StrongAuthenticationRequirement.RelyingParty = "*"
    $StrongAuthenticationRequirement.State = "Enabled"

    try {
        Set-MsolUser -UserPrincipalName $EmailAddress -UsageLocation "US" -ErrorAction:Stop
        Set-MsolUser -UserPrincipalName $EmailAddress -StrongAuthenticationRequirements @($StrongAuthenticationRequirement) -ErrorAction:Stop
    } catch {
        Write-Error "MFA was not enabled on account $EmailAdress due to $($_.Exception.Message)"
    }
}
function Add-Office365License {
    [CmdletBinding()]
    Param (
        [Parameter(Mandatory)]
        [string]$EmailAddress
    )

    try {
        $ConsumedLicenses = (Get-MgSubscribedSku | Where-Object { $_.SkuPartNumber -eq 'STANDARDPACK' } -ErrorAction:Stop).ConsumedUnits
        $ActiveLicenses = (Get-MgSubscribedSku | Where-Object { $_.SkuPartNumber -eq 'STANDARDPACK' } | Select-Object -ExpandProperty PrepaidUnits -ErrorAction:Stop).Enabled
        $E1LicensesRemaining = $ActiveLicenses - $ConsumedLicenses
        $E1Sku = Get-MgSubscribedSku -All | Where-Object SkuPartNumber -eq 'STANDARDPACK'

        Update-MgUser -UserId $EmailAddress -UsageLocation "US"
        if ($E1LicensesRemaining -gt 0) { 
            Set-MgUserLicense -UserId $EmailAddress -AddLicenses @{SkuId = $E1SKU.SkuId } -RemoveLicenses @() -ErrorAction:Stop
        } else {
            Throw "No available licenses remaining, purchase another license then assign it to this account manually."
        }

    } catch {
        Write-Error "Unable to assign license to user $EmailAddress due to $($_.Exception.Message)"
    }
}
function New-StringPassword($length) {
    $characters = '1234567890!@#$%^&*()qazwsxedcrfvtgbyhnujmikolpQAZWSXEDCRFVTGBYHNUJMIKOLP'
    $random = 1..$length | ForEach-Object { Get-Random -Maximum $characters.length }
    $private:ofs = ""
    return [string]$characters[$random]
}
11

… small tip …

Instead of querying the new AD account immediately after you created it you could use the parameter -PassThru for the New-AdUser command and use the object you just created.

1 Like
12

A couple of small tips and thoughts from me as well…

You may want to restrict the $Company and $City parameters with ValidateSet and the acceptable values. A bit more typing for you up front, but it will minimize the risk of users mistyping them once the script is put into use.

Another thing is that your script assumes that the user will always feed it a $FullName parameter consisting of exactly one first name and one last name.
There are a number of blind spots about names, but let’s just take a very simple counterexample: “John David Smith”
Your script would create the user name “jdavid” and ignore the last name entirely.
You could change the $LastName assignment to this to grab the last word in the $FullName variable:
$LastName = $FullName.Split(" ")[-1]

But that could still cause you problems with a compound last name like “le Bon” or “van Gogh”.

Have you given any thought to how you’d manage users with accented or non-anglizied names?
I’m pretty sure that a user name like “jcalderón” or “djørgensen” could cause problems in the AD.
Instead of splitting the name in the script I would probably just grab the first and last names separately and then sanitize them before creating the user name.

2 Likes