diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1c269dbd..d45e9cf2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,7 +14,15 @@ Change Log Unreleased ********** -0.13.1 - 2025-11-06 +0.14.0 - 2025-11-10 +******************** + +Added +===== + +* Implement custom matcher to check for staff and superuser status. + +0.13.1 - 2025-11-10 ******************** Fixed diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index 6438c962..7a509bf1 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "0.13.1" +__version__ = "0.14.0" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__)) diff --git a/openedx_authz/engine/config/model.conf b/openedx_authz/engine/config/model.conf index 89bb2bd8..b4707590 100644 --- a/openedx_authz/engine/config/model.conf +++ b/openedx_authz/engine/config/model.conf @@ -92,4 +92,4 @@ e = some(where (p.eft == allow)) && !some(where (p.eft == deny)) # 1. Subject must have role in scope OR global role # 2. Scope must match pattern # 3. Action must match OR inherit via action grouping -m = (g(r.sub, p.sub, r.scope) || g(r.sub, p.sub, "*")) && keyMatch(r.scope, p.scope) && (r.act == p.act || g2(p.act, r.act)) +m = is_staff_or_superuser(r.sub, r.act, r.scope) || (g(r.sub, p.sub, r.scope) || g(r.sub, p.sub, "*")) && keyMatch(r.scope, p.scope) && (r.act == p.act || g2(p.act, r.act)) diff --git a/openedx_authz/engine/enforcer.py b/openedx_authz/engine/enforcer.py index 866842a2..ee58b81f 100644 --- a/openedx_authz/engine/enforcer.py +++ b/openedx_authz/engine/enforcer.py @@ -22,6 +22,7 @@ from django.conf import settings from openedx_authz.engine.adapter import ExtendedAdapter +from openedx_authz.engine.matcher import is_admin_or_superuser_check def libraries_v2_enabled() -> bool: @@ -195,5 +196,6 @@ def _initialize_enforcer(cls) -> SyncedEnforcer: adapter = ExtendedAdapter() enforcer = SyncedEnforcer(settings.CASBIN_MODEL, adapter) + enforcer.add_function("is_staff_or_superuser", is_admin_or_superuser_check) return enforcer diff --git a/openedx_authz/engine/matcher.py b/openedx_authz/engine/matcher.py new file mode 100644 index 00000000..dea6373d --- /dev/null +++ b/openedx_authz/engine/matcher.py @@ -0,0 +1,40 @@ +"""Custom condition checker. Note only used for data_library scope""" + +from django.contrib.auth import get_user_model + +from openedx_authz.api.data import ContentLibraryData, ScopeData, UserData +from openedx_authz.rest_api.utils import get_user_by_username_or_email + +User = get_user_model() + + +def is_admin_or_superuser_check(request_user: str, request_action: str, request_scope: str) -> bool: # pylint: disable=unused-argument + """ + Evaluates custom, non-role-based conditions for authorization checks. + + Checks attribute-based conditions that don't rely on role assignments. + Currently handles ContentLibraryData scopes by granting access to staff + and superusers. + + Args: + request_user (str): Namespaced user key (format: "user::") + request_action (str): Namespaced action key (format: "action::") + request_scope (str): Namespaced scope key (format: "scope_type::") + + Returns: + bool: True if the condition is satisfied (user is staff/superuser for + ContentLibraryData scopes), False otherwise (including when user + doesn't exist or scope type is not supported) + """ + try: + username = UserData(namespaced_key=request_user).external_key + user = get_user_by_username_or_email(username) + except User.DoesNotExist: + return False + + scope = ScopeData(namespaced_key=request_scope) + + if isinstance(scope, ContentLibraryData): + return user.is_staff or user.is_superuser + + return False diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index fabd14fa..4a962d3a 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -195,6 +195,36 @@ def test_permission_validation_success(self, request_data: list[dict], permissio self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data, expected_response) + @data( + ("lib:AnyOrg1:ANYLIB1", True), + ("lib:AnyOrg2:ANYLIB2", True), + ("lib:AnyOrg3:ANYLIB3", True), + ("global:AnyScope1", False), + ) + @unpack + def test_permission_validation_staff_superuser_access(self, scope: str, expected_result: bool): + """Test that staff/superuser users have guaranteed permissions for ContentLibrary scopes. + + Test cases: + - ContentLibrary scopes (lib:*): Staff/superuser automatically allowed + - Generic scopes (global:*): No automatic access granted + + Expected result: + - Returns 200 OK status + - For library scopes: All permissions are allowed (True) + - For non-library scopes: Permissions follow normal authorization (False) + """ + self.client.force_authenticate(user=self.admin_user) + request_data = [{"action": perm.identifier, "scope": scope} for perm in roles.LIBRARY_ADMIN_PERMISSIONS] + expected_response = request_data.copy() + for item in expected_response: + item["allowed"] = expected_result + + 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) + @data( # Single permission [{"action": "edit_library"}], diff --git a/openedx_authz/tests/test_enforcement.py b/openedx_authz/tests/test_enforcement.py index ccb2144e..13adfe57 100644 --- a/openedx_authz/tests/test_enforcement.py +++ b/openedx_authz/tests/test_enforcement.py @@ -10,10 +10,13 @@ from unittest import TestCase import casbin +import pytest from ddt import data, ddt, unpack +from django.contrib.auth import get_user_model from openedx_authz import ROOT_DIRECTORY from openedx_authz.constants import roles +from openedx_authz.engine.matcher import is_admin_or_superuser_check from openedx_authz.tests.test_utils import ( make_action_key, make_library_key, @@ -22,6 +25,8 @@ make_user_key, ) +User = get_user_model() + class AuthRequest(TypedDict): """ @@ -44,6 +49,7 @@ class AuthRequest(TypedDict): ] +@pytest.mark.django_db @ddt class CasbinEnforcementTestCase(TestCase): """ @@ -65,6 +71,7 @@ def setUpClass(cls) -> None: raise FileNotFoundError(f"Model file not found: {model_file}") cls.enforcer = casbin.Enforcer(model_file) + cls.enforcer.add_function("is_staff_or_superuser", is_admin_or_superuser_check) def _load_policy(self, policy: list[str]) -> None: """ @@ -573,3 +580,84 @@ def test_wildcard_library_access(self, scope: str, expected_result: bool): "expected_result": expected_result, } self._test_enforcement(self.POLICY, request) + + +@pytest.mark.django_db +@ddt +class StaffSuperuserAccessTests(CasbinEnforcementTestCase): + """ + Tests for staff and superuser automatic permission grants via is_staff_or_superuser. + + This test class verifies that staff members and superusers are automatically + granted access to ContentLibrary scopes through the is_admin_or_superuser_check function, + without requiring explicit role assignments. + """ + + # Empty policy - no role assignments for staff/superuser users + POLICY = [] + + def setUp(self) -> None: + """Set up the test environment.""" + super().setUp() + User.objects.create_user(username="staff_user", email="staff@example.com", password="test", is_staff=True) + User.objects.create_superuser(username="superuser", email="super@example.com", password="test") + User.objects.create_user(username="regular_user", email="regular@example.com", password="test") + + @data( + # Staff user has automatic access to any library scope + ( + make_user_key("staff_user"), + make_action_key("view_library"), + make_library_key("lib:TestOrg:TestLib"), + True, + ), + ( + make_user_key("staff_user"), + make_action_key("edit_library"), + make_library_key("lib:AnyOrg:AnyLib"), + True, + ), + # Superuser has automatic access to any library scope + ( + make_user_key("superuser"), + make_action_key("view_library"), + make_library_key("lib:TestOrg:TestLib"), + True, + ), + ( + make_user_key("superuser"), + make_action_key("delete_library"), + make_library_key("lib:AnyOrg:AnyLib"), + True, + ), + # Regular user without role assignment has no access + ( + make_user_key("regular_user"), + make_action_key("view_library"), + make_library_key("lib:TestOrg:TestLib"), + False, + ), + # Non existent library scope access denied + ( + make_user_key("regular_user"), + make_action_key("view_library"), + make_library_key("lib:NonExistent:NoLib"), + False, + ), + ) + @unpack + def test_staff_superuser_guaranteed_permissions(self, subject: str, action: str, scope: str, expected_result: bool): + """Test that staff and superusers have guaranteed permissions for ContentLibrary scopes. + + This test validates that: + - Staff users automatically have access to all library scopes without role assignments + - Superusers automatically have access to all library scopes without role assignments + - Regular users require explicit role assignments to access libraries + - Access is granted through the is_staff_or_superuser matcher function + + Expected result: + - Staff and superusers can perform any action on any ContentLibrary scope + - Regular users are denied access without role assignments + """ + request = {"subject": subject, "action": action, "scope": scope, "expected_result": expected_result} + self._test_enforcement(self.POLICY, request) diff --git a/tests/test_models.py b/tests/test_models.py index 38c29ce8..4239fd72 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -2,12 +2,3 @@ """ Tests for the `openedx-authz` models module. """ - -import pytest - - -@pytest.mark.skip(reason="Placeholder to allow pytest to succeed before real tests are in place.") -def test_placeholder(): - """ - TODO: Delete this test once there are real tests. - """