2

This script is supposed to grab users from several OUs and assign the users to one variable, then it takes the users from that variable and filters through each one based on last logon dates over 30 days of age. Then it exports to a CSV with some info I'd like.

The problem is, when I get to the foreach part, it searches through the entire directory and doesn't use the users in the variable I supplied.

Any criticism is greatly appreciated as well.

$30days = (get-date).adddays(-30)

$Users1 = Get-ADUser -SearchBase 'OU=Users,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com' -LdapFilter '(UserPrincipalName=*)(extensionAttribute9=*)'
$Users2 = Get-ADUser -SearchBase 'OU=Users-Remote,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com' -LdapFilter '(UserPrincipalName=*)(extensionAttribute9=*)'
$Users3 = Get-ADUser -SearchBase 'OU=Contractors,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com' -LdapFilter '(UserPrincipalName=*)(extensionAttribute9=*)'
$Users4 = Get-ADUser -SearchBase 'OU=Temps,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com' -LdapFilter '(UserPrincipalName=*)(extensionAttribute9=*)'
$Users5 = Get-ADUser -SearchBase 'OU=Users,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com' -LdapFilter '(UserPrincipalName=*)(extensionAttribute9=*)'
$Users6 = Get-ADUser -SearchBase 'OU=Users,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com' -LdapFilter '(UserPrincipalName=*)(extensionAttribute9=*)'

$Users = $Users1,$Users2,$Users3,$Users4,$Users5,$Users6 
$useraccountsover30days = foreach ($user in $($Users)){Get-ADUser -filter {lastlogondate -le $30days} -Properties lastlogondate}

$lastlogonreadable = $useraccountsover30days | Select-Object SamAccountName,lastlogondate

    $lastlogonreadable | Export-Csv C:/Users/myname/Desktop/Usersover30days.csv 
Andrew Ryan Davis
  • 123
  • 1
  • 1
  • 5

3 Answers3

5

There are a few recommendations I'd make with your current script.

First off, a single large query is almost always going to perform better than many smaller queries. So rather than running get-aduser separately for each target OU, I would combine them into a single call using a higher level common OU as the search base. Obviously, this may end up returning results from OUs you didn't want to include. But it's much faster to filter those out later.

You're also calling get-aduser again for each result from the first set of queries just to filter on lastLogonDate. But you could instead combine that filter with the -ldapfilter from your original queries. It's just a matter of converting the -filter version with an equivalent -ldapfilter version. The secret to doing this is knowing that lastLogonDate is just a Powershell converted version of the lastLogonTimestamp attribute. And you can convert a normal Powershell DateTime value into the format that lastLogonTimestamp uses with the ToFileTime() method.

The last thing that confused me was the (UserPrincipalName=*) portion of your ldapfilter. In every domain I've ever touched, this attribute will always have a value (just like SamAccountName or DistinguishedName). It may be different than the default value of <SamAccoutnName>@<DomainFQDN>, but it's never empty. The filter isn't hurting anything necessarily. It's just one extra thing for AD to spend CPU cycles evaluating when it doesn't need to. But if you have reason to believe it might be empty in your environment, by all means leave it in.

So here's how I'd modify your script if I understand your intentions correctly.

# make the comparison value using ToFileTime()
$30daysago = (Get-Date).AddDays(-30).ToFileTime()

# make the combined ldapfilter value
$LdapFilter = "(&(lastLogonTimestamp<=$30daysago)(extensionAttribute9=*)"

# make an array of the OU DNs you care about
$TargetOUs = @(
    "OU=Users,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com"
    "OU=Users-Remote,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com"
    "OU=Contractors,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com"
    "OU=Temps,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com"
)

# define your common search base
$base = "OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com"

# get your combined results and the additional attributes you care about
$OldUsers = get-aduser -ldapfilter $LdapFilter -searchbase $base -pr lastLogonDate

# convert the target OU list into a regular expression we can compare each DN against in a single comparison call
$regex = ""
$TargetOUs | %{ $regex += ".*," + [Regex]::Escape($_) + "$|" }
$regex = $regex.Substring(0,$regex.Length-1)

# filter the results that aren't in your target OUs
# (depending on your OU layout, it might be more efficient to flip this
#  and define the OUs you explicitly want to leave out)
$FilteredUsers = $OldUsers | ?{ $_.DistinguishedName -match $regex }

# export your CSV (sorted for good measure)
$FilteredUsers | select SamAccountName,LastLogonDate | sort LastLogonDate | export-csv C:/Users/myname/Desktop/Usersover30days.csv

P.S. Be wary of treating lastLogonTimestamp (or lastLogonDate) as 100% accurate. It may be anywhere from 9 to 14 days out of date by design.

Ryan Bolger
  • 16,755
  • 4
  • 42
  • 64
0

I solved a similar request for multiple OUs with repeating the same set like the following multiple times with the OU specified:

get-aduser -filter 'enabled -eq $true' -Properties Name, LastLogonDate, AccountExpirationDate, description, Department, title, UserPrincipalName -SearchBase "OU=Users,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com" | Where-object {$_.lastlogondate -lt (get-date).AddDays(-30)} | Select-Object Name, LastLogonDate, AccountExpirationDate, description, Department, title, UserPrincipalName | Export-csv C:\temp\users-nologin-30.csv -Encoding UTF8 -Append -NoTypeInformation -Force -Delimiter ";"

It gets all enabled Accounts, with the properties Name, LastLogonDate, AccountExpirationDate, description, Department, title, UserPrincipalName and appends it to a csv-File. As stated in the other Answer 30 days can be not precise, so better call for 44 days just to be sure.

If "LastLogonDate" is empty, the user has never logged on, perhaps it is a new Account.

Crujach
  • 101
  • 2
-1

You never use the $user value in the Get-ADUser call...try changing the $UsersX value to the parameters you need to send to Get-ADUser and substitute them in...and since you're reusing the same LdapFilter, might as well make that a variable as well. (Also, you're reusing the "OU=Users,OU=US-Location" three times, I assume that's just for example purposes?)

$30days = (get-date).adddays(-30)

$LdapFilter = '(UserPrincipalName=*)(extensionAttribute9=*)'

$Users1 = 'OU=Users,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com'

$Users2 = 'OU=Users-Remote,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com' 

$Users3 = 'OU=Contractors,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com'

$Users4 = 'OU=Temps,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com'

$Users5 = 'OU=Users,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com'

$Users6 = 'OU=Users,OU=US-Location,OU=Americas,DC=Domain,DC=Domain,DC=com' 

$Users = $Users1,$Users2,$Users3,$Users4,$Users5,$Users6 
$useraccountsover30days = foreach ($user in $($Users)){Get-ADUser -SearchBase $user -LdapFilter $LdapFilter -filter {lastlogondate -le $30days} -Properties lastlogondate}

$lastlogonreadable = $useraccountsover30days | Select-Object SamAccountName,lastlogondate

    $lastlogonreadable | Export-Csv C:/Users/myname/Desktop/Usersover30days.csv