Request Help - Inactive ADUser Query and Manipulation Encountering Issues...

I’m trying (initially) to query ADUsers in a specific OU; identify those that are 90-days inactive; document their group memberships; make a note in the Description field that the account is being Disabled as of x-date; Disable identified accounts; and move disabled accounts to a “Parking” OU.

I’ve made notes in the Gist as well, but would appreciate any help getting the Group Membership piece working (pretty please)

https://gist.github.com/rsmith7712/fdfe025d989508102044fdbbf5d3b9a8

Can you be more specific about what the group membership piece should be doing, and isn’t?

Desired Result:
The ADUsers identified inactive have their Group Memberships added in the last column, before each user's Description is updated with "Disabled as of x-date", the account is disabled, and then moved to another OU.

Here is the output in the CSV:

Name User Account Pswd Exp Pswd Nvr Exp When Created Password Last Set Last Logon Date Group
MIT-Mickey Mouse Mmouse TRUE FALSE 4/17/2014 19:27 7/17/2014 18:06 7/10/2014 10:23

Error message:

Get-ADGroup : Cannot validate argument on parameter ‘Identity’. The argument is null. Supply a non-null argument and try the command again.
At C:_E_\scripts\powerShell\helpRequested\help_ZombieAcct_90dayRpt_n_Move.ps1:62 char:37

  • Groups = ($_.memberof | Get-ADGroup <<<<  | Select -ExpandProperty Name) -join ","
    
    • CategoryInfo : InvalidData: (:slight_smile: [Get-ADGroup], ParameterBindingValidationException
    • FullyQualifiedErrorId : ParameterArgumentValidationError,Microsoft.ActiveDirectory.Management.Commands.GetADGroup

Get-ADGroup is complaining that it’s being given a null value for the -Identity parameter. So, whatever you’re piping in is either (A) empty or (B) unacceptable to the command. I will note that memberOf is a collection of objects, not just a single group; Get-ADGroup doesn’t like being given a collection. -Identity is only rigged up to accept a single value.

And that’s an appalling way to create a CSV :).

$objects = @()
$output = @{'Name'='whatever' ; 'Password'='this' ; 'Something' = 'Else'}
$objects += (New-Object -Type PSObject -Prop $output)
...
$objects | Export-CSV

Would be a lot closer. Still far from ideal. But manually forming a CSV by concatenating strings is making me cry.

Don… Any Thoughts?

Here’s the (near) final script,

Issue still having:

  • Collecting ADUser Group Memberships is still for everyone in the specified $SearchBase OU, instead of being able to JUST query those ADUser accounts who meet the $xDays variable.

https://gist.github.com/rsmith7712/fdfe025d989508102044fdbbf5d3b9a8

You’re not applying a filter to the Get-ADUser command on line 56. You do it on line 64, so…? But why query AD twice for the same info? C:\ADUser_GroupMembership_Rpt.csv will contain Names and Groups but so does your log file?

Jeff - Lines 56-61 were put in to capture the Group Memberships because the query on the end of Line 73 fails to capture the info and put it with the rest of the requested data in the CSV. I would love to simplify this to generate a single CSV with all the requested data, but so far I just haven’t figured out how.

Another issue, with the 56-61 query is that it pulls data on all the users in the specified OU instead of those that meet the requirements in the $xDays variable - Yet another reason I would love to find a way to get group memberships collection working through the main query.

Any ideas?

“Another issue, with the 56-61 query is that it pulls data on all the users in the specified OU instead of those that meet the requirements in the $xDays variable – Yet another reason I would love to find a way to get group memberships collection working through the main query.”

Here’s what I’d do to troubleshoot this.

First, I’d check and see what was in $xDays. It looks from your script like it’s a [datetime] object. Is that what the LastLogonDate property expects? Just running $xDays from the console doesn’t count, here, because PowerShell’s going to render it to a string for console display. But what is the shell passing to Active Directory?

I’d spend some time querying users from the command-line, using different date/time values, to verify what’s actually happening. I mean, if it’s pulling all users, then obviously your query criteria isn’t working, so I’d spend some time interactively running the command and refining it.

Don,

So following your suggestion I spent yesterday doing searches in different formats to see what they returned. It eventually led me to an article where I replaced:

old - Ln:58 -Filter *

new - Ln:58 -Filter {LastLogonDate -like $xDays -and Enabled -eq “true”}

Now, here’s what I don’t get… The CSV that Lines 58-63 generate is now empty (file created but no contents), whereas the end ADGroup query on Line 76 now appears to be working because the CSV that it generates now had the Group Memberships for only the ADUsers that meet the time requirement. I’m not complaining - this is exactly the result I have wanted but I would like to know WHY.

I updated the Gist and it can be referenced from an earlier post (instead of posting it again) - pls forgive, still learning

((get-aduser me -Properties memberof).memberof.trimstart(‘CN=’)|%{$_.split(‘,’)[0]}) -join [char]9786

Dan - Where would that line go? At the end of Ln:76 or after the -Filter and before the -Properties on Ln:58?

That was an example of how to turn the memberof attribute into a string without using get-adgroup.

I suspect you are feeding something to get-adgroup that can’t be enumerated under the current domain.

btw, the correct comparison operators for your filter are gt and lt.

((get-aduser me -Properties lastlogondate).lastlogondate  -gt [datetime]::today)

Here you go, play with this. Still room for improvement.


