From 560c17f9b067120c34b880cd223611402f2faf72 Mon Sep 17 00:00:00 2001 From: Maxim Zhiltsov Date: Tue, 5 Dec 2023 14:42:03 +0300 Subject: [PATCH] Fix list GT settings permissions in an org (#7190) - Fixed a 500 error for supervisors and workers when they tried to list settings in an org context - Added possible 403 and 404 errors for the task filter --- ...4_mzhiltsov_fix_gt_settings_permissions.md | 4 ++ cvat/apps/iam/permissions.py | 9 ++++- cvat/apps/iam/rules/quality_settings.rego | 2 +- tests/python/rest_api/test_quality_control.py | 39 ++++++++++--------- 4 files changed, 34 insertions(+), 20 deletions(-) create mode 100644 changelog.d/20231127_163524_mzhiltsov_fix_gt_settings_permissions.md diff --git a/changelog.d/20231127_163524_mzhiltsov_fix_gt_settings_permissions.md b/changelog.d/20231127_163524_mzhiltsov_fix_gt_settings_permissions.md new file mode 100644 index 000000000000..12a04febdd66 --- /dev/null +++ b/changelog.d/20231127_163524_mzhiltsov_fix_gt_settings_permissions.md @@ -0,0 +1,4 @@ +### Fixed + +- Server error in list quality settings API, when called in an org + () diff --git a/cvat/apps/iam/permissions.py b/cvat/apps/iam/permissions.py index d79979747930..ed044519f4dc 100644 --- a/cvat/apps/iam/permissions.py +++ b/cvat/apps/iam/permissions.py @@ -54,7 +54,7 @@ class PermissionResult: def get_organization(request, obj): # Try to get organization from an object otherwise, return the organization that is specified in query parameters - if obj is not None and isinstance(obj, Organization): + if isinstance(obj, Organization): return obj if obj: @@ -1942,6 +1942,13 @@ def create(cls, request, view, obj, iam_context): permissions.append(TaskPermission.create_base_perm( request, view, iam_context=iam_context, scope=task_scope, obj=obj.task )) + elif scope == cls.Scopes.LIST: + if task_id := request.query_params.get("task_id", None): + permissions.append(TaskPermission.create_scope_view( + request, int(task_id), iam_context=iam_context, + )) + + permissions.append(cls.create_scope_list(request, iam_context)) else: permissions.append(cls.create_base_perm(request, view, scope, iam_context, obj)) diff --git a/cvat/apps/iam/rules/quality_settings.rego b/cvat/apps/iam/rules/quality_settings.rego index 97f8ca138a47..1ed7a6bded37 100644 --- a/cvat/apps/iam/rules/quality_settings.rego +++ b/cvat/apps/iam/rules/quality_settings.rego @@ -92,7 +92,7 @@ filter := [] { # Django Q object to filter list of entries user := input.auth.user org := input.auth.organization qobject := [ - {"task__organization": org.id}, "|", + {"task__organization": org.id}, {"task__project__organization": org.id}, "|", {"task__owner_id": user.id}, diff --git a/tests/python/rest_api/test_quality_control.py b/tests/python/rest_api/test_quality_control.py index acd1d38f1d25..58df82b1b8f5 100644 --- a/tests/python/rest_api/test_quality_control.py +++ b/tests/python/rest_api/test_quality_control.py @@ -741,21 +741,23 @@ def test_can_use_simple_filter_for_object_list(self, field): @pytest.mark.usefixtures("restore_db_per_class") class TestListSettings(_PermissionTestBase): def _test_list_settings_200( - self, user: str, obj_id: int, *, expected_data: Optional[Dict[str, Any]] = None, **kwargs + self, user: str, task_id: int, *, expected_data: Optional[Dict[str, Any]] = None, **kwargs ): with make_api_client(user) as api_client: - (_, response) = api_client.quality_api.retrieve_settings(obj_id, **kwargs) - assert response.status == HTTPStatus.OK + actual = get_paginated_collection( + api_client.quality_api.list_settings_endpoint, + task_id=task_id, + **kwargs, + return_json=True, + ) if expected_data is not None: - assert DeepDiff(expected_data, json.loads(response.data), ignore_order=True) == {} - - return response + assert DeepDiff(expected_data, actual, ignore_order=True) == {} - def _test_list_settings_403(self, user: str, obj_id: int, **kwargs): + def _test_list_settings_403(self, user: str, task_id: int, **kwargs): with make_api_client(user) as api_client: - (_, response) = api_client.quality_api.retrieve_settings( - obj_id, **kwargs, _parse_response=False, _check_status=False + (_, response) = api_client.quality_api.list_settings( + task_id=task_id, **kwargs, _parse_response=False, _check_status=False ) assert response.status == HTTPStatus.FORBIDDEN @@ -776,13 +778,12 @@ def test_user_list_settings_in_sandbox( else: user = next(u for u in users if not is_task_staff(u["id"], task["id"]))["username"] - settings = next(s for s in quality_settings if s["task_id"] == task["id"]) - settings_id = settings["id"] + settings = [s for s in quality_settings if s["task_id"] == task["id"]] if allow: - self._test_list_settings_200(user, settings_id, expected_data=settings) + self._test_list_settings_200(user, task_id=task["id"], expected_data=settings) else: - self._test_list_settings_403(user, settings_id) + self._test_list_settings_403(user, task_id=task["id"]) @pytest.mark.parametrize( "org_role, is_staff, allow", @@ -797,7 +798,7 @@ def test_user_list_settings_in_sandbox( ("worker", False, False), ], ) - def test_user_get_settings_in_org_task( + def test_user_list_settings_in_org_task( self, tasks, users, @@ -827,13 +828,15 @@ def test_user_get_settings_in_org_task( assert task - settings = next(s for s in quality_settings if s["task_id"] == task["id"]) - settings_id = settings["id"] + settings = [s for s in quality_settings if s["task_id"] == task["id"]] + org_id = task["organization"] if allow: - self._test_list_settings_200(user["username"], settings_id, expected_data=settings) + self._test_list_settings_200( + user["username"], task_id=task["id"], expected_data=settings, org_id=org_id + ) else: - self._test_list_settings_403(user["username"], settings_id) + self._test_list_settings_403(user["username"], task_id=task["id"], org_id=org_id) @pytest.mark.usefixtures("restore_db_per_class")