ansible-collections / microsoft.ad

Ansible collection for Active Directory management
GNU General Public License v3.0
39 stars 22 forks source link

Unable to add built-in group members #130

Closed AllRWeak closed 2 months ago

AllRWeak commented 3 months ago
SUMMARY

Starting from version 1.6.0 microsoft.ad.group module errors when trying to add members to a built-in group like Administrators. The same code worked with the previous version of the collection.

ISSUE TYPE
COMPONENT NAME

microsoft.ad.group

ANSIBLE VERSION
ansible [core 2.17.0]
  config file = /srv/ansible.cfg
  configured module search path = ['/home/builder/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/builder/.venv/lib/python3.11/site-packages/ansible
  ansible collection location = /srv/ansible:/home/builder/ansible
  executable location = /home/builder/.venv/bin/ansible
  python version = 3.11.9 (main, Jun 27 2024, 00:06:49) [GCC 12.2.0] (/home/builder/.venv/bin/python3)
  jinja version = 3.1.4
  libyaml = True
COLLECTION VERSION
Collection   Version
------------ -------
microsoft.ad 1.6.0  
OS / ENVIRONMENT

Docker container based on python:3.11.9-slim-bookworm

STEPS TO REPRODUCE

The error occurs when trying to include the following task on a domain controller (the Domain_Administrators is already pre-created):

  - name: Adding required group members...
    microsoft.ad.group:
      name: Administrators
      path: CN=Builtin,DC=example,DC=com
      members:
        add:
          - CN=Domain_Administrators,OU=Groups,DC=example,DC=com
      state: present
EXPECTED RESULTS

Add the Domain_Administrators to the built-in Administrators group.

ACTUAL RESULTS

The command errors with the following message:

The full traceback is:
Cannot perform this operation on built-in accounts
At line:1447 char:38
+ ...     $finalADObject = & $setCommand @commonParams @setParams @adParams
+                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (17799b9b-be36-42dc-8c25-2bdabd020d0c:ADGroup) [Set-ADGroup], ADException
    + FullyQualifiedErrorId : ActiveDirectoryServer:1371,Microsoft.ActiveDirectory.Management.Commands.SetADGroup

ScriptStackTrace:
at Invoke-AnsibleADObject, <No file>: line 1447
at <ScriptBlock>, <No file>: line 84
fatal: [dc1]: FAILED! => changed=false 
  distinguished_name: CN=Administrators,CN=Builtin,DC=example,DC=com
  invocation:
    module_args:
      attributes:
        add: {}
        remove: {}
        set: {}
      category: null
      description: null
      display_name: null
      domain_credentials: []
      domain_password: null
      domain_server: null
      domain_username: null
      homepage: null
      identity: null
      managed_by: null
      members:
        add:
        - CN=Domain_Administrators,OU=Groups,DC=example,DC=com
        lookup_failure_action: fail
        remove: null
        set: null
      name: Administrators
      path: CN=Builtin,DC=example,DC=com
      protect_from_deletion: null
      sam_account_name: null
      scope: null
      state: present
  msg: 'Set-ADGroup failed: Cannot perform this operation on built-in accounts'
  object_guid: 17799b9b-be36-42dc-8c25-2bdabd020d0c
Yannik commented 3 months ago

@AllRWeak Please try adding this user account to the group manually using Set-ADGroup to see if the issue occurs in this case as well.

AllRWeak commented 3 months ago

Yes using this command manually works: Set-ADGroup -Identity Administrators -Add @{member="CN=Domain_Administrators,OU=Groups,DC=example,DC=com"}

jborean93 commented 2 months ago

Thanks for the bug report, it looks like v1.6.0 which unified the DN attribute based value lookups to some common code changed how the members option for the group module worked. In the old code it was using the -Add and -Remove parameters to add/remove the members required. https://github.com/ansible-collections/microsoft.ad/blob/0de35154f82f6bf8092845df8a48e4a35ea91f2d/plugins/modules/group.ps1#L148-L159

In the latest version with the unified code it uses the -Replace parameter to just set it once but it seems like for the "builtin" groups the -Replace parameter cannot be used. I've got some local code which reverts the logic for this specific option to using -Add/-Remove but I'll need to write some tests before pushing it to make sure this doesn't regress in the future.