function get-oldpeeps {
	
	param (
		
		[Parameter(Mandatory = $True)]
		[string]$searchbase,
		[Parameter(Mandatory = $False)]
		[Int]$xdays,
		[Parameter(Mandatory = $False)]
		[Switch]$allusers
		
	)
	
	$today = get-date -uformat "%Y/%m/%d"
	if ($xdays) { $age = (get-date).AddDays(- $xdays) } else { $age = (get-date).AddDays(-365) }
	$expire = (get-date).AddDays(-1)
	$ParkingOU = "OU=30Days, OU=Disabled Accounts, OU=Domain Services, DC=Domain, DC=com"
	
	#use a switch statement so you don't have to continuously comment sections of code. 
	
	switch ($searchbase) {
		
		MIT{ $ou = "OU=MIT, OU=Service Accounts, OU=Domain Services, DC=Domain, DC=com" }
		Laptop{ $ou = "OU=Laptop, OU=IS, OU=Corporate Computers, DC=Domain, DC=com" }
		Remote{ $ou = "OU=Remote Accounts, DC=Domain, DC=com" }
		All{ $ou = "DC=Domain, DC=com" }
		
	}
	
	#This is called a here string. Play with it. Easier to code without losing track of qoutes and plus's
	
	$userDesc = @"
Disabled Inactive $today Moved From OU $SearchBase
"@
	
	if ($allusers) {
		Get-ADUser -SearchBase $ou -Filter * -Properties DisplayName, MemberOf | % {
			
			[PSCustomObject]@{
				UserName = $_.DisplayName
				Groups = ($_.MemberOf | Get-ADGroup | Select -ExpandProperty Name) -join ","
			}
			
		}
		
		#dont hardcode export file into scripts. return info from function then | Export-Csv C:\ADUser_GroupMembership_Rpt.csv -NTI
		
	} else {
		
		$Users = Get-ADUser -SearchBase $ou -Properties memberof, PasswordNeverExpires, WhenCreated, PasswordLastSet, LastLogonDate -Filter {
			(LastLogonDate -gt $age)
			-AND (PasswordLastSet -gt $age)
			-AND (Enabled -eq $True)
			-AND (PasswordNeverExpires -eq $false)
			-AND (WhenCreated -le $age)
		}
		
		#revise filter whencreated less than 90 could not result in users lastlogondate being greater than 90?
		
		$users | ForEach-Object {
			
			Set-ADUser $_ -AccountExpirationDate $expire -Description $userdesc -WhatIf
			Move-ADObject $_ -TargetPath $ParkingOU -WhatIf
			$_ | select *, @{ l = 'Groups'; e = { (($_.memberof | Get-ADGroup).Name) -join '; ' } }
		}
		
	}
	
	
}


Looks like I started with the switch idea + the searchbase param and didn’t implement it correctly down the line. Searchbase not mandatory. No need to define ou with AllUsers switch.

function get-oldpeeps {
	
	param (
		
		[Parameter(Mandatory = $False)]
		[string]$searchbase,
		[Parameter(Mandatory = $False)]
		[Int]$xdays,
		[Parameter(Mandatory = $False)]
		[Switch]$allusers
		
	)
	

	$today = get-date -uformat "%Y/%m/%d"
	if ($xdays) { $age = (get-date).AddDays(- $xdays) } else { $age = (get-date).AddDays(-365) }
	$expire = (get-date).AddDays(-1)
	$ParkingOU = "OU=30Days, OU=Disabled Accounts, OU=Domain Services, DC=Domain, DC=com"
	
	#use a switch statement so you don't have to continuously comment sections of code. 
	
	switch ($searchbase) {
		
		MIT{ $ou = "OU=MIT, OU=Service Accounts, OU=Domain Services, DC=Domain, DC=com" }
		Laptop{ $ou = "OU=Laptop, OU=IS, OU=Corporate Computers, DC=Domain, DC=com" }
		Remote{ $ou = "OU=Remote Accounts, DC=Domain, DC=com" }
	        ''{$ou = 'dc=domain,dc=com'}
		
	}
	
	#This is called a here string. Play with it. Easier to code without losing track of qoutes and plus's
	
	$userDesc = @"
Disabled Inactive $today Moved From OU $ou
"@
	
	if ($allusers) {
		Get-ADUser -Filter * -Properties DisplayName, MemberOf | % {
			
			[PSCustomObject]@{
				UserName = $_.DisplayName
				Groups = ($_.MemberOf | Get-ADGroup | Select -ExpandProperty Name) -join ","
			}
			
		}
		
		#dont hardcode export file into scripts. return info from function then | Export-Csv C:\ADUser_GroupMembership_Rpt.csv -NTI
		
	} else {
		
		$Users = Get-ADUser -SearchBase $ou -Properties memberof, PasswordNeverExpires, WhenCreated, PasswordLastSet, LastLogonDate -Filter {
			(LastLogonDate -gt $age)
			-AND (PasswordLastSet -gt $age)
			-AND (Enabled -eq $True)
			-AND (PasswordNeverExpires -eq $false)
			-AND (WhenCreated -le $age)
		}
		
		#revise filter whencreated less than 90 could not result in users lastlogondate being greater than 90?
		
		$users | ForEach-Object {
			
			Set-ADUser $_ -AccountExpirationDate $expire -Description $userdesc -WhatIf
			Move-ADObject $_ -TargetPath $ParkingOU -WhatIf
			$_ | select *, @{ l = 'Groups'; e = { (($_.memberof | Get-ADGroup).Name) -join '; ' } }
		}
		
	}
	
	
}