Users being created more than once

Viewed 106

I am trying to make a script to create groups, user and OUs for my DC. It is supposed to add 250 users and have the first 60 placed in the RODC and then randomly split the other 250 into the OUs but after it has placed the first 60 users into the RODC OU it will then try to add the same users to other OUs with this error:

dsadd failed:cn=amohn1,ou=DB_Engineers,ou=Programmers,ou=BRAAN,dc=BRAAN,dc=COM:The specified account already exists.

I am not sure why it is doing this. Here is my script:

$scriptDir = Split-Path -Path $MyInvocation.MyCommand.Definition -Parent ##Grabs the directory path of the script to be used as a relevant path.##
$domainname = (Get-ADDomain -Current LocalComputer).NetBIOSName ##Grabs the computers domainname##
$filecontent = Get-Content $scriptDir\Users.txt ##Reads the .txt file##

##Setting global variables.##
$itteration = 0
$AdminCount = 0
$ComputerName = "default"
$ComputerNumber = 1
$ComputerType = "default"
$ServicePack = ""

if($domainname -eq "BRAAN") ##Is the computer on the BRAAN domain?##
{
    ##Build OU and Group Structure inside the domain.##
    dsadd ou "ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd ou "ou=CustomDom,dc=BRAAN,dc=COM"
    dsadd ou "ou=BRAANComputers,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd ou "ou=IT,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd ou "ou=HR,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd ou "ou=Sales,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd ou "ou=Executive,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd ou "ou=Programmers,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd ou "ou=DB_Engineers,ou=Programmers,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd ou "ou=Content_Experts,ou=Programmers,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd ou "ou=Animators,ou=Programmers,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd ou "ou=Operations,ou=CustomDom,dc=BRAAN,dc=COM"
    dsadd ou "ou=RODC,dc=BRAAN,dc=COM"
    dsadd group "cn=IT_Staff,ou=IT,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd group "cn=HR_Staff,ou=HR,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd group "cn=Sales_Staff,ou=Sales,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd group "cn=Executive_Staff,ou=Executive,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd group "cn=DB_Engineers_Staff,ou=BRAAN,dc=BRAAN,dc=COM"
    dsadd group "cn=Content_Experts_Staff,ou=BRAAN,dc=BRAAN,dc=COM" 
    dsadd group "cn=Animators_Staff,ou=BRAAN,dc=BRAAN,dc=COM"  
    dsadd group "cn=Operations_Staff,ou=Operations,ou=CustomDom,dc=BRAAN,dc=COM" 
    dsadd group "cn=RODC_Staff,ou=RODC,dc=BRAAN,dc=COM" 

    ##Sets the computer name##
    $ComputerName = "Braan-"
    while($itteration -ne 80) ##While loop used to create ComputerObjects.##
    {
        $random = Get-Random -Minimum 1 -Maximum 3 ##Random number used to randomly assign OperatingSystem to the computers.##
        if($random -eq 1)
        {
            $ComputerType = "Windows 10"
        }
        ##Creation of PCs.##
        $Computer = $ComputerName+$ComputerNumber
        New-ADComputer -Name $Computer -OperatingSystem $ComputerType -OperatingSystemServicePack $ServicePack -Path "ou=BRAANComputers,ou=BRAAN,dc=BRAAN,dc=COM"
        $itteration++
        $ComputerNumber++
    }
    ##Resetting the itteration variable.##
    $itteration = 0
    ##Resetting the random variable.##
    $random = 0
    while($itteration -ne 250) ##Begins loop for user creation.##
    {     
        while($itteration -lt 60) ##Places first 60 users into the RODC OU.##
        {
            ##Splits the values in the .txt file via spaces.##
            $values = $filecontent[$itteration] -split(' ')
            $usrname = $values[0]
            dsadd user "cn=$usrname,ou=RODC,dc=BRAAN,dc=COM" -fn $values[1] -ln $values[2] -disabled no -pwd "Password1" -memberof "cn=RODC_Staff,ou=RODC,dc=BRAAN,dc=com" ##Create user and add to group.##
            $itteration++
        }

        $values = $filecontent[$itteration] -split(' ')
        ##Random used to determine which OUs a user is put in.##
        $random = Get-Random -Minimum 1 -Maximum 9
        $usrname = $values[0]
        ##Sets itteration to 70 above zero so we have no repeat accounts.##
        $itteration = 70

        while($itteration -ne 180)##Begins loop for user creation.##
        {
            if($random -eq 1) ##IT OU.##
            {
                dsadd user "cn=$usrname,ou=IT,ou=BRAAN,dc=BRAAN,dc=COM" -fn $values[1] -ln $values[2] -disabled no -pwd "Password1" -memberof "cn=IT_Staff,ou=IT,ou=BRAAN,dc=BRAAN,dc=com" ##Create user and add to group.##
            }elseif($random -eq 2) ##HR OU.##
            {
                dsadd user "cn=$usrname,ou=HR,ou=BRAAN,dc=BRAAN,dc=COM" -fn $values[1] -ln $values[2] -disabled no -pwd "Password1" -memberof "cn=HR_Staff,ou=HR,ou=BRAAN,dc=BRAAN,dc=com" ##Create user and add to group.##
            }elseif($random -eq 3) ##Sales OU.##
            {
                dsadd user "cn=$usrname,ou=Sales,ou=BRAAN,dc=BRAAN,dc=COM" -fn $values[1] -ln $values[2] -disabled no -pwd "Password1" -memberof "cn=Sales_Staff,ou=Sales,ou=BRAAN,dc=BRAAN,dc=com" ##Create user and add to group.##
            }elseif($random -eq 4) ##Finance OU.##
            {
                dsadd user "cn=$usrname,ou=Finance,ou=BRAAN,dc=BRAAN,dc=COM" -fn $values[1] -ln $values[2] -disabled no -pwd "Password1" -memberof "cn=Finance_Staff,ou=Finance,ou=BRAAN,dc=BRAAN,dc=com" ##Create user and add to group.##
            }elseif($random -eq 5) ##Executives OU.##
            {
                dsadd user "cn=$usrname,ou=Executives,ou=BRAAN,dc=BRAAN,dc=COM" -fn $values[1] -ln $values[2] -disabled no -pwd "Password1" -memberof "cn=Executives_Staff,ou=Executives,ou=BRAAN,dc=BRAAN,dc=com" ##Create user and add to group.##
            }elseif($random -eq 6) ##DB_Engineers Group.##
            {
                dsadd user "cn=$usrname,ou=DB_Engineers,ou=Programmers,ou=BRAAN,dc=BRAAN,dc=COM" -fn $values[1] -ln $values[2] -disabled no -pwd "Password1" -memberof "cn=DB_Engineers_Staff,ou=DB_Engineers,ou=Programmers,ou=BRAAN,dc=BRAAN,dc=com" ##Create user and add to group.##
            }elseif($random -eq 7) ##Content_Experts Group.##
            {
                dsadd user "cn=$usrname,ou=Content_Experts,ou=Programmers,ou=BRAAN,dc=BRAAN,dc=COM" -fn $values[1] -ln $values[2] -disabled no -pwd "Password1" -memberof "cn=Content_Experts_Staff,ou=Content_Experts,ou=Programmers,ou=BRAAN,dc=BRAAN,dc=com" ##Create user and add to group.##
            }
            elseif($random -eq 8)##Animators Group.##
            {
                dsadd user "cn=$usrname,ou=Animators,ou=BRAAN,dc=BRAAN,dc=COM" -fn $values[1] -ln $values[2] -disabled no -pwd "Password1" -memberof "cn=Animators_Staff,ou=Animators,ou=Programmers,ou=BRAAN,dc=BRAAN,dc=com" ##Create user and add to group.##
            }
            ##Checks if admin accounts are needed, if so then adds one admin account given that they are also in the Management OU.##
            if($AdminCount -ne 2 -and $random -eq 2)
            {
                $AdminCount++
                dsmod group "cn=Domain Admins,cn=Users,dc=BRAAN,dc=COM" -addmbr "cn=$usrname,ou=Management,ou=BRAAN,dc=BRAAN,dc=com"    ##Adds account to Domain Admins.##
                dsmod group "cn=Enterprise Admins,cn=Users,dc=BRAAN,dc=COM" -addmbr "cn=$usrname,ou=Management,ou=BRAAN,dc=BRAAN,dc=com" ##Adds account to Enterprise Admins.##
            }
            $itteration++
        }
    }
}else ##Computer is not on any of the above domains and script will exit.##
{
    Write-Host "Domain not listed, exiting script."
}
2 Answers

