internet/ansible-freeipa
4
0
Fork 0
mirror of https://github.com/freeipa/ansible-freeipa.git synced 2026-03-26 21:33:05 +00:00
Code Issues Projects Releases Wiki Activity

ipagroup: Fix management of AD objects

Browse Source 
When using AD objects, a user expects to use the more human readable
form, like "user@ad.domain", but this impose some dificulties on
evaluating which object is being referenced as AD has several forms to
refer to the same object.

Each object is AD is identified uniquely by its SID, and this is the
identifier that IPA stores in its database. When managing AD objects,
IPA finds its SID and works with that value.

ansible-freeipa tried to process these objects using the human readable
values, and it cause idempontence error when ensuring the values were
present or modified, and, at least in some cases, prevented the objects
to be made absent, as the object list created didn't match the SID to
the value used as module parameter.

By using SID to process the AD objects in ipagroup, the addition or
removal of members works and idempotence of these members is ensured.

The only issue with thils approach is that it only works no server
nodes. In client nodes, the conversion to SID is not available and the
same issues that existed before will still be present.

Tests were updated to reflect these changes, a new test, specific to
idempotence issues of AD objects was added:

   tests/group/test_group_ad_users.yml

Resolves: https://issues.redhat.com/browse/RHEL-70023
This commit is contained in:
Rafael Guterres Jeffman
2025-01-29 00:32:16 -03:00
parent 8581b79eba
commit 6925503a10
7 changed files with 308 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
README-group.md
View File

