Skip to content

Commit d8b47d8

Browse files
committed
feat: make scope optional for action validation
1 parent e5b097e commit d8b47d8

5 files changed

Lines changed: 160 additions & 11 deletions

File tree

openedx_authz/api/users.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
"get_visible_role_assignments_for_user",
6363
"get_visible_user_role_assignments_filtered_by_current_user",
6464
"is_user_allowed",
65+
"is_user_allowed_in_any_scope",
6566
"get_scopes_for_user_and_permission",
6667
"get_users_for_role_in_scope",
6768
"unassign_all_roles_from_user",
@@ -413,6 +414,28 @@ def is_user_allowed(
413414
)
414415

415416

417+
def is_user_allowed_in_any_scope(
418+
user_external_key: str,
419+
action_external_key: str,
420+
) -> bool:
421+
"""Check if a user has a specific permission in at least one scope.
422+
423+
Staff and superusers are always allowed, since they implicitly have every
424+
permission across all scopes.
425+
426+
Args:
427+
user_external_key (str): ID of the user (e.g., 'john_doe').
428+
action_external_key (str): The action to check (e.g., 'manage_library_team').
429+
430+
Returns:
431+
bool: True if the user is staff/superuser or has the specified permission
432+
in any scope, False otherwise.
433+
"""
434+
if is_user_staff_or_superuser(user_external_key):
435+
return True
436+
return bool(get_scopes_for_user_and_permission(user_external_key, action_external_key))
437+
438+
416439
def get_users_for_role_in_scope(role_external_key: str, scope_external_key: str) -> list[UserData]:
417440
"""Get all the users assigned to a specific role in a specific scope.
418441

openedx_authz/rest_api/v1/serializers.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,14 +64,27 @@ class OrgMixin(serializers.Serializer): # pylint: disable=abstract-method
6464

6565

6666
class PermissionValidationSerializer(ActionMixin, ScopeMixin): # pylint: disable=abstract-method
67-
"""Serializer for permission validation request."""
67+
"""Serializer for permission validation request.
68+
69+
The ``scope`` is optional: when provided, the permission is validated against that
70+
scope; when omitted, the permission is validated across any scope.
71+
"""
72+
73+
scope = serializers.CharField(max_length=255, required=False, allow_null=True)
6874

6975

7076
class PermissionValidationResponseSerializer(PermissionValidationSerializer): # pylint: disable=abstract-method
7177
"""Serializer for permission validation response."""
7278

7379
allowed = serializers.BooleanField()
7480

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+
7588

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

openedx_authz/rest_api/v1/views.py

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,16 @@ class PermissionValidationMeView(APIView):
104104
Expects a list of permission objects, each containing:
105105
106106
- action: The action to validate (e.g., 'content_libraries.edit_library_content')
107-
- scope: The authorization scope (e.g., 'lib:DemoX:CSPROB')
107+
- scope (Optional): The authorization scope (e.g., 'lib:DemoX:CSPROB'). When omitted,
108+
the permission is validated across any scope (the user is allowed if they have the
109+
permission in at least one scope).
108110
109111
**Response Format**
110112
111113
Returns a list of validation results, each containing:
112114
113115
- action: The requested action
114-
- scope: The requested scope
116+
- scope: The requested scope (only present when a scope was provided in the request)
115117
- allowed: Boolean indicating if the user has the permission
116118
117119
**Authentication and Permissions**
@@ -133,6 +135,22 @@ class PermissionValidationMeView(APIView):
133135
{"action": "edit_library", "scope": "lib:DemoX:CSPROB", "allowed": true},
134136
{"action": "delete_library_content", "scope": "lib:OpenedX:CS50", "allowed": false}
135137
]
138+
139+
**Example Request (without scope)**
140+
141+
POST /api/authz/v1/permissions/validate/me::
142+
143+
[
144+
{"action": "content_libraries.manage_library_team"},
145+
{"action": "courses.manage_course_team"}
146+
]
147+
148+
**Example Response (without scope)**::
149+
150+
[
151+
{"action": "content_libraries.manage_library_team", "allowed": true},
152+
{"action": "courses.manage_course_team", "allowed": false}
153+
]
136154
"""
137155

138156
@apidocs.schema(
@@ -155,9 +173,13 @@ def post(self, request: HttpRequest) -> Response:
155173
for permission in data:
156174
try:
157175
action = permission["action"]
158-
scope = permission["scope"]
159-
allowed = api.is_user_allowed(username, action, scope)
160-
response_data.append({"action": action, "scope": scope, "allowed": allowed})
176+
scope = permission.get("scope")
177+
if scope:
178+
allowed = api.is_user_allowed(username, action, scope)
179+
response_data.append({"action": action, "scope": scope, "allowed": allowed})
180+
else:
181+
allowed = api.is_user_allowed_in_any_scope(username, action)
182+
response_data.append({"action": action, "allowed": allowed})
161183
except ValueError as e:
162184
logger.error(f"Error validating permission for user {username}: {e}")
163185
return Response(data={"message": "Invalid scope format"}, status=status.HTTP_400_BAD_REQUEST)

openedx_authz/tests/api/test_users.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
get_visible_role_assignments_for_user,
3232
get_visible_user_role_assignments_filtered_by_current_user,
3333
is_user_allowed,
34+
is_user_allowed_in_any_scope,
3435
unassign_all_roles_from_user,
3536
unassign_role_from_user,
3637
validate_users,
@@ -605,6 +606,45 @@ def test_is_user_allowed(self, username, action, scope_name, expected_result):
605606
)
606607
self.assertEqual(result, expected_result)
607608

609+
@data(
610+
# alice is library_admin on lib:Org1:math_101, so she holds these in at least one scope.
611+
("alice", permissions.DELETE_LIBRARY.identifier, True),
612+
("alice", permissions.MANAGE_LIBRARY_TEAM.identifier, True),
613+
# jane is library_user on lib:Org1:english_101, which never grants these permissions.
614+
("jane", permissions.DELETE_LIBRARY.identifier, False),
615+
("jane", permissions.MANAGE_LIBRARY_TEAM.identifier, False),
616+
# A user without any assignment is not allowed in any scope.
617+
("nonexistent_user", permissions.MANAGE_LIBRARY_TEAM.identifier, False),
618+
)
619+
@unpack
620+
def test_is_user_allowed_in_any_scope(self, username, action, expected_result):
621+
"""Test checking if a user holds a permission in at least one scope.
622+
623+
Expected result:
624+
- The function returns True when the user has the permission in any scope,
625+
and False when the user has it in no scope.
626+
"""
627+
result = is_user_allowed_in_any_scope(
628+
user_external_key=username,
629+
action_external_key=action,
630+
)
631+
self.assertEqual(result, expected_result)
632+
633+
def test_is_user_allowed_in_any_scope_staff_always_allowed(self):
634+
"""Staff/superusers are allowed for any action regardless of explicit assignments.
635+
636+
Expected result:
637+
- The function returns True for a staff user that has no role assignments.
638+
"""
639+
User = get_user_model()
640+
User.objects.create_user(username="staff_member", email="staff_member@example.com", is_staff=True)
641+
642+
result = is_user_allowed_in_any_scope(
643+
user_external_key="staff_member",
644+
action_external_key=permissions.MANAGE_LIBRARY_TEAM.identifier,
645+
)
646+
self.assertTrue(result)
647+
608648

609649
@ddt
610650
class TestValidateUsersAPI(UserAssignmentsSetupMixin):

openedx_authz/tests/rest_api/test_views.py

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,6 @@ def test_permission_validation_staff_superuser_access(self, scope: str, expected
259259

260260
@data(
261261
# Single permission
262-
[{"action": "edit_library"}],
263262
[{"scope": "lib:Org1:LIB1"}],
264263
[{"action": "edit_library", "scope": ""}],
265264
[{"action": "edit_library", "scope": "s" * 256}],
@@ -281,10 +280,6 @@ def test_permission_validation_staff_superuser_access(self, scope: str, expected
281280
{"action": "edit_library", "scope": "lib:Org1:LIB1"},
282281
{"scope": "lib:Org1:LIB1"},
283282
],
284-
[
285-
{"action": "edit_library", "scope": "lib:Org1:LIB1"},
286-
{"action": "edit_library"},
287-
],
288283
)
289284
def test_permission_validation_invalid_data(self, invalid_data: list[dict]):
290285
"""Test permission validation with invalid request data.
@@ -296,6 +291,62 @@ def test_permission_validation_invalid_data(self, invalid_data: list[dict]):
296291

297292
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
298293

294+
@data(
295+
# Single action the user has in some scope (LIBRARY_USER on lib:Org1:LIB1) - allowed
296+
([{"action": permissions.VIEW_LIBRARY.identifier}], [True]),
297+
# Single action the user has in no scope - denied
298+
([{"action": permissions.MANAGE_LIBRARY_TEAM.identifier}], [False]),
299+
# Multiple actions - mixed results
300+
(
301+
[
302+
{"action": permissions.VIEW_LIBRARY.identifier},
303+
{"action": permissions.MANAGE_LIBRARY_TEAM.identifier},
304+
{"action": permissions.COURSES_MANAGE_COURSE_TEAM.identifier},
305+
],
306+
[True, False, False],
307+
),
308+
)
309+
@unpack
310+
def test_permission_validation_any_scope_success(self, request_data: list[dict], permission_map: list[bool]):
311+
"""Test permission validation without a scope (any-scope check).
312+
313+
When the scope is omitted, the permission is validated across any scope: the user
314+
is allowed if they hold the permission in at least one scope.
315+
316+
Expected result:
317+
- Returns 200 OK status
318+
- Response omits the scope key and reports the any-scope result
319+
"""
320+
self.client.force_authenticate(user=self.regular_user)
321+
expected_response = [
322+
{"action": perm["action"], "allowed": allowed}
323+
for perm, allowed in zip(request_data, permission_map)
324+
]
325+
326+
response = self.client.post(self.url, data=request_data, format="json")
327+
328+
self.assertEqual(response.status_code, status.HTTP_200_OK)
329+
self.assertEqual(response.data, expected_response)
330+
331+
def test_permission_validation_any_scope_staff_always_allowed(self):
332+
"""Staff/superusers are allowed for any action when no scope is provided.
333+
334+
Expected result:
335+
- Returns 200 OK status
336+
- Every action is allowed regardless of explicit assignments
337+
"""
338+
self.client.force_authenticate(user=self.admin_user)
339+
request_data = [
340+
{"action": permissions.MANAGE_LIBRARY_TEAM.identifier},
341+
{"action": permissions.COURSES_MANAGE_COURSE_TEAM.identifier},
342+
]
343+
expected_response = [{"action": perm["action"], "allowed": True} for perm in request_data]
344+
345+
response = self.client.post(self.url, data=request_data, format="json")
346+
347+
self.assertEqual(response.status_code, status.HTTP_200_OK)
348+
self.assertEqual(response.data, expected_response)
349+
299350
def test_permission_validation_unauthenticated(self):
300351
"""Test permission validation without authentication.
301352

0 commit comments

Comments
 (0)