asked on

Review my code please

This runs great from what I can tell. Its a script to create active directory accounts, add the users to groups and then create the home folders. Can you check my script and let me know of any issues you might see that I do not. Or any thing bad that might happen if something goes wrong.

Import-Module ActiveDirectory
$error.clear()
$MaximumErrorCount = 10000
Import-Csv "C:\Scripts\accounts\activestaff.csv" | ForEach-Object {

        switch ($_.Location)
	{
		Amgs {
			$OU = "OU=Staff,OU=As,dc=homelab,dc=com"
			$Description = "Staff - AS"
		}
		Bldwn {
			$OU = "OU=Staff,OU=Bn,dc=homelab,dc=com"
			$Description = "Staff - BN"
		}
		"CAM PPR" {
			$OU = "OU=Staff,OU=Cg,dc=homelab,dc=com"
			$Description = "Staff - CA"
		}
		CMBPRT {
			$OU = "OU=Staff,OU=Cmbrdgprt,dc=homelab,dc=com"
			$Description = "Staff - Ct"
		}
		"SCH DMN" {
			$OU = "OU=Staff,OU=Cc,dc=homelab,dc=com"
			$Description = "Staff - Ce"
		}
		CS {
			$OU = "OU=Staff,OU=CR,dc=homelab,dc=com"
			$Description = "Staff - CR"
		}
		"FLT/MN" {
			$OU = "OU=Staff,OU=FMA,dc=homelab,dc=com"
			$Description = "Staff - FM"
		}
		"J & P" {
			$OU = "OU=Staff,OU=Gs,dc=homelab,dc=com"
			$Description = "Staff - Gs"
		}
		Hggrt {
			$OU = "OU=Staff,OU=Hy,dc=homelab,dc=com"
			$Description = "Staff - HY"
		}
		Kng {
			$OU = "OU=Staff,OU=Kg,dc=homelab,dc=com"
			$Description = "Staff - KG"
		}
		"Kng pn" {
			$OU = "OU=Staff,OU=Kn,dc=homelab,dc=com"
			$Description = "Staff - KN"
		}
		"KNN/LNG" {
			$OU = "OU=Staff,OU=KL,dc=homelab,dc=com"
			$Description = "Staff - KLO"
		}
		MRS {
			$OU = "OU=Staff,OU=Me,dc=homelab,dc=com"
			$Description = "Staff - ME"
		}
		Pbdy {
			$OU = "OU=Staff,OU=Py,dc=homelab,dc=com"
			$Description = "Staff - PY"
		}
		"PTNM V"{
			$OU = "OU=Staff,OU=Pm,dc=homelab,dc=com"
			$Description = "Staff - Pm"
		}
		"RNDG V" {
			$OU = "OU=Staff,OU=Re,dc=homelab,dc=com"
			$Description = "Staff - Re"
		}
		Tbn {
			$OU = "OU=Staff,OU=Tn,dc=homelab,dc=com"
			$Description = "Staff - TN"
		}
		'VSSL LN' {
			$OU = "OU=Staff,OU=Vl,dc=homelab,dc=com"
			$Description = "Staff - Vl"
		}
		"SLMN" {
			$OU = "OU=Staff,dc=homelab,dc=com"
			$Description = "Staff - "
		}
		default
		{
			$OU = "OU=Staff,dc=homelab,dc=com"
			$Description = "Staff - "
		}
	}
if (dsquery user -samid $_.Login)
		{
			$LogonName = $_.Login2
		}
		else
       		 {
         		   $LogonName = $_.Login
       		 }
      
    $ADUser = [ordered]@{ }
	$ADUser['Name'] = $_.FirstName + " " + $_.LastName
	$ADUser['SamAccountName'] = $LogonName
	$ADUser['GivenName'] = $_.FirstName
	$ADUser['Surname'] = $_.LastName
    $ADUser['Description'] = $Description
	$ADUser['DisplayName'] = $_.FirstName + " " + $_.LastName
	$ADUser['UserPrincipalName'] = $LogonName + "@homelab.com"
	$ADUser['AccountPassword'] = ConvertTo-SecureString -AsPlainText 'P@ssw0rd' -Force
	$ADUser['Title'] = $_."Job Title"
	$ADUser['EmailAddress'] = $LogonName + "@homelab.com"
	$ADUser['Office'] = $_.EID
    $ADUser['Path'] = $OU
	$ADUser['Enabled'] = $True
	$ADUser['HomeDirectory'] = "\\dc-pc\share\$LogonName"
	$ADUser['HomeDrive'] = 'H:'
	New-ADUser @ADUser 


	Add-ADGroupMember "googleapps" –Members $LogonName
	Add-ADGroupMember "yard-staff" –Members $LogonName

    
	New-Item -type directory -path "\\dc-pc\share\$LogonName"
	$Acl = Get-Acl "\\dc-pc\share\$LogonName"
	$Ar = New-Object system.security.accesscontrol.filesystemaccessrule ("$LogonName", "Modify", "ContainerInherit, ObjectInherit", "None", "Allow")
	$Acl.SetAccessRule($Ar)
	Set-Acl "\\dc-pc\share\$LogonName" $Acl

}