Because you keep resetting your $itteration variable to 70!

During the first run of the outer while($itteration -ne 250) loop:

while ($itteration -ne 250) {

    # runs 60 times ($itteration -in 0..59)
    while ($itteration -lt 60) {
        $itteration++
    }

    # $itteration is reset (to a higher value)
    $itteration = 70

    # runs 110 times ($itteration -in 70..179)
    while ($itteration -ne 180) {
        $itteration++
    }
}

Alright, so far so good - $itteration is not 250, so the loop body runs again:

while ($itteration -ne 250) {

    # $itteration is now 180, so we skip this loop
    while ($itteration -lt 60) {
        $itteration++
    }

    # $itteration is reset (this time to a lower value!)
    $itteration = 70 # <-- endless loop detected

    # runs another 110 times ($itteration -in 70..179)
    while ($itteration -ne 180) {
        $itteration++
    }
}

Your math isn't adding up here. You say...

It is supposed to add 250 users and have the first 60 placed in the rodc and then randomly split the other 250 into the ous

...but there is no "other 250". An initial 60 would require an "other 190" for a total of 250.

Further, given this skeleton code...

##Resetting the itteration variable.##
$itteration = 0
$userCount = 0

while ($itteration -ne 250) { ##Begins loop for user creation.##     
    while ($itteration -lt 60) { ##Places first 60 users into the RODC OU.##
        $itteration++
        $userCount++
    }

    ##Sets itteration to 70 above zero so we have no repeat accounts.##
    $itteration = 70

    while ($itteration -ne 180) { ##Begins loop for user creation.##
        $itteration++
        $userCount++
    }
}

