Create share folder and AD security group and apply security to new folder

I think I'm finally done with my code and was hoping for some pointers. My goal for this script was to:

• Create new folder

• Create AD group FS-TESTSHARE-R

• Create AD group FS-TESTSHARE-RW

• Apply both groups to the new share folder

• Set full read permissions to FS-TESTSHARE-R

• Set full read/Rights permissions to FS-TESTSHARE-RW

• Set full access permissions for local machine admins and domain admins

Everything seems to be working ok, but I was wondering if there was a cleaner way to do this or if I'm on the right track. Right before posting this I was doing the "FS-$($NAME.replace(' ',''))-RW".toupper() in all of the individual spots where I now have $ADNameRW (and $ADNameR), but I realized that it would work better to create the string first.

Any other tips or suggestions on my creation?

```
$FileShare = Read-Host -prompt "Verify Fileshare Server Name - (ie ECCOFS01)"
if ( ($FileShare -eq $null) -or ($FileShare -eq "") -or ($FileShare -eq " ") -or ($FileShare -eq "c:") -or ($FileShare -eq "d:"))
{
Write-Host "You entered $FileShare which is an incorrect value: This Script is now Exiting." -Foreground "White" -Background "Red"
Exit
}
else
{
Write-Host "You Entered $FileShare"
}

$Parent = Read-Host -prompt "Enter full parent path that will contain the new folder (ie. \\eccofs01\Groups\ECCO IT\ or C:\Test Share)"
if ( ($Parent -eq $null) -or ($Parent -eq "") -or ($Parent -eq " ") -or ($Parent -eq "c:") -or ($Parent -eq "d:"))
{
Write-Host "You entered $Parent which is an incorrect value: This Script is now Exiting." -Foreground "White" -Background "Red"
Exit
}
else
{
Write-Host "You Entered $Parent"
}

$Name = Read-Host -prompt "Enter New Folder Name."
if ( ($Name -eq $null) -or ($Name -eq "") -or ($Name -eq " "))
{
Write-Host "You entered $Name which is an incorrect value: This Script is now Exiting." -Foreground "White" -Background "Red"
Exit
}
else
{
Write-Host "You Entered $Name"
}

Advanced Functions

I would have a serious look at the about_advanced_function_parameters. In the beginning you are doing some parameter validation on read-host Input. I would consider making this into an advanced script capable of accepting parameters so that you can make a series of calls to it and you don't need to rely on Read-Host which can get tedious if you are going to be running this several times at once.

Another set back in this is that if a user gets the first 2 prompts correct and makes a mistake on the third they have to start over again. You could go a couple of different routes but in its simplest form perhaps you should give users a couple of chances so they have a margin for error. If you used parameters like I mentioned earlier then the could just call the command again and fix the mistake without going through all the prompts.

Input Validation

I see logic this several times which is you trying to verify that a string is not null or empty space....

($Name -eq $null) -or ($Name -eq "") -or ($Name -eq " ")

The string static method [string]::IsNullOrWhiteSpace() would cover all those bases nicely.

If([string]::IsNullOrWhiteSpace($name)){"Do Stuff"}else{"Do Less Stuff"}

String Concatenation for Creating File Paths

You gather both parent folder and folder name separately and put them together later. Consider using the [io.path] method Combine() which will make it so you don't have to worry about the presence of a slash in the input from the user.

$Path = [IO.Path]::Combine($parent, $Name)

Continuing on this path I also see that you ask for the server name and then the full share path anyway (which contains the server path). Instead of asking for them separately you can cast the string as a URI to get just the host name without much hassle. Saves asking the user for something twice.

([uri]"\\eccofs01\Groups\ECCO IT\").Host

Split-Path would also help when taking a single sting and separating a path from its parent. Just something to keep in the back pocket.

split-path "\\eccofs01\Groups\ECCO IT\" -Parent
\\eccofs01\Groups

split-path "\\eccofs01\Groups\ECCO IT\" -Leaf
ECCO IT

-Parent can be omitted and is used by default.

Maybe a Little Regex Would Help

You are removing spaces using the string method `.Replace()` which is fine. Depending on your local other white-space characters are possible to run into. You can cover them with a simple `-replace '\s'` which will replace all white-space characters with nothing. 

$FileShareFull = $FileShare -replace '\s'

Avoid duplicate code

I read your previous post and we helped to remove some redundancy I would like to point out a couple other of areas to consider improvement.

$ADNameRW = "FS-$FileShareFull-$Name-RW".toupper()
$ADNameR = "FS-$FileShareFull-$Name-R".toupper()

You are making almost the same string accept that one has a W at the end. Why no make one a basic copy of the other. This way, in the unlikely event you change your naming convention you only really have to edit the one line. THINK OF THE SAVINGS!

$ADNameR = "FS-$FileShareFull-$Name-R".toupper()
$ADNameRW = $ADNameR + "W"

I love splatting an applaud your use of it. In the cases of $GroupParams1 and $GroupParams2 both share similar params. Why not more that shared group out and add it to the unique param groups.

$sharedParams = @{
    'GroupCategory' = "Security"
    'GroupScope' = "Global"
    'Path' = "OU=$Country,OU=FILE SHARE GROUPS,OU=Security Groups,DC=esg,DC=intl"
}

$GroupParams1 = $sharedParams + @{
    'Name' = $ADNameRW
    'SamAccountName' = $ADNameRW
    'DisplayName' = "$NAME Read-Write Access"
    'Description' = "Members of this group have read-write access to $Path."
}

$GroupParams2= $sharedParams + @{
    'Name' = $ADNameR
    'SamAccountName' = $ADNameR
    'DisplayName' = "$NAME Read-Write Access"
    'Description' = "Members of this group have read access to $Path"
}

There are others that are so close to being able to consolidate but making it work would hamper readability which ultimately is important.

Potentially unwanted output

The New-Item cmdlet would return the directory object that was created to the console as output. That can sometimes be seen as a nuisance. If that is the case an easy approach is to cast the whole command to [void] or pipe into Out-Null. Both have the same result.

[void](New-Item -Path $Path -ItemType Directory)
New-Item -Path $Path -ItemType Directory | Out-Null

These suggestions don't all have to be implemented but would be good to at least consider. I do like the overall feel of the code and I have always been procrastinating about doing something very similar. Nice work.

StackExchange Code Review Q#125493, answer score: 2