Skip to content

Commit ae6d992

Browse files
dcoadcoa
committed
refactor: remove def in serialized and add more test
1 parent d8b47d8 commit ae6d992

3 files changed

Lines changed: 97 additions & 11 deletions

File tree

‎openedx_authz/rest_api/v1/serializers.py‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,21 +70,14 @@ class PermissionValidationSerializer(ActionMixin, ScopeMixin): # pylint: disabl
7070
scope; when omitted, the permission is validated across any scope.
7171
"""
7272

73-
scope = serializers.CharField(max_length=255, required=False, allow_null=True)
73+
scope = serializers.CharField(max_length=255, required=False)
7474

7575

7676
class PermissionValidationResponseSerializer(PermissionValidationSerializer): # pylint: disable=abstract-method
7777
"""Serializer for permission validation response."""
7878

7979
allowed = serializers.BooleanField()
8080

81-
def to_representation(self, instance: dict) -> dict:
82-
"""Serialize the result, omitting ``scope`` when the request had no scope."""
83-
representation = super().to_representation(instance)
84-
if instance.get("scope") is None:
85-
representation.pop("scope", None)
86-
return representation
87-
8881

8982
class RoleScopeValidationMixin(serializers.Serializer): # pylint: disable=abstract-method
9083
"""Mixin providing role and scope validation logic."""

‎openedx_authz/tests/api/test_users.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,8 +613,21 @@ def test_is_user_allowed(self, username, action, scope_name, expected_result):
613613
# jane is library_user on lib:Org1:english_101, which never grants these permissions.
614614
("jane", permissions.DELETE_LIBRARY.identifier, False),
615615
("jane", permissions.MANAGE_LIBRARY_TEAM.identifier, False),
616+
# daniel is course_staff on course-v1:TestOrg+TestCourse+2024_T1, so he holds the
617+
# staff course permissions in at least one scope.
618+
("daniel", permissions.COURSES_VIEW_COURSE.identifier, True),
619+
("daniel", permissions.COURSES_EDIT_COURSE_CONTENT.identifier, True),
620+
("daniel", permissions.COURSES_PUBLISH_COURSE_CONTENT.identifier, True),
621+
# course_staff never grants team management, so daniel holds it in no scope.
622+
("daniel", permissions.COURSES_MANAGE_COURSE_TEAM.identifier, False),
623+
# carlos is course_staff on three different courses; he still holds these in some scope.
624+
("carlos", permissions.COURSES_VIEW_COURSE.identifier, True),
625+
("carlos", permissions.COURSES_MANAGE_ADVANCED_SETTINGS.identifier, True),
626+
# jane only holds a library role, so she has no course permission in any scope.
627+
("jane", permissions.COURSES_VIEW_COURSE.identifier, False),
616628
# A user without any assignment is not allowed in any scope.
617629
("nonexistent_user", permissions.MANAGE_LIBRARY_TEAM.identifier, False),
630+
("nonexistent_user", permissions.COURSES_VIEW_COURSE.identifier, False),
618631
)
619632
@unpack
620633
def test_is_user_allowed_in_any_scope(self, username, action, expected_result):

‎openedx_authz/tests/rest_api/test_views.py‎

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,14 +199,17 @@ def setUp(self):
199199
([{"action": permissions.VIEW_LIBRARY.identifier, "scope": "lib:Org2:LIB2"}], [False]),
200200
# Single permission - denied (action not assigned to user)
201201
([{"action": "content_libraries.edit_library", "scope": "lib:Org1:LIB1"}], [False]),
202+
# Course permission - denied (user holds no course role in the scope)
203+
([{"action": permissions.COURSES_VIEW_COURSE.identifier, "scope": COURSE_SCOPE_ORG1}], [False]),
202204
# Multiple permissions - mixed results
203205
(
204206
[
205207
{"action": permissions.VIEW_LIBRARY.identifier, "scope": "lib:Org1:LIB1"},
206208
{"action": permissions.VIEW_LIBRARY.identifier, "scope": "lib:Org2:LIB2"},
207209
{"action": "content_libraries.edit_library", "scope": "lib:Org1:LIB1"},
210+
{"action": permissions.COURSES_VIEW_COURSE.identifier, "scope": COURSE_SCOPE_ORG1},
208211
],
209-
[True, False, False],
212+
[True, False, False, False],
210213
),
211214
)
212215
@unpack
@@ -296,14 +299,18 @@ def test_permission_validation_invalid_data(self, invalid_data: list[dict]):
296299
([{"action": permissions.VIEW_LIBRARY.identifier}], [True]),
297300
# Single action the user has in no scope - denied
298301
([{"action": permissions.MANAGE_LIBRARY_TEAM.identifier}], [False]),
299-
# Multiple actions - mixed results
302+
# Course actions the user holds in no scope (LIBRARY_USER only) - denied
303+
([{"action": permissions.COURSES_VIEW_COURSE.identifier}], [False]),
304+
([{"action": permissions.COURSES_EDIT_COURSE_CONTENT.identifier}], [False]),
305+
# Multiple actions across namespaces - mixed results
300306
(
301307
[
302308
{"action": permissions.VIEW_LIBRARY.identifier},
303309
{"action": permissions.MANAGE_LIBRARY_TEAM.identifier},
310+
{"action": permissions.COURSES_VIEW_COURSE.identifier},
304311
{"action": permissions.COURSES_MANAGE_COURSE_TEAM.identifier},
305312
],
306-
[True, False, False],
313+
[True, False, False, False],
307314
),
308315
)
309316
@unpack
@@ -347,6 +354,79 @@ def test_permission_validation_any_scope_staff_always_allowed(self):
347354
self.assertEqual(response.status_code, status.HTTP_200_OK)
348355
self.assertEqual(response.data, expected_response)
349356

357+
def test_permission_validation_course_scoped(self):
358+
"""Test scoped permission validation for course permissions.
359+
360+
A non-staff user assigned course_staff on a course holds the staff course
361+
permissions in that scope, but not in an unrelated course scope.
362+
363+
Expected result:
364+
- Returns 200 OK status
365+
- The scope is echoed back and the result reflects the user's course role
366+
"""
367+
assign_role_to_user_in_scope(
368+
user_external_key=self.regular_user.username,
369+
role_external_key=roles.COURSE_STAFF.external_key,
370+
scope_external_key=COURSE_SCOPE_ORG1,
371+
)
372+
self.client.force_authenticate(user=self.regular_user)
373+
request_data = [
374+
# Held in the assigned scope
375+
{"action": permissions.COURSES_VIEW_COURSE.identifier, "scope": COURSE_SCOPE_ORG1},
376+
# course_staff does not manage the course team
377+
{"action": permissions.COURSES_MANAGE_COURSE_TEAM.identifier, "scope": COURSE_SCOPE_ORG1},
378+
# Not held in an unrelated course scope
379+
{
380+
"action": permissions.COURSES_VIEW_COURSE.identifier,
381+
"scope": "course-v1:Org2+COURSE2+2024",
382+
},
383+
]
384+
expected_response = [
385+
{**request_data[0], "allowed": True},
386+
{**request_data[1], "allowed": False},
387+
{**request_data[2], "allowed": False},
388+
]
389+
390+
response = self.client.post(self.url, data=request_data, format="json")
391+
392+
self.assertEqual(response.status_code, status.HTTP_200_OK)
393+
self.assertEqual(response.data, expected_response)
394+
395+
def test_permission_validation_any_scope_course_permissions(self):
396+
"""Test any-scope permission validation for course permissions.
397+
398+
A non-staff user assigned course_staff in at least one course scope holds the
399+
staff course permissions across any scope, but not the permissions the role
400+
never grants.
401+
402+
Expected result:
403+
- Returns 200 OK status
404+
- Response omits the scope key and reports the any-scope result
405+
"""
406+
assign_role_to_user_in_scope(
407+
user_external_key=self.regular_user.username,
408+
role_external_key=roles.COURSE_STAFF.external_key,
409+
scope_external_key=COURSE_SCOPE_ORG1,
410+
)
411+
self.client.force_authenticate(user=self.regular_user)
412+
request_data = [
413+
# Held in some scope via the course_staff role
414+
{"action": permissions.COURSES_VIEW_COURSE.identifier},
415+
{"action": permissions.COURSES_EDIT_COURSE_CONTENT.identifier},
416+
# Never granted by course_staff
417+
{"action": permissions.COURSES_MANAGE_COURSE_TEAM.identifier},
418+
]
419+
expected_response = [
420+
{"action": permissions.COURSES_VIEW_COURSE.identifier, "allowed": True},
421+
{"action": permissions.COURSES_EDIT_COURSE_CONTENT.identifier, "allowed": True},
422+
{"action": permissions.COURSES_MANAGE_COURSE_TEAM.identifier, "allowed": False},
423+
]
424+
425+
response = self.client.post(self.url, data=request_data, format="json")
426+
427+
self.assertEqual(response.status_code, status.HTTP_200_OK)
428+
self.assertEqual(response.data, expected_response)
429+
350430
def test_permission_validation_unauthenticated(self):
351431
"""Test permission validation without authentication.
352432

0 commit comments

Comments
 (0)