Managing user names in AD

PowerShell Help
1

Hi,

I’m working on a module for managing users in our AD.

One part of the script is creating user names for new users and I have a couple of questions about the best way of doing some things.

We have two simple templates for user names. Staff is preferably lower case first name and if that’s taken we use first name plus last name initial.
The $fixedFirst variable in the snippet below is a sanitized version of the first name (changing it to lower case, removing accents and spaces and converting Swedish characters ‘äöå’ to ‘aoa’ respectively).

$samAccountName = $fixedFirst
if ((Get-ADUser -Filter {sAMAccountName -eq $fixedFirst})) {
Write-Warning -Message "$samAccountName används redan."
$samAccountName = $fixedFirst + $fixedLast.Substring(0,1)

As you can see it attempts to set the username as $fixedFirst and if that user name already exists it uses the first name + initial combo. As of right now I have no fallback in case that name is taken. I know I could nest another if in there, where it would then ask the user to type in the desired user name. I just wonder if there could be a better and simpler way. Especially since this is already nested in an if checking for user type.

Students are another matter. The template there is the last two digits of the year they start and the two first letters of the first name and the first four letters of the last name. In a conflict (rare) we use year and three and three. The user name must always be 8 characters long.

$samAccountName = $FirstName.Substring(0,2)
$samAccountName = $samAccountName + $LastName.Substring(0,4)
$samAccountName = $StudentYear + $samAccountName

if ((Get-ADUser -Filter {sAMAccountName -eq $samAccountName}) -or ($samAccountName.Length -lt 8)) {
Write-Warning -Message "$samAccountName används redan."
$samAccountName = $FirstName.Substring(0,3)
$samAccountName = $samAccountName + $LastName.Substring(0,3)
$samAccountName = $StudentYear + $samAccountName
}

This basically does the same for students as the above does for staff. It tries the default format first and if there’s a conflict or the user name is shorter than eight characters it jumps to the three plus three format with no option for getting a name from the user.

To summarize. I wish to have a three step process where the script tests first the basic template, then if that fails it tests the backup template, and finally if that fails it should ask the user for the desired final username… and of course test that too before continuing.
I know I can nest a number of ifs, but I think there ought to be a better way of doing all this.

2

Well there are a number of ways to go about this.
In general if you’re nesting a lot of IF’s then it’s most likely time to break out those scenarios into seperate functions.
E.g. if you test each scenario first and then just use an if-statement with multiple conditions, that should limit the if-levels.

Assuming the test functions returns true if the tested template can be used. (meaning no conflict).
[pre]
$basicTemplate = Test-BasicNameTemplate -Name $UserName # this function need to be written of course
$backupTemplate = Test-BackupNameTemplate -Name $UserName

if(($userType -eq ‘staff’) -and ($basicTemplate)){

Do stuff

} elseif (($userType -eq ‘staff’) -and ($backupTemplate)) {

Do other stuff

} else {

last resort, you will probably need to nest an if here to check if the manually entered name works.

or return something that will then be caught in a different if statement.

}
[/pre]

Just a heads up, if the CN is the same in the same OU you will also get a conflict.
So you’re probably going to need to test for that as well depending on the structure in the AD.

3

Thank you for your reply.

I might be missing something, if I’m sending the username to be tested separately would it not be better to just have one test that would validate the username, as otherwise I would need four or more username tests: Test-StaffBasicTemplate, Test-StaffBackupTemplate, Test-StudentBasicTemplate and Test-StudentBackupTemplate.

I’ll look into separating the tests out to separate functions, but it feels as if the basic issue remains. That I will be doing the same nesting, just that the condition will be a call to the function instead of performing the logic directly in the larger function.

Validating/testing for a username externally is a good idea, that I can use elsewhere in the module. So I’ll write a quick function for that.

I’ve appended the complete function as it looks right now and added comments where it would fail when both username templates were taken.

https://gist.github.com/laage/e9e25bfd1cc88e2863c81f8aed24584d

4

The thing is that nested IF’s are not inheritantly bad.
But in general once you go beyond the second IF it’s much harder to read and maintain over time.
So if you break out parts that you want in a nested IF to a seperate function, you literaly remove X levels of IF’s in that first function.
It doesn’t mean that you remove the total amount of IF’s, I mean if you have to make a logical decision and based on that decision make another logical choice.
Then that is what you are going to need to do.
Another option is to lower the amount of levels by having multiple conditions in the IF-statements, if that is feasible.

Of course you don’t have to make a seperate function for each test case, if you want you can combine the tests into a single function and return the result.

This is very quick and dirty but it will really take some of time to write it all with all the bells and whistles and error handling etc.
So please only use this as a food for thought or example, not a production solution.
E.g. don’t use Test-AccountName in its current form since it will decide which samAccountName should be used even if you don’t have get-aduser installed.

[pre]
function Test-AccountName {
Param(
$BasicTemplate,
$BackupTemplate
)

# We are only interested in triggering the return value.
$null = try{Get-Aduser $BasicTemplate}catch{return $BasicTemplate}
$null = try{Get-Aduser $BackupTemplate}catch{return $BackupTemplate}

return "fail"

}
[/pre]

So you supply the function with the basic-template and the backup-template of the samAccountName
If Get-Aduser finds the name it will not give an error and will go to the next statement.
If it’s not found Get-Aduser will produce an error (meaning the name is available for use) and return the samAccountName that should be used.
If both are available the function will return fail, meaning both commands where successfull and therefore they are already taken.

For the main function you could basically replace 66->103 with

[pre]
if ($UserType.ToLower() -eq ‘elev’) {
# create the two variables $basicTemplate and $backupTemplate for elev according to your rules.
$samAccountName = Test-AccountName -BasicTemplate $basicTemplate -BackupTemplate $backupTemplate

$userOU = $studentOU
$gsUserOU = "/Elever/" + $StudentYear

}

if ($UserType.ToLower() -eq ‘lärare’) {
# create the two variables $basicTemplate and $backupTemplate for lärare according to your rules.
$samAccountName = Test-AccountName -BasicTemplate $basicTemplate -BackupTemplate $backupTemplate

$userOU = $teacherOU

}

if ($UserType.ToLower() -eq ‘personal’) {
# create the two variables $basicTemplate and $backupTemplate for personal according to your rules.
$samAccountName = Test-AccountName -BasicTemplate $basicTemplate -BackupTemplate $backupTemplate

$userOU = $staffOU

}

if($samAccountName -eq ‘fail’) {
$samAccountName = Read-Host -Prompt “Please enter a username manually”
}
[/pre]

One thing that you have to consider is the ‘New-MGStudentAccountName’ function for which I can’t see how it works.

5

Ok, this forum doesn’t really handle edits well since both the original and edit flies into cyberspace.
Will see if my post comes back or I’ll repost tomorrow.

6

I’d appreciate any thoughts you have on the matter if you have the time and inclination to revisit this.

I understand how frustrating it is when the text you’ve been working on decides to /dev/null itself.

7

Yeah it has happened before here so I saved a copy of the text, will see if it gets stuck again :slight_smile:

The thing is that nested IF’s are not inheritantly bad.
But in general once you go beyond the second IF it’s much harder to read and maintain over time.
So if you break out parts that you want in a nested IF to a seperate function, you literaly remove X levels of IF’s in that first function.
It doesn’t mean that you remove the total amount of IF’s, I mean if you have to make a logical decision and based on that decision make another logical choice.
Then that is what you are going to need to do.
Another option is to lower the amount of levels by having multiple conditions in the IF-statements, if that is feasible.

Of course you don’t have to make a seperate function for each test case, if you want you can combine the tests into a single function and return the result.

This is very quick and dirty but it will really take some of time to write it all with all the bells and whistles and error handling etc.
So please only use this as a food for thought or example, not a production solution.
E.g. don’t use Test-AccountName in its current form since it will decide which samAccountName should be used even if you don’t have get-aduser installed.

[pre]
function Test-AccountName {
Param(
$BasicTemplate,
$BackupTemplate
)

# We are only interested in triggering the return value.
$null = try{Get-Aduser $BasicTemplate}catch{return $BasicTemplate}
$null = try{Get-Aduser $BackupTemplate}catch{return $BackupTemplate}

return "fail"

}
[/pre]

So you supply the function with the basic-template and the backup-template of the samAccountName
If Get-Aduser finds the name it will not give an error and will go to the next statement.
If it’s not found Get-Aduser will produce an error (meaning the name is available for use) and return the samAccountName that should be used.
If both are available the function will return fail, meaning both commands where successfull and therefore they are already taken.

For the main function you could basically replace 66->103 with

[pre]
if ($UserType.ToLower() -eq ‘elev’) {
# create the two variables $basicTemplate and $backupTemplate for elev according to your rules.
$samAccountName = Test-AccountName -BasicTemplate $basicTemplate -BackupTemplate $backupTemplate

$userOU = $studentOU
$gsUserOU = "/Elever/" + $StudentYear

}

if ($UserType.ToLower() -eq ‘lärare’) {
# create the two variables $basicTemplate and $backupTemplate for lärare according to your rules.
$samAccountName = Test-AccountName -BasicTemplate $basicTemplate -BackupTemplate $backupTemplate

$userOU = $teacherOU

}

if ($UserType.ToLower() -eq ‘personal’) {
# create the two variables $basicTemplate and $backupTemplate for personal according to your rules.
$samAccountName = Test-AccountName -BasicTemplate $basicTemplate -BackupTemplate $backupTemplate

$userOU = $staffOU

}

if($samAccountName -eq ‘fail’) {
$samAccountName = Read-Host -Prompt “Please enter a username manually”
}
[/pre]

One thing that you have to consider is the ‘New-MGStudentAccountName’ function for which I can’t see how it works.
So possibly you need to refactor that or handle the ‘elev’ case differently.

8

Sorry for the late response, I’ve been pretty busy lately.

Thank you again for your response. I appreciate the time and effort you’ve put into this.

I will be looking at this more closely when I get the time.

The ‘New-MGStudentAccountName’ is basically a version of the ‘$samAccountName = $fixedFirst’/‘$samAccountName = $fixedFirst + $fixedLast.Substring(0,1)’ which builds a student samAccountName - the template there is just a bit more complicated than lowercase first name so I put it in separate function.