by Chris_J at 2013-04-08 20:24:59
Hi all,by Chris_J at 2013-04-08 20:45:09
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
Think I got it working…by Chris_J at 2013-04-08 20:50:27
[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.
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.by Chris_J at 2013-04-08 21:02:44
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
Apologies, I’ve fixed itby happysysadm at 2013-04-09 01:18:51
[code2=powershell]If (-not $User.SamAccountName)
{$Result += New-Object PSObject -Property @{
User = $_
DN = "N/A"
Status = "User not found"
}
$NotFoundCount ++[/code2]
Hi Chris,by MasterOfTheHat at 2013-04-09 08:09:31
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
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.by Chris_J at 2013-04-09 16:41:18$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.
That’s brilliant Thank you guys.by Chris_J at 2013-04-09 17:01:02
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:
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]by MasterOfTheHat at 2013-04-10 07:17:04
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
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??by Chris_J at 2013-04-10 15:58:32
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…
All for the sake of an @by Chris_J at 2013-04-10 16:19:38
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?
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.