diff --git a/README-group.md b/README-group.md index 75b8bb54fa..05fd605482 100644 --- a/README-group.md +++ b/README-group.md @@ -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 diff --git a/plugins/modules/ipagroup.py b/plugins/modules/ipagroup.py index 7251fda2eb..d5f701dbee 100644 --- a/plugins/modules/ipagroup.py +++ b/plugins/modules/ipagroup.py @@ -113,13 +113,14 @@ 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 @@ 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 @@ 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 @@ 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 + and 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." diff --git a/tests/group/test_group.yml b/tests/group/test_group.yml index 8cb76946d2..a64b4cdedb 100644 --- a/tests/group/test_group.yml +++ b/tests/group/test_group.yml @@ -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 diff --git a/tests/group/test_group_ad_users.yml b/tests/group/test_group_ad_users.yml new file mode 100644 index 0000000000..0c84db6c25 --- /dev/null +++ b/tests/group/test_group_ad_users.yml @@ -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 diff --git a/tests/group/test_group_client_context.yml b/tests/group/test_group_client_context.yml index 8d0132def0..2a5b89e439 100644 --- a/tests/group/test_group_client_context.yml +++ b/tests/group/test_group_client_context.yml @@ -18,6 +18,15 @@ 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: nouse@ad.test + 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. diff --git a/tests/group/test_group_external_members.yml b/tests/group/test_group_external_members.yml index f9214d3dfd..b1f9f85d1d 100644 --- a/tests/group/test_group_external_members.yml +++ b/tests/group/test_group_external_members.yml @@ -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 + state: absent + register: result + failed_when: result.failed or result.changed + + - name: Ensure external group is present, with AD users. ipagroup: name: extgroup - nonposix: true external: true - external_member: "AD\\Domain Users" + external_member: "{{ ad_user }}" register: result failed_when: result.failed or not result.changed - - name: Add nonposix, external group, with AD users, again. + - 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 - nonposix: true external: true - external_member: "AD\\Domain Users" + external_member: "{{ ad_user }}" register: result failed_when: result.failed or result.changed - - name: Remove group + - 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: "{{ 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\\Domain Users" + 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 diff --git a/tests/group/test_group_idoverrideuser.yml b/tests/group/test_group_idoverrideuser.yml index ce4c0bb4ee..2b028919b8 100644 --- a/tests/group/test_group_idoverrideuser.yml +++ b/tests/group/test_group_idoverrideuser.yml @@ -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,13 +90,7 @@ register: result failed_when: result.failed or result.changed - - name: Cleanup idoverrideuser member. - ipagroup: - ipaadmin_password: SomeADMINpassword - name: idovergroup - idoverrideuser: "{{ ad_user }}" - state: absent - + always: - name: Remove testing groups. ipagroup: ipaadmin_password: SomeADMINpassword @@ -96,10 +98,9 @@ - idovergroup state: absent - always: - 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