computer - Improve sAMAccountName handling

Add extra logic to handle searching for a computer account by identity without the $ suffix. Also adds the ability to create a computer account without the $ suffix on the sAMAccountName attribute. This should be a niche scenario but it is useful for testing and is added for completion sakes.
ansible-collections · Jul 17, 2024 · 5236659 · 5236659
1 parent d8b0a6a
commit 5236659
Show file tree

Hide file tree

Showing 7 changed files with 245 additions and 14 deletions.
diff --git a/changelogs/fragments/124-computer-identity-dollar.yml b/changelogs/fragments/124-computer-identity-dollar.yml
@@ -0,0 +1,5 @@
+bugfixes:
+  - >-
+    microsoft.ad.computer - Added fallback ``identity`` lookup for ``sAMAccountName`` with the ``$`` suffix. This
+    ensures that finding the computer object will work with or without the ``$`` suffix.
+    - https://github.com/ansible-collections/microsoft.ad/issues/124
diff --git a/changelogs/fragments/computer-sam.yml b/changelogs/fragments/computer-sam.yml
@@ -0,0 +1,4 @@
+minor_changes:
+  - >-
+    microsoft.ad.computer - Added the ``do_not_append_dollar_to_sam`` option which can create a computer account
+    without the ``$`` suffix when an explicit ``sam_account_name` was provided without one.
diff --git a/plugins/doc_fragments/ad_object.py b/plugins/doc_fragments/ad_object.py
@@ -155,6 +155,10 @@ class ModuleDocFragment:
     - If omitted, the AD object to manage is selected by the
       C(distinguishedName) using the format C(CN={{ name }},{{ path }}). If
       I(path) is not defined, the C(defaultNamingContext) is used instead.
+    - When using the M(microsoft.ad.computer) module, the identity will
+      automatically append C($) to the end of the C(sAMAccountName) if the
+      provided value did not result in a match and did not already have a C($)
+      at the end.
     type: str
   name:
     description:

diff --git a/plugins/module_utils/_ADObject.psm1 b/plugins/module_utils/_ADObject.psm1
@@ -708,6 +708,7 @@ Function Get-AnsibleADObject {
     # The -Identity parameter is used where possible as LDAPFilter is limited
     # to just the defaultNamingContext as defined by -SearchBase.
     $objectGuid = [Guid]::Empty
+    $tryDollarFallback = $false
     if ([System.Guid]::TryParse($Identity, [ref]$objectGuid)) {
         $getParams.Identity = $objectGuid
     }
@@ -727,6 +728,7 @@ Function Get-AnsibleADObject {
         }
         catch [System.ArgumentException] {
             if ($Identity -match '^(?:[^:*?""<>|\/\\]+\\)?(?<username>[^;:""<>|?,=\*\+\\\(\)]+)$') {
+                $tryDollarFallback = -not $Matches.username.EndsWith('$')
                 $getParams.LDAPFilter = "(sAMAccountName=$($Matches.username))"
             }
             else {
@@ -746,7 +748,19 @@ Function Get-AnsibleADObject {
         $obj = & $GetCommand @getParams | Select-Object -First 1
     }
     catch [Microsoft.ActiveDirectory.Management.ADIdentityNotFoundException] {
-        $obj = $null
+        # AD cmdlets only raise this error when -Identity was used.
+        # This means the account wasn't found and we don't need the fallback
+        # logic below.
+        return
+    }
+
+    # If we are dealing with a known object type where the sAMAccountName
+    # typically ends with $ we want to try again with the $ append to the
+    # filter.
+    # https://github.com/ansible-collections/microsoft.ad/issues/124
+    if (-not $obj -and $tryDollarFallback -and $GetCommand.Name -in @('Get-ADComputer')) {
+        $getParams.LDAPFilter = $getParams.LDAPFilter.Substring(0, $getParams.LDAPFilter.Length - 1) + '$)'
+        $obj = & $GetCommand @getParams | Select-Object -First 1
     }
 
     $obj

diff --git a/plugins/modules/computer.ps1 b/plugins/modules/computer.ps1
@@ -22,6 +22,13 @@ $setParams = @{
             Option = @{ type = 'str' }
             Attribute = 'DNSHostName'
         }
+        [PSCustomObject]@{
+            Name = 'do_not_append_dollar_to_sam'
+            Option = @{
+                default = $false
+                type = 'bool'
+            }
+        }
         [PSCustomObject]@{
             Name = 'enabled'
             Option = @{ type = 'bool' }
@@ -94,6 +101,25 @@ $setParams = @{
             Name = 'sam_account_name'
             Option = @{ type = 'str' }
             Attribute = 'sAMAccountName'
+            # New handling is done in PostAction as New-ADComputer cannot
+            # set a SAM without the $ suffix.
+            Set = {
+                param($Module, $ADParams, $SetParams, $ADObject)
+
+                # Using the -SAMAccountName parameter will automatically append
+                # '$' to the value. We want to set the provided user value
+                # which may not have the suffix so we use the raw attribute
+                # replacement method.
+                $sam = $Module.Params.sam_account_name
+                if ($sam -ne $ADObject.SAMAccountName) {
+                    if (-not $SetParams.ContainsKey('Replace')) {
+                        $SetParams['Replace'] = @{}
+                    }
+                    $SetParams.Replace['sAMAccountName'] = $sam
+                }
+
+                $module.Diff.after.sam_account_name = $sam
+            }
         }
         [PSCustomObject]@{
             Name = 'spn'
@@ -131,7 +157,11 @@ $setParams = @{
     PreAction = {
         param ($Module, $ADParams, $ADObject)
 
-        if ($Module.Params.sam_account_name -and -not $Module.Params.sam_account_name.EndsWith('$')) {
+        if (
+            $Module.Params.sam_account_name -and
+            -not $Module.Params.sam_account_name.EndsWith('$') -and
+            -not $Module.Params.do_not_append_dollar_to_sam
+        ) {
             $Module.Params.sam_account_name = "$($Module.Params.sam_account_name)$"
         }
     }
@@ -140,6 +170,20 @@ $setParams = @{
 
         if ($ADObject) {
             $Module.Result.sid = $ADObject.SID.Value
+
+            # This should only happen when the computer account was created.
+            # The code will set sam_account_name to the desired value without
+            # the '$' suffix.
+            if (
+                $Module.Params.state -eq 'present' -and
+                $Module.Params.sam_account_name -and
+                $Module.Params.do_not_append_dollar_to_sam -and
+                $Module.Params.sam_account_name -ne $ADObject.SAMAccountName
+            ) {
+                $ADObject | Set-ADComputer -Replace @{
+                    sAMAccountName = $module.Params.sam_account_name
+                } -WhatIf:$Module.CheckMode @ADParams
+            }
         }
         elseif ($Module.Params.state -eq 'present') {
             # Use dummy value for check mode when creating a new user

diff --git a/plugins/modules/computer.py b/plugins/modules/computer.py
@@ -70,10 +70,18 @@
     - Specifies the fully qualified domain name (FQDN) of the computer.
     - This is the value set on the C(dNSHostName) LDAP attribute.
     type: str
+  do_not_append_dollar_to_sam:
+    description:
+    - Do not automatically append C($) to the I(sam_account_name) value.
+    - This only applies when I(sam_account_name) is explicitly set and can be
+      used to create a computer account without the C($) suffix.
+    default: false
+    type: bool
+    version_added: 1.7.0
   enabled:
     description:
-    - C(yes) will enable the group.
-    - C(no) will disable the group.
+    - C(yes) will enable the computer.
+    - C(no) will disable the computer.
     type: bool
   kerberos_encryption_types:
     description:
@@ -150,8 +158,10 @@
       operating systems compatibility.
     - If ommitted the value is the same as C(name$) when the computer is
       created.
-    - Note that all computer C(sAMAccountName) values need to end with a C($).
-    - If C($) is omitted, it will be added to the end.
+    - Note that all computer C(sAMAccountName) values typically end with a C($).
+    - By default if C($) is omitted, it will be added to the end. If
+      I(do_not_append_dollar_to_sam=True) then the provided value will be used
+      as is without adding C($) to the end.
     type: str
   spn:
     description:
@@ -201,6 +211,11 @@
   module.
 - This module must be run on a Windows target host with the C(ActiveDirectory)
   module installed.
+- When matching by I(identity) with a C(sAMAccountName) value, the value should
+  end with C($). If the provided value does not end with C($) the module will
+  still attempt to find the computer account with the provided value before
+  attempting a fallback lookup with C($) appended to the end. This fallback
+  behaviour was added with version C(1.7.0) of this collection.
 extends_documentation_fragment:
 - microsoft.ad.ad_object
 - ansible.builtin.action_common_attributes
@@ -240,12 +255,12 @@
 
 - name: Remove linux computer from Active Directory using a windows machine
   microsoft.ad.computer:
-    identity: one_linux_server
+    identity: one_linux_server$
     state: absent
 
 - name: Add SPNs to computer
   microsoft.ad.computer:
-    identity: TheComputer
+    identity: TheComputer$
     spn:
       add:
         - HOST/TheComputer
@@ -254,7 +269,7 @@
 
 - name: Remove SPNs on the computer
   microsoft.ad.computer:
-    identity: TheComputer
+    identity: TheComputer$
     spn:
       remove:
         - HOST/TheComputer
@@ -263,11 +278,17 @@
 
 - name: Set the principals the computer trusts for delegation from
   microsoft.ad.computer:
-    identity: TheComputer
+    identity: TheComputer$
     delegates:
       set:
         - CN=FileShare,OU=Computers,DC=domain,DC=test
         - OtherServer$  # Lookup by sAMAaccountName
+
+- name: Add computer with sAMAccountName without $ suffix
+  microsoft.ad.computer:
+    identity: TheComputer
+    sam_account_name: TheComputer
+    do_not_append_dollar_to_sam: true
 """
 
 RETURN = r"""

diff --git a/tests/integration/targets/computer/tasks/tests.yml b/tests/integration/targets/computer/tasks/tests.yml
@@ -293,8 +293,7 @@
 
 - name: add and remove list options
   computer:
-    name: MyComputer
-    path: CN=Users,{{ setup_domain_info.output[0].defaultNamingContext }}
+    identity: MyComputer2
     delegates:
       add:
       - CN=Administrator,CN=Users,{{ setup_domain_info.output[0].defaultNamingContext }}
@@ -343,8 +342,7 @@
 
 - name: add and remove list options - idempotent
   computer:
-    name: MyComputer
-    path: CN=Users,{{ setup_domain_info.output[0].defaultNamingContext }}
+    identity: MyComputer2$
     delegates:
       add:
       - CN=Administrator,CN=Users,{{ setup_domain_info.output[0].defaultNamingContext }}
@@ -473,3 +471,144 @@
     that:
     - spn_add is changed
     - spn_add_actual.objects[0].servicePrincipalName == ['HTTP/fake', 'HTTP/host.domain:8080', 'HTTP/host']
+
+- name: remove computer before sAMAccountName tests
+  computer:
+    identity: '{{ object_identity }}'
+    state: absent
+
+- name: create computer without sAMAccountName $ suffix - check mode
+  computer:
+    identity: MyComputer
+    sam_account_name: MyComputer
+    do_not_append_dollar_to_sam: true
+    state: present
+  register: create_comp_no_sam_check
+  check_mode: true
+
+- name: get result of create computer without sAMAccountName $ suffix - check mode
+  object_info:
+    identity: '{{ create_comp_no_sam_check.distinguished_name }}'
+    properties:
+    - sAMAccountName
+  register: create_comp_no_sam_check_actual
+
+- name: assert create computer without sAMAccountName $ suffix - check mode
+  assert:
+    that:
+    - create_comp_no_sam_check is changed
+    - create_comp_no_sam_check_actual.objects == []
+
+- name: create computer without sAMAccountName $ suffix
+  computer:
+    identity: MyComputer
+    sam_account_name: MyComputer
+    do_not_append_dollar_to_sam: true
+    state: present
+  register: create_comp_no_sam
+
+- set_fact:
+    object_identity: '{{ create_comp_no_sam.object_guid }}'
+
+- name: get result of create computer without sAMAccountName $ suffix
+  object_info:
+    identity: '{{ object_identity }}'
+    properties:
+    - sAMAccountName
+  register: create_comp_no_sam_actual
+
+- name: assert create computer without sAMAccountName $ suffix
+  assert:
+    that:
+    - create_comp_no_sam is changed
+    - create_comp_no_sam_actual.objects[0].sAMAccountName == 'MyComputer'
+
+- name: create computer without sAMAccountName $ suffix - idempotent
+  computer:
+    identity: MyComputer
+    sam_account_name: MyComputer
+    do_not_append_dollar_to_sam: true
+    state: present
+  register: create_comp_no_sam_again
+
+- name: assert create computer without sAMAccountName $ suffix - idempotent
+  assert:
+    that:
+    - not create_comp_no_sam_again is changed
+
+- name: change computer without sAMAccountName $ suffix - check mode
+  computer:
+    name: MyComputer
+    sam_account_name: MyComputerFoo
+    do_not_append_dollar_to_sam: true
+    state: present
+  register: change_comp_no_sam_check
+  check_mode: true
+
+- name: get result of change computer without sAMAccountName $ suffix - check mode
+  object_info:
+    identity: '{{ object_identity }}'
+    properties:
+    - sAMAccountName
+  register: change_comp_no_sam_check_actual
+
+- name: assert change computer without sAMAccountName $ suffix - check mode
+  assert:
+    that:
+    - change_comp_no_sam_check is changed
+    - change_comp_no_sam_check_actual.objects[0].sAMAccountName == 'MyComputer'
+
+- name: change computer without sAMAccountName $ suffix
+  computer:
+    name: MyComputer
+    sam_account_name: MyComputerFoo
+    do_not_append_dollar_to_sam: true
+    state: present
+  register: change_comp_no_sam
+
+- name: get result of change computer without sAMAccountName $ suffix
+  object_info:
+    identity: '{{ object_identity }}'
+    properties:
+    - sAMAccountName
+  register: change_comp_no_sam_actual
+
+- name: assert change computer without sAMAccountName $ suffix - check mode
+  assert:
+    that:
+    - change_comp_no_sam is changed
+    - change_comp_no_sam_actual.objects[0].sAMAccountName == 'MyComputerFoo'
+
+- name: change computer without sAMAccountName $ suffix - idempotent
+  computer:
+    name: MyComputer
+    sam_account_name: MyComputerFoo
+    do_not_append_dollar_to_sam: true
+    state: present
+  register: change_comp_no_sam_again
+
+- name: assert change computer without sAMAccountName $ suffix - idempotent
+  assert:
+    that:
+    - not change_comp_no_sam_again is changed
+
+- name: find computer by identity sAMAccountName without $ suffix
+  computer:
+    identity: MyComputerFoo
+    display_name: Test
+    state: present
+  register: find_by_sam_no_suffix
+
+- name: get result of find computer by identity sAMAccountName without $ suffix
+  object_info:
+    identity: '{{ object_identity }}'
+    properties:
+    - displayName
+  register: find_by_sam_no_suffix_actual
+
+- name: assert find computer by identity sAMAccountName without $ suffix
+  assert:
+    that:
+    - find_by_sam_no_suffix is changed
+    - find_by_sam_no_suffix.object_guid == object_identity
+    - find_by_sam_no_suffix_actual.objects[0].DisplayName == 'Test'