Pablo's Powershell Pow-Wow

Discussion in 'Business & Enterprise Computing' started by PabloEscobar, Sep 18, 2014.

  1. colmaz

    colmaz Member

    Joined:
    Jan 8, 2007
    Messages:
    443
    Location:
    Perth, WA
    It's when I review past me's code, that I remember I should comment my code more often.

    As mentioned above, my preferred approach (when I remember) is to write out what the script does as pseudocode comments. Then I start writing the actual code around it.

    My top tips:
    • Be consistent in all things as it makes it easier to follow
      • Refactor older code if you have to
    • Use variables that make sense
      • $object is bad, $AD_User is better
    • Use of Hungarian notation is typically avoided
      • $strThing, $intCound, etc.
      • It gets kind of hard after a while when everything becomes $objThis, $objThat
    • Use consistent casing
      • snake_case vs Title_Case vs camel_Case, etc.
    • Have a look at PS Script Analyzer and take it's recommendations with a pinch of salt
    I'd have a look at the Practice and Style guide (it most likely covers the above) but work blocks GitHub.
     
  2. bart5986

    bart5986 Member

    Joined:
    Jan 31, 2006
    Messages:
    5,093
    Location:
    Brisbane
    Seeing as we seem to work in the same job, I'm curious, what are you planning to do with your powershell scripts at logon?

    If only there was a script to install printer drivers without administrative access :lol:
     
  3. miicah

    miicah Member

    Joined:
    Jun 3, 2010
    Messages:
    7,494
    Location:
    Mount Cotton, QLD
    Puts a logon record in our DB, updates PaperCut config files, make sure all language packs are installed (although I don't think I need this any more), remove drive mappings (for normal and exam accounts).
     
  4. bart5986

    bart5986 Member

    Joined:
    Jan 31, 2006
    Messages:
    5,093
    Location:
    Brisbane
    Oh ok thanks for the information, I've been planning to do a logon record for a while.
     
  5. Gunna

    Gunna Member

    Joined:
    Dec 25, 2001
    Messages:
    7,901
    Location:
    Brisbane
    I am trying to create a termination script and some of our users may have Admin accounts.

    Their standard accounts are in format AB1234 and admin accounts are similar to AB1234.admin.
    I need a dynamic menu to return 1 or more results

    Testing my script if there are 2 AD objects it is fine but if there is 1 I get an error. The menu code I found online and isnt mine.

    Code:
    $TeamMemberNumber = Read-Host "Please Enter the Team Members username or team member ID"
    $UserQuery = Get-ADUser -Filter "SamAccountName -like '*$TeamMemberNumber*'" -server "%domain controller"
    echo "`n"
    Echo "These were the users found with AD Username like $TeamMemberNumber :"
    echo "`n"
    $menu = @{}
    for ($i=1;$i -le $UserQuery.count; $i++) {
        Write-Host "$i. $($UserQuery[$i-1].UserPrincipalName)"
        $menu.Add($i,($UserQuery[$i-1].UserPrincipalName))
        }
    Echo "`n"
    [int]$ans = Read-Host 'Enter selection of account to terminate'
    $selection = $menu.Item($ans)
    $AUSADuser = Get-ADUser -filter "UserPrincipalName -eq '$selection'"-properties MemberOf, Description, manager -server "%DC%"
    $Firstname = $AUSADuser.GivenName
    $LastName = $AUSADuser.surname
    $Verify = Read-Host "Please verify new hires name is: $FirstName $Lastname before proceeding. Press Y to proceed or any key to cancel"
    if ($verify -eq "Y")
    {Write-host "Script is proceeding"}
    else
    {write-host "BROKEN!!!!"}
    I get this error if there is a single item to return
    Any ideas on a dynamic menu that allows a single result?
     
    Last edited: Sep 16, 2021
  6. looktall

    looktall Working Class Doughnut

    Joined:
    Sep 17, 2001
    Messages:
    26,660
    not a clue but in my exit script i have the person running it enter the samaccount of the person to be exited and then the script generates a bunch of variables (most of which i have removed in the example below) before asking the person to confirm if they have the right account.

    Code:
    $SAMAccountName = Read-Host 'Account name to disable'
    
    # Get the properties of the account and set variables
    $user = Get-ADuser $SAMAccountName -properties canonicalName, distinguishedName, displayName, Mail
    $DistinguishedName = $user.distinguishedName
    $CanonicalName = $user.canonicalName
    $DisplayName = $user.displayName
    $mail = $user.mail
    
    <...Bunch of Stuff...>
    <# --- Confirm correct account to exit --- #>
    
    $Message = "This will exit the user account for $($CanonicalName)."
    $Question = "Are you sure you wish to continue?"
    $Choices = "&Yes", "&No"
    $Decision = $Host.UI.PromptForChoice($Message, $Question, $Choices, 1)
    if ($Decision -eq 0) {
    write-host "Exiting User..."}
    else {
    write-host "Aborting Script. No changes have been made."
    exit}
     
  7. Gunna

    Gunna Member

    Joined:
    Dec 25, 2001
    Messages:
    7,901
    Location:
    Brisbane
    Cheers, I wanted it to do a search based on username or Team member ID to display all accounts as a reminder that this person may have an admin account we forgot about.

    There are reasons where sometimes the user needs a local admin account for a specific machine or could be an I.T team member who has an admin account so this would serve as validation of their accounts.
     
    looktall likes this.
  8. miicah

    miicah Member

    Joined:
    Jun 3, 2010
    Messages:
    7,494
    Location:
    Mount Cotton, QLD
    It's because PowerShell returns the single user object instead of an array of objects with 1 item if it only finds one result. I can't find where this is documented, but I have replicated your problem on my end.

    Get-Member for the single AD object still exposes the Count method, but it just returns nothing. So I did this:

    Code:
    if ($UserQuery.Count) {
        for ($i=1;$i -le $UserQuery.count; $i++) {
            Write-Host "$i. $($UserQuery[$i-1].UserPrincipalName)"
            $menu.Add($i,($UserQuery[$i-1].UserPrincipalName))
            }
    }
    else {
        Write-Host "1. $UserQuery.UserPrincipalName)"
        $menu.Add(1, $UserQuery.UserPrincipalName)
    }
    It's not very flexible, but the only use cases you have for this are 0 results (not handled), 1 result (handled) or more than 1 (handled).
     
    Gunna likes this.
  9. Gunna

    Gunna Member

    Joined:
    Dec 25, 2001
    Messages:
    7,901
    Location:
    Brisbane
    Good catch and adding [array] to before the variable that runs the Get-ADuser query forces it to use an array, even if a single entry is returned. That solves my issue
     
    miicah likes this.

Share This Page

Advertisement: