Disabling users from csv with error check

This topic contains 0 replies, has 1 voice, and was last updated by  Forums Archives 5 years, 8 months ago.

  • Author
    Posts
  • #6319

    by Chris_J at 2013-04-08 20:24:59

    Hi all,

    New to PowerShell so not sure about the syntax in my script or how to make it more efficient. I want to disable a list of users from a text file (just a list of Sam Account Names), but also check whether the user is already disabled or the user even exists.

    Here is my script

    [code2=powershell]$InputFile = ".\users.txt"
    $OutputFile = ".\disabledusers.csv"

    Add-PSSnapin Quest.ActiveRoles.ADManagement -ErrorAction SilentlyContinue

    $Result = @()
    $DisabledCount = 0
    $AlreadyDisabledCount = 0
    $NotFoundCount = 0

    Get-Content $InputFile | ForEach-Object {
    $User = $null
    $User = Get-QADUser $_ -SearchRoot "DC=*,DC=*"
    If (-not $User.AccountIsDisabled)
    {$User | Disable-QADUser
    $Result += New-Object PSObject -Property @{
    User = $User.SamAccountName
    DN = $User.DN
    Status = "Disabled"
    }
    $DisabledCount ++
    }
    }
    Get-Content $InputFile | ForEach-Object {
    $User = $null
    $User = Get-QADUser $_ -SearchRoot "DC=*,DC=*"
    If ($User.AccountIsDisabled)
    {$Result += New-Object PSObject -Property @{
    User = $User.SamAccountName
    DN = $User.DN
    Status = "Already Disabled"
    }
    $AlreadyDisabledCount ++
    }
    }
    Get-Content $InputFile | ForEach-Object {
    $User = $null
    $User = Get-QADUser $_ -SearchRoot "DC=*,DC=*"
    If (-not $User)
    {$Result += New-Object PSObject -Property @{
    User = $User.SamAccountName
    DN = "N/A"
    Status = "User not found"
    }
    $NotFoundCount ++
    }
    }

    $Result = $Result | Select $_
    $Result | Out-GridView
    $Result | Where-Object {$_} | Export-Csv $OutputFile -NoTypeInformation -Force

    Write-Host "Accounts in File": ((Get-Content $InputFile).Length)
    Write-Host "Users Disabled": $DisabledCount
    Write-Host "Already Disabled": $AlreadyDisabledCount
    Write-Host "User not found": $NotFoundCount[/code2]

    Issues I get – When I put a user id in the text file, which doesn't exist I get – Disable-QADUser : Cannot validate argument on parameter 'Identity'. The argument is null or empty. Supply an argument that
    is not null or empty and then try the command again.

    I found if I only have 1 user ID in the file, the Accounts in File count is the number of characters.

    When my script disabled an account, it shows the account as being disabled, but also outputs that it is "already disabled" too.

    This was a powershell script I found on thesurlyadmin which I have tried to rehash for Quest Powershell

    by Chris_J at 2013-04-08 20:45:09

    Think I got it working..

    [code2=powershell]$InputFile = ".\users.txt"
    $OutputFile = ".\disabledusers.csv"

    Add-PSSnapin Quest.ActiveRoles.ADManagement -ErrorAction SilentlyContinue

    $Result = @()
    $DisabledCount = 0
    $AlreadyDisabledCount = 0
    $NotFoundCount = 0

    Get-Content $InputFile | ForEach-Object {
    $User = $null
    $User = Get-QADUser $_ -SearchRoot "DC=*,DC=*"
    If ($User.SamAccountName -and -not $User.AccountIsDisabled)
    {$User | Disable-QADUser
    $Result += New-Object PSObject -Property @{
    User = $User.SamAccountName
    DN = $User.DN
    Status = "Disabled"
    }
    $DisabledCount ++
    }
    If ($User.AccountIsDisabled)
    {$Result += New-Object PSObject -Property @{
    User = $User.SamAccountName
    DN = $User.DN
    Status = "Already Disabled"
    }
    $AlreadyDisabledCount ++
    }
    If (-not $User.SamAccountName)
    {$Result += New-Object PSObject -Property @{
    User = $User.SamAccountName
    DN = "N/A"
    Status = "User not found"
    }
    $NotFoundCount ++
    }
    }

    $Result = $Result | Select $_
    $Result | Out-GridView
    $Result | Where-Object {$_} | Export-Csv $OutputFile -NoTypeInformation -Force

    Write-Host "Accounts in File": ((Get-Content $InputFile).Length)
    Write-Host "Users Disabled": $DisabledCount
    Write-Host "Already Disabled": $AlreadyDisabledCount
    Write-Host "User not found": $NotFoundCount[/code2]

    Not sure I want the -error silentlycontinue on the disable user though.

    by Chris_J at 2013-04-08 20:50:27

    Where it's outputting to the csv and out-gridview, where I have put a non-existent user id how would I get that to show instead of a blank user. DN is showing correctly as N/A. it's not a real big issue, so not too bothered if I can't fix that bit.

    Any input much appreciated, as I really don't understand PS. I've only just starting getting the grasp of basic VB script for deploying software on SCCM 😉

    by Chris_J at 2013-04-08 21:02:44

    Apologies, I've fixed it

    [code2=powershell]If (-not $User.SamAccountName)
    {$Result += New-Object PSObject -Property @{
    User = $_
    DN = "N/A"
    Status = "User not found"
    }
    $NotFoundCount ++[/code2]

    by happysysadm at 2013-04-09 01:18:51

    Hi Chris,

    good to know you fixed it by yourself.

    Anyway I think it's better to use a 'if...elseif...else' construct for decision making:

    [code2=powershell]Get-Content $InputFile | ForEach-Object {
    $User = Get-QADUser $_ -SearchRoot "DC=*,DC=*"
    If(-not $User.SamAccountName){'user not found'}
    elseif($User.AccountIsDisabled){'user already disabled'}
    else{$user | Disable-QADUser}
    }[/code2]
    No need to run get-content tree times as in your first piece of code. If you do so it is normal that it disables the account and then it show that it is disabled.

    Hope this helps
    Carlo

    by MasterOfTheHat at 2013-04-09 08:09:31

    Not really a big advantage, but I would probably have written it more like this. It's easier for me to read and less repetition of code. happy make a valid point about the if, elseif stuff. By using multiple if statements, you have the potential of executing multiple blocks of code if your input happens to meet the criteria in more than one if. By using if/else, you are making them mutually exclusive.
    $InputFile = ".\users.txt"
    $OutputFile = ".\disabledusers.csv"

    Add-PSSnapin Quest.ActiveRoles.ADManagement -ErrorAction SilentlyContinue

    $Result = @()
    $DisabledCount = 0
    $AlreadyDisabledCount = 0
    $NotFoundCount = 0
    $Status = ""
    $DN = ""

    Get-Content $InputFile | ForEach-Object {
    $User = Get-QADUser $_ -SearchRoot "DC=*,DC=*"
    if($User -eq $null)
    {
    # User does not exist
    $DN = "N/A"
    $Status = "User not found"
    $NotFoundCount ++
    }
    else
    {
    # User exists
    if($User.AccountIsDisabled)
    {
    # Account is already disabled
    $DN = $User.DN
    $Status = "Already Disabled"
    $AlreadyDisabledCount ++
    }
    else
    {
    # Account is enabled
    try {
    # Try to disable account
    Disable-QADUser $User
    $success = $true
    }
    catch
    {
    "$($User.SamAccountName) - $($_.Exception.Message)"
    $success = $false
    }
    finally
    {
    $DN = $User.DN
    if($success)
    {
    $Status = "Account disable successful"
    $DisabledCount ++
    }
    else
    {
    $Status = "Account disable failed"
    $DisableFailedCount ++
    }
    }
    }
    }
    $Result += New-Object PSObject -Property @{
    User = $_
    DN = $DN
    Status = $Staus
    }
    }
    $Result | Out-GridView
    $Result | Export-Csv $OutputFile -NoTypeInformation -Force

    "Accounts in File $((Get-Content $InputFile).Length)"
    "Users Disabled: $DisabledCount"
    "Already Disabled: $AlreadyDisabledCount"
    "User not found: $NotFoundCount"

    A couple of notes:
    [list=][*]Not sure why you were piping $Results to Select-Object and Where-Object at the end of the script. You were essentially running a filter to get everything, so it wasn't really helpful.[*]You weren't checking for any errors when you tried to disable the user, so you would never know if the user running the script didn't have permissions, etc, etc unless you looked at the console[*]You usually want to check to see if an account exists as soon as you try to retrieve the account. (Actually, I really should have wrapped the Get-QADUser call in a try/catch instead of using the if($User -eq $null) condition)[/list]

    Again, not trying to say that my way is the right way, but I wanted to give you a little different perspective.

    by Chris_J at 2013-04-09 16:41:18

    That's brilliant Thank you guys.

    Master of The Hat, beautiful and better than what I had. The status wasn't showing on the output, but it was just a spelling mistake in the code

    [code2=powershell]$Result += New-Object PSObject -Property @{
    User = $_
    DN = $DN
    Status = $Staus[/code2]

    Changed Status = $Staus to $Status and works perfect. I wasn't too sure my code was very efficient or all that good, but doing what I wanted. Any idea, why when I only have 1 account in the text file it shows the number of account in the file as the number of characters? If I have 2 lines it shows as 2. When I change the code to use a csv file, I have SamAccountName on the first column and use Get-QADUser $_.SamAccountName and Write-Host "Accounts in File": ((Get-Content $InputFile).Length -1) which gives me the correct answer otherwise it sees SamAccountName as an input too.

    Here is the output in the PowerShell console with 1 user ID in the text file. The UserID is 8 characters long 🙂

    Accounts in File 8
    Users Disabled: 0
    Already Disabled: 1
    User not found: 0
    Failed to disable:

    by Chris_J at 2013-04-09 17:01:02

    How did you get else to work too? I tried with else, and Quest didn't like it.

    by happysysadm at 2013-04-10 00:19:57

    [quote="Chris_J"]Any idea, why when I only have 1 account in the text file it shows the number of account in the file as the number of characters? If I have 2 lines it shows as 2. [/quote]
    Hi Chris,
    replace this:
    [code2=powershell]Write-Host "Accounts in File": ((Get-Content $InputFile).Length)[/code2]
    with this:
    [code2=powershell]Write-Host "Accounts in File": (@(Get-Content $InputFile).Length)[/code2]
    and this way you are forcing get-content to pipe its elements into an array, which in powershell is declared with @(). Length will then return the number of element in the array.
    Carlo

    by MasterOfTheHat at 2013-04-10 07:17:04

    happy's answer fixes your problem with 1 user in the input file. Get-Content only returns a collection of objects representing the lines of text in a file if it reads more than one line. If it sees only one line, it returns an object of the type of data it's reading, (in this case, a string). Maybe one of the MVPs can explain why it does that??

    Here's a simple example:
    PS C:\> gc C:\temp\test.txt
    test1
    PS C:\> $a = gc C:\temp\test.txt
    PS C:\> $a.GetType()

    IsPublic IsSerial Name BaseType
    -------- -------- ---- --------
    True True String System.Object

    PS C:\> $a.Length
    5
    PS C:\> @($a).Length
    1
    PS C:\> gc C:\temp\test2.txt
    test1
    test2
    PS C:\> $b = gc C:\temp\test2.txt
    PS C:\> $b.GetType()

    IsPublic IsSerial Name BaseType
    -------- -------- ---- --------
    True True Object[] System.Array

    PS C:\> $b.Length
    2
    PS C:\> @($b).Length
    2

    Not sure what you mean about Quest not liking the "else". Can you provide an example?

    And if you haven't yet, you'll learn that there are usually million ways to do anything in code. What you end up running depends on personal preferences, coding/scripting background, education, whose best practices you're following, what "new thing" you've been playing with, how long you've been writing in that language, and just how you felt that day in general! 🙂 Mine was just a suggestion, and I'm sure that several of the other posters would say my code isn't all that great. But it works, and I like it...

    by Chris_J at 2013-04-10 15:58:32

    All for the sake of an @ 🙂

    I can't remember how I had the else errors, but I know I had too many {} don't know what those symbols are called. Would I have closed the if statement and then tried using else outside of that?

    by Chris_J at 2013-04-10 16:19:38

    It was this code I was trying to rehash for Quest – http://thesurlyadmin.com/2012/10/01/dis ... rs-by-csv/

    by MasterOfTheHat at 2013-04-11 06:18:45

    There is definitely something to be said about tabs and spacing in code... That post is a good example. With every line starting at the same spot, my eyes get crossed and I spend more time trying to figure out where the next line is that I lose the logic! Nothing against the guy's code; if it works, that's what is important. But I have to write mine so that I can easily read it later.

You must be logged in to reply to this topic.