Author Posts

September 16, 2014 at 2:49 am

Hello all

I'm trying to automatize some things I'm doing in my job and I cmae with couple scripts. One of them is to check who is logged on which machine in domain.


$ADSearchBase = "OU=Computers,DC=domain,DC=local"
$ADFilter = "*"

Function Get-LoggedUsersOnComputers
    {
    $ADComputersList = Get-ADComputer -SearchBase $ADSearchBase -Filter $ADFilter 
    foreach ($ADComputer in $ADComputersList)
        {
        if ((Test-NetConnection $ADComputer.Name -WarningAction SilentlyContinue).PingSucceeded)
            {
            Write-host -ForegroundColor Green $ADComputer.Name
            $ExplorerProcesses = Get-WmiObject -ComputerName $ADComputer.Name -Class win32_process | where { $_.name -Match "explorer" }
            foreach ($ExplorerProcess in $ExplorerProcesses)
                {
                Write-Host -ForegroundColor Yellow " " $ExplorerProcess.GetOwner().User
                }
            }
        else
            {
            Write-Host -ForegroundColor Red $ADComputer.Name
            }
        }
    }

Do you think it can be changed to something more cute or faster? Any professional suggestions?

Thank you,

September 16, 2014 at 4:25 am

I'm not a huge fan of Write-Host. If the goal is just to produce some indications of success, I'd rely on Write-Verbose. Your function isn't outputting text or an object – it's drawing directly on the screen. So you can't redirect that to a file, or export it to a CSV, or do anything with it. I'd suggest moving to a custom object for the output – it's much more in fitting PowerShell's native patterns.

I'd use -Filter on Get-WmiObject vs. using Where-Object – better performance to have the filtering happen at the source.

I'd probably rely on Test-Connection -Quiet vs. the command you're using to do the ping. However, a valid ping doesn't mean WmiObject will succeed. It'd make more sense to just use WmiObject, and trap any errors. A computer can have ICMP blocked, but still allow WMI, and vice-versa.

Heh, "cuter." 🙂

September 17, 2014 at 12:18 am

Is this better?

$ADSearchBase = "OU=Computers,DC=domain,DC=local"
$ADFilter = "*"

Function Get-LoggedUsersOnComputers
    {
    $ADComputersList = Get-ADComputer -SearchBase $ADSearchBase -Filter $ADFilter 
    foreach ($ADComputer in $ADComputersList)
        {
        Write-Output $ADComputer.Name
        Try
            {
            $ExplorerProcesses = Get-WmiObject -ComputerName $ADComputer.Name -Class win32_process -Filter "Name='explorer.exe'" -ErrorAction SilentlyContinue
            }
        Finally
            {
            If ($?)
                {
                foreach ($ExplorerProcess in $ExplorerProcesses)
                    {
                    Write-Output "  $($ExplorerProcess.GetOwner().User)"
                    }
                }
            Else
                {
                Write-Output "  < << Could not connect. "
                }
            }
        }
    }


Get-LoggedUsersOnComputers

September 17, 2014 at 3:50 am

Hi. I think that you should also replace the -ErrorAction SilentlyContinue part with -ErrorAction Stop at the Get-WMIObject command in order to catch the error.
Also instead of using $?, you could put a catch block after try in which you can write the error and use the Continue command to go to the next PC.
The finally block should contain only the user info; no if statement needed anymore if you use catch.

September 17, 2014 at 3:56 am

I had problem with Get-WMIObject cmdlet and catching error. It kinda acts weird way.

September 17, 2014 at 4:16 am

In that case you can also remove in my opinion the try and finally keywords because they do not do anything.
Place the code from those blocks directly in the function.