Author Posts

February 14, 2018 at 7:39 pm

Hello,
I am completely new to using Powershell and have started an online class to learn to use it. I am working on a project that is unrelated to the learning site that I am struggling with. The project goal is to add modify rights to the 'Admins' and 'Users' groups of 3 folders; one at the root of 'c:\mydirectory', one in 'Program Files\mydirectory', and one in 'Program Files (x86)\mydirectory' if they exist. This is what I have so far:

#region section globals
# Declare and set globals
$RootPath =0
$32Path =0
$64Path =0
# Call the VarTest function to reset the above variables
VarTest
#endregion

#region section varcheck
# Clear Variables
function VarTest ()
{
if ($RootPath -ne 0) {$RootPath -eq 0; write-host "The RootPath Variable has been Reset" + Write-Host $RootPath}
else {Write-Host "The RootPath Variable is Empty" + $RootPath};
if ($32Path -ne 0) {$32Path =0; write-host "The 32Path Variable has been Reset" + Write-Host $32Path}
else {Write-Host "The 32Path Variable is Empty" + $32Path};
if ($64Path -ne 0) {$64Path -eq 0; write-host "The 64Path Variable has been Reset" + Write-Host $64Path}
else {Write-Host "The 64Path Variable is Empty" + $64Path};
# Call the PathTest function
PathTest
}
#endregion

#region section pathtest
# Test the directory paths to make sure they exist and set the variable to add the modify rights to the AD Groups
function PathTest () {
# Test Root mydirectory Folder
if  (Test-Path -Path "C:\mydirectory" -IsValid) {$RootPath =1; write-host "Root mydirectory is here" + $RootPath}
Else {write-host "Root mydirectory is not here"; Exit;}
# Test Program Files\mydirectory Folder
if (Test-Path -Path "c:\program files\mydirectory" -IsValid) {$32Path =1; Write-Host "program files\mydirectory is here" + $32Path}
Else {write-host "32Bit mydirectory folder is not here"}
# Test Program Files x86\mydirectory Folder
if (Test-Path -Path "C:\program files (x86)\mydirectory" -IsValid) {$64Path =1; Write-Host "program files x86\mydirectory is here" + $64Path}
Else {Write-Host "64Bit mydirectory folder is not here"}
PermissionChange
}
#endregion

#region section root permissions
# Set the permissions on the root directory
function PermissionChange () {
$Acl = Get-Acl "C:\mydirectory"

$Ar = New-Object System.Security.AccessControl.FileSystemAccessRule("Users", "Modify", "ContainerInherit,ObjectInherit", "None", "Allow")
$Ar2 = New-Object System.Security.AccessControl.FileSystemAccessRule("Admins", "Modify", "ContainerInherit,ObjectInherit", "None", "Allow")

$Acl.SetAccessRule($Ar)
Set-Acl "c:\mydirectory" $Acl

$Acl.SetAccessRule($Ar2)
Set-Acl "c:\mydirectory" $Acl
Write-Host "Root Directory Updated"
}
#endregion

#region section 32permissions
# Set the permissions on the 32bit directory
function 32Permissions () {
if ($32Path =1) {
$Acl = Get-Acl "C:\mydirectory"
Set-Acl "C:\Program Files\mydirectory" $Acl
Write-Host "32bit mydirectory folder has been update"
}
}
#endregion

#region section 64permissions
# Set the permissions on the 64bit directory
function 64Permissions () {
if ($64Path =1) {
$Acl = Get-Acl "C:\mydirectory"
Set-Acl "C:\Program Files (x86)\mydirectory" $Acl
Write-Host "64bit mydirectory folder has been update"
}
}
#endregion

What I have seems way more complex than I think it should be (I'd rather use a ForEach statement if I could but I haven't gotten that far yet), and when I test it the variables seem to get stuck with 1 after it is run. I am also having an issue on the test system that if the 'Program Files\mydirectory' exists when the script is ran and then the directory is deleted, on subsequent runs of the script it will still say the directory is there and set the variable to 1 (I have not checked to see if that is the case with the other 2 directories).

February 15, 2018 at 5:27 am

Everyone will / can have an opinion, and what follows is just mine. There are always better more definitive ways to do just about anything, but that is directly relevant to and of knowledge, training, experience, belief, preference and acceptance.

All that being said. You need to work under the concepts of:

Is this code easily readable / understandable?
Is this code well formatted?
Is this code easily maintained?
Is this code follow a development standard?
Is this code going to be share / used by others who need to understand and maintain it?
Is this code as optimized as it cade be?
Is this code as short as it can be without being recondite / abstruse?
Was unit testing leveraged?
Was code validation leveraged?
Was proper error handling implemented?
Was code logging implemented?
Was built-in help added?
Was version control leveraged?
etc...

As for the code itself...

  • You have some formatting issues, which make the code hard to read / follow.
  • You are calling functions before they are ever loaded.
  • You are calling variables with an assignment that already exists.
  • You are using Write-Host unnecessarily and incorrectly.
  • Functions should follow the normal verb-noun construct of normal cmdlets.
  • If you are using a whole bunch of If/Else statements, it's time to look at select case.
  • Overuse if double quotes, you only really need them when you need variable expansion in a quoted string. Plain text strings use single quotes.
  • You are using semicolons for no real purpose. They, mean, in the case you are using them, that the next command is independent of the proceeding one. If it's a separate command, then put it on the next line. Cramming things on one line, because you can is really not a thing in scripts. At the command line, I get that, but not scripts / code.
  • Are you sure you wan to use the EXIT where you have it?
  • You have function in the code which are never used.
  • When building code, never do it all at once. That is why modular programming / unit testing exists.
  • You do this one line / segment at a time, test that and make sure you are getting what you'd expect before moving on.

