Improve ipapermission member management.

In `ipapermission` plugin, Some attributtes were not being managed
when `action: member` was enabled.

This patch enable member management for `right`, `rawfilter`,
`filter, and fixes management of `memberof`.

Fix issue #489

plugins/modules/ipapermission.py

@@ -277,10 +277,8 @@ def main():
             ansible_module.fail_json(
                 msg="Only one permission can be added at a time.")
         if action == "member":
-            invalid = ["right", "bindtype", "subtree",
-                       "extra_target_filter", "rawfilter", "target",
-                       "targetto", "targetfrom", "memberof", "targetgroup",
-                       "object_type", "rename"]
+            invalid = ["bindtype", "target", "targetto", "targetfrom",
+                       "subtree", "targetgroup", "object_type", "rename"]
         else:
             invalid = ["rename"]
 
@@ -299,13 +297,12 @@ def main():
     if state == "absent":
         if len(names) < 1:
             ansible_module.fail_json(msg="No name given.")
-        invalid = ["right",
-                   "bindtype", "subtree",
-                   "extra_target_filter", "rawfilter", "target", "targetto",
-                   "targetfrom", "memberof", "targetgroup", "object_type",
+        invalid = ["bindtype", "subtree", "target", "targetto",
+                   "targetfrom", "targetgroup", "object_type",
                    "no_members", "rename"]
         if action != "member":
-            invalid += ["attrs"]
+            invalid += ["right", "attrs", "memberof",
+                        "extra_target_filter", "rawfilter"]
 
     for x in invalid:
         if vars()[x] is not None:
@@ -317,6 +314,11 @@ def main():
         ansible_module.fail_json(
             msg="Bindtype 'self' is not supported by your IPA version.")
 
+    if all([extra_target_filter, rawfilter]):
+        ansible_module.fail_json(
+            msg="Cannot specify target filter and extra target filter "
+                "simultaneously.")
+
     # Init
 
     changed = False
@@ -359,16 +361,31 @@ def main():
                         ansible_module.fail_json(
                             msg="No permission '%s'" % name)
 
-                    # attrs
-                    if attrs is not None:
-                        _attrs = list(set(list(res_find["attrs"]) + attrs))
-                        if len(_attrs) > len(res_find["attrs"]):
-                            commands.append([name, "permission_mod",
-                                             {"attrs": _attrs}])
+                    member_attrs = {}
+                    check_members = {
+                        "attrs": attrs,
+                        "memberof": memberof,
+                        "ipapermright": right,
+                        "ipapermtargetfilter": rawfilter,
+                        "extratargetfilter": extra_target_filter,
+                        # subtree member management is currently disabled.
+                        # "ipapermlocation": subtree,
+                    }
+
+                    for _member, _member_change in check_members.items():
+                        if _member_change is not None:
+                            _res_list = res_find[_member]
+                            _new_set = set(_res_list + _member_change)
+                            if _new_set != set(_res_list):
+                                member_attrs[_member] = list(_new_set)
+
+                    if member_attrs:
+                        commands.append([name, "permission_mod", member_attrs])
 
                 else:
                     ansible_module.fail_json(
                         msg="Unknown action '%s'" % action)
+
             elif state == "renamed":
                 if action == "permission":
                     # Generate args
@@ -393,6 +410,7 @@ def main():
                 else:
                     ansible_module.fail_json(
                         msg="Unknown action '%s'" % action)
+
             elif state == "absent":
                 if action == "permission":
                     if res_find is not None:
@@ -403,20 +421,26 @@ def main():
                         ansible_module.fail_json(
                             msg="No permission '%s'" % name)
 
-                    # attrs
-                    if attrs is not None:
-                        # New attribute list (remove given ones from find
-                        # result)
-                        # Make list with unique entries
-                        _attrs = list(set(res_find["attrs"]) - set(attrs))
-                        if len(_attrs) < 1:
-                            ansible_module.fail_json(
-                                msg="At minimum one attribute is needed.")
+                    member_attrs = {}
+                    check_members = {
+                        "attrs": attrs,
+                        "memberof": memberof,
+                        "ipapermright": right,
+                        "ipapermtargetfilter": rawfilter,
+                        "extratargetfilter": extra_target_filter,
+                        # subtree member management is currently disabled.
+                        # "ipapermlocation": subtree,
+                    }
 
-                        # Entries New number of attributes is smaller
-                        if len(_attrs) < len(res_find["attrs"]):
-                            commands.append([name, "permission_mod",
-                                             {"attrs": _attrs}])
+                    for _member, _member_change in check_members.items():
+                        if _member_change is not None:
+                            _res_set = set(res_find[_member])
+                            _new_set = _res_set - set(_member_change)
+                            if _new_set != _res_set:
+                                member_attrs[_member] = list(_new_set)
+
+                    if member_attrs:
+                        commands.append([name, "permission_mod", member_attrs])
 
             else:
                 ansible_module.fail_json(msg="Unknown state '%s'" % state)

tests/permission/test_permission.yml

@@ -6,6 +6,15 @@
   tasks:
   - include_tasks: ../env_freeipa_facts.yml
 
+  - name: Ensure testing groups are present.
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      name: "{{ item }}"
+      state: present
+    with_items:
+      - rbacgroup1
+      - rbacgroup2
+
   # CLEANUP TEST ITEMS
 
   - name: Ensure permission perm-test-1 is absent
@@ -24,6 +33,8 @@
       ipaadmin_password: SomeADMINpassword
       name: perm-test-1
       object_type: host
+      memberof: rbacgroup1
+      filter: '(cn=*.ipa.*)'
       right: all
     register: result
     failed_when: not result.changed or result.failed
@@ -33,10 +44,106 @@
       ipaadmin_password: SomeADMINpassword
       name: perm-test-1
       object_type: host
+      memberof: rbacgroup1
+      filter: '(cn=*.ipa.*)'
       right: all
     register: result
     failed_when: result.changed or result.failed
 
+  - name: Ensure permission perm-test-1 has an extra filter '(cn=*.internal.*)'
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      filter: '(cn=*.internal.*)'
+      action: member
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 has an extra filter '(cn=*.internal.*)', again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      filter: '(cn=*.internal.*)'
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 `right` has `write`
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      right: write
+      action: member
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 `right` has `write`, again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      right: write
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 `right` has no `write`
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      right: write
+      action: member
+      state: absent
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 `right` has no `write`, again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      right: write
+      action: member
+      state: absent
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 `memberof` has `rbackgroup2`
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      memberof: rbacgroup2
+      action: member
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 `memberof` has `rbackgroup2`, again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      memberof: rbacgroup2
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 `memberof` item `rbackgroup1` is absent
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      memberof: rbacgroup1
+      action: member
+      state: absent
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 `memberof` item `rbackgroup1` is absent, again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      memberof: rbacgroup1
+      action: member
+      state: absent
+    register: result
+    failed_when: result.changed or result.failed
+
   - name: Ensure permission perm-test-1 is present with attr carlicense
     ipapermission:
       ipaadmin_password: SomeADMINpassword
@@ -163,6 +270,34 @@
     register: result
     failed_when: result.changed or result.failed
 
+  - name: Ensure permission perm-test-1 has rawfilter '(objectclass=ipagroup)'
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      rawfilter: '(objectclass=ipagroup)'
+      action: member
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 has rawfilter '(objectclass=ipagroup)', again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      rawfilter: '(objectclass=ipagroup)'
+      action: member
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure filter and rawfilter cannot be used together.
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      name: perm-test-1
+      rawfilter: '(objectclass=ipagroup)'
+      filter: '(cn=*.internal.*)'
+      action: member
+    register: result
+    failed_when: not result.failed or "Cannot specify target filter and extra target filter simultaneously" not in result.msg
+
   - name: Rename permission perm-test-1 to perm-test-renamed
     ipapermission:
       ipaadmin_password: SomeADMINpassword
@@ -213,7 +348,7 @@
 
   # CLEANUP TEST ITEMS
 
-  - name: Ensure permission perm-test-1 is absent
+  - name: Ensure testing permissions are absent
     ipapermission:
       ipaadmin_password: SomeADMINpassword
       name:
@@ -221,3 +356,12 @@
       - perm-test-bindtype-test
       - perm-test-renamed
       state: absent
+
+  - name: Ensure testing groups are absent.
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      name: "{{ item }}"
+      state: absent
+    with_items:
+      - rbacgroup1
+      - rbacgroup2