ansible-collections / microsoft.ad

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

Adding members to a group requires an explicit path to be set #87

Closed timstipdonk closed 7 months ago

timstipdonk commented 8 months ago
SUMMARY

When using version 1.1 of the module, the microsoft.ad.group module returns an access denied, unless a "Path" variable is defined. I suspect that the module tries to move the specific AD group. When the "Path" variable is added, the code works as expected, and adds the user to the AD group.

The task is delegated to a server with the correct AD tools, and has been verified to work correctly with the usage of the previous module (community.windows.win_domain_group). The username and password are also correct and verified using the community.windows.win_domain_group module.

ISSUE TYPE
COMPONENT NAME

microsoft.ad.group

ANSIBLE VERSION
ansible [core 2.15.6]
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/runner/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.9/site-packages/ansible
  ansible collection location = /home/runner/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.9.18 (main, Sep 22 2023, 17:58:34) [GCC 8.5.0 20210514 (Red Hat 8.5.0-20)] (/usr/bin/python3.9)
  jinja version = 3.1.2
  libyaml = True
COLLECTION VERSION
Collection   Version
------------ -------
microsoft.ad 1.1.0
CONFIGURATION
CONFIG_FILE() = /etc/ansible/ansible.cfg
OS / ENVIRONMENT

Target OS is a mix of 2019 and 2022 servers, behavior does not change between them.

STEPS TO REPRODUCE

The following code fails, and returns an error (see screenshot below the ansible code). When the Path variable is added, the playbook seems to work as expected.

image

- name: Add user's AD- account to group membership
  microsoft.ad.group:
    name: "AD-Group-Name"
    members:
      add:
      - "{{ form_username }}"
    scope: global
    category: security
    domain_username: "{{ domain_username }}"
    domain_password: "{{ domain_password }}"
  delegate_to: "{{ delegation_server_win }}"
  vars:
    ansible_user: "{{ ansible_user }}"
    ansible_password: "{{ ansible_password }}"
    ansible_winrm_server_cert_validation: ignore
    ansible_winrm_transport: kerberos
EXPECTED RESULTS

User added to group

ACTUAL RESULTS

Received the following fatal error:

image

FAILED! => {"changed": false, "distinguished_name": null, "msg": "New-ADGroup failed: Access is denied", "object_guid": null}
jborean93 commented 8 months ago

You need to update to a newer version and use the identity option instead of name. Currently if you specify name without a path it will attempt to find the LDAP object with that name in the default path which is why it's trying to use New-ADGroup to create it. Since v1.4.0 you can use identity to find a group by the sAMAccountName and manage it that way https://github.com/ansible-collections/microsoft.ad/blob/main/CHANGELOG.rst#minor-changes

- name: Add user's AD- account to group membership
  microsoft.ad.group:
    identity: "AD-Group-Name"
    members:
      add:
      - "{{ form_username }}"
    scope: global
    category: security
    domain_username: "{{ domain_username }}"
    domain_password: "{{ domain_password }}"
  delegate_to: "{{ delegation_server_win }}"
  vars:
    ansible_user: "{{ ansible_user }}"
    ansible_password: "{{ ansible_password }}"
    ansible_winrm_server_cert_validation: ignore
    ansible_winrm_transport: kerberos
timstipdonk commented 8 months ago

@jborean93 I see that the "Name" parameter now mentions that an explicit path must be passed "It is strictly the name of the object in the path specified.". The documentation however does not mention that the "identity" parameter must be used explicitely. The documentation only mentions that the identity parameter should be defined if the "name" attribute is not defined.

The documentation (Microsoft.Ad.Group) mentions the following:

Name parameter documentation: image

Identity parameter documentation: image

Path documentation: image

Could you please update the documentation + the examples found on the Ansible documentation page? If the newer functionality calls for the "identity" parameter, rather than the "name" parameter, this should be documented and included in the examples.

jborean93 commented 8 months ago

Thanks for bringing that to my attention, the docs definitely need to be updated. Essentially that's the historical behaviour where name had to be set.

Just to clarify here, identity is not required to be set, it's that either identity or name must be set. In the past you always had to set name but as that referred to the name of the LDAP object it caused confusion.

There are two parts to both identity and name, the first first is figuring out what group to manage is. If identity is set then it will lookup the group by sAMAccountName, objectSid, objectGuid, distinguishedName with this value. If identity is not set (name is required) then name is used to find the group under {{ path | default(default_group_path) }}/{{ name }}.

The second part is editing the found group which can be used to rename the group if identity is set. If both name and identity is set then the group will be renamed (LDAP object wise) if the name doesn't match up with what identity is pointed to. The path of the group will also be changed if path is explicitly set.

I'll have to think about a good way to document this to help clarify it all.