Change default path handling for existing AD objs (#59)

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.
ansible-collections · Aug 7, 2023 · eadd6b7 · eadd6b7
1 parent a89e2b0
commit eadd6b7
Show file tree

Hide file tree

Showing 5 changed files with 235 additions and 8 deletions.
diff --git a/changelogs/fragments/default-path.yml 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
@@ -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
@@ -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
@@ -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
@@ -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: