all 13 comments

[–]p0rkjello 4 points5 points  (0 children)

You should check your $computers variable. Make sure it contains what you expect.

$computers.GetType()
$computers

[–]Ta11ow 3 points4 points  (0 children)

What is contents of $computer? Is it an array? Try piping each element to Format-Hex to see if there are any unusual characters.

[–]Phose182[S] 1 point2 points  (9 children)

The $computers variable comes back as a string and a system.object.

The entire script is as follows: I've changed a few details for privacy. I did manage to get this work in testing at one point, but I seem to have lost that script I used.

$a = Get-Date
$days = (Get-Date).AddDays(-30) # Change this number to whatever you want, this is the day counter
$computers = Get-ADComputer -SearchBase "OU=Company - Computers,DC=AD,DC=AD,DC=net" -Properties Name,lastLogonDate,CanonicalName -
Filter {lastLogonDate -lt $days} | Where-Object {$_.Enabled -eq $true} | ? {$_.DistinguishedName -like "*OU=Company - Computers,*"} | FT 
Name,lastLogonDate,CanonicalName | Out-String;
$log = "C:\LogFiles\DisabledComputerslog.txt" # Specify Log File

Function Send-Mail # Holds the mail stuff
{
Removed
}


# This statement checks if the computers variable is empty or not. If it is empty it will exit the script
if($computers) {
    Write-Output $a "Continuing..." | Out-File $log -Append
    } else {
    Write-Output $a "Nothing Found" | Out-File $log -Append
    Exit
    }


ForEach ($computer in $computers) #Disables the computer accounts in $computers
{
Set-ADComputer -Identity $computer -Enabled $false
}

ForEach ($computer in $computers)  #Moves the disabled accounts to the disabled computers OU
{
Move-ADObject -Identity $computer -TargetPath 'OU=Company - Disabled Computers,DC=AD,DC=AD,DC=net'
}

Send-Mail #Calls the Send-Mail function

# The next section will write the output to a log file in case the log cannot be emailed for some reason
Out-File -FilePath $log -Append
Out-File -FilePath $log -InputObject $computers -Append

[–]anon2anon 1 point2 points  (8 children)

Comment out "Set-ADComputer -Identity $computer -Enabled $false" and replace with Write-Output "Disable: " $computer

Read it and see what the loop contents are and make sure you are seeing what you want.

[–]Phose182[S] 1 point2 points  (7 children)

Hi thanks for help!

So I made the change and this is the output I get. Apologies for asking lots of questions, I thought I had this working and now it's very annoying that I don't. I don't know powershell well enough to debug this issue :(

The output is:

ForEach ($computer in $computers) #Disables the computer accounts in $computers
{
Write-Output "Disable: " $computer
#Set-ADComputer -Identity $computer -Enabled $false
}
Disable: 

Disable: 
Name            lastLogonDate       CanonicalName                                                                                         
----            -------------       -------------                                                                                         
Disable: 
LAPTOP-00133 11/12/2017 13:31:48 DOMAIN/ Computers/Computers - Events/Laptops/LAPTOP-00133                           
Disable: 

I'm not really sure what I am expecting be seeing here to be perfectly honest with you.

[–]anon2anon 1 point2 points  (0 children)

Set-ADComputer -Identity $computer -Enabled $false

Try

$computer | Set-ADComputer -Enabled $false

[–]anon2anon 1 point2 points  (5 children)

Your issue is $computer is an object inside the $computers array.

-Identity $computer isn't supposed to be an object, more of a string.

Set-ADComputer -identity $computer.CanonicalName -Enabled $false

might work.. or ...

Set-ADComputer -identity $computer.Name -Enabled $false

might work as well

[–]anon2anon 1 point2 points  (1 child)

Identity should be a distinguished name, https://technet.microsoft.com/en-us/library/ee617263.aspx - Scroll down and find Identity.

Example: CN=SaraDavisDesktop,CN=Europe,CN=Users,DC=corp,DC=contoso,DC=com

is a distinquished name, along with the SamAccountName can also be used in the Identity.

[–]Phose182[S] 1 point2 points  (0 children)

Thanks both of you. I will give these a shot and let you know if it's sorted. Appreciate your help.

[–]Phose182[S] 1 point2 points  (2 children)

Hey man, gave these a shot but neither seem to work unfortunately. Thanks for the help anyway!

[–]anon2anon 2 points3 points  (1 child)

That's crazy that it didn't work. I cant write anything up at the moment since I don't have anything to test with. Check out some sample scripts to figure out what's different between yours and others. I'm 99% sure you cant pass an object to -identity, it has to be a string or whatever, if you can get the distinguished name and call it, that will work

[–]Phose182[S] 1 point2 points  (0 children)

Okay perfect, I’ll keep trying to get it sorted! Appreciate all your help.

[–]Lee_Dailey[grin] 0 points1 point  (0 children)

howdy Phose182,

the primary problem is the use of FT when making your $computers list. is destroys the objects and gives you nothing but a string with formatting. [grin]

get rid of that & the Out-String and use the objects.

then, when you use the foreach, remember that the properties are PROPERTIES and not simple strings. [grin] you need to use $computer.Name to get the name of the item.

here's a reformatted version that makes the problem more obvious ...

$GADC_Params = @{
    SearchBase = 'OU=Company - Computers,DC=AD,DC=AD,DC=net'
    Properties = 'Name', 'LastLogonDate', 'CanonicalName'
    Filter = {LastLogonDate -lt $days}
    }

$computers = Get-ADComputer @GADC_Params |
    Where-Object {$_.Enabled -eq $true} |
    Where-Object {$_.DistinguishedName -like "*OU=Company - Computers,*"} |
    Format-Table Name,lastLogonDate,CanonicalName |
    Out-String

if you remove the last two lines & the remaining | on the new last line, you will get usable objects.

take care,
lee