From d8b47d84dfd4ef439dddd46d2cf8fdf832cb1f5a Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Mon, 29 Jun 2026 21:27:44 +1000 Subject: [PATCH 1/2] feat: make scope optional for action validation --- openedx_authz/api/users.py | 23 ++++++++ openedx_authz/rest_api/v1/serializers.py | 15 +++++- openedx_authz/rest_api/v1/views.py | 32 ++++++++++-- openedx_authz/tests/api/test_users.py | 40 ++++++++++++++ openedx_authz/tests/rest_api/test_views.py | 61 ++++++++++++++++++++-- 5 files changed, 160 insertions(+), 11 deletions(-) diff --git a/openedx_authz/api/users.py b/openedx_authz/api/users.py index e6d3a491..e9bf8a73 100644 --- a/openedx_authz/api/users.py +++ b/openedx_authz/api/users.py @@ -62,6 +62,7 @@ "get_visible_role_assignments_for_user", "get_visible_user_role_assignments_filtered_by_current_user", "is_user_allowed", + "is_user_allowed_in_any_scope", "get_scopes_for_user_and_permission", "get_users_for_role_in_scope", "unassign_all_roles_from_user", @@ -413,6 +414,28 @@ def is_user_allowed( ) +def is_user_allowed_in_any_scope( + user_external_key: str, + action_external_key: str, +) -> bool: + """Check if a user has a specific permission in at least one scope. + + Staff and superusers are always allowed, since they implicitly have every + permission across all scopes. + + Args: + user_external_key (str): ID of the user (e.g., 'john_doe'). + action_external_key (str): The action to check (e.g., 'manage_library_team'). + + Returns: + bool: True if the user is staff/superuser or has the specified permission + in any scope, False otherwise. + """ + if is_user_staff_or_superuser(user_external_key): + return True + return bool(get_scopes_for_user_and_permission(user_external_key, action_external_key)) + + def get_users_for_role_in_scope(role_external_key: str, scope_external_key: str) -> list[UserData]: """Get all the users assigned to a specific role in a specific scope. diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index 0d7e83bc..e178d02b 100644 --- a/openedx_authz/rest_api/v1/serializers.py +++ b/openedx_authz/rest_api/v1/serializers.py @@ -64,7 +64,13 @@ class OrgMixin(serializers.Serializer): # pylint: disable=abstract-method class PermissionValidationSerializer(ActionMixin, ScopeMixin): # pylint: disable=abstract-method - """Serializer for permission validation request.""" + """Serializer for permission validation request. + + The ``scope`` is optional: when provided, the permission is validated against that + scope; when omitted, the permission is validated across any scope. + """ + + scope = serializers.CharField(max_length=255, required=False, allow_null=True) class PermissionValidationResponseSerializer(PermissionValidationSerializer): # pylint: disable=abstract-method @@ -72,6 +78,13 @@ class PermissionValidationResponseSerializer(PermissionValidationSerializer): # allowed = serializers.BooleanField() + def to_representation(self, instance: dict) -> dict: + """Serialize the result, omitting ``scope`` when the request had no scope.""" + representation = super().to_representation(instance) + if instance.get("scope") is None: + representation.pop("scope", None) + return representation + class RoleScopeValidationMixin(serializers.Serializer): # pylint: disable=abstract-method """Mixin providing role and scope validation logic.""" diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index 3f061104..62bd2d4f 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -104,14 +104,16 @@ class PermissionValidationMeView(APIView): Expects a list of permission objects, each containing: - action: The action to validate (e.g., 'content_libraries.edit_library_content') - - scope: The authorization scope (e.g., 'lib:DemoX:CSPROB') + - scope (Optional): The authorization scope (e.g., 'lib:DemoX:CSPROB'). When omitted, + the permission is validated across any scope (the user is allowed if they have the + permission in at least one scope). **Response Format** Returns a list of validation results, each containing: - action: The requested action - - scope: The requested scope + - scope: The requested scope (only present when a scope was provided in the request) - allowed: Boolean indicating if the user has the permission **Authentication and Permissions** @@ -133,6 +135,22 @@ class PermissionValidationMeView(APIView): {"action": "edit_library", "scope": "lib:DemoX:CSPROB", "allowed": true}, {"action": "delete_library_content", "scope": "lib:OpenedX:CS50", "allowed": false} ] + + **Example Request (without scope)** + + POST /api/authz/v1/permissions/validate/me:: + + [ + {"action": "content_libraries.manage_library_team"}, + {"action": "courses.manage_course_team"} + ] + + **Example Response (without scope)**:: + + [ + {"action": "content_libraries.manage_library_team", "allowed": true}, + {"action": "courses.manage_course_team", "allowed": false} + ] """ @apidocs.schema( @@ -155,9 +173,13 @@ def post(self, request: HttpRequest) -> Response: for permission in data: try: action = permission["action"] - scope = permission["scope"] - allowed = api.is_user_allowed(username, action, scope) - response_data.append({"action": action, "scope": scope, "allowed": allowed}) + scope = permission.get("scope") + if scope: + allowed = api.is_user_allowed(username, action, scope) + response_data.append({"action": action, "scope": scope, "allowed": allowed}) + else: + allowed = api.is_user_allowed_in_any_scope(username, action) + response_data.append({"action": action, "allowed": allowed}) except ValueError as e: logger.error(f"Error validating permission for user {username}: {e}") return Response(data={"message": "Invalid scope format"}, status=status.HTTP_400_BAD_REQUEST) diff --git a/openedx_authz/tests/api/test_users.py b/openedx_authz/tests/api/test_users.py index 6da3f193..93ea6f74 100644 --- a/openedx_authz/tests/api/test_users.py +++ b/openedx_authz/tests/api/test_users.py @@ -31,6 +31,7 @@ get_visible_role_assignments_for_user, get_visible_user_role_assignments_filtered_by_current_user, is_user_allowed, + is_user_allowed_in_any_scope, unassign_all_roles_from_user, unassign_role_from_user, validate_users, @@ -605,6 +606,45 @@ def test_is_user_allowed(self, username, action, scope_name, expected_result): ) self.assertEqual(result, expected_result) + @data( + # alice is library_admin on lib:Org1:math_101, so she holds these in at least one scope. + ("alice", permissions.DELETE_LIBRARY.identifier, True), + ("alice", permissions.MANAGE_LIBRARY_TEAM.identifier, True), + # jane is library_user on lib:Org1:english_101, which never grants these permissions. + ("jane", permissions.DELETE_LIBRARY.identifier, False), + ("jane", permissions.MANAGE_LIBRARY_TEAM.identifier, False), + # A user without any assignment is not allowed in any scope. + ("nonexistent_user", permissions.MANAGE_LIBRARY_TEAM.identifier, False), + ) + @unpack + def test_is_user_allowed_in_any_scope(self, username, action, expected_result): + """Test checking if a user holds a permission in at least one scope. + + Expected result: + - The function returns True when the user has the permission in any scope, + and False when the user has it in no scope. + """ + result = is_user_allowed_in_any_scope( + user_external_key=username, + action_external_key=action, + ) + self.assertEqual(result, expected_result) + + def test_is_user_allowed_in_any_scope_staff_always_allowed(self): + """Staff/superusers are allowed for any action regardless of explicit assignments. + + Expected result: + - The function returns True for a staff user that has no role assignments. + """ + User = get_user_model() + User.objects.create_user(username="staff_member", email="staff_member@example.com", is_staff=True) + + result = is_user_allowed_in_any_scope( + user_external_key="staff_member", + action_external_key=permissions.MANAGE_LIBRARY_TEAM.identifier, + ) + self.assertTrue(result) + @ddt class TestValidateUsersAPI(UserAssignmentsSetupMixin): diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index c2d3eb5e..1fda2870 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -259,7 +259,6 @@ def test_permission_validation_staff_superuser_access(self, scope: str, expected @data( # Single permission - [{"action": "edit_library"}], [{"scope": "lib:Org1:LIB1"}], [{"action": "edit_library", "scope": ""}], [{"action": "edit_library", "scope": "s" * 256}], @@ -281,10 +280,6 @@ def test_permission_validation_staff_superuser_access(self, scope: str, expected {"action": "edit_library", "scope": "lib:Org1:LIB1"}, {"scope": "lib:Org1:LIB1"}, ], - [ - {"action": "edit_library", "scope": "lib:Org1:LIB1"}, - {"action": "edit_library"}, - ], ) def test_permission_validation_invalid_data(self, invalid_data: list[dict]): """Test permission validation with invalid request data. @@ -296,6 +291,62 @@ def test_permission_validation_invalid_data(self, invalid_data: list[dict]): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + @data( + # Single action the user has in some scope (LIBRARY_USER on lib:Org1:LIB1) - allowed + ([{"action": permissions.VIEW_LIBRARY.identifier}], [True]), + # Single action the user has in no scope - denied + ([{"action": permissions.MANAGE_LIBRARY_TEAM.identifier}], [False]), + # Multiple actions - mixed results + ( + [ + {"action": permissions.VIEW_LIBRARY.identifier}, + {"action": permissions.MANAGE_LIBRARY_TEAM.identifier}, + {"action": permissions.COURSES_MANAGE_COURSE_TEAM.identifier}, + ], + [True, False, False], + ), + ) + @unpack + def test_permission_validation_any_scope_success(self, request_data: list[dict], permission_map: list[bool]): + """Test permission validation without a scope (any-scope check). + + When the scope is omitted, the permission is validated across any scope: the user + is allowed if they hold the permission in at least one scope. + + Expected result: + - Returns 200 OK status + - Response omits the scope key and reports the any-scope result + """ + self.client.force_authenticate(user=self.regular_user) + expected_response = [ + {"action": perm["action"], "allowed": allowed} + for perm, allowed in zip(request_data, permission_map) + ] + + response = self.client.post(self.url, data=request_data, format="json") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data, expected_response) + + def test_permission_validation_any_scope_staff_always_allowed(self): + """Staff/superusers are allowed for any action when no scope is provided. + + Expected result: + - Returns 200 OK status + - Every action is allowed regardless of explicit assignments + """ + self.client.force_authenticate(user=self.admin_user) + request_data = [ + {"action": permissions.MANAGE_LIBRARY_TEAM.identifier}, + {"action": permissions.COURSES_MANAGE_COURSE_TEAM.identifier}, + ] + expected_response = [{"action": perm["action"], "allowed": True} for perm in request_data] + + response = self.client.post(self.url, data=request_data, format="json") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data, expected_response) + def test_permission_validation_unauthenticated(self): """Test permission validation without authentication. From 78f034b7a24bb7dc151861bd09902b26d7b923a7 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Wed, 1 Jul 2026 08:49:43 +1000 Subject: [PATCH 2/2] refactor: address review feedback --- CHANGELOG.rst | 10 ++++++++ openedx_authz/__init__.py | 2 +- openedx_authz/rest_api/v1/serializers.py | 9 +------ openedx_authz/tests/api/test_users.py | 31 +++++++++++++++++++++--- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9fa3398f..55ecf256 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,16 @@ Change Log Unreleased ********** +1.20.0 - 2026-07-01 +******************* + +Added +===== + +* Make ``scope`` optional when validating actions: the permission validation API and + ``/permissions/validate/me`` endpoint now allow checking whether a user holds a + permission in any scope (via ``is_user_allowed_in_any_scope``) when no scope is provided. + 1.19.0 - 2026-06-17 ******************* diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index c49521e0..d8f4c041 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "1.19.0" +__version__ = "1.20.0" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__)) diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index e178d02b..a62572a8 100644 --- a/openedx_authz/rest_api/v1/serializers.py +++ b/openedx_authz/rest_api/v1/serializers.py @@ -70,7 +70,7 @@ class PermissionValidationSerializer(ActionMixin, ScopeMixin): # pylint: disabl scope; when omitted, the permission is validated across any scope. """ - scope = serializers.CharField(max_length=255, required=False, allow_null=True) + scope = serializers.CharField(max_length=255, required=False) class PermissionValidationResponseSerializer(PermissionValidationSerializer): # pylint: disable=abstract-method @@ -78,13 +78,6 @@ class PermissionValidationResponseSerializer(PermissionValidationSerializer): # allowed = serializers.BooleanField() - def to_representation(self, instance: dict) -> dict: - """Serialize the result, omitting ``scope`` when the request had no scope.""" - representation = super().to_representation(instance) - if instance.get("scope") is None: - representation.pop("scope", None) - return representation - class RoleScopeValidationMixin(serializers.Serializer): # pylint: disable=abstract-method """Mixin providing role and scope validation logic.""" diff --git a/openedx_authz/tests/api/test_users.py b/openedx_authz/tests/api/test_users.py index 93ea6f74..9bb236f6 100644 --- a/openedx_authz/tests/api/test_users.py +++ b/openedx_authz/tests/api/test_users.py @@ -613,8 +613,21 @@ def test_is_user_allowed(self, username, action, scope_name, expected_result): # jane is library_user on lib:Org1:english_101, which never grants these permissions. ("jane", permissions.DELETE_LIBRARY.identifier, False), ("jane", permissions.MANAGE_LIBRARY_TEAM.identifier, False), + # daniel is course_staff on course-v1:TestOrg+TestCourse+2024_T1, so he holds the + # staff course permissions in at least one scope. + ("daniel", permissions.COURSES_VIEW_COURSE.identifier, True), + ("daniel", permissions.COURSES_EDIT_COURSE_CONTENT.identifier, True), + ("daniel", permissions.COURSES_PUBLISH_COURSE_CONTENT.identifier, True), + # course_staff never grants team management, so daniel holds it in no scope. + ("daniel", permissions.COURSES_MANAGE_COURSE_TEAM.identifier, False), + # carlos is course_staff on three different courses; he still holds these in some scope. + ("carlos", permissions.COURSES_VIEW_COURSE.identifier, True), + ("carlos", permissions.COURSES_MANAGE_ADVANCED_SETTINGS.identifier, True), + # jane only holds a library role, so she has no course permission in any scope. + ("jane", permissions.COURSES_VIEW_COURSE.identifier, False), # A user without any assignment is not allowed in any scope. ("nonexistent_user", permissions.MANAGE_LIBRARY_TEAM.identifier, False), + ("nonexistent_user", permissions.COURSES_VIEW_COURSE.identifier, False), ) @unpack def test_is_user_allowed_in_any_scope(self, username, action, expected_result): @@ -630,17 +643,27 @@ def test_is_user_allowed_in_any_scope(self, username, action, expected_result): ) self.assertEqual(result, expected_result) - def test_is_user_allowed_in_any_scope_staff_always_allowed(self): + @data( + # Staff-only user + ("staff_member", {"is_staff": True}), + # Superuser-only user + ("superuser_member", {"is_superuser": True}), + # Both staff and superuser + ("staff_superuser_member", {"is_staff": True, "is_superuser": True}), + ) + @unpack + def test_is_user_allowed_in_any_scope_staff_always_allowed(self, username, flags): """Staff/superusers are allowed for any action regardless of explicit assignments. Expected result: - - The function returns True for a staff user that has no role assignments. + - The function returns True for a staff and/or superuser user that has no + role assignments. """ User = get_user_model() - User.objects.create_user(username="staff_member", email="staff_member@example.com", is_staff=True) + User.objects.create_user(username=username, email=f"{username}@example.com", **flags) result = is_user_allowed_in_any_scope( - user_external_key="staff_member", + user_external_key=username, action_external_key=permissions.MANAGE_LIBRARY_TEAM.identifier, ) self.assertTrue(result)