From d0e75c7d52bdd9a0309188dd6b10766685a3d61a Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Mon, 7 Aug 2023 16:00:32 +1000 Subject: [PATCH] Change default path handling for existing AD objs Changes the behaviour of path for existing objects when no path was specified and identity points to a valid object. The old behaviour was to treat no path value as the default container for that object type which would have resulted in moving the object. The new behaviour will treat no path or empty path value as keep the existing path. The object will only be moved if an explicit path value was set. Adds support for the sentinel value microsoft.ad.default_path to be specified as the path which will be the default container for that object type. --- changelogs/fragments/default-path.yml | 4 + plugins/doc_fragments/ad_object.py | 10 +- plugins/module_utils/_ADObject.psm1 | 14 ++- .../targets/object/tasks/tests.yml | 107 +++++++++++++++++ .../integration/targets/user/tasks/tests.yml | 108 +++++++++++++++++- 5 files changed, 235 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/default-path.yml diff --git a/changelogs/fragments/default-path.yml b/changelogs/fragments/default-path.yml new file mode 100644 index 0000000..be47067 --- /dev/null +++ b/changelogs/fragments/default-path.yml @@ -0,0 +1,4 @@ +minor_changes: +- >- + AD objects will no longer be moved to the default AD path for their type if no ``path`` was specified. Use the value + ``microsoft.ad.default_path`` to explicitly set the path to the default path if that behaviour is desired. diff --git a/plugins/doc_fragments/ad_object.py b/plugins/doc_fragments/ad_object.py index 31ed8ea..e70cf63 100644 --- a/plugins/doc_fragments/ad_object.py +++ b/plugins/doc_fragments/ad_object.py @@ -119,11 +119,17 @@ class ModuleDocFragment: path: description: - The path of the OU or the container where the new object should exist in. - - If no path is specified, the default is the C(defaultNamingContext) of - domain for most objects. + - If creating a new object, the new object will be created at the path + specified. If no path is specified then the C(defaultNamingContext) of + the domain will be used as the path for most object types. + - If managing an existing object found by I(identity), the path of the + found object will be moved to the one specified by this option. If no + path is specified, the object will not be moved. - The modules M(microsoft.ad.computer), M(microsoft.ad.user), and M(microsoft.ad.group) have their own default path that is configured on the Active Directory domain controller. + - This can be set to C(microsoft.ad.default_path) which will equal the + default value used when creating a new object. type: str protect_from_deletion: description: diff --git a/plugins/module_utils/_ADObject.psm1 b/plugins/module_utils/_ADObject.psm1 index e9dd94b..49d87ef 100644 --- a/plugins/module_utils/_ADObject.psm1 +++ b/plugins/module_utils/_ADObject.psm1 @@ -685,6 +685,8 @@ Function Invoke-AnsibleADObject { $PostAction ) + $defaultPathSentinel = 'microsoft.ad.default_path' + $spec = @{ options = @{ attributes = @{ @@ -828,7 +830,7 @@ Function Invoke-AnsibleADObject { } else { $ouPath = $defaultObjectPath - if ($module.Params.path) { + if ($module.Params.path -and $module.Params.path -ne $defaultPathSentinel) { $ouPath = $module.Params.path } "$namePrefix=$($Module.Params.name -replace ',', '\,'),$ouPath" @@ -911,7 +913,7 @@ Function Invoke-AnsibleADObject { } $objectPath = $null - if ($module.Params.path) { + if ($module.Params.path -and $module.Params.path -ne $defaultPathSentinel) { $objectPath = $path $newParams.Path = $module.Params.path } @@ -1081,8 +1083,12 @@ Function Invoke-AnsibleADObject { $module.Result.changed = $true } - if ($module.Params.path -and $module.Params.path -ne $objectPath) { - $objectPath = $module.Params.path + $desiredPath = $module.Params.path + if ($desiredPath -eq $defaultPathSentinel) { + $desiredPath = $defaultObjectPath + } + if ($desiredPath -and $desiredPath -ne $objectPath) { + $objectPath = $desiredPath $module.Diff.after.path = $objectPath $addProtection = $false diff --git a/tests/integration/targets/object/tasks/tests.yml b/tests/integration/targets/object/tasks/tests.yml index b642ce6..b181609 100644 --- a/tests/integration/targets/object/tasks/tests.yml +++ b/tests/integration/targets/object/tasks/tests.yml @@ -441,6 +441,59 @@ - move_ou_actual.objects[0].DistinguishedName == 'OU=TestOU 2,' ~ sub_ous.results[0].distinguished_name - move_ou_actual.objects[0].ProtectedFromAccidentalDeletion == true +- name: do not move object in non default path without path - check + object: + name: TestOU 2 + identity: '{{ sub_ous.results[1].object_guid }}' + type: organizationalUnit + attributes: + set: + description: Test comment + register: dont_move_no_path_check + check_mode: true + +- name: get result of do not move object in non default path without path - check + object_info: + identity: '{{ sub_ous.results[1].object_guid }}' + properties: + - description + register: dont_move_no_path_check_actual + +- name: assert do not move object in non default path without path - check + assert: + that: + - dont_move_no_path_check is changed + - dont_move_no_path_check.distinguished_name == 'OU=TestOU 2,' ~ sub_ous.results[0].distinguished_name + - dont_move_no_path_check_actual.objects[0].Name == 'TestOU 2' + - dont_move_no_path_check_actual.objects[0].DistinguishedName == 'OU=TestOU 2,' ~ sub_ous.results[0].distinguished_name + - dont_move_no_path_check_actual.objects[0].Description == None + +- name: do not move object in non default path without path + object: + name: TestOU 2 + identity: '{{ sub_ous.results[1].object_guid }}' + type: organizationalUnit + attributes: + set: + description: Test comment + register: dont_move_no_path + +- name: get result of do not move object in non default path without path + object_info: + identity: '{{ sub_ous.results[1].object_guid }}' + properties: + - description + register: dont_move_no_path_actual + +- name: assert do not move object in non default path without path - check + assert: + that: + - dont_move_no_path is changed + - dont_move_no_path.distinguished_name == 'OU=TestOU 2,' ~ sub_ous.results[0].distinguished_name + - dont_move_no_path_actual.objects[0].Name == 'TestOU 2' + - dont_move_no_path_actual.objects[0].DistinguishedName == 'OU=TestOU 2,' ~ sub_ous.results[0].distinguished_name + - dont_move_no_path_actual.objects[0].Description == 'Test comment' + - name: remove object that is protected from deletion - check object: name: My, Container @@ -1444,3 +1497,57 @@ assert: that: - not unset_normal_again is changed + +- name: move object back into the default path - check + object: + name: My, Container + identity: '{{ object_identity }}' + type: container + path: microsoft.ad.default_path + register: move_into_default_check + check_mode: true + +- name: get result of move object back into the default path - check + object_info: + identity: '{{ object_identity }}' + register: move_into_default_check_actual + +- name: assert move object back into the default path - check + assert: + that: + - move_into_default_check is changed + - move_into_default_check.distinguished_name == 'CN=My\, Container,' ~ setup_domain_info.output[0].defaultNamingContext + - move_into_default_check_actual.objects[0].DistinguishedName == 'CN=My\, Container,CN=Users,' ~ setup_domain_info.output[0].defaultNamingContext + +- name: move object back into the default path + object: + name: My, Container + identity: '{{ object_identity }}' + type: container + path: microsoft.ad.default_path + register: move_into_default + +- name: get result of move object back into the default path + object_info: + identity: '{{ object_identity }}' + register: move_into_default_actual + +- name: assert move object back into the default path + assert: + that: + - move_into_default is changed + - move_into_default.distinguished_name == 'CN=My\, Container,' ~ setup_domain_info.output[0].defaultNamingContext + - move_into_default_actual.objects[0].DistinguishedName == 'CN=My\, Container,' ~ setup_domain_info.output[0].defaultNamingContext + +- name: move object back into the default path - idempotent + object: + name: My, Container + identity: '{{ object_identity }}' + type: container + path: microsoft.ad.default_path + register: move_into_default_again + +- name: assert move object back into the default path - idempotent + assert: + that: + - not move_into_default_again is changed diff --git a/tests/integration/targets/user/tasks/tests.yml b/tests/integration/targets/user/tasks/tests.yml index 928dd3b..d64d0e7 100644 --- a/tests/integration/targets/user/tasks/tests.yml +++ b/tests/integration/targets/user/tasks/tests.yml @@ -177,11 +177,115 @@ that: - not move_user_again is changed +- name: update user not in default path by identity - check + user: + name: MyUser2 + identity: '{{ object_sid }}' + firstname: first name + register: dont_move_no_path_check + check_mode: true + +- name: get result of update user not in default path by identity - check + object_info: + identity: '{{ object_identity }}' + properties: + - givenName + register: dont_move_no_path_check_actual + check_mode: true + +- name: assert update user not in default path by identity - check + assert: + that: + - dont_move_no_path_check is changed + - dont_move_no_path_check.distinguished_name == 'CN=MyUser2,' ~ setup_domain_info.output[0].defaultNamingContext + - dont_move_no_path_check_actual.objects[0].DistinguishedName == 'CN=MyUser2,' ~ setup_domain_info.output[0].defaultNamingContext + - dont_move_no_path_check_actual.objects[0].Name == 'MyUser2' + - dont_move_no_path_check_actual.objects[0].givenName == None + +- name: update user not in default path by identity + user: + name: MyUser2 + identity: '{{ object_sid }}' + firstname: first name + register: dont_move_no_path + +- name: get result of update user not in default path by identity + object_info: + identity: '{{ object_identity }}' + properties: + - givenName + register: dont_move_no_path_actual + check_mode: true + +- name: assert update user not in default path by identity - check + assert: + that: + - dont_move_no_path is changed + - dont_move_no_path.distinguished_name == 'CN=MyUser2,' ~ setup_domain_info.output[0].defaultNamingContext + - dont_move_no_path_actual.objects[0].DistinguishedName == 'CN=MyUser2,' ~ setup_domain_info.output[0].defaultNamingContext + - dont_move_no_path_actual.objects[0].Name == 'MyUser2' + - dont_move_no_path_actual.objects[0].givenName == 'first name' + +- name: move user back - check + user: + name: MyUser + identity: MyUser + path: microsoft.ad.default_path + register: move_with_path_sentinel_check + check_mode: true + +- name: get result of move user back - check + object_info: + identity: '{{ object_identity }}' + properties: + - sAMAccountName + register: move_with_path_sentinel_check_actual + +- name: assert move user back - check + assert: + that: + - move_with_path_sentinel_check is changed + - move_with_path_sentinel_check.distinguished_name == 'CN=MyUser,CN=Users,' ~ setup_domain_info.output[0].defaultNamingContext + - move_with_path_sentinel_check_actual.objects[0].DistinguishedName == 'CN=MyUser2,' ~ setup_domain_info.output[0].defaultNamingContext + - move_with_path_sentinel_check_actual.objects[0].Name == 'MyUser2' + - move_with_path_sentinel_check_actual.objects[0].sAMAccountName == 'MyUser' + - name: move user back user: name: MyUser - identity: MyUser # By sAMAccountName - path: CN=Users,{{ setup_domain_info.output[0].defaultNamingContext }} + identity: MyUser + path: microsoft.ad.default_path + register: move_with_path_sentinel + +- name: get result of move user back + object_info: + identity: '{{ object_identity }}' + properties: + - sAMAccountName + register: move_with_path_sentinel_actual + +- name: assert move user back + assert: + that: + - move_with_path_sentinel is changed + - move_with_path_sentinel.distinguished_name == 'CN=MyUser,CN=Users,' ~ setup_domain_info.output[0].defaultNamingContext + - move_with_path_sentinel_actual.objects[0].DistinguishedName == 'CN=MyUser,CN=Users,' ~ setup_domain_info.output[0].defaultNamingContext + - move_with_path_sentinel_actual.objects[0].Name == 'MyUser' + - move_with_path_sentinel_actual.objects[0].sAMAccountName == 'MyUser' + +- name: move user back - idempotent + user: + name: MyUser + identity: MyUser + path: microsoft.ad.default_path + register: move_with_path_sentinel_again + +- name: assert move user back - idempotent + assert: + 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: