Deduplicate access permissions list returned to clients

rbac/management/access/view.py

@@ -23,6 +23,7 @@
 from management.role.serializer import AccessSerializer
 from management.utils import (
     APPLICATION_KEY,
+    deduplicate_access_queryset,
     get_principal_from_request,
     validate_and_get_key,
     validate_limit_and_offset,
@@ -121,7 +122,9 @@ def get(self, request):
         access_policy = cache.get_policy(principal.uuid, sub_key)
         if access_policy is None:
             queryset = self.get_queryset(ordering)
-            access_policy = self.serializer_class(queryset, many=True).data
+            access_policy = self.serializer_class(
+                deduplicate_access_queryset(queryset), many=True
+            ).data
             cache.save_policy(principal.uuid, sub_key, access_policy)
 
         page = self.paginate_queryset(access_policy)

rbac/management/permission/model.py

@@ -31,6 +31,20 @@ class Permission(TenantAwareModel):
     description = models.TextField(default="")
     permissions = models.ManyToManyField("self", symmetrical=False, related_name="requiring_permissions")
 
+    def __eq__(self, other):
+        return self.permission == other.permission
+
+    def __contains__(self, other):
+        # Note that the sense of 'contains' is opposite to the sense of 'in':
+        # A is in B if B contains A.
+        if self.permission == other.permission:
+            return True
+        if not (self.application == other.application or self.application == '*'):
+            return False
+        if not (self.resource_type == other.resource_type or self.resource_type == '*'):
+            return False
+        return (self.verb == other.verb or self.verb == '*' or (other.verb == 'read' and self.verb == 'write'))
+
     def save(self, *args, **kwargs):
         """Populate the application, resource_type and verb field before saving."""
         context = self.permission.split(":")

rbac/management/utils.py

@@ -192,6 +192,45 @@ def filter_queryset_by_tenant(queryset, tenant):
     return queryset.filter(tenant=tenant)
 
 
+def deduplicate_access_queryset(queryset):
+    """
+    Deduplicate the access queryset.
+
+    Takes a queryset on the Access model, and returns a list of the minimal
+    permissions described in the original queryset.  These deduplications are
+    performed:
+
+    1: two exactly matching permissions are combined - e.g. 'app:*:read' and
+       'app:*:read' - including joining together their resourceDefinitions.
+
+    Others may be added later, such as combining permissions that wholly
+    include others.  For now let's stick with what's simple.
+    """
+    # Since the ordering does not matter, we record these in a dict by the
+    # permission, for ease of access.
+    deduplicated_access = dict()
+
+    def matching_perms(this_access, sought_access):
+        """Find all access objects that match the app and resource given."""
+        tap = this_access.permission
+        sap = sought_access.permission
+        return tap.app == sap.app and tap.resource_type == sap.resource_type
+
+    for access in queryset:
+        if not access.permission:
+            # Cannot emit an access permission if it doesn't have one
+            continue
+        if access.permission.permission in deduplicated_access:
+            # Combine this access object's resource definitions into that
+            # already stored.  Note that at this stage we DO NOT attempt to
+            # merge attributeFilter structures.
+            deduplicated_access[access.permission.permission].resourceDefinitions.extend(
+                access.resourceDefinitions
+            )
+    # Return access object list in order by permission for testing consistency
+    return (deduplicated_access[perm] for perm in sorted(deduplicated_access.keys()))
+
+
 def validate_and_get_key(params, query_key, valid_values, default_value=None, required=True):
     """Validate the key."""
     value = params.get(query_key, default_value)

tests/management/test_utils.py

@@ -23,6 +23,7 @@
     policies_for_principal,
     roles_for_principal,
     account_id_for_tenant,
+    deduplicate_access_queryset,
 )
 from tests.identity_request import IdentityRequest
 
@@ -130,3 +131,29 @@ def test_account_number_from_tenant_name(self):
         """Test that we get the expected account number from a tenant name."""
         tenant = Tenant.objects.create(tenant_name="acct1234")
         self.assertEqual(account_id_for_tenant(tenant), "1234")
+
+    def test_deduplicate_access_queryset(self):
+        """
+        Test a variety of scenarios where we need to deduplicate the data
+        """
+        ih_hosts_read = Permission.objects.create(permission='inventory:hosts:read', tenant=self.tenant)
+        ihhr_access1 = Access.objects.create(permission=ih_hosts_read, role=self.roleA, tenant=self.tenant)
+        groups_rdef2 = ResourceDefinition.objects.create(access=ihhr_access1, attributeFilter={
+            'key': 'group.id', 'operation': 'in', 'value': [
+                '78e3dc30-cec3-4b49-be2d-37482c74a9a1',
+                '78e3dc30-cec3-4b49-be2d-37482c74a9b1'
+            ]
+        })
+        ihhr_access2 = Access.objects.create(permission=ih_hosts_read, role=self.roleA, tenant=self.tenant)
+        groups_rdef2 = ResourceDefinition.objects.create(access=ihhr_access2, attributeFilter={
+            'key': 'group.id', 'operation': 'in', 'value': [
+                '78e3dc30-cec3-4b49-be2d-37482c74a9a2',
+                '78e3dc30-cec3-4b49-be2d-37482c74a9b2'
+            ]
+        })
+
+        # First basic test: what do these get merged into?
+        queryset = Access.objects.filter(tenant=self.tenant)
+        deduped = deduplicate_access_queryset(queryset)
+        print("deduped:", deduped)
+        self.assertEqual(len(deduped), 1)