@@ -8,8 +8,12 @@ The group module allows to ensure presence and absence of groups and members of
The group module is as compatible as possible to the Ansible upstream `ipa_group` module, but additionally offers to add users to a group and also to remove users from a group.
## Note
Ensuring presence (adding) of several groups with mixed types (`external`, `nonposix` and `posix`) requires a fix in FreeIPA. The module implements a workaround to automatically use `client` context if the fix is not present in the target node FreeIPA and if more than one group is provided to the task using the `groups` parameter. If `ipaapi_context` is forced to be `server`, the module will fail in this case.
Notes
-----
* Ensuring presence (adding) of several groups with mixed types (`external`, `nonposix` and `posix`) requires a fix in FreeIPA. The module implements a workaround to automatically use `client` context if the fix is not present in the target node FreeIPA and if more than one group is provided to the task using the `groups` parameter. If `ipaapi_context` is forced to be `server`, the module will fail in this case.
* Using `externalmember` or `idoverrideuser` is only supported with `ipaapi_context: server`. With 'client' context, module execution will fail.
Features
@@ -213,7 +217,7 @@ Example playbook to add members from a trusted realm to an external group:
---
- name: Playbook to handle groups.
hosts: ipaserver
tasks:
- name: Create an external group and add members from a trust to it.
ipagroup:
@@ -276,6 +280,7 @@ Example playbook to ensure groups are absent:
state: absent
```
Variables
=========
@@ -299,8 +304,8 @@ Variable | Description | Required
`service` | List of service name strings assigned to this group. Only usable with IPA versions 4.7 and up. | no
`membermanager_user` | List of member manager users assigned to this group. Only usable with IPA versions 4.8.4 and up. | no
`membermanager_group` | List of member manager groups assigned to this group. Only usable with IPA versions 4.8.4 and up. | no
`externalmember` \| `ipaexternalmember` \| `external_member`| List of members of a trusted domain in DOM\\name or name@domain form. | no
`idoverrideuser` | List of user ID overrides to manage. Only usable with IPA versions 4.8.7 and up.| no
`externalmember` \| `ipaexternalmember` \| `external_member`| List of members of a trusted domain in DOM\\name or name@domain form. Requires "server" context. | no
`idoverrideuser` | List of user ID overrides to manage. Only usable with IPA versions 4.8.7 and up. Requires "server" context. | no
`rename` \| `new_name` | Rename the user object to the new name string. Only usable with `state: renamed`. | no
`action` | Work on group or member level. It can be on of `member` or `group` and defaults to `group`. | no
`state` | The state to ensure. It can be one of `present`, `absent` or `renamed`, default: `present`. | yes

76
plugins/modules/ipagroup.py
View File

@@ -113,13 +113,14 @@ options:
externalmember:
description:
- List of members of a trusted domain in DOM\\name or name@domain form.
Requires "server" context.
required: false
type: list
elements: str
aliases: ["ipaexternalmember", "external_member"]
idoverrideuser:
description:
- User ID overrides to add
- User ID overrides to add. Requires "server" context.
required: false
type: list
elements: str
@@ -188,13 +189,14 @@ options:
externalmember:
description:
- List of members of a trusted domain in DOM\\name or name@domain form.
Requires "server" context.
required: false
type: list
elements: str
aliases: ["ipaexternalmember", "external_member"]
idoverrideuser:
description:
- User ID overrides to add
- User ID overrides to add. Requires "server" context.
required: false
type: list
elements: str
@@ -297,6 +299,7 @@ EXAMPLES = """
posix: yes
# Create an external group and add members from a trust to it.
# Module will fail if running under 'client' context.
- ipagroup:
ipaadmin_password: SomeADMINpassword
name: extgroup
@@ -327,7 +330,8 @@ RETURN = """
from ansible.module_utils._text import to_text
from ansible.module_utils.ansible_freeipa_module import \
IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, \
gen_add_list, gen_intersection_list, api_check_param
gen_add_list, gen_intersection_list, api_check_param, \
convert_to_sid
from ansible.module_utils import six
if six.PY3:
unicode = str
@@ -562,21 +566,29 @@ def main():
# The simple solution is to switch to client context for ensuring
# several groups simply if the user was not explicitly asking for
# the server context no matter if mixed types are used.
context = None
context = ansible_module.params_get("ipaapi_context")
if state == "present" and groups is not None and len(groups) > 1 \
and not FIX_6741_DEEPCOPY_OBJECTCLASSES:
_context = ansible_module.params_get("ipaapi_context")
if _context is None:
if context is None:
context = "client"
ansible_module.debug(
"Switching to client context due to an unfixed issue in "
"your IPA version: https://pagure.io/freeipa/issue/9349")
elif _context == "server":
elif context == "server":
ansible_module.fail_json(
msg="Ensuring several groups with server context is not "
"supported by your IPA version: "
"https://pagure.io/freeipa/issue/9349")
if (
externalmember is not None
or idoverrideuser is not None
and context == "client"
):
ansible_module.fail_json(
msg="Cannot use externalmember in client context."
)
# Use groups if names is None
if groups is not None:
names = groups
@@ -676,6 +688,23 @@ def main():
# Make sure group exists
res_find = find_group(ansible_module, name)
# external members must de handled as SID
externalmember = convert_to_sid(externalmember)
# idoverrides need to be compared through SID
idoverrideuser_sid = convert_to_sid(idoverrideuser)
res_idoverrideuser_sid = convert_to_sid(
(res_find or {}).get("member_idoverrideuser", []))
idoverride_set = dict(
list(zip(idoverrideuser_sid or [], idoverrideuser or [])) +
list(
zip(
res_idoverrideuser_sid or [],
(res_find or {}).get("member_idoverrideuser", [])
)
)
)
user_add, user_del = [], []
group_add, group_del = [], []
service_add, service_del = [], []
@@ -723,11 +752,12 @@ def main():
res_find = {}
# if we just created/modified the group, update res_find
res_find.setdefault("objectclass", [])
classes = list(res_find.setdefault("objectclass", []))
if external and not is_external_group(res_find):
res_find["objectclass"].append("ipaexternalgroup")
classes.append("ipaexternalgroup")
if posix and not is_posix_group(res_find):
res_find["objectclass"].append("posixgroup")
classes.append("posixgroup")
res_find["objectclass"] = classes
member_args = gen_member_args(
user, group, service, externalmember, idoverrideuser
@@ -752,11 +782,19 @@ def main():
)
)
# There are multiple ways to name an AD User, and any
# can be used in idoverrides, so we create the add/del
# lists based on SID, and then use the given user name
# to the idoverride.
(idoverrides_add,
idoverrides_del) = gen_add_del_lists(
idoverrideuser,
res_find.get("member_idoverrideuser")
)
idoverrideuser_sid, res_idoverrideuser_sid)
idoverrides_add = [
idoverride_set[sid] for sid in set(idoverrides_add)
]
idoverrides_del = [
idoverride_set[sid] for sid in set(idoverrides_del)
]
membermanager_user_add, membermanager_user_del = \
gen_add_del_lists(
@@ -790,7 +828,10 @@ def main():
)
)
idoverrides_add = gen_add_list(
idoverrideuser, res_find.get("member_idoverrideuser"))
idoverrideuser_sid, res_idoverrideuser_sid)
idoverrides_add = [
idoverride_set[sid] for sid in set(idoverrides_add)
]
membermanager_user_add = gen_add_list(
membermanager_user,
@@ -829,7 +870,10 @@ def main():
)
)
idoverrides_del = gen_intersection_list(
idoverrideuser, res_find.get("member_idoverrideuser"))
idoverrideuser_sid, res_idoverrideuser_sid)
idoverrides_del = [
idoverride_set[sid] for sid in set(idoverrides_del)
]
membermanager_user_del = gen_intersection_list(
membermanager_user, res_find.get("membermanager_user"))
@@ -872,7 +916,7 @@ def main():
if len(externalmember_del) > 0:
del_member_args["ipaexternalmember"] = \
externalmember_del
elif externalmember or external:
elif externalmember:
ansible_module.fail_json(
msg="Cannot add external members to a "
"non-external group."

30
tests/group/test_group.yml
View File

@@ -1,8 +1,8 @@
---
- name: Test group
hosts: "{{ ipa_test_host | default('ipaserver') }}"
become: true
gather_facts: true
become: false
gather_facts: false
module_defaults:
ipauser:
ipaadmin_password: SomeADMINpassword
@@ -10,6 +10,9 @@
ipagroup:
ipaadmin_password: SomeADMINpassword
ipaapi_context: "{{ ipa_context | default(omit) }}"
ipaservice:
ipaadmin_password: SomeADMINpassword
ipaapi_context: "{{ ipa_context | default(omit) }}"
tasks:
# setup
@@ -51,6 +54,16 @@
register: result
failed_when: not result.changed or result.failed
- name: Ensure test service HTTP is present
ipaservice:
name: "{{ 'HTTP/' + fqdn_at_domain }}"
notify: Cleanup http service
- name: Ensure test service LDAP is present
ipaservice:
name: "{{ 'ldap/' + fqdn_at_domain }}"
notify: Cleanup ldap service
# TESTS
- name: Ensure group1 is present
@@ -437,3 +450,16 @@
state: absent
register: result
failed_when: not result.changed or result.failed
# ansible-lint is complaining on the use of 'when' and requiring
# the use of handlers.
handlers:
- name: Cleanup http service
ipaservice:
name: "{{ 'HTTP/' + fqdn_at_domain }}"
state: absent
- name: Cleanup ldap service
ipaservice:
name: "{{ 'ldap/' + fqdn_at_domain }}"
state: absent

73
tests/group/test_group_ad_users.yml Normal file
View File

@@ -0,0 +1,73 @@
---
- name: Test group AD external members idempotence
hosts: ipaserver
become: false
gather_facts: false
module_defaults:
ipagroup:
ipaadmin_password: SomeADMINpassword
ipaapi_context: server # external_member requires 'server' context
vars:
ad_user: "{{ test_ad_user | default('AD\\aduser') }}"
alt_user: "{{ test_alt_user | default('aduser@ad.ipa.test') }}"
tasks:
- name: Include tasks ../env_freeipa_facts.yml
ansible.builtin.include_tasks: ../env_freeipa_facts.yml
- name: Ensure test group is absent.
ipagroup:
name: extgroup
state: absent
- name: Execute group tests if trust test environment is supported
when: trust_test_is_supported | default(false)
block:
- name: Ensure external group, with AD users, is present.
ipagroup:
name: extgroup
external: true
external_member: "{{ ad_user }}"
register: result
failed_when: result.failed or not result.changed
- name: Ensure external group, with AD users, is present, again
ipagroup:
name: extgroup
external: true
external_member: "{{ ad_user }}"
register: result
failed_when: result.failed or result.changed
- name: Ensure external group, with alternate name AD users, is present
ipagroup:
name: extgroup
external: true
external_member: "{{ alt_user }}"
register: result
failed_when: result.failed or result.changed
- name: Ensure external_member is absent
ipagroup:
name: extgroup
external_member: "{{ ad_user }}"
action: member
state: absent
register: result
failed_when: result.failed or not result.changed
- name: Ensure external_member is absent, again
ipagroup:
name: extgroup
external_member: "{{ alt_user }}"
action: member
state: absent
register: result
failed_when: result.failed or result.changed
always:
- name: Cleanup environment.
ipagroup:
name: extgroup
state: absent

13
tests/group/test_group_client_context.yml
View File

@@ -4,6 +4,9 @@
become: no
gather_facts: no
vars:
ad_user: "{{ test_ad_user | default('AD\\aduser') }}"
tasks:
- name: Include FreeIPA facts.
ansible.builtin.include_tasks: ../env_freeipa_facts.yml
@@ -18,6 +21,16 @@
failed_when: not (result.failed and result.msg is regex("No module named '*ipaserver'*"))
when: ipa_host_is_client
- name: Ensuref fail if externalmember is used in client context.
ipagroup:
ipaadmin_password: SomeADMINpassword
ipaapi_context: client
name: ThisShouldNotWork
external_member: "{{ ad_user }}"
external: true
register: result
failed_when: not (result.failed and result.msg == "Cannot use externalmember in client context.")
# Import basic module tests, and execute with ipa_context set to 'client'.
# If ipaclients is set, it will be executed using the client, if not,
# ipaserver will be used.

114
tests/group/test_group_external_members.yml
View File

@@ -1,12 +1,16 @@
---
- name: Find trust
hosts: ipaserver
- name: Test groups with external members
hosts: "{{ ipa_test_host | default('ipaserver') }}"
become: false
gather_facts: false
module_defaults:
ipagroup:
ipaadmin_password: SomeADMINpassword
ipaapi_context: "{{ ipa_context | default(omit) }}"
ipaapi_context: server # external_member requires 'server' context
vars:
ad_user: "{{ test_ad_user | default('AD\\aduser') }}"
alt_user: "{{ test_alt_user | default('aduser@ad.ipa.test') }}"
tasks:
@@ -24,86 +28,121 @@
when: trust_test_is_supported | default(false)
block:
- name: Add nonposix group.
- name: Ensure nonposix group is present
ipagroup:
name: extgroup
nonposix: true
register: result
failed_when: result.failed or not result.changed
- name: Set group to be external
- name: Ensure nonposix group is present, again
ipagroup:
name: extgroup
nonposix: true
register: result
failed_when: result.failed or result.changed
- name: Ensure nonposix group is external
ipagroup:
name: extgroup
external: true
register: result
failed_when: result.failed or not result.changed
- name: Add AD users to group
- name: Ensure nonposix group has AD users
ipagroup:
name: extgroup
external_member: "AD\\Domain Users"
external_member: "{{ ad_user }}"
register: result
failed_when: result.failed or not result.changed
- name: Add AD users to group, again
- name: Ensure nonposix group has AD users, again
ipagroup:
name: extgroup
external_member: "AD\\Domain Users"
external_member: "{{ ad_user }}"
register: result
failed_when: result.failed or result.changed
- name: Remove external group
- name: Ensure nonposix group is absent.
ipagroup:
name: extgroup
state: absent
register: result
failed_when: result.failed or not result.changed
- name: Add nonposix, external group, with AD users.
- name: Ensure nonposix group is absent, again.
ipagroup:
name: extgroup
nonposix: true
external: true
external_member: "AD\\Domain Users"
register: result
failed_when: result.failed or not result.changed
- name: Add nonposix, external group, with AD users, again.
ipagroup:
name: extgroup
nonposix: true
external: true
external_member: "AD\\Domain Users"
state: absent
register: result
failed_when: result.failed or result.changed
- name: Remove group
- name: Ensure external group is present, with AD users.
ipagroup:
name: extgroup
external: true
external_member: "{{ ad_user }}"
register: result
failed_when: result.failed or not result.changed
- name: Ensure external group is present, with AD alternate users.
ipagroup:
name: extgroup
external: true
external_member: "{{ alt_user }}"
register: result
failed_when: result.failed or result.changed
- name: Ensure external group is present, with AD users, again.
ipagroup:
name: extgroup
external: true
external_member: "{{ ad_user }}"
register: result
failed_when: result.failed or result.changed
- name: Ensure external group is absent
ipagroup:
name: extgroup
state: absent
register: result
failed_when: result.failed or not result.changed
- name: Add nonposix group.
- name: Ensure external group is absent, again
ipagroup:
name: extgroup
state: absent
register: result
failed_when: result.failed or result.changed
- name: Ensure nonposix group is present.
ipagroup:
name: extgroup
nonposix: true
register: result
failed_when: result.failed or not result.changed
- name: Set group to be external, and add users.
- name: Ensure group is external, and has AD users.
ipagroup:
name: extgroup
external: true
external_member: "AD\\Domain Users"
external_member: "{{ ad_user }}"
register: result
failed_when: result.failed or not result.changed
- name: Set group to be external, and add users, again.
- name: Ensure group is external, and has AD alternate users.
ipagroup:
name: extgroup
external: true
external_member: "AD\\Domain Users"
external_member: "{{ alt_user }}"
register: result
failed_when: result.failed or result.changed
- name: Ensure group is external, and has AD users, again.
ipagroup:
name: extgroup
external: true
external_member: "{{ ad_user }}"
register: result
failed_when: result.failed or result.changed
@@ -117,7 +156,7 @@
- name: Ensure external group members are present
ipagroup:
name: extgroup_members
external_member: "AD\\Domain Users"
external_member: "{{ ad_user }}"
action: member
register: result
failed_when: result.failed or not result.changed
@@ -125,7 +164,7 @@
- name: Ensure external group members are present, again
ipagroup:
name: extgroup_members
external_member: "AD\\Domain Users"
external_member: "{{ ad_user }}"
action: member
register: result
failed_when: result.failed or result.changed
@@ -133,16 +172,25 @@
- name: Ensure external group members are absent
ipagroup:
name: extgroup_members
external_member: "AD\\Domain Users"
external_member: "{{ ad_user }}"
action: member
state: absent
register: result
failed_when: result.failed or not result.changed
- name: Ensure external group alternate members are absent
ipagroup:
name: extgroup_members
external_member: "{{ alt_user }}"
action: member
state: absent
register: result
failed_when: result.failed or result.changed
- name: Ensure external group members are absent, again
ipagroup:
name: extgroup_members
external_member: "AD\\Domain Users"
external_member: "{{ ad_user }}"
action: member
state: absent
register: result

85
tests/group/test_group_idoverrideuser.yml
View File

@@ -1,12 +1,17 @@
---
- name: Test group
- name: Test group idoverrideuser
hosts: ipaserver
become: yes
gather_facts: yes
become: false
gather_facts: false
module_defaults:
ipagroup:
ipaadmin_password: SomeADMINpassword
ipaidoverrideuser:
ipaadmin_password: SomeADMINpassword
vars:
ad_user: "{{ test_ad_user | default('AD\\aduser') }}"
ad_domain: "{{ test_ad_domain | default('ad.ipa.test') }}"
ad_user: "{{ test_ad_user | default('AD\\aduser') }}"
alt_user: "{{ test_alt_user | default('aduser@ad.ipa.test') }}"
tasks:
- name: Include tasks ../env_freeipa_facts.yml
@@ -15,38 +20,42 @@
- name: Execute tests if ipa_verison >= 4.8.7 and trust test environment is supported
when: ipa_version is version("4.8.7", ">=") and trust_test_is_supported | default(false)
block:
- name: Create idoverrideuser.
ansible.builtin.shell: |
kinit -c idoverride_cache admin <<< SomeADMINpassword
ipa idoverrideuser-add "Default Trust View" {{ ad_user }}
kdestroy -A -q -c idoverride_cache
- name: Ensure test idoverrideuser is present
ipaidoverrideuser:
idview: "Default Trust View"
anchor: "{{ ad_user }}"
register: result
failed_when: result.failed and "no modifications to be performed" not in result.msg
- name: Remove testing groups.
- name: Ensure test groups are absent
ipagroup:
ipaadmin_password: SomeADMINpassword
name:
- idovergroup
- idovergroup
state: absent
- name: Add group with idoverrideuser.
- name: Ensure group with idoverrideuser is present.
ipagroup:
ipaadmin_password: SomeADMINpassword
name: idovergroup
idoverrideuser: "{{ ad_user }}"
register: result
failed_when: result.failed or not result.changed
- name: Add group with idoverrideuser, again.
- name: Ensure group with idoverrideuser is present, again.
ipagroup:
ipaadmin_password: SomeADMINpassword
name: idovergroup
idoverrideuser: "{{ ad_user }}"
register: result
failed_when: result.failed or result.changed
- name: Remove idoverrideuser member.
- name: Ensure group with alternative idoverrideuser is present.
ipagroup:
name: idovergroup
idoverrideuser: "{{ alt_user }}"
register: result
failed_when: result.failed or result.changed
- name: Ensure idoverrideuser member is absent.
ipagroup:
ipaadmin_password: SomeADMINpassword
name: idovergroup
idoverrideuser: "{{ ad_user }}"
action: member
@@ -54,9 +63,8 @@
register: result
failed_when: result.failed or not result.changed
- name: Remove idoverrideuser member, again.
- name: Ensure idoverrideuser member is absent, again.
ipagroup:
ipaadmin_password: SomeADMINpassword
name: idovergroup
idoverrideuser: "{{ ad_user }}"
action: member
@@ -64,7 +72,7 @@
register: result
failed_when: result.failed or result.changed
- name: Add idoverrideuser member.
- name: Ensure idoverrideuser member is present.
ipagroup:
ipaadmin_password: SomeADMINpassword
name: idovergroup
@@ -73,7 +81,7 @@
register: result
failed_when: result.failed or not result.changed
- name: Add idoverrideuser member, again.
- name: Ensure idoverrideuser member is present, again.
ipagroup:
ipaadmin_password: SomeADMINpassword
name: idovergroup
@@ -82,24 +90,17 @@
register: result
failed_when: result.failed or result.changed
- name: Cleanup idoverrideuser member.
ipagroup:
ipaadmin_password: SomeADMINpassword
name: idovergroup
idoverrideuser: "{{ ad_user }}"
state: absent
- name: Remove testing groups.
ipagroup:
ipaadmin_password: SomeADMINpassword
name:
- idovergroup
state: absent
always:
- name: Remove testing groups.
ipagroup:
ipaadmin_password: SomeADMINpassword
name:
- idovergroup
state: absent
- name: Remove idoverrideuser.
ansible.builtin.shell:
cmd: |
kinit -c idoverride_cache admin <<< SomeADMINpassword
ipa idoverrideuser-del "Default Trust View" {{ ad_user }}
kdestroy -A -q -c idoverride_cache
ipaidoverrideuser:
idview: "Default Trust View"
anchor: "{{ ad_user }}"
continue: true
state: absent