...if only one iteration of while ($itteration -ne 250) is supposed to execute then, firstly, it shouldn't be a loop at all, but on that first iteration, the counting goes like this...

  • while ($itteration -lt 60): 60 iterations (0..59)
    • 60 total users, 60 RODC users
  • $itteration = 70: 0 iterations (60..69 skipped)
    • 60 total users, 60 RODC users
  • while ($itteration -ne 180): 110 iterations (70..179)
    • 170 total users, 60 RODC users

As you can see, that nets us only 170 users. Due to the bug pointed out in @MathiasR.Jessen's answer, the outer while is an infinite loop. On each subsequent iteration, the while ($itteration -lt 60) loop will not be entered but the while ($itteration -ne 180) loop will, so the user count will grow by 70..179 = 110 users each time: 170, 280, 390, etc.; none of which are 250.

In short, don't do this. It should not require this level of complexity (i.e. nested loops with an iterator that skips around) to count from 0..60 and from 60..250, nor this level of analysis to confirm if that's what it's actually doing. Instead, one simple fix is to convert the inner while loops into branches of an if/else and only modify $itteration at the end of the loop...

while ($itteration -ne 250) { ##Begins loop for user creation.##     
    if ($itteration -lt 60) {
        ##Places first 60 users into the RODC OU.##
        # dsadd user "cn=$usrname,ou=RODC,dc=BRAAN,dc=COM" ...
    } else {
        ##Begins user creation.##
        # dsadd user ...
    }
    $itteration++
}

That can be readily translated into a for loop, too.

Better still, since there's minimal overlap in the logic of ranges 0..60 and 60..250, they don't even need to occupy the same loop. I also think this makes the intent more explicit, which is a good thing. If you want 60 RODC users, then write a loop to do that...

for ($rodcUserCount = 0; $rodcUserCount -lt 60; $rodcUserCount++)
{
    ##Places first 60 users into the RODC OU.##
    # dsadd user "cn=$usrname,ou=RODC,dc=BRAAN,dc=COM" ...
}

If you want another 190 general users, then follow that up with another loop...

for ($generalUserCount = 0; $generalUserCount -lt 190; $generalUserCount++)
{
    ##Begins user creation.##
    # dsadd user ...
}

There you have two simple and straightforward loops to create 250 users. Note that I am naming the iteration variable to make it clear what's being counted.

Writing code like this that is clear and easy to understand will be doing a big favor to your current self, your future self, and anyone else that might have to work with this code.


A couple other things I found that I think are bugs...

  • You are setting $random outside of the user creation loop...

    $random = Get-Random -Minimum 1 -Maximum 9
    $usrname = $values[0]
    ##Sets itteration to 70 above zero so we have no repeat accounts.##
    $itteration = 70
    
    while ($itteration -ne 180) { ##Begins loop for user creation.##
    

    This means that each of the 110 iterations of that loop will have the same value for $random and, therefore, create those users all in the same department. Instead, I think you want to set $random at the beginning of the loop body.

  • When $random -eq 8, you are creating a user at cn=$usrname,ou=Animators,ou=BRAAN,dc=BRAAN,dc=COM but adding them to the cn=Animators_Staff,ou=Animators,ou=Programmers,ou=BRAAN,dc=BRAAN,dc=com group (notice the extra ou=Programmers). This is the peril of code duplication.

For the second point, instead of that long chain of if/elseif statements that test $random you could define this at the top of the script...

$departmentBaseDN = 'ou=BRAAN,dc=BRAAN,dc=COM'
$departments = @(
    'IT',
    'HR',
    'Sales',
    'Finance',
    'Executives',
    'Programmers\DB_Engineers',
    'Programmers\Content_Experts',
    'Animators'
)

...and then adjust your Get-Random call to return an index within the bounds of that array...

$random = Get-Random -Maximum $departments.Length # 0 ≤ $random < $departments.Length

...and then use this in your loop to dynamically build the distinguished names of the user, department OU, and department group...

# This returns a one-element array for departments with no \
$departmentParts = $departments[$random] -split '\\'
# The last part is the department name
$departmentName = $departmentParts[-1]

$departmentUnitDN = $departmentBaseDN
# Prepend to the base DN the OU for each part in order
foreach ($departmentPart in $departmentParts)
{
    $departmentUnitDN = "ou=$departmentPart,$departmentUnitDN"
}
$departmentGroupDN = "cn=$departmentName`_Staff,$departmentUnitDN"
$userDN = "cn=$usrname,$departmentUnitDN"

dsadd user "$userDN" -fn $values[1] -ln $values[2] -disabled no -pwd "Password1" -memberof "$departmentGroupDN"  ##Create user and add to group.##

You could take advantage of the same $departments variable to replace some of the repetitive dsadd ou and dsadd group commands at the beginning of the script, too.

Finally, be aware that there is an ActiveDirectory module for PowerShell that, in addition to the New-ADComputer cmdlet you are already using, provides many other cmdlets you could use here...

Related