As far as this...

'when I test it the variables seem to get stuck with 1 after it is run.'

It is that way, because that is what you told the code to do. You are not using the comparison and assignment operators correctly for what you appear to be trying to accomplish. I made that adjustment below.

This would be my take on what you show here.
Note, this can be optimized more, to reduce this code, and you do have ohter errors in the code becasue of the behaviour you are seeing.
That is another homework assignment for you to resolve.

If you make needed position changes with the functions and some code clean-up, you'd end up with something like this, which is far more readable, though I did not correct all the errors, just some of them.
Well, and used style that I commonly use for the code layout. Everyone has their take on that though.

    #region Begin section globals

        # Declare and set globals
        $RootPath = 0 
        $32Path   = 0 
        $64Path   = 0

    #endregion  End section globals



    #region Begin section root permissions

        # Set the permissions on the root directory
        function Set-PermissionChange() 
        {
            [cmdletbinding()]

            Param
            (
                $Acl = (Get-Acl -Path 'C:\mydirectory')
            )

            $AccessRuleUser = New-Object System.Security.AccessControl.FileSystemAccessRule('Users', 'Modify', 'ContainerInherit,ObjectInherit', 'None', 'Allow')
            $AccessRuleAdmin = New-Object System.Security.AccessControl.FileSystemAccessRule('Admins', 'Modify', 'ContainerInherit,ObjectInherit', 'None', 'Allow')

            $Acl.SetAccessRule($AccessRuleUser)
            Set-Acl -Path "C:\mydirectory $Acl" -WhatIf

            $Acl.SetAccessRule($AccessRuleAdmin)
            Set-Acl -Path "C:\mydirectory $Acl" -WhatIf
            'Root Directory Updated'
        }

    #endregion End section root permissions

    #region Begin section 32permissions

        # Set the permissions on the 32bit directory
        function Set-32Permissions() 
        {
            [cmdletbinding()]

            Param
            (
        
            )

            if ($32Path = 1) 
            {
                $Acl = Get-Acl -Path 'C:\mydirectory'
                Set-Acl -Path "C:\Program Files\mydirectory $Acl" -WhatIf
                '32bit mydirectory folder has been update'
            }
        }

    #endregion End section 32permissions


    #region Begin section 64permissions

        # Set the permissions on the 64bit directory
        function Set-64Permissions() 
        {
            [cmdletbinding()]

            Param
            (
        
            )

            if ($64Path = 1) 
            {
                $Acl = Get-Acl -Path 'C:\mydirectory'
                Set-Acl -Path "C:\Program Files (x86)\mydirectory $Acl" -WhatIf
                '64bit mydirectory folder has been update'
            }
        }

    #endregion End section 64permissions



    #region Begin section pathtest

        # Test the directory paths to make sure they exist and set the variable to add 
        # the modify rights to the AD Groups

        function New-PathTest() 
        {
            [cmdletbinding()]

            Param
            (
        
            )

            # Test Root mydirectory Folder
            if  (Test-Path -Path 'C:\mydirectory') 
            {
                $RootPath = 1
                'Root mydirectory is here $RootPath'
            }
            Else 
            {
                Write-Warning -Message 'Root mydirectory is not here'
                Exit
            }

            # Test Program Files\mydirectory Folder
            if (Test-Path -Path 'C:\Program Files\mydirectory') 
            {
                $32Path = 1
                'program files\mydirectory is here $32Path'
            }
            Else 
            {
                Write-Warning -Message '32Bit mydirectory folder is not here'
            }

            # Test Program Files x86\mydirectory Folder
            if (Test-Path -Path 'C:\Program Files (x86)\mydirectory') 
            {
                $64Path = 1
                'program files x86\mydirectory is here $64Path'
            }
            Else 
            {
                Write-Warning -Message '64Bit mydirectory folder is not here'
            }
            Set-PermissionChange
        }

    #endregion End section pathtest





    #region Begin section varcheck

        # Clear Variables

        function New-VarTest()
        {
            [cmdletbinding()]

            Param
            (
        
            )

            if ($RootPath -gt 0) 
            {
                $RootPath = 0
                Write-Warning -Message 'The RootPath Variable has been Reset $RootPath'
            }
            else 
            {
    
            }


            if ($32Path -gt 0) 
            {
                $32Path = 0
                Write-Warning -Message 'The 32Path Variable has been Reset $32Path'
            }
            else 
            {
                "The 32Path Variable is Empty $32Path"
            }


            if ($64Path -gt 0) 
            {
                $64Path = 0
                'The 64Path Variable has been Reset $64Path'
            }
            else 
            {
                "The 64Path Variable is Empty $64Path"
            }

            # Call the PathTest function
            New-PathTest
        }

    #endregion End section varcheck


    New-VarTest

February 20, 2018 at 5:36 pm

postanote Thank you for the reply and suggestions. As you can see from my script, my knowledge on powershell is minimal at best and I don't know that I am getting my $$ worth from the online class... then again maybe some of this is covered in later modules and I am just being overzealous in what I am trying to do at my present level. Thanks again for the response, suggestions, and the clean up on the code.