1

SO I wrote a script that gets some information from AD about a user and their associated computer(s). I have read online about a few of the problems people face with the Get-ADComputer cmdlet and some possible bugs with it. At first I thought that was what I had run into here but now I'm not so sure.

My problem is that my code works fine, but if I add an unrelated snippet of code to get some info from a variable, it breaks something else. Take a look.

Entire code: http://paste.ofcode.org/Hgbiukp2XYqKnv2sdUGBKb

The bit of code in question:

## Prompt host for input
$username = Read-Host -Prompt "Enter the Username"

## Get list of computers
$ComputerList = Get-ADComputer -Filter {ManagedBy -eq $username} -Properties ManagedBy | Select-Object -ExpandProperty Name

## Compute and format results
Foreach ($Computer in $ComputerList)
{
    $OnlineStatus = Test-Connection -ComputerName $Computer -BufferSize 16 -Count 1 -Quiet
    If ($OnlineStatus -like "True") {$OnlineStatus = "$True"} else {$OnlineStatus = "$False"}

    Get-ADComputer -Identity $Computer -Properties ManagedBy,DNSHostName,LastLogonTimestamp |
    Select-Object DNSHostName,@{Name="Active";Expression={$OnlineStatus}},@{Name="LastLogonTimestamp";Expression={[datetime]::FromFileTime($_.LastLogonTimestamp)}}
}

So this part works perfect. Now if you add a snippet that gets some info about the username, you can see that it stops displaying the output about the AD computer.

## Prompt host for input
$username = Read-Host -Prompt "Enter the Username"

## Get Username info
Get-ADUser -Identity $username –Properties “DisplayName”, “msDS-UserPasswordExpiryTimeComputed”, "LockedOut" |
Select-Object -Property @{Name="Name";Expression={$_.DisplayName}},@{Name=“PWD Expiration Timestamp”;Expression={[datetime]::FromFileTime($_.“msDS-UserPasswordExpiryTimeComputed”)}},LockedOut

## Get list of computers
$ComputerList = Get-ADComputer -Filter {ManagedBy -eq $username} -Properties ManagedBy | Select-Object -ExpandProperty Name

## Compute and format results
Foreach ($Computer in $ComputerList)
{
    $OnlineStatus = Test-Connection -ComputerName $Computer -BufferSize 16 -Count 1 -Quiet
    If ($OnlineStatus -like "True") {$OnlineStatus = "$True"} else {$OnlineStatus = "$False"}

    Get-ADComputer -Identity $Computer -Properties ManagedBy,DNSHostName,LastLogonTimestamp |
    Select-Object DNSHostName,@{Name="Active";Expression={$OnlineStatus}},@{Name="LastLogonTimestamp";Expression={[datetime]::FromFileTime($_.LastLogonTimestamp)}}
}

Can someone tell me why this is? And how to fix it?

Normally I'm able to figure it out just by playing with it but this one really has me stumped. I think the issue must be with the formatting of the $ComputerList variable but every item in the list is a String, which is what Get-ADComputer requires. So I just don't know.

Thank you all in advance.

1 Answers1

1

I believe your problem revolves around the fact that you're writing multiple different types of objects to the output pipeline (implicitly by not assigning them to a variable) and relying on default Powershell formatting rules to display them properly to the console (which it can't).

It's displaying the first type of object and then trying to display the second type of object using the same formatter as the first and just writing blank lines. You'd have the opposite problem if you move your Get-ADUser call below the loop with the Get-ADComputer calls.

Assuming the output of this script is only ever intended to be displayed on the console and not further processed by other scripts or chained with other cmdlets, you may want to use Write-Host with explicit formatting for each part of your output. Normally, this is a bad idea though. Something like this.

$user = Get-ADUser -Identity $username –Properties “DisplayName”, “msDS-UserPasswordExpiryTimeComputed”, "LockedOut"
Write-Host "User: $($user.DisplayName)`tPWD Expiration: $([datetime]::FromFileTime($_.“msDS-UserPasswordExpiryTimeComputed”))`tLockedOut: $($user.LockedOut)"

I'd also like to offer some additional advice on the general structure of your script.

First, I'd make your $username variable a mandatory parameter instead of explicitly calling Read-Host. Powershell will automatically prompt for it if it's not specified on the command line already.

When you check for the existence of the usere, you are changing the object type of your $username variable from a string to an ADUser object. I'm guessing this wasn't your intention and it's just a lucky that the later references to it work using the object instead of the string. You also make a second Get-ADUser call right after that to get the attributes you care about which costs an extra round-trip to the domain controller. I'd just combine the two and move your ## Get Username info section into the try/catch block. It will still throw the same exception if the user doesn't exist. I'd also assign the output to a new variable like $user.

Optionally, you could also skip all the exception handling and change the Get-ADUser call to use the -Filter {SamAccountName -eq $username} parameter and then just test for $null like you do later with Get-ADComputer.

You also end up duplicating your calls to Get-ADComputer as well. First you are calling with the filter, and then you're re-calling for each result. It would be more efficient to just add the DNSHostName,LastLogonTimestamp attributes to the initial call and then iterate directly over the resulting objects with no need to re-call Get-ADComputer.

Here's what a resulting script might look like:

param(
    [Parameter(Mandatory=$true,Position=0)]
    [string]$username
)

# Import AD Module
Import-Module ActiveDirectory

Write-Host "Active Domain: $((Get-ADDomain).DNSRoot)"

# Get user details
$user = Get-ADUser -Filter {SamAccountName -eq $username} -Properties DisplayName,msDS-UserPasswordExpiryTimeComputed,LockedOut
If ($user -eq $null) {
    Write-Error "Error: Username not found. Exiting"
    Exit
}
Write-Host "Name: $($user.DisplayName)"
Write-Host "PWD Expiration Timestamp: $([datetime]::FromFileTime($_.'msDS-UserPasswordExpiryTimeComputed'))"
Write-Host "LockedOut: $($user.LockedOut)"

# Get managed computers
$ComputerList = Get-ADComputer -Filter {ManagedBy -eq $username} -Properties ManagedBy,DNSHostName,LastLogonTimestamp

# Error-Handling: If no computers found
If ($ComputerList -eq $null) {
    Write-Error "No computers found"
    Exit
}

# Compute and format results
ForEach ($computer in $ComputerList) {

    $OnlineStatus = Test-Connection -ComputerName $computer.Name -BufferSize 16 -Count 1 -Quiet

    # since this is now the only type of object we will be putting on the pipeline,
    # it's ok to just do that now
    $computer | Select-Object DNSHostName,@{L="Active";E={$OnlineStatus}},@{L="LastLogonTimestamp";E={[datetime]::FromFileTime($_.LastLogonTimestamp)}} | Write-Output
}
Ryan Bolger
  • 16,755
  • 4
  • 42
  • 64
  • It seems you are correct. The problem is that you can't have two select statements with different fields printing to host in the same script. It will throw blank fields. Or in my case because none of the fields align - won't print anything. – Michael Timmerman Dec 22 '16 at 23:58
  • And thank you for all of the feedback. I made several changes to my script based on that and I feel much better about it. I had no idea how much emphases was placed on object types. Now everything runs much smoother and more natural. No weird hacks just to make it work. – Michael Timmerman Dec 23 '16 at 00:00
  • Indeed. It's easy to fall into the trap of thinking Powershell is just another scripting language when you're first getting started. But the way it treats everything as objects and not just text is really powerful once you wrap your head around it. – Ryan Bolger Dec 23 '16 at 18:13