From 4a5e4659a35253d69d2a5d55dd5cf484189e2a55 Mon Sep 17 00:00:00 2001 From: Amit Upadhye Date: Thu, 21 Sep 2023 22:39:12 +0530 Subject: [PATCH 1/2] Refs #RHIROS-1312 - handle inventory and hosts admin --- ros/api/common/add_group_filter.py | 15 +++++- ros/lib/rbac_interface.py | 14 ++++- ...s_inventory_hosts_read_without_groups.json | 53 +++++++++++++++++++ ...s_inventory_hosts_star_without_groups.json | 53 +++++++++++++++++++ ...ns_inventory_star_read_without_groups.json | 53 +++++++++++++++++++ ...ns_inventory_star_star_without_groups.json | 53 +++++++++++++++++++ tests/test_api_endpoints.py | 30 +++++++++++ 7 files changed, 269 insertions(+), 2 deletions(-) create mode 100644 tests/data_files/mock_rbac_returns_inventory_hosts_read_without_groups.json create mode 100644 tests/data_files/mock_rbac_returns_inventory_hosts_star_without_groups.json create mode 100644 tests/data_files/mock_rbac_returns_inventory_star_read_without_groups.json create mode 100644 tests/data_files/mock_rbac_returns_inventory_star_star_without_groups.json diff --git a/ros/api/common/add_group_filter.py b/ros/api/common/add_group_filter.py index 4db89691..c50d41d6 100644 --- a/ros/api/common/add_group_filter.py +++ b/ros/api/common/add_group_filter.py @@ -8,6 +8,9 @@ def group_filtered_query(query): + if able_to_access_all_systems(): + return query + total_groups_from_request = get_host_groups() len_of_total_groups = len(total_groups_from_request) no_none_groups = [grp for grp in total_groups_from_request if grp is not None] @@ -29,5 +32,15 @@ def get_host_groups(): try: host_groups = [gid for gid in request.host_groups] except AttributeError as e: - logger.debug(f"Can't parse the host groups, inventory groups feature is not available?: {e}") + logger.debug(f"Can't parse the host_groups, setting host_groups attr to default empty array([])!: {e}") return host_groups + + +def able_to_access_all_systems(): + access_all_systems = False + try: + access_all_systems = request.able_to_access_all_systems + except AttributeError as e: + logger.debug("Can't parse the able_to_access_all_systems," + f"setting able_to_access_all_systems attr to default False value!: {e}") + return access_all_systems diff --git a/ros/lib/rbac_interface.py b/ros/lib/rbac_interface.py index 2725107c..cb6fc2f6 100644 --- a/ros/lib/rbac_interface.py +++ b/ros/lib/rbac_interface.py @@ -12,6 +12,7 @@ VALID_HTTP_VERBS = ["get", "options", "head", "post", "put", "patch", "delete"] LOG = get_logger(__name__) host_group_attr = 'host_groups' +access_all_systems = 'able_to_access_all_systems' def fetch_url(url, auth_header, logger, method="get"): @@ -158,18 +159,26 @@ def set_host_groups(rbac_response): role_list = rbac_response['data'] host_groups = [] - + able_to_access_all_systems = False for role in role_list: if 'permission' not in role: continue 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 if 'resourceDefinitions' not in role: continue if not isinstance(role['resourceDefinitions'], list): continue + + if len(role['resourceDefinitions']) == 0 and role['permission'] in ['inventory:hosts:*', 'inventory:hosts:read', + 'inventory:*:*', 'inventory:*:read']: + able_to_access_all_systems = True + # If user is inventory or hosts admin then we break the loop and don't check for next roles + break + for rscdef in role['resourceDefinitions']: if not isinstance(rscdef, dict): continue @@ -198,3 +207,6 @@ def set_host_groups(rbac_response): if host_groups: setattr(request, host_group_attr, host_groups) LOG.info(f"User has host groups {host_groups}") + + # Set admin even if we don't find it true + setattr(request, access_all_systems, able_to_access_all_systems) diff --git a/tests/data_files/mock_rbac_returns_inventory_hosts_read_without_groups.json b/tests/data_files/mock_rbac_returns_inventory_hosts_read_without_groups.json new file mode 100644 index 00000000..5e8a0dc8 --- /dev/null +++ b/tests/data_files/mock_rbac_returns_inventory_hosts_read_without_groups.json @@ -0,0 +1,53 @@ +{ + "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": [], + "permission": "inventory:hosts:read" + }, + { + "resourceDefinitions": [ + { + "attributeFilter": { + "key": "group.id", + "_comment": "The test-group id", + "value": [ + "abcdefgh-d97e-4ed0-9095-ef07d73b4839" + ], + "operation": "in" + } + } + ], + "permission": "inventory:hosts:read" + } + ] +} \ No newline at end of file diff --git a/tests/data_files/mock_rbac_returns_inventory_hosts_star_without_groups.json b/tests/data_files/mock_rbac_returns_inventory_hosts_star_without_groups.json new file mode 100644 index 00000000..4024635d --- /dev/null +++ b/tests/data_files/mock_rbac_returns_inventory_hosts_star_without_groups.json @@ -0,0 +1,53 @@ +{ + "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": [], + "permission": "inventory:hosts:*" + }, + { + "resourceDefinitions": [ + { + "attributeFilter": { + "key": "group.id", + "_comment": "The test-group id", + "value": [ + "abcdefgh-d97e-4ed0-9095-ef07d73b4839" + ], + "operation": "in" + } + } + ], + "permission": "inventory:hosts:read" + } + ] +} \ No newline at end of file diff --git a/tests/data_files/mock_rbac_returns_inventory_star_read_without_groups.json b/tests/data_files/mock_rbac_returns_inventory_star_read_without_groups.json new file mode 100644 index 00000000..2fc66da6 --- /dev/null +++ b/tests/data_files/mock_rbac_returns_inventory_star_read_without_groups.json @@ -0,0 +1,53 @@ +{ + "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": [], + "permission": "inventory:*:read" + }, + { + "resourceDefinitions": [ + { + "attributeFilter": { + "key": "group.id", + "_comment": "The test-group id", + "value": [ + "abcdefgh-d97e-4ed0-9095-ef07d73b4839" + ], + "operation": "in" + } + } + ], + "permission": "inventory:hosts:read" + } + ] +} \ No newline at end of file diff --git a/tests/data_files/mock_rbac_returns_inventory_star_star_without_groups.json b/tests/data_files/mock_rbac_returns_inventory_star_star_without_groups.json new file mode 100644 index 00000000..89b70c63 --- /dev/null +++ b/tests/data_files/mock_rbac_returns_inventory_star_star_without_groups.json @@ -0,0 +1,53 @@ +{ + "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": [], + "permission": "inventory:*:*" + }, + { + "resourceDefinitions": [ + { + "attributeFilter": { + "key": "group.id", + "_comment": "The test-group id", + "value": [ + "abcdefgh-d97e-4ed0-9095-ef07d73b4839" + ], + "operation": "in" + } + } + ], + "permission": "inventory:hosts:read" + } + ] +} \ No newline at end of file diff --git a/tests/test_api_endpoints.py b/tests/test_api_endpoints.py index d4626931..552f2385 100644 --- a/tests/test_api_endpoints.py +++ b/tests/test_api_endpoints.py @@ -629,3 +629,33 @@ def test_systems_array_of_groups_with_ungrouped( assert response.json["data"][1]["groups"][0]["name"] == "test-group" assert response.json["data"][2]["groups"][0]["name"] == "example-group" assert response.json["data"][3]["groups"] == [] + + +def test_access_of_all_systems( + auth_token, + db_setup, + db_create_account, + db_create_system, + system_with_example_group, + system_with_test_group, + system_with_foo_group, + db_create_performance_profile, + create_performance_profiles, + mocker): + """When user has 'inventory:hosts:*' or 'inventory:hosts:read' or 'inventory:*:*' or 'inventory:*:read' permissions + and resource definition does not have any groups it means user can see all the systems on ROS""" + with app.test_client() as client: + mock_enable_rbac(mocker) + rbac_mocks = ['mock_rbac_returns_inventory_hosts_star_without_groups.json', + 'mock_rbac_returns_inventory_hosts_read_without_groups.json', + 'mock_rbac_returns_inventory_star_star_without_groups.json', + 'mock_rbac_returns_inventory_star_read_without_groups.json'] + for mocks in rbac_mocks: + mock_rbac(get_rbac_mock_file(mocks), mocker) + response = client.get('/api/ros/v1/systems', headers={"x-rh-identity": auth_token}) + assert response.status_code == 200 + assert response.json["meta"]["count"] == 4 + assert response.json["data"][0]["groups"][0]["name"] == "foo-group" + assert response.json["data"][1]["groups"][0]["name"] == "test-group" + assert response.json["data"][2]["groups"][0]["name"] == "example-group" + assert response.json["data"][3]["groups"] == [] From 988ee325bd2100ae222f6b8261bd5c9800c251ef Mon Sep 17 00:00:00 2001 From: Amit Upadhye Date: Fri, 22 Sep 2023 20:46:47 +0530 Subject: [PATCH 2/2] Use set instead of list to avoid dups --- ros/lib/rbac_interface.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ros/lib/rbac_interface.py b/ros/lib/rbac_interface.py index cb6fc2f6..1cd05141 100644 --- a/ros/lib/rbac_interface.py +++ b/ros/lib/rbac_interface.py @@ -158,7 +158,7 @@ def set_host_groups(rbac_response): return role_list = rbac_response['data'] - host_groups = [] + host_groups = set() able_to_access_all_systems = False for role in role_list: if 'permission' not in role: @@ -201,7 +201,7 @@ def set_host_groups(rbac_response): continue # Finally, we have the right key: add them to our list # The host_groups may have duplicate group_ids - host_groups.extend(value) + host_groups.update(value) # If we found any host groups at the end of that, store them if host_groups: