Reputation:
trying to compare dates if its todate or greater then do not enable, if its less then or equal to today, enable account based on the description
$newemployees = Get-aduser -Filter * -Properties whencreated, Description, Samaccountname | Where-Object {$_.whenCreated -gt $Datecutoff} `
| Select-Object Description, enabled, Samaccountname | Where-Object {$_.Enabled -eq $False}
$DateCutOff=(Get-Date).AddDays(-14)
$lastpasstest = Get-ADUser -Identity lastpasstest -Properties Description, Samaccountname | Select-Object Description, enabled, Samaccountname
$today = Get-Date -format MM/dd/yyyy
Write-Output "todays Date: $today"
$newpwd = ConvertTo-SecureString -String "apasswordwemightuse!" -AsPlainText –Force
ForEach ( $user in $newemployees ) {
if ($user.description -ge $today)
{
Write-Output "Start Date: $($user.description)"
Write-Output "Users will be enabled on their start date: $($user.SamAccountName)"
}
if ($user.description -le $today) {
Write-Output "Enabling User: $($user.SamAccountName) $($user.description)"
$user.SamAccountName | Enable-ADAccount
Set-ADAccountPassword $user -NewPassword $newpwd -Reset
Set-ADuser $user -ChangePasswordAtLogon $True
}
}
Upvotes: 0
Views: 919
Reputation: 36277
There's a few things here, so let's go over them one at a time, but first for the TLDR folks: The answer to your original question is that you're comparing strings. The $User.description
value is a string, and you converted that date to a string for $today
when you formatted it. In order to compare dates you would need the value type on the left of the operator to be a [datetime]
object.
Now let's get down to the business of really fixing this here.
The first issue I see is that you define $DateCutOff
in line 3 after you try to use it in a comparison in line 1. So we move line 3 to the top, simple enough.
Next you get all of the users in AD, then whittle them down with Where-Object
statements. That's asking for a lot, it is far better to use the -Filter
parameter to only ask AD for the users you want. Let's convert the lines that you query AD to do that with this:
$DateCutOff = (get-date).AddDays(-14)
$newemployees = Get-aduser -Filter {Enabled -eq $false -and whenCreated -gt $DateCutOff} -Properties whencreated, Description, Samaccountname |
Select-Object Description, enabled, Samaccountname
That should only get the users that you want from AD, which should result in fewer timeouts and faster responses in general.
Then we have a line that appears to serve no purpose so I'm skipping it. Moving down to when you define $today
. You use the -f
parameter, which is used to format the date as a string. In order to keep it a date you would need to leave that part off. If you want it formatted you can always do that later.
$today = get-date
Personally, and this is my preference, I skip defining things like that and just use [datetime]::today
in their place. That's just me, but I thought I'd put it in here in case you wanted to do so as well.
Now, because we changed that to a date, your write-output
line won't look the same. For that matter, using Write-Output
is redundant. Anything not otherwise explicitly redirected is sent to the output. So we can just change that to this:
"todays Date: {0}" -f $today.ToString("MM/dd/yyyy")
After that you define the password, which is in plain text, and in general is a no-no, but it's functional so I'll leave it alone.
$newpwd = ConvertTo-SecureString -String "apasswordwemightuse!" -AsPlainText –Force
Now we get to the loop where you check if users should be enabled. I mentioned above that you are comparing strings, but it is worth noting that when it comes to comparisons PowerShell looks at the value on the left's type and tries to convert the value on the right to match. So really the simplest way to fix your original script is to type cast the description as a [datetime]
object like If([datetime]($user.description) -gt $today){
, but that seems dirty to me since it would then have to convert $today
to a datetime each time as well as the description. Better to have at least one be a datetime already and work from there. So what I would do is put today's datetime object first, and compare the description to that, like this:
ForEach ( $user in $newemployees ) {
if ([datetime]::today -lt $user.description)
Beyond that you run users through If($user.description -ge $today)
and If($user.description -le $today)
, but really it's going to be one or the other, so you could just do the second part as an else
clause. That said, here's what I'd have done:
ForEach ( $user in $newemployees ) {
if ([datetime]::today -lt $user.description)
{
"Start Date: $($user.description)"
"Users will be enabled on their start date: $($user.SamAccountName)"
}
else
{
"Enabling User: $($user.SamAccountName) $($user.description)"
$user.SamAccountName | Enable-ADAccount
Set-ADAccountPassword $user -NewPassword $newpwd -Reset
Set-ADuser $user -ChangePasswordAtLogon $True
}
}
Lastly, you write a fair bit to screen. You might want to convert that to write-verbose
so that it quietly runs, but if you need to keep a closer eye on things you can run it with the -verbose
switch, or change your $VerbosePreference
for the session.
Upvotes: 2