Skip to content

Commit

Permalink
Fix datetime comparisons and other user checks (#62)
Browse files Browse the repository at this point in the history
Fixes the idempotent comparisons of date_time values. Also fixes a few
problems with the user module around diff output and handling of expired
accounts.
  • Loading branch information
jborean93 authored Aug 8, 2023
1 parent eadd6b7 commit bc1476e
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 5 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/datetime-attributes.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- Fix up date_time attribute comparisons to be idempotent - https://github.com/ansible-collections/microsoft.ad/issues/57
2 changes: 2 additions & 0 deletions changelogs/fragments/user-account-expired-password.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- microsoft.ad.user - treat an expired account as a password that needs to be changed
2 changes: 2 additions & 0 deletions changelogs/fragments/user-spn-diff.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- microsoft.ad.user - Ensure the ``spn`` diff after key is ``spn`` and not ``kerberos_encryption_types``
2 changes: 1 addition & 1 deletion plugins/module_utils/_ADObject.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ Function Compare-AnsibleADAttribute {
[System.Convert]::ToBase64String($_)
}
elseif ($_ -is [System.DateTime]) {
$_.ToUniversalTime().ToString('o')
$_.ToUniversalTime().ToFileTimeUtc()
}
elseif ($_ -is [System.DirectoryServices.ActiveDirectorySecurity]) {
$_.GetSecurityDescriptorSddlForm([System.Security.AccessControl.AccessControlSections]::All)
Expand Down
3 changes: 2 additions & 1 deletion plugins/modules/user.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Function Test-Credential {
$failed_codes = @(
0x0000052E, # ERROR_LOGON_FAILURE
0x00000532, # ERROR_PASSWORD_EXPIRED
0x00000701, # ERROR_ACCOUNT_EXPIRED
0x00000773, # ERROR_PASSWORD_MUST_CHANGE
0x00000533 # ERROR_ACCOUNT_DISABLED
)
Expand Down Expand Up @@ -278,7 +279,7 @@ $setParams = @{
$SetParams.ServicePrincipalNames.Remove = $res.ToRemove
}
}
$module.Diff.after.kerberos_encryption_types = @($res.Value | Sort-Object)
$module.Diff.after.spn = @($res.Value | Sort-Object)
}
}

Expand Down
3 changes: 2 additions & 1 deletion plugins/modules/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@
- C(always) will always update passwords.
- C(on_create) will only set the password for newly created users.
- C(when_changed) will only set the password when changed.
- Using C(when_changed) will not work if the account is not enabled.
- Using C(when_changed) will not work if the account is not enabled or is
expired.
choices:
- always
- on_create
Expand Down
46 changes: 44 additions & 2 deletions tests/integration/targets/user/tasks/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,6 @@
that:
- not move_with_path_sentinel_again is changed

# CN=Users,{{ setup_domain_info.output[0].defaultNamingContext }}

- name: update password from blank - skip for on_create
user:
name: MyUser
Expand Down Expand Up @@ -378,6 +376,29 @@
- always_update_password is changed
- always_update_password_actual.objects[0].pwdLastSet > change_pass_actual.objects[0].pwdLastSet

- name: expire account for subsequent password check
user:
name: MyUser
attributes:
set:
accountExpires:
type: date_time
value: '2000-01-01T00:00:00.0000000Z'

# There's no way to validate a password on an expired account, this will
# result in a change even if the password is the same
- name: update password for expired account
user:
name: MyUser
password: Password123!
update_password: when_changed
register: update_password_on_expired_account

- name: assert update password for expired account
assert:
that:
- update_password_on_expired_account is changed

- name: remove user - check
user:
name: MyUser
Expand Down Expand Up @@ -509,6 +530,9 @@
attributes:
set:
comment: My comment
accountExpires:
type: date_time
value: '3023-07-31T00:00:00.0000000Z'
register: create_user_check
check_mode: true

Expand Down Expand Up @@ -559,6 +583,9 @@
attributes:
set:
comment: My comment
accountExpires:
type: date_time
value: '3023-07-31T00:00:00.0000000Z'
register: create_user

- set_fact:
Expand All @@ -569,6 +596,7 @@
object_info:
identity: '{{ object_identity }}'
properties:
- accountExpires
- c
- comment
- company
Expand Down Expand Up @@ -619,6 +647,7 @@
- create_user_actual.objects[0].Description == 'User Description'
- create_user_actual.objects[0].DisplayName == 'User Name'
- create_user_actual.objects[0].DistinguishedName == 'CN=MyUser,' ~ setup_domain_info.output[0].defaultNamingContext
- create_user_actual.objects[0].accountExpires == 448921440000000000
- create_user_actual.objects[0].c == 'au'
- create_user_actual.objects[0].comment == 'My comment'
- create_user_actual.objects[0].company == 'Red Hat'
Expand Down Expand Up @@ -677,6 +706,9 @@
attributes:
set:
comment: My comment
accountExpires:
type: date_time
value: '3023-07-31T00:00:00.0000000Z'
register: create_user_again

- name: assert create user with extra info - idempotent
Expand Down Expand Up @@ -718,13 +750,17 @@
attributes:
set:
comment: My Comment
accountExpires:
type: date_time
value: '3023-07-31T00:00:00.0000001Z'
register: update_user_check
check_mode: true

- name: get result of update user settings - check
object_info:
identity: '{{ object_identity }}'
properties:
- accountExpires
- c
- comment
- company
Expand Down Expand Up @@ -759,6 +795,7 @@
- update_user_check_actual.objects[0].Description == 'User Description'
- update_user_check_actual.objects[0].DisplayName == 'User Name'
- update_user_check_actual.objects[0].DistinguishedName == 'CN=MyUser,' ~ setup_domain_info.output[0].defaultNamingContext
- update_user_check_actual.objects[0].accountExpires == 448921440000000000
- update_user_check_actual.objects[0].c == 'au'
- update_user_check_actual.objects[0].comment == 'My comment'
- update_user_check_actual.objects[0].company == 'Red Hat'
Expand Down Expand Up @@ -814,12 +851,16 @@
attributes:
set:
comment: My Comment
accountExpires:
type: date_time
value: '3023-07-31T00:00:00.0000001Z'
register: update_user

- name: get result of update user settings
object_info:
identity: '{{ object_identity }}'
properties:
- accountExpires
- c
- comment
- company
Expand Down Expand Up @@ -868,6 +909,7 @@
- update_user_actual.objects[0].Description == 'User description'
- update_user_actual.objects[0].DisplayName == 'User name'
- update_user_actual.objects[0].DistinguishedName == 'CN=MyUser,' ~ setup_domain_info.output[0].defaultNamingContext
- update_user_actual.objects[0].accountExpires == 448921440000000001
- update_user_actual.objects[0].c == 'us'
- update_user_actual.objects[0].comment == 'My Comment'
- update_user_actual.objects[0].company == 'Ansible'
Expand Down

0 comments on commit bc1476e

Please sign in to comment.