Fixes #RHIROS-1312 - update RBAC filters (#355)

* Fixes #RHIROS-1312 - update RBAC filters * Fix nits and add extra test case
RedHatInsights · Sep 3, 2023 · b5c1c68 · b5c1c68
1 parent f3703be
commit b5c1c68
Show file tree

Hide file tree

Showing 9 changed files with 335 additions and 16 deletions.
diff --git a/ros/lib/rbac_interface.py b/ros/lib/rbac_interface.py
@@ -157,11 +157,12 @@ def set_host_groups(rbac_response):
         return
 
     role_list = rbac_response['data']
+    host_groups = []
 
     for role in role_list:
         if 'permission' not in role:
             continue
-        if role['permission'] != 'inventory:hosts:read':
+        if role['permission'] not in ['inventory:hosts:read', 'inventory:hosts:*', 'inventory:*:read', 'inventory:*:*']:
             continue
         # ignore the failure modes, try moving on to other roles that
         # also match this permission
@@ -189,10 +190,11 @@ def set_host_groups(rbac_response):
                 value = json.loads(value)
             if not isinstance(value, list):
                 continue
-            # Finally, we have the right key: set its value (a list) as
-            # the 'host_group_attr' property
-            # Maybe it would be good to set values as json here instead of list
-            setattr(request, host_group_attr, value)
-            LOG.info(f"User has host groups {value}")
-            # and we can leave in triumph
-            return
+            # Finally, we have the right key: add them to our list
+            # The host_groups may have duplicate group_ids
+            host_groups.extend(value)
+
+    # If we found any host groups at the end of that, store them
+    if host_groups:
+        setattr(request, host_group_attr, host_groups)
+        LOG.info(f"User has host groups {host_groups}")
diff --git a/tests/data_files/mock_rbac_returns_array_of_groups.json b/tests/data_files/mock_rbac_returns_array_of_groups.json
@@ -0,0 +1,63 @@
+{
+    "meta": {
+        "count": 3,
+        "limit": 1000,
+        "offset": 0
+    },
+    "links": {
+        "first": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0",
+        "next": null,
+        "previous": null,
+        "last": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0"
+    },
+    "data": [
+        {
+            "resourceDefinitions": [],
+            "permission": "ros:*:*"
+        },
+        {
+            "resourceDefinitions": [
+                {
+                    "attributeFilter": {
+                        "key": "group.id",
+                        "value": [
+                            "12345678-fe1b-4191-8408-cbadbd47f7a3",
+                            "abcdefgh-d97e-4ed0-9095-ef07d73b4839",
+                            "d4e2fc0f-617d-49d5-8d1b-acbb423f0fbe"
+                        ],
+                        "operation": "in"
+                    }
+                }
+            ],
+            "permission": "inventory:hosts:read"
+        },
+        {
+            "resourceDefinitions": [
+                {
+                    "attributeFilter": {
+                        "key": "group.id",
+                        "value": [
+                            "d4e2fc0f-617d-49d5-8d1b-acbb423f0fbe"
+                        ],
+                        "operation": "in"
+                    }
+                }
+            ],
+            "permission": "inventory:hosts:read"
+        },
+        {
+            "resourceDefinitions": [
+                {
+                    "attributeFilter": {
+                        "key": "group.id",
+                        "value": [
+                            "d4e2fc0f-617d-49d5-8d1b-acbb423f0fbe"
+                        ],
+                        "operation": "in"
+                    }
+                }
+            ],
+            "permission": "inventory:groups:read"
+        }
+    ]
+}
diff --git a/tests/data_files/mock_rbac_returns_emtpy_group.json b/tests/data_files/mock_rbac_returns_emtpy_group.json
@@ -5,10 +5,10 @@
    "offset": 0
  },
  "links": {
-   "first": "/api/rbac/v1/access/?application=advisor%2Cinventory&limit=7&offset=0",
+   "first": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0",
    "next": null,
    "previous": null,
-   "last": "/api/rbac/v1/access/?application=advisor%2Cinventory&limit=7&offset=0"
+   "last": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0"
  },
   "data": [
     {

diff --git a/tests/data_files/mock_rbac_returns_groups_including_example_group.json b/tests/data_files/mock_rbac_returns_groups_including_example_group.json
@@ -5,10 +5,10 @@
    "offset": 0
  },
  "links": {
-   "first": "/api/rbac/v1/access/?application=advisor%2Cinventory&limit=7&offset=0",
+   "first": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0",
    "next": null,
    "previous": null,
-   "last": "/api/rbac/v1/access/?application=advisor%2Cinventory&limit=7&offset=0"
+   "last": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0"
  },
  "data": [
    {
@@ -20,9 +20,9 @@
        {
          "attributeFilter": {
            "key": "group.id",
+           "_comment": "The example-group id",
            "value": [
              "12345678-fe1b-4191-8408-cbadbd47f7a3",
-             "99999999-d97e-4ed0-9095-ef07d73b4839",
              null
            ],
            "operation": "in"

diff --git a/tests/data_files/mock_rbac_returns_multiple_read_permissions.json b/tests/data_files/mock_rbac_returns_multiple_read_permissions.json
@@ -0,0 +1,78 @@
+{
+  "meta": {
+    "count": 5,
+    "limit": 1000,
+    "offset": 0
+  },
+  "links": {
+
+    "first": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0",
+    "next": null,
+    "previous": null,
+    "last": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0"
+  },
+  "data": [
+    {
+      "resourceDefinitions": [],
+      "permission": "ros:*:*"
+    },
+    {
+      "resourceDefinitions": [
+        {
+          "attributeFilter": {
+            "key": "group.id",
+            "value": [
+              "12345678-fe1b-4191-8408-cbadbd47f7a3"
+            ],
+            "operation": "in"
+          }
+        }
+      ],
+      "permission": "inventory:groups:read"
+    },
+    {
+      "resourceDefinitions": [
+        {
+          "attributeFilter": {
+            "key": "group.id",
+            "value": [
+              "155860d5-648c-4529-847a-690cbf198934"
+            ],
+            "operation": "in"
+          }
+        }
+      ],
+      "permission": "inventory:groups:write"
+    },
+    {
+      "resourceDefinitions": [
+        {
+          "attributeFilter": {
+            "key": "group.id",
+            "_comment": "The test-group id",
+            "value": [
+              "abcdefgh-d97e-4ed0-9095-ef07d73b4839"
+            ],
+            "operation": "in"
+          }
+        }
+      ],
+      "permission": "inventory:hosts:read"
+    },
+    {
+      "resourceDefinitions": [
+        {
+          "attributeFilter": {
+            "key": "group.id",
+            "_comment": "The foo-group id",
+            "value": [
+              "d4e2fc0f-617d-49d5-8d1b-acbb423f0fbe"
+            ],
+            "operation": "in"
+          }
+        }
+      ],
+      "permission": "inventory:hosts:read"
+    }
+  ]
+}
diff --git a/tests/data_files/mock_rbac_returns_multiple_types_of_read_permissions.json b/tests/data_files/mock_rbac_returns_multiple_types_of_read_permissions.json
@@ -0,0 +1,91 @@
+{
+  "meta": {
+    "count": 5,
+    "limit": 1000,
+    "offset": 0
+  },
+  "links": {
+    "first": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0",
+    "next": null,
+    "previous": null,
+    "last": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0"
+  },
+  "data": [
+    {
+      "resourceDefinitions": [],
+      "permission": "ros:*:*"
+    },
+    {
+      "resourceDefinitions": [
+        {
+          "attributeFilter": {
+            "key": "group.id",
+            "value": [
+              "404c617d-3dc7-4de6-912b-bde1312e1ce5"
+            ],
+            "operation": "in"
+          }
+        }
+      ],
+      "permission": "inventory:groups:read"
+    },
+    {
+      "resourceDefinitions": [
+        {
+          "attributeFilter": {
+            "key": "group.id",
+            "value": [
+              "155860d5-648c-4529-847a-690cbf198934"
+            ],
+            "operation": "in"
+          }
+        }
+      ],
+      "permission": "inventory:groups:write"
+    },
+    {
+      "resourceDefinitions": [
+        {
+          "attributeFilter": {
+            "key": "group.id",
+            "_comment": "The example-group id",
+            "value": [
+              "12345678-fe1b-4191-8408-cbadbd47f7a3"
+            ],
+            "operation": "in"
+          }
+        }
+      ],
+      "permission": "inventory:hosts:read"
+    },
+    {
+      "resourceDefinitions": [
+        {
+          "attributeFilter": {
+            "key": "group.id",
+            "value": [
+              "abcdefgh-d97e-4ed0-9095-ef07d73b4839"
+            ],
+            "operation": "in"
+          }
+        }
+      ],
+      "permission": "inventory:hosts:*"
+    },
+    {
+      "resourceDefinitions": [
+        {
+          "attributeFilter": {
+            "key": "group.id",
+            "_comment": "The foo-group id",
+            "value": [
+              "d4e2fc0f-617d-49d5-8d1b-acbb423f0fbe"
+            ],
+            "operation": "in"
+          }
+        }
+      ],
+      "permission": "inventory:*:read"
+    }
+  ]
+}
diff --git a/tests/data_files/mock_rbac_returns_no_groups.json b/tests/data_files/mock_rbac_returns_no_groups.json
@@ -5,10 +5,10 @@
    "offset": 0
  },
  "links": {
-   "first": "/api/rbac/v1/access/?application=advisor%2Cinventory&limit=7&offset=0",
+   "first": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0",
    "next": null,
    "previous": null,
-   "last": "/api/rbac/v1/access/?application=advisor%2Cinventory&limit=7&offset=0"
+   "last": "/api/rbac/v1/access/?application=ros%2Cinventory&limit=1000&offset=0"
  },
   "data": [
     {

diff --git a/tests/fixtures/db_fixtures_for_inventory_groups.py b/tests/fixtures/db_fixtures_for_inventory_groups.py
@@ -48,9 +48,31 @@ def system_with_test_group():
     db.session.commit()
 
 
+@pytest.fixture
+def system_with_foo_group():
+    system = System(
+        id=4,
+        tenant_id=1,
+        inventory_id='88888888-d97e-4ed0-9095-ef07d73b4839',
+        display_name='ip-181-36-37-38.ap-north-1.compute.internal',
+        fqdn='ip-181-36-37-38.ap-north-1.compute.internal',
+        cloud_provider='aws',
+        instance_type='t2.micro',
+        state='Idling',
+        region='ap-north-1',
+        operating_system={"name": "RHEL", "major": 8, "minor": 9},
+        cpu_states=['CPU_UNDERSIZED', 'CPU_UNDERSIZED_BY_PRESSURE'],
+        io_states=['IO_UNDERSIZED_BY_PRESSURE'],
+        memory_states=['MEMORY_UNDERSIZED', 'MEMORY_UNDERSIZED_BY_PRESSURE'],
+        groups=[{"id": "d4e2fc0f-617d-49d5-8d1b-acbb423f0fbe", "name": "foo-group"}]
+    )
+    db.session.add(system)
+    db.session.commit()
+
+
 @pytest.fixture
 def create_performance_profiles():
-    for sys_id in range(2, 4):
+    for sys_id in range(2, 5):
         db_create_performance_profile(sys_id)