Skip to content

Commit

Permalink
Fix list GT settings permissions in an org (#7190)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
zhiltsov-max authored Dec 5, 2023
1 parent e025b20 commit 560c17f
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Fixed

- Server error in list quality settings API, when called in an org
(<https://github.com/opencv/cvat/pull/7190>)
9 changes: 8 additions & 1 deletion cvat/apps/iam/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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))

Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/iam/rules/quality_settings.rego
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
39 changes: 21 additions & 18 deletions tests/python/rest_api/test_quality_control.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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",
Expand All @@ -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,
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit 560c17f

Please sign in to comment.