Open in new window

Dustin Saunders

Is this the whole script? What is:

$error.clear()
$MaximumErrorCount = 10000

Open in new window

for?

Dustin Saunders

A few suggestions:

If you're using this to create AD users, I'd add in validation to make sure essential fields aren't blank or invalid. Otherwise, this can cause your user creation to fail.

You should log create successes and fails somewhere, so if one user fails you can fix that. I prefer to create an log writer function then I can be as verbose as I'd like as to where a problem occurred.

You can additionally add in a verification run at the end of the script, to double check that all directories were created and that users exist in AD, and add to your log any verification issues.

SOLUTION

Joseph Moody

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

Roccat

ASKER

Dustin, This is the whole script. I was using those "$error.clear()
$MaximumErrorCount = 10000 " in my testing to clear the error count and set the error count maximum to 10,000. I was going to use the $error variable to count and log errors.

Dustin Saunders

{
			$LogonName = $_.Login2
		}
		else
       	{
         	$LogonName = $_.Login
       	}

Open in new window

What is $_.Login2 ?

Roccat

ASKER

Dustin Saunders

Is that something you can add in to the code rather than CSV? The less that people need to input the better (because human error is the biggest cause of issues in automated user account creation from my experience.)

For logging, something similar to this:

$log = "C:\log\userCreate.log"

function WriteLog($message)
{
    $d = Get-Date
    $out = $d + " -- " + $message
    Add-Content -Path $log -Value $out
}

Open in new window

try 
    {    
        $ADUser = [ordered]@{ }
	    $ADUser['Name'] = $_.FirstName + " " + $_.LastName
	    $ADUser['SamAccountName'] = $LogonName
	    $ADUser['GivenName'] = $_.FirstName
	    $ADUser['Surname'] = $_.LastName
        $ADUser['Description'] = $Description
	    $ADUser['DisplayName'] = $_.FirstName + " " + $_.LastName
	    $ADUser['UserPrincipalName'] = $LogonName + "@homelab.com"
	    $ADUser['AccountPassword'] = ConvertTo-SecureString -AsPlainText 'P@ssw0rd' -Force
	    $ADUser['Title'] = $_."Job Title"
	    $ADUser['EmailAddress'] = $LogonName + "@homelab.com"
	    $ADUser['Office'] = $_.EID
        $ADUser['Path'] = $OU
	    $ADUser['Enabled'] = $True
	    $ADUser['HomeDirectory'] = "\\dc-pc\share\$LogonName"
	    $ADUser['HomeDrive'] = 'H:'
	    New-ADUser ADUser
        WriteLog("Successfully created user $LogonName.")
    }
    catch
    {
        WriteLog("Failed creating user $LogonName.")
        $error++
    }

Open in new window

Roccat

ASKER

I was going to create the second login option in the script but the other admin offered to generate the second login column when he generates the reports that populates the csv file.

ASKER CERTIFIED SOLUTION

Dustin Saunders

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

Roccat

ASKER

Thank you!!