From f7a21b999483f057b541cbd5eeefad2bc06ed0e6 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Wed, 11 Mar 2026 17:25:23 -0500 Subject: [PATCH 01/32] feat: enhance ScopeMeta to support glob patterns --- openedx_authz/api/data.py | 304 +++++++++++++++++++++++- openedx_authz/engine/enforcer.py | 2 + openedx_authz/models/scopes.py | 21 +- openedx_authz/tests/test_enforcement.py | 84 ++++++- openedx_authz/tests/test_utils.py | 22 +- 5 files changed, 421 insertions(+), 12 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 859e8e31..8ba2f96c 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -33,6 +33,9 @@ "ScopeData", "SubjectData", "ContentLibraryData", + "CourseOverviewData", + "OrgLevelLibraryGlob", + "OrgLevelCourseGlob", ] AUTHZ_POLICY_ATTRIBUTES_SEPARATOR = "^" @@ -40,6 +43,11 @@ GLOBAL_SCOPE_WILDCARD = "*" NAMESPACED_KEY_PATTERN = rf"^.+{re.escape(AUTHZ_POLICY_ATTRIBUTES_SEPARATOR)}.+$" +# Pattern for allowed characters in organization identifiers (from opaque-keys library) +# Matches: word characters (letters, digits, underscore), hyphens, tildes, periods, colons +# Reference: opaque_keys.edx.locator.Locator.ALLOWED_ID_CHARS +ALLOWED_CHARS_PATTERN = re.compile(r"^[\w\-~.:]+$", re.UNICODE) + class GroupingPolicyIndex(Enum): """Index positions for fields in a Casbin grouping policy (g or g2). @@ -148,13 +156,21 @@ class ScopeMeta(type): """Metaclass for ScopeData to handle dynamic subclass instantiation based on namespace.""" scope_registry: ClassVar[dict[str, Type["ScopeData"]]] = {} + glob_registry: ClassVar[dict[str, Type["ScopeData"]]] = {} def __init__(cls, name, bases, attrs): """Initialize the metaclass and register subclasses.""" super().__init__(name, bases, attrs) if not hasattr(cls, "scope_registry"): cls.scope_registry = {} - cls.scope_registry[cls.NAMESPACE] = cls + if not hasattr(cls, "glob_registry"): + cls.glob_registry = {} + + # Register glob classes (they have 'Glob' in their name) + if "Glob" in name and cls.NAMESPACE: + cls.glob_registry[cls.NAMESPACE] = cls + else: + cls.scope_registry[cls.NAMESPACE] = cls def __call__(cls, *args, **kwargs): """Instantiate the appropriate ScopeData subclass dynamically. @@ -166,9 +182,11 @@ def __call__(cls, *args, **kwargs): 1. external_key: Determines subclass from the key format. The namespace prefix before the first ':' is used to look up the appropriate subclass. Example: ScopeData(external_key='lib:DemoX:CSPROB') → ContentLibraryData + Example: ScopeData(external_key='lib:DemoX*') → OrgLevelLibraryGlob 2. namespaced_key: Determines subclass from the namespace prefix before '^'. Example: ScopeData(namespaced_key='lib^lib:DemoX:CSPROB') → ContentLibraryData + Example: ScopeData(namespaced_key='lib^lib:DemoX*') → OrgLevelLibraryGlob Usage patterns: - namespaced_key: Used when retrieving objects from the policy store @@ -179,6 +197,10 @@ def __call__(cls, *args, **kwargs): >>> scope = ScopeData(external_key='lib:DemoX:CSPROB') >>> isinstance(scope, ContentLibraryData) True + >>> # From glob external key + >>> scope = ScopeData(external_key='lib:DemoX*') + >>> isinstance(scope, OrgLevelLibraryGlob) + True >>> # From namespaced key (e.g., policy store) >>> scope = ScopeData(namespaced_key='lib^lib:DemoX:CSPROB') >>> isinstance(scope, ContentLibraryData) @@ -210,18 +232,21 @@ def get_subclass_by_namespaced_key(mcs, namespaced_key: str) -> Type["ScopeData" """Get the appropriate ScopeData subclass from the namespaced key. Extracts the namespace prefix (before '^') and returns the registered subclass. + If the key contains a wildcard, returns the appropriate glob subclass. Args: - namespaced_key: The namespaced key (e.g., 'lib^lib:DemoX:CSPROB', 'global^generic'). + namespaced_key: The namespaced key (e.g., 'lib^lib:DemoX:CSPROB', 'lib^lib:DemoX*', 'global^generic'). Returns: The ScopeData subclass for the namespace, or ScopeData if namespace not recognized. Examples: - >>> ScopeMeta.get_subclass_by_namespaced_key('course-v1^course-v1:WGU+CS002+2025_T1') + >>> ScopeMeta.get_subclass_by_namespaced_key('course-v1^course-v1:WGU+CS002+2025_T1') >>> ScopeMeta.get_subclass_by_namespaced_key('lib^lib:DemoX:CSPROB') + >>> ScopeMeta.get_subclass_by_namespaced_key('lib^lib:DemoX*') + >>> ScopeMeta.get_subclass_by_namespaced_key('global^generic') """ @@ -229,7 +254,16 @@ def get_subclass_by_namespaced_key(mcs, namespaced_key: str) -> Type["ScopeData" if not re.match(NAMESPACED_KEY_PATTERN, namespaced_key): raise ValueError(f"Invalid namespaced_key format: {namespaced_key}") - namespace = namespaced_key.split(AUTHZ_POLICY_ATTRIBUTES_SEPARATOR, 1)[0] + namespace, external_key = namespaced_key.split(AUTHZ_POLICY_ATTRIBUTES_SEPARATOR, 1) + + # Check if this is a glob pattern (contains wildcard) + is_glob = GLOBAL_SCOPE_WILDCARD in external_key + + if is_glob: + # Try to get glob-specific class first + return mcs.glob_registry.get(namespace, ScopeData) + + # Fall back to standard scope class return mcs.scope_registry.get(namespace, ScopeData) @classmethod @@ -239,11 +273,15 @@ def get_subclass_by_external_key(mcs, external_key: str) -> Type["ScopeData"]: Extracts the namespace from the external key (before the first ':') and validates the key format using the subclass's validate_external_key method. + This method now also handles glob patterns by detecting wildcards and returning + the appropriate glob subclass instead of the standard scope class. + Args: - external_key: The external key (e.g., 'lib:DemoX:CSPROB', 'global:generic'). + external_key: The external key (e.g., 'lib:DemoX:CSPROB', 'lib:DemoX*', 'global:generic'). Returns: The ScopeData subclass corresponding to the namespace. + If the key contains a wildcard, returns the corresponding glob subclass. Raises: ValueError: If the external_key format is invalid or namespace is not recognized. @@ -251,10 +289,17 @@ def get_subclass_by_external_key(mcs, external_key: str) -> Type["ScopeData"]: Examples: >>> ScopeMeta.get_subclass_by_external_key('lib:DemoX:CSPROB') + >>> ScopeMeta.get_subclass_by_external_key('course-v1:OpenedX+CS101+2024') + + >>> ScopeMeta.get_subclass_by_external_key('lib:DemoX*') + + >>> ScopeMeta.get_subclass_by_external_key('course-v1:OpenedX*') + Notes: - The external_key format should be 'namespace:some-identifier' (e.g., 'lib:DemoX:CSPROB'). - The namespace prefix before ':' is used to determine the subclass. + - If a wildcard is detected, the glob_registry is consulted first. - Each subclass must implement validate_external_key() to verify the full key format. - This won't work for org scopes that don't have explicit namespace prefixes. TODO: Handle org scopes differently. @@ -263,6 +308,17 @@ def get_subclass_by_external_key(mcs, external_key: str) -> Type["ScopeData"]: raise ValueError(f"Invalid external_key format: {external_key}") namespace = external_key.split(EXTERNAL_KEY_SEPARATOR, 1)[0] + + # Check if this is a glob pattern (contains wildcard) + is_glob = GLOBAL_SCOPE_WILDCARD in external_key + + if is_glob: + # Try to get glob-specific class first + glob_subclass = mcs.glob_registry.get(namespace) + if glob_subclass and glob_subclass.validate_external_key(external_key): + return glob_subclass + + # Fall back to standard scope class scope_subclass = mcs.scope_registry.get(namespace) if not scope_subclass: @@ -470,6 +526,125 @@ def __repr__(self): return self.namespaced_key +@define +class OrgLevelLibraryGlob(ContentLibraryData): + """Organization-level glob pattern for content libraries. + + This class represents glob patterns that match multiple libraries within an organization. + Format: 'lib:ORG*' where ORG is a valid organization identifier. + + The glob pattern allows granting permissions to all libraries within a specific organization + without needing to specify each library individually. + + Attributes: + NAMESPACE (str): Inherited 'lib' from ContentLibraryData. + external_key (str): The glob pattern (e.g., 'lib:DemoX*'). + namespaced_key (str): The pattern with namespace (e.g., 'lib^lib:DemoX*'). + + Validation Rules: + - Must end with GLOBAL_SCOPE_WILDCARD (*) + - Must have format 'lib:ORG*' (exactly one organization identifier) + - The organization must exist in at least one ContentLibrary + - Wildcard can only appear at the end after org identifier + - Cannot have wildcards at slug level (lib:ORG:SLUG* is invalid) + + Examples: + >>> glob = OrgLevelLibraryGlob(external_key='lib:DemoX*') + >>> glob.org + 'DemoX' + + Note: + This class is automatically instantiated by the ScopeMeta metaclass when + a library scope with a wildcard is created. + """ + + @property + def org(self) -> str | None: + """Get the organization identifier from the glob pattern. + + Returns: + str: The organization identifier (e.g., 'DemoX' from 'lib:DemoX*'), None otherwise. + """ + return self.get_org(self.external_key) + + @classmethod + def validate_external_key(cls, external_key: str) -> bool: + """Validate the external_key format for organization-level library globs. + + Args: + external_key (str): The external key to validate (e.g., 'lib:DemoX*'). + + Returns: + bool: True if the format is valid, False otherwise. + """ + if not external_key.startswith(cls.NAMESPACE + EXTERNAL_KEY_SEPARATOR): + return False + + if not external_key.endswith(GLOBAL_SCOPE_WILDCARD): + return False + + org = cls.get_org(external_key) + if org is None: + return False + + if not ALLOWED_CHARS_PATTERN.match(org): + return False + + return True + + @classmethod + def get_org(cls, external_key: str) -> str | None: + """Extract the organization identifier from the glob pattern. + + Args: + external_key (str): The external key to extract the organization identifier from. + + Returns: + str: The organization identifier (e.g., 'DemoX' from 'lib:DemoX*'), None otherwise. + """ + scope_prefix = external_key[: -len(GLOBAL_SCOPE_WILDCARD)] + parts = scope_prefix.split(EXTERNAL_KEY_SEPARATOR) + + if len(parts) != 2 or not parts[1]: + return None + + return parts[1] + + @classmethod + def org_exists(cls, org: str) -> bool: + """Check if at least one content library exists with the given organization. + + Args: + org (str): Organization identifier to check. + + Returns: + bool: True if at least one library with this org exists, False otherwise. + """ + lib_obj = ContentLibrary.objects.filter(org__short_name=org).only("org").last() + return lib_obj is not None and lib_obj.org.short_name == org + + def get_object(self) -> None: + """Glob patterns don't represent a single object. + + Returns: + None: Glob patterns match multiple objects, not a single one. + """ + return None + + def exists(self) -> bool: + """Check if the glob pattern is valid. + + For glob patterns, existence means the organization exists, + not that a specific library exists. + + Returns: + bool: True if the organization exists, False otherwise. + """ + if self.org is None: + return False + return self.org_exists(self.org) + + @define class CourseOverviewData(ScopeData): """A course scope for authorization in the Open edX platform. @@ -572,6 +747,125 @@ def __repr__(self): return self.namespaced_key +@define +class OrgLevelCourseGlob(CourseOverviewData): + """Organization-level glob pattern for courses. + + This class represents glob patterns that match multiple courses within an organization. + Format: 'course-v1:ORG*' where ORG is a valid organization identifier. + + The glob pattern allows granting permissions to all courses within a specific organization + without needing to specify each course individually. + + Attributes: + NAMESPACE: Inherited 'course-v1' from CourseOverviewData. + external_key: The glob pattern (e.g., 'course-v1:OpenedX*'). + namespaced_key: The pattern with namespace (e.g., 'course-v1^course-v1:OpenedX*'). + + Validation Rules: + - Must end with GLOBAL_SCOPE_WILDCARD (*) + - Must have format 'course-v1:ORG*' (exactly one organization identifier) + - The organization must exist in at least one CourseOverview + - Wildcard can only appear at the end after org identifier + - Cannot have wildcards at course or run level (course-v1:ORG+COURSE* is invalid) + + Examples: + >>> glob = OrgLevelCourseGlob(external_key='course-v1:OpenedX*') + >>> glob.org + 'OpenedX' + + Note: + This class is automatically instantiated by the ScopeMeta metaclass when + a course scope with a wildcard is created. + """ + + @property + def org(self) -> str | None: + """Get the organization identifier from the glob pattern. + + Returns: + str | None: The organization identifier (e.g., 'OpenedX' from 'course-v1:OpenedX*'), None otherwise. + """ + return self.get_org(self.external_key) + + @classmethod + def validate_external_key(cls, external_key: str) -> bool: + """Validate the external_key format for organization-level course globs. + + Args: + external_key (str): The external key to validate (e.g., 'course-v1:OpenedX*'). + + Returns: + bool: True if the format is valid, False otherwise. + """ + if not external_key.startswith(cls.NAMESPACE + EXTERNAL_KEY_SEPARATOR): + return False + + if not external_key.endswith(GLOBAL_SCOPE_WILDCARD): + return False + + org = cls.get_org(external_key) + if org is None: + return False + + if not ALLOWED_CHARS_PATTERN.match(org): + return False + + return True + + @classmethod + def get_org(cls, external_key: str) -> str | None: + """Extract the organization identifier from the glob pattern. + + Args: + external_key (str): The external key to extract the organization identifier from. + + Returns: + str | None: The organization identifier (e.g., 'OpenedX' from 'course-v1:OpenedX*'), None otherwise. + """ + scope_prefix = external_key[: -len(GLOBAL_SCOPE_WILDCARD)] + parts = scope_prefix.split(EXTERNAL_KEY_SEPARATOR) + + if len(parts) != 2 or not parts[1]: + return None + + return parts[1] + + @classmethod + def org_exists(cls, org: str) -> bool: + """Check if at least one course exists with the given organization. + + Args: + org (str): Organization identifier to check. + + Returns: + bool: True if at least one course with this org exists, False otherwise. + """ + course_obj = CourseOverview.objects.filter(org=org).only("org").last() + return course_obj is not None and course_obj.org == org + + def get_object(self) -> None: + """Glob patterns don't represent a single object. + + Returns: + None: Glob patterns match multiple objects, not a single one. + """ + return None + + def exists(self) -> bool: + """Check if the glob pattern is valid. + + For glob patterns, existence means the organization exists, + not that a specific course exists. + + Returns: + bool: True if the organization exists, False otherwise. + """ + if self.org is None: + return False + return self.org_exists(self.org) + + class SubjectMeta(type): """Metaclass for SubjectData to handle dynamic subclass instantiation based on namespace.""" diff --git a/openedx_authz/engine/enforcer.py b/openedx_authz/engine/enforcer.py index 123b1d75..c8f7ace9 100644 --- a/openedx_authz/engine/enforcer.py +++ b/openedx_authz/engine/enforcer.py @@ -20,6 +20,7 @@ from uuid import uuid4 from casbin import SyncedEnforcer +from casbin.util import key_match_func from casbin.util.log import DEFAULT_LOGGING, configure_logging from casbin_adapter.enforcer import initialize_enforcer from django.conf import settings @@ -279,5 +280,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) + enforcer.add_named_domain_matching_func("g", key_match_func) return enforcer diff --git a/openedx_authz/models/scopes.py b/openedx_authz/models/scopes.py index 24b01556..f34927f4 100644 --- a/openedx_authz/models/scopes.py +++ b/openedx_authz/models/scopes.py @@ -11,6 +11,7 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 +from openedx_authz.api.data import GLOBAL_SCOPE_WILDCARD, ScopeData from openedx_authz.models.core import Scope @@ -81,7 +82,7 @@ class ContentLibraryScope(Scope): ) @classmethod - def get_or_create_for_external_key(cls, scope): + def get_or_create_for_external_key(cls, scope: ScopeData) -> "ContentLibraryScope": """Get or create a ContentLibraryScope for the given external key. Args: @@ -89,8 +90,14 @@ def get_or_create_for_external_key(cls, scope): a LibraryLocatorV2-compatible string. Returns: - ContentLibraryScope: The Scope instance for the given ContentLibrary + ContentLibraryScope: The Scope instance for the given ContentLibrary, + or None if the scope is a glob pattern (contains wildcard). """ + # For glob scopes we don't create a Scope object since + # they don't represent a specific content library + if GLOBAL_SCOPE_WILDCARD in scope.external_key: + return None + library_key = LibraryLocatorV2.from_string(scope.external_key) content_library = ContentLibrary.objects.get_by_key(library_key) scope, _ = cls.objects.get_or_create(content_library=content_library) @@ -124,7 +131,7 @@ class CourseScope(Scope): ) @classmethod - def get_or_create_for_external_key(cls, scope): + def get_or_create_for_external_key(cls, scope: ScopeData) -> "CourseScope": """Get or create a CourseScope for the given external key. Args: @@ -132,8 +139,14 @@ def get_or_create_for_external_key(cls, scope): a CourseKey string. Returns: - CourseScope: The Scope instance for the given CourseOverview + CourseScope: The Scope instance for the given CourseOverview, + or None if the scope is a glob pattern (contains wildcard). """ + # For glob scopes we don't create a Scope object + # since they don't represent a specific course + if GLOBAL_SCOPE_WILDCARD in scope.external_key: + return None + course_key = CourseKey.from_string(scope.external_key) course_overview = CourseOverview.get_from_id(course_key) scope, _ = cls.objects.get_or_create(course_overview=course_overview) diff --git a/openedx_authz/tests/test_enforcement.py b/openedx_authz/tests/test_enforcement.py index d227f800..b571c7fc 100644 --- a/openedx_authz/tests/test_enforcement.py +++ b/openedx_authz/tests/test_enforcement.py @@ -11,19 +11,22 @@ import casbin import pytest +from casbin.util import key_match_func from ddt import data, ddt, unpack from django.contrib.auth import get_user_model from openedx_authz import ROOT_DIRECTORY -from openedx_authz.api.data import GLOBAL_SCOPE_WILDCARD -from openedx_authz.constants import roles +from openedx_authz.api.data import GLOBAL_SCOPE_WILDCARD, ContentLibraryData, CourseOverviewData +from openedx_authz.constants import permissions, roles from openedx_authz.engine.matcher import is_admin_or_superuser_check from openedx_authz.tests.test_utils import ( make_action_key, + make_course_key, make_library_key, make_role_key, make_scope_key, make_user_key, + make_wildcard_key, ) User = get_user_model() @@ -73,6 +76,7 @@ def setUpClass(cls) -> None: cls.enforcer = casbin.Enforcer(model_file) cls.enforcer.add_function("is_staff_or_superuser", is_admin_or_superuser_check) + cls.enforcer.add_named_domain_matching_func("g", key_match_func) def _load_policy(self, policy: list[str]) -> None: """ @@ -583,6 +587,82 @@ def test_wildcard_library_access(self, scope: str, expected_result: bool): self._test_enforcement(self.POLICY, request) +@ddt +class OrgGlobEnforcementTests(CasbinEnforcementTestCase): + """ + Tests for organization-level glob patterns in course and library scopes. + + This test class verifies that policies defined with org-level glob patterns + (e.g., "course-v1:OpenedX*" or "lib:DemoX*") are correctly enforced for + concrete course and library scopes that belong to those organizations. + """ + + POLICY = [ + # Policies + [ + "p", + make_role_key(roles.COURSE_STAFF.external_key), + make_action_key("courses.view_course"), + make_wildcard_key(CourseOverviewData.NAMESPACE), + "allow", + ], + [ + "p", + make_role_key(roles.LIBRARY_ADMIN.external_key), + make_action_key("content_libraries.view_library"), + make_wildcard_key(ContentLibraryData.NAMESPACE), + "allow", + ], + # Role assignments + [ + "g", + make_user_key("user-1"), + make_role_key(roles.COURSE_STAFF.external_key), + make_course_key("course-v1:OpenedX*"), + ], + [ + "g", + make_user_key("user-2"), + make_role_key(roles.LIBRARY_ADMIN.external_key), + make_library_key("lib:DemoX*"), + ], + ] + + CASES = [ + # Permission granted + { + "subject": make_user_key("user-1"), + "action": make_action_key(permissions.COURSES_VIEW_COURSE.action.external_key), + "scope": make_course_key("course-v1:OpenedX+DemoCourse+2026_T1"), + "expected_result": True, + }, + { + "subject": make_user_key("user-2"), + "action": make_action_key(permissions.VIEW_LIBRARY.action.external_key), + "scope": make_library_key("lib:DemoX:OrgLevelGlobLib"), + "expected_result": True, + }, + # Permission denied + { + "subject": make_user_key("user-1"), + "action": make_action_key(permissions.COURSES_VIEW_COURSE.action.external_key), + "scope": make_course_key("course-v1:InexistentOrg+DemoCourse+2026_T1"), + "expected_result": False, + }, + { + "subject": make_user_key("user-2"), + "action": make_action_key(permissions.VIEW_LIBRARY.action.external_key), + "scope": make_library_key("lib:InexistentOrg:OrgLevelGlobLib"), + "expected_result": False, + }, + ] + + @data(*CASES) + def test_org_level_glob_enforcement(self, request: AuthRequest): + """Test that org-level glob patterns in scopes are enforced correctly.""" + self._test_enforcement(self.POLICY, request) + + @pytest.mark.django_db @ddt class StaffSuperuserAccessTests(CasbinEnforcementTestCase): diff --git a/openedx_authz/tests/test_utils.py b/openedx_authz/tests/test_utils.py index b6b04ad1..52c29a3c 100644 --- a/openedx_authz/tests/test_utils.py +++ b/openedx_authz/tests/test_utils.py @@ -1,6 +1,14 @@ """Test utilities for creating namespaced keys using class constants.""" -from openedx_authz.api.data import GLOBAL_SCOPE_WILDCARD, ActionData, ContentLibraryData, RoleData, ScopeData, UserData +from openedx_authz.api.data import ( + GLOBAL_SCOPE_WILDCARD, + ActionData, + ContentLibraryData, + CourseOverviewData, + RoleData, + ScopeData, + UserData, +) def make_user_key(key: str) -> str: @@ -51,6 +59,18 @@ def make_library_key(key: str) -> str: return f"{ContentLibraryData.NAMESPACE}{ContentLibraryData.SEPARATOR}{key}" +def make_course_key(key: str) -> str: + """Create a namespaced course key. + + Args: + key: The course identifier (e.g., 'course-v1:DemoX+DemoCourse+2026_T1') + + Returns: + str: Namespaced course key (e.g., 'course-v1^course-v1:DemoX+DemoCourse+2026_T1') + """ + return f"{CourseOverviewData.NAMESPACE}{CourseOverviewData.SEPARATOR}{key}" + + def make_scope_key(namespace: str, key: str) -> str: """Create a namespaced scope key with custom namespace. From 8cce7055241855b0ea67f117a44cfec8a13c9ed3 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 08:35:59 -0500 Subject: [PATCH 02/32] refactor: rename org glob scopes --- openedx_authz/api/data.py | 24 ++++++++++++------------ openedx_authz/tests/test_enforcement.py | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 8ba2f96c..ecd642b3 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -34,8 +34,8 @@ "SubjectData", "ContentLibraryData", "CourseOverviewData", - "OrgLevelLibraryGlob", - "OrgLevelCourseGlob", + "OrgLibraryGlobData", + "OrgCourseGlobData", ] AUTHZ_POLICY_ATTRIBUTES_SEPARATOR = "^" @@ -182,11 +182,11 @@ def __call__(cls, *args, **kwargs): 1. external_key: Determines subclass from the key format. The namespace prefix before the first ':' is used to look up the appropriate subclass. Example: ScopeData(external_key='lib:DemoX:CSPROB') → ContentLibraryData - Example: ScopeData(external_key='lib:DemoX*') → OrgLevelLibraryGlob + Example: ScopeData(external_key='lib:DemoX*') → OrgLibraryGlobData 2. namespaced_key: Determines subclass from the namespace prefix before '^'. Example: ScopeData(namespaced_key='lib^lib:DemoX:CSPROB') → ContentLibraryData - Example: ScopeData(namespaced_key='lib^lib:DemoX*') → OrgLevelLibraryGlob + Example: ScopeData(namespaced_key='lib^lib:DemoX*') → OrgLibraryGlobData Usage patterns: - namespaced_key: Used when retrieving objects from the policy store @@ -199,7 +199,7 @@ def __call__(cls, *args, **kwargs): True >>> # From glob external key >>> scope = ScopeData(external_key='lib:DemoX*') - >>> isinstance(scope, OrgLevelLibraryGlob) + >>> isinstance(scope, OrgLibraryGlobData) True >>> # From namespaced key (e.g., policy store) >>> scope = ScopeData(namespaced_key='lib^lib:DemoX:CSPROB') @@ -246,7 +246,7 @@ def get_subclass_by_namespaced_key(mcs, namespaced_key: str) -> Type["ScopeData" >>> ScopeMeta.get_subclass_by_namespaced_key('lib^lib:DemoX:CSPROB') >>> ScopeMeta.get_subclass_by_namespaced_key('lib^lib:DemoX*') - + >>> ScopeMeta.get_subclass_by_namespaced_key('global^generic') """ @@ -292,9 +292,9 @@ def get_subclass_by_external_key(mcs, external_key: str) -> Type["ScopeData"]: >>> ScopeMeta.get_subclass_by_external_key('course-v1:OpenedX+CS101+2024') >>> ScopeMeta.get_subclass_by_external_key('lib:DemoX*') - + >>> ScopeMeta.get_subclass_by_external_key('course-v1:OpenedX*') - + Notes: - The external_key format should be 'namespace:some-identifier' (e.g., 'lib:DemoX:CSPROB'). @@ -527,7 +527,7 @@ def __repr__(self): @define -class OrgLevelLibraryGlob(ContentLibraryData): +class OrgLibraryGlobData(ContentLibraryData): """Organization-level glob pattern for content libraries. This class represents glob patterns that match multiple libraries within an organization. @@ -549,7 +549,7 @@ class OrgLevelLibraryGlob(ContentLibraryData): - Cannot have wildcards at slug level (lib:ORG:SLUG* is invalid) Examples: - >>> glob = OrgLevelLibraryGlob(external_key='lib:DemoX*') + >>> glob = OrgLibraryGlobData(external_key='lib:DemoX*') >>> glob.org 'DemoX' @@ -748,7 +748,7 @@ def __repr__(self): @define -class OrgLevelCourseGlob(CourseOverviewData): +class OrgCourseGlobData(CourseOverviewData): """Organization-level glob pattern for courses. This class represents glob patterns that match multiple courses within an organization. @@ -770,7 +770,7 @@ class OrgLevelCourseGlob(CourseOverviewData): - Cannot have wildcards at course or run level (course-v1:ORG+COURSE* is invalid) Examples: - >>> glob = OrgLevelCourseGlob(external_key='course-v1:OpenedX*') + >>> glob = OrgCourseGlobData(external_key='course-v1:OpenedX*') >>> glob.org 'OpenedX' diff --git a/openedx_authz/tests/test_enforcement.py b/openedx_authz/tests/test_enforcement.py index b571c7fc..d7a378ed 100644 --- a/openedx_authz/tests/test_enforcement.py +++ b/openedx_authz/tests/test_enforcement.py @@ -639,7 +639,7 @@ class OrgGlobEnforcementTests(CasbinEnforcementTestCase): { "subject": make_user_key("user-2"), "action": make_action_key(permissions.VIEW_LIBRARY.action.external_key), - "scope": make_library_key("lib:DemoX:OrgLevelGlobLib"), + "scope": make_library_key("lib:DemoX:OrgGlobLib"), "expected_result": True, }, # Permission denied @@ -652,7 +652,7 @@ class OrgGlobEnforcementTests(CasbinEnforcementTestCase): { "subject": make_user_key("user-2"), "action": make_action_key(permissions.VIEW_LIBRARY.action.external_key), - "scope": make_library_key("lib:InexistentOrg:OrgLevelGlobLib"), + "scope": make_library_key("lib:InexistentOrg:OrgGlobLib"), "expected_result": False, }, ] From d51a1a4883583439fa8caf9d4e8498c051324639 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 08:53:35 -0500 Subject: [PATCH 03/32] feat: add IS_GLOB attribute in scope classes --- openedx_authz/api/data.py | 16 ++++++++-------- openedx_authz/models/scopes.py | 9 ++++----- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index ecd642b3..f92153f6 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -12,15 +12,10 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 -try: - from openedx.core.djangoapps.content_libraries.models import ContentLibrary -except ImportError: - ContentLibrary = None +from openedx_authz.models.scopes import get_content_library_model, get_course_overview_model -try: - from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -except ImportError: - CourseOverview = None +ContentLibrary = get_content_library_model() +CourseOverview = get_course_overview_model() __all__ = [ "UserData", @@ -381,6 +376,7 @@ class ScopeData(AuthZData, metaclass=ScopeMeta): # 2. Custom global scopes that don't map to specific domain objects (e.g., 'global:some_scope') # Subclasses like ContentLibraryData ('lib') represent concrete resource types with their own namespaces. NAMESPACE: ClassVar[str] = "global" + IS_GLOB: ClassVar[bool] = False @classmethod def validate_external_key(cls, _: str) -> bool: @@ -558,6 +554,8 @@ class OrgLibraryGlobData(ContentLibraryData): a library scope with a wildcard is created. """ + IS_GLOB: ClassVar[bool] = True + @property def org(self) -> str | None: """Get the organization identifier from the glob pattern. @@ -779,6 +777,8 @@ class OrgCourseGlobData(CourseOverviewData): a course scope with a wildcard is created. """ + IS_GLOB: ClassVar[bool] = True + @property def org(self) -> str | None: """Get the organization identifier from the glob pattern. diff --git a/openedx_authz/models/scopes.py b/openedx_authz/models/scopes.py index f34927f4..b9e7d951 100644 --- a/openedx_authz/models/scopes.py +++ b/openedx_authz/models/scopes.py @@ -11,7 +11,6 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 -from openedx_authz.api.data import GLOBAL_SCOPE_WILDCARD, ScopeData from openedx_authz.models.core import Scope @@ -82,7 +81,7 @@ class ContentLibraryScope(Scope): ) @classmethod - def get_or_create_for_external_key(cls, scope: ScopeData) -> "ContentLibraryScope": + def get_or_create_for_external_key(cls, scope) -> "ContentLibraryScope": """Get or create a ContentLibraryScope for the given external key. Args: @@ -95,7 +94,7 @@ def get_or_create_for_external_key(cls, scope: ScopeData) -> "ContentLibraryScop """ # For glob scopes we don't create a Scope object since # they don't represent a specific content library - if GLOBAL_SCOPE_WILDCARD in scope.external_key: + if scope.IS_GLOB: return None library_key = LibraryLocatorV2.from_string(scope.external_key) @@ -131,7 +130,7 @@ class CourseScope(Scope): ) @classmethod - def get_or_create_for_external_key(cls, scope: ScopeData) -> "CourseScope": + def get_or_create_for_external_key(cls, scope) -> "CourseScope": """Get or create a CourseScope for the given external key. Args: @@ -144,7 +143,7 @@ def get_or_create_for_external_key(cls, scope: ScopeData) -> "CourseScope": """ # For glob scopes we don't create a Scope object # since they don't represent a specific course - if GLOBAL_SCOPE_WILDCARD in scope.external_key: + if scope.IS_GLOB: return None course_key = CourseKey.from_string(scope.external_key) From 8b584e2e3bd14570e523f747862aaccc11f647fd Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 09:10:58 -0500 Subject: [PATCH 04/32] test: add unit tests for new org glob scopes --- openedx_authz/tests/api/test_data.py | 135 ++++++++++++++++++++++++++- 1 file changed, 134 insertions(+), 1 deletion(-) diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 9807f4e9..9312b032 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -3,13 +3,15 @@ from unittest.mock import Mock, patch from ddt import data, ddt, unpack -from django.test import TestCase +from django.test import TestCase, override_settings from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_authz.api.data import ( ActionData, ContentLibraryData, CourseOverviewData, + OrgCourseGlobData, + OrgLibraryGlobData, PermissionData, RoleAssignmentData, RoleData, @@ -19,6 +21,7 @@ UserData, ) from openedx_authz.constants import permissions, roles +from openedx_authz.tests.stubs.models import ContentLibrary, CourseOverview, Organization @ddt @@ -233,9 +236,17 @@ def test_scope_data_registration(self): self.assertIn("course-v1", ScopeData.scope_registry) self.assertIs(ScopeData.scope_registry["course-v1"], CourseOverviewData) + # Glob registries for organization-level scopes + self.assertIn("lib", ScopeMeta.glob_registry) + self.assertIs(ScopeMeta.glob_registry["lib"], OrgLibraryGlobData) + self.assertIn("course-v1", ScopeMeta.glob_registry) + self.assertIs(ScopeMeta.glob_registry["course-v1"], OrgCourseGlobData) + @data( ("course-v1^course-v1:WGU+CS002+2025_T1", CourseOverviewData), ("lib^lib:DemoX:CSPROB", ContentLibraryData), + ("lib^lib:DemoX*", OrgLibraryGlobData), + ("course-v1^course-v1:OpenedX*", OrgCourseGlobData), ("global^generic_scope", ScopeData), ) @unpack @@ -254,6 +265,8 @@ def test_dynamic_instantiation_via_namespaced_key(self, namespaced_key, expected @data( ("course-v1^course-v1:WGU+CS002+2025_T1", CourseOverviewData), ("lib^lib:DemoX:CSPROB", ContentLibraryData), + ("lib^lib:DemoX*", OrgLibraryGlobData), + ("course-v1^course-v1:OpenedX*", OrgCourseGlobData), ("global^generic", ScopeData), ("unknown^something", ScopeData), ) @@ -273,6 +286,8 @@ def test_get_subclass_by_namespaced_key(self, namespaced_key, expected_class): @data( ("course-v1:WGU+CS002+2025_T1", CourseOverviewData), ("lib:DemoX:CSPROB", ContentLibraryData), + ("lib:DemoX*", OrgLibraryGlobData), + ("course-v1:OpenedX*", OrgCourseGlobData), ("lib:edX:Demo", ContentLibraryData), ("global:generic_scope", ScopeData), ) @@ -654,3 +669,121 @@ def test_exists_returns_false_when_library_does_not_exist(self, mock_content_lib result = library_scope.exists() self.assertFalse(result) + + +@ddt +@override_settings(OPENEDX_AUTHZ_CONTENT_LIBRARY_MODEL="content_libraries.ContentLibrary") +class TestOrgLibraryGlobData(TestCase): + """Tests for the OrgLibraryGlobData scope.""" + + @data( + ("lib:DemoX*", True), + ("lib:Org-123*", True), + ("lib:Org.with.dots*", True), + ("lib:Org:With:Colon*", False), + ("lib:Org", False), + ("other:DemoX*", False), + ("lib:DemoX**", False), + ) + @unpack + def test_validate_external_key(self, external_key, expected_valid): + """Validate organization-level library glob external keys.""" + self.assertEqual(OrgLibraryGlobData.validate_external_key(external_key), expected_valid) + + @data( + ("lib:DemoX*", "DemoX"), + ("lib:Org-123*", "Org-123"), + ("lib:Org.with.dots*", "Org.with.dots"), + ("lib:Org:With:Colon*", None), + ("lib:*", None), + ) + @unpack + def test_get_org(self, external_key, expected_org): + """Test organization extraction from library glob pattern.""" + self.assertEqual(OrgLibraryGlobData.get_org(external_key), expected_org) + + def test_exists_true_when_org_has_libraries_in_db(self): + """exists() returns True when at least one library with the org exists in the DB.""" + org_name = "DemoX" + organization = Organization.objects.create(short_name=org_name) + ContentLibrary.objects.create(org=organization, slug="testlib", title="Test Library") + + result = OrgLibraryGlobData(external_key=f"lib:{org_name}*").exists() + + self.assertTrue(result) + + def test_exists_false_when_org_does_not_exist_in_db(self): + """exists() returns False when the org does not exist in the DB.""" + org_name = "DemoX" + + result = OrgLibraryGlobData(external_key=f"lib:{org_name}*").exists() + + self.assertFalse(result) + + def test_exists_false_when_org_exists_but_no_libraries_in_db(self): + """exists() returns False when the org exists but no libraries exist in the DB.""" + org_name = "DemoX" + Organization.objects.create(short_name=org_name) + + result = OrgLibraryGlobData(external_key=f"lib:{org_name}*").exists() + + self.assertFalse(result) + + +@ddt +@override_settings(OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL="course_overviews.CourseOverview") +class TestOrgCourseGlobData(TestCase): + """Tests for the OrgCourseGlobData scope.""" + + @data( + ("course-v1:OpenedX*", True), + ("course-v1:My-Org_1*", True), + ("course-v1:Org.with.dots*", True), + ("course-v1:Org:With:Plus*", False), + ("course-v1:OpenedX", False), + ("other:OpenedX*", False), + ("course-v1:OpenedX**", False), + ) + @unpack + def test_validate_external_key(self, external_key, expected_valid): + """Validate organization-level course glob external keys.""" + self.assertEqual(OrgCourseGlobData.validate_external_key(external_key), expected_valid) + + @data( + ("course-v1:OpenedX*", "OpenedX"), + ("course-v1:My-Org_1*", "My-Org_1"), + ("course-v1:Org.with.dots*", "Org.with.dots"), + ("course-v1:Org:With:Plus*", None), + ("course-v1:*", None), + ) + @unpack + def test_get_org(self, external_key, expected_org): + """Test organization extraction from course glob pattern.""" + self.assertEqual(OrgCourseGlobData.get_org(external_key), expected_org) + + def test_exists_true_when_org_has_courses(self): + """exists() returns True when at least one course with the org exists.""" + org_name = "OpenedX" + Organization.objects.create(short_name=org_name) + CourseOverview.objects.create(org=org_name, display_name="Test Course") + + result = OrgCourseGlobData(external_key=f"course-v1:{org_name}*").exists() + + self.assertTrue(result) + + def test_exists_false_when_org_does_not_exist(self): + """exists() returns False when the org does not exist.""" + org_name = "OpenedX" + + result = OrgCourseGlobData(external_key=f"course-v1:{org_name}*").exists() + + self.assertFalse(result) + + def test_exists_false_when_org_exists_but_no_courses(self): + """exists() returns False when the org exists but no courses exist.""" + org_name = "OpenedX" + Organization.objects.create(short_name=org_name) + + result = OrgCourseGlobData(external_key=f"course-v1:{org_name}*").exists() + + self.assertFalse(result) From a154226d756866e4731f8557e106a445c37c2a6e Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 10:19:51 -0500 Subject: [PATCH 05/32] feat: add ID_SEPARATOR attribute --- openedx_authz/api/data.py | 81 ++++++++++++++++++++++++--------------- 1 file changed, 51 insertions(+), 30 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index f92153f6..92c019dc 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -425,11 +425,12 @@ class ContentLibraryData(ScopeData): Content libraries use the LibraryLocatorV2 format for identification. Attributes: - NAMESPACE: 'lib' for content library scopes. - external_key: The content library identifier (e.g., 'lib:DemoX:CSPROB'). + NAMESPACE (str): 'lib' for content library scopes. + ID_SEPARATOR (str): ':' for content library scopes. + external_key (str): The content library identifier (e.g., 'lib:DemoX:CSPROB'). Must be a valid LibraryLocatorV2 format. - namespaced_key: The library identifier with namespace (e.g., 'lib^lib:DemoX:CSPROB'). - library_id: Property alias for external_key. + namespaced_key (str): The library identifier with namespace (e.g., 'lib^lib:DemoX:CSPROB'). + library_id (str): Property alias for external_key. Examples: >>> library = ContentLibraryData(external_key='lib:DemoX:CSPROB') @@ -443,6 +444,7 @@ class ContentLibraryData(ScopeData): """ NAMESPACE: ClassVar[str] = "lib" + ID_SEPARATOR: ClassVar[str] = ":" @property def library_id(self) -> str: @@ -527,25 +529,27 @@ class OrgLibraryGlobData(ContentLibraryData): """Organization-level glob pattern for content libraries. This class represents glob patterns that match multiple libraries within an organization. - Format: 'lib:ORG*' where ORG is a valid organization identifier. + Format: 'lib:ORG:*' where ORG is a valid organization identifier. The glob pattern allows granting permissions to all libraries within a specific organization without needing to specify each library individually. Attributes: NAMESPACE (str): Inherited 'lib' from ContentLibraryData. - external_key (str): The glob pattern (e.g., 'lib:DemoX*'). - namespaced_key (str): The pattern with namespace (e.g., 'lib^lib:DemoX*'). + ID_SEPARATOR (str): ':' for content library scopes. + IS_GLOB (bool): True for organization-level glob patterns. + external_key (str): The glob pattern (e.g., 'lib:DemoX:*'). + namespaced_key (str): The pattern with namespace (e.g., 'lib^lib:DemoX:*'). Validation Rules: - Must end with GLOBAL_SCOPE_WILDCARD (*) - - Must have format 'lib:ORG*' (exactly one organization identifier) + - Must have format 'lib:ORG:*' (exactly one organization identifier) - The organization must exist in at least one ContentLibrary - Wildcard can only appear at the end after org identifier - Cannot have wildcards at slug level (lib:ORG:SLUG* is invalid) Examples: - >>> glob = OrgLibraryGlobData(external_key='lib:DemoX*') + >>> glob = OrgLibraryGlobData(external_key='lib:DemoX:*') >>> glob.org 'DemoX' @@ -561,7 +565,7 @@ def org(self) -> str | None: """Get the organization identifier from the glob pattern. Returns: - str: The organization identifier (e.g., 'DemoX' from 'lib:DemoX*'), None otherwise. + str: The organization identifier (e.g., 'DemoX' from 'lib:DemoX:*'), None otherwise. """ return self.get_org(self.external_key) @@ -570,7 +574,7 @@ def validate_external_key(cls, external_key: str) -> bool: """Validate the external_key format for organization-level library globs. Args: - external_key (str): The external key to validate (e.g., 'lib:DemoX*'). + external_key (str): The external key to validate (e.g., 'lib:DemoX:*'). Returns: bool: True if the format is valid, False otherwise. @@ -578,7 +582,9 @@ def validate_external_key(cls, external_key: str) -> bool: if not external_key.startswith(cls.NAMESPACE + EXTERNAL_KEY_SEPARATOR): return False - if not external_key.endswith(GLOBAL_SCOPE_WILDCARD): + # Enforce explicit org-level separator: 'lib:ORG:*' + suffix = cls.ID_SEPARATOR + GLOBAL_SCOPE_WILDCARD + if not external_key.endswith(suffix): return False org = cls.get_org(external_key) @@ -598,9 +604,13 @@ def get_org(cls, external_key: str) -> str | None: external_key (str): The external key to extract the organization identifier from. Returns: - str: The organization identifier (e.g., 'DemoX' from 'lib:DemoX*'), None otherwise. + str: The organization identifier (e.g., 'DemoX' from 'lib:DemoX:*'), None otherwise. """ - scope_prefix = external_key[: -len(GLOBAL_SCOPE_WILDCARD)] + suffix = cls.ID_SEPARATOR + GLOBAL_SCOPE_WILDCARD + if not external_key.endswith(suffix): + return None + + scope_prefix = external_key[: -len(suffix)] parts = scope_prefix.split(EXTERNAL_KEY_SEPARATOR) if len(parts) != 2 or not parts[1]: @@ -650,11 +660,14 @@ class CourseOverviewData(ScopeData): Courses uses the CourseKey format for identification. Attributes: - NAMESPACE: 'course-v1' for course scopes. - external_key: The course identifier (e.g., 'course-v1:TestOrg+TestCourse+2024_T1'). + NAMESPACE (str): 'course-v1' for course scopes. + ID_SEPARATOR (str): '+' for course scopes. + IS_GLOB (bool): False for course scopes. + external_key (str): The course identifier (e.g., 'course-v1:TestOrg+TestCourse+2024_T1'). Must be a valid CourseKey format. - namespaced_key: The course identifier with namespace (e.g., 'course-v1^course-v1:TestOrg+TestCourse+2024_T1'). - course_id: Property alias for external_key. + namespaced_key (str): The course identifier with namespace + (e.g., 'course-v1^course-v1:TestOrg+TestCourse+2024_T1'). + course_id (str): Property alias for external_key. Examples: >>> course = CourseOverviewData(external_key='course-v1:TestOrg+TestCourse+2024_T1') @@ -662,10 +675,10 @@ class CourseOverviewData(ScopeData): 'course-v1^course-v1:TestOrg+TestCourse+2024_T1' >>> course.course_id 'course-v1:TestOrg+TestCourse+2024_T1' - """ NAMESPACE: ClassVar[str] = "course-v1" + ID_SEPARATOR: ClassVar[str] = "+" @property def course_id(self) -> str: @@ -750,25 +763,27 @@ class OrgCourseGlobData(CourseOverviewData): """Organization-level glob pattern for courses. This class represents glob patterns that match multiple courses within an organization. - Format: 'course-v1:ORG*' where ORG is a valid organization identifier. + Format: 'course-v1:ORG+*' where ORG is a valid organization identifier. The glob pattern allows granting permissions to all courses within a specific organization without needing to specify each course individually. Attributes: - NAMESPACE: Inherited 'course-v1' from CourseOverviewData. - external_key: The glob pattern (e.g., 'course-v1:OpenedX*'). - namespaced_key: The pattern with namespace (e.g., 'course-v1^course-v1:OpenedX*'). + NAMESPACE (str): Inherited 'course-v1' from CourseOverviewData. + ID_SEPARATOR (str): '+' for course scopes. + IS_GLOB (bool): True for organization-level glob patterns. + external_key (str): The glob pattern (e.g., 'course-v1:OpenedX+*'). + namespaced_key (str): The pattern with namespace (e.g., 'course-v1^course-v1:OpenedX+*'). Validation Rules: - Must end with GLOBAL_SCOPE_WILDCARD (*) - - Must have format 'course-v1:ORG*' (exactly one organization identifier) + - Must have format 'course-v1:ORG+*' (exactly one organization identifier) - The organization must exist in at least one CourseOverview - Wildcard can only appear at the end after org identifier - Cannot have wildcards at course or run level (course-v1:ORG+COURSE* is invalid) Examples: - >>> glob = OrgCourseGlobData(external_key='course-v1:OpenedX*') + >>> glob = OrgCourseGlobData(external_key='course-v1:OpenedX+*') >>> glob.org 'OpenedX' @@ -784,7 +799,7 @@ def org(self) -> str | None: """Get the organization identifier from the glob pattern. Returns: - str | None: The organization identifier (e.g., 'OpenedX' from 'course-v1:OpenedX*'), None otherwise. + str | None: The organization identifier (e.g., 'OpenedX' from 'course-v1:OpenedX+*'), None otherwise. """ return self.get_org(self.external_key) @@ -793,7 +808,7 @@ def validate_external_key(cls, external_key: str) -> bool: """Validate the external_key format for organization-level course globs. Args: - external_key (str): The external key to validate (e.g., 'course-v1:OpenedX*'). + external_key (str): The external key to validate (e.g., 'course-v1:OpenedX+*'). Returns: bool: True if the format is valid, False otherwise. @@ -801,7 +816,9 @@ def validate_external_key(cls, external_key: str) -> bool: if not external_key.startswith(cls.NAMESPACE + EXTERNAL_KEY_SEPARATOR): return False - if not external_key.endswith(GLOBAL_SCOPE_WILDCARD): + # Enforce explicit org-level separator: 'course-v1:ORG+*' + glob_suffix = cls.ID_SEPARATOR + GLOBAL_SCOPE_WILDCARD + if not external_key.endswith(glob_suffix): return False org = cls.get_org(external_key) @@ -821,9 +838,13 @@ def get_org(cls, external_key: str) -> str | None: external_key (str): The external key to extract the organization identifier from. Returns: - str | None: The organization identifier (e.g., 'OpenedX' from 'course-v1:OpenedX*'), None otherwise. + str | None: The organization identifier (e.g., 'OpenedX' from 'course-v1:OpenedX+*'), None otherwise. """ - scope_prefix = external_key[: -len(GLOBAL_SCOPE_WILDCARD)] + suffix = cls.ID_SEPARATOR + GLOBAL_SCOPE_WILDCARD + if not external_key.endswith(suffix): + return None + + scope_prefix = external_key[: -len(suffix)] parts = scope_prefix.split(EXTERNAL_KEY_SEPARATOR) if len(parts) != 2 or not parts[1]: From a08f22c441a9b28023452cbf8263149e54ade998 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 10:25:02 -0500 Subject: [PATCH 06/32] fix: update glob patterns in test data --- openedx_authz/tests/api/test_data.py | 57 ++++++++++++++-------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 9312b032..3fd18413 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -265,8 +265,8 @@ def test_dynamic_instantiation_via_namespaced_key(self, namespaced_key, expected @data( ("course-v1^course-v1:WGU+CS002+2025_T1", CourseOverviewData), ("lib^lib:DemoX:CSPROB", ContentLibraryData), - ("lib^lib:DemoX*", OrgLibraryGlobData), - ("course-v1^course-v1:OpenedX*", OrgCourseGlobData), + ("lib^lib:DemoX:*", OrgLibraryGlobData), + ("course-v1^course-v1:OpenedX+*", OrgCourseGlobData), ("global^generic", ScopeData), ("unknown^something", ScopeData), ) @@ -286,8 +286,8 @@ def test_get_subclass_by_namespaced_key(self, namespaced_key, expected_class): @data( ("course-v1:WGU+CS002+2025_T1", CourseOverviewData), ("lib:DemoX:CSPROB", ContentLibraryData), - ("lib:DemoX*", OrgLibraryGlobData), - ("course-v1:OpenedX*", OrgCourseGlobData), + ("lib:DemoX:*", OrgLibraryGlobData), + ("course-v1:OpenedX+*", OrgCourseGlobData), ("lib:edX:Demo", ContentLibraryData), ("global:generic_scope", ScopeData), ) @@ -677,13 +677,12 @@ class TestOrgLibraryGlobData(TestCase): """Tests for the OrgLibraryGlobData scope.""" @data( - ("lib:DemoX*", True), - ("lib:Org-123*", True), - ("lib:Org.with.dots*", True), - ("lib:Org:With:Colon*", False), + ("lib:DemoX:*", True), + ("lib:Org-123:*", True), + ("lib:Org.with.dots:*", True), ("lib:Org", False), - ("other:DemoX*", False), - ("lib:DemoX**", False), + ("other:DemoX:*", False), + ("lib:DemoX:*:*", False), ) @unpack def test_validate_external_key(self, external_key, expected_valid): @@ -691,10 +690,10 @@ def test_validate_external_key(self, external_key, expected_valid): self.assertEqual(OrgLibraryGlobData.validate_external_key(external_key), expected_valid) @data( - ("lib:DemoX*", "DemoX"), - ("lib:Org-123*", "Org-123"), - ("lib:Org.with.dots*", "Org.with.dots"), - ("lib:Org:With:Colon*", None), + ("lib:DemoX:*", "DemoX"), + ("lib:Org-123:*", "Org-123"), + ("lib:Org.with.dots:*", "Org.with.dots"), + ("lib:Org:With:Colon:*", None), ("lib:*", None), ) @unpack @@ -708,7 +707,7 @@ def test_exists_true_when_org_has_libraries_in_db(self): organization = Organization.objects.create(short_name=org_name) ContentLibrary.objects.create(org=organization, slug="testlib", title="Test Library") - result = OrgLibraryGlobData(external_key=f"lib:{org_name}*").exists() + result = OrgLibraryGlobData(external_key=f"lib:{org_name}:*").exists() self.assertTrue(result) @@ -716,7 +715,7 @@ def test_exists_false_when_org_does_not_exist_in_db(self): """exists() returns False when the org does not exist in the DB.""" org_name = "DemoX" - result = OrgLibraryGlobData(external_key=f"lib:{org_name}*").exists() + result = OrgLibraryGlobData(external_key=f"lib:{org_name}:*").exists() self.assertFalse(result) @@ -725,7 +724,7 @@ def test_exists_false_when_org_exists_but_no_libraries_in_db(self): org_name = "DemoX" Organization.objects.create(short_name=org_name) - result = OrgLibraryGlobData(external_key=f"lib:{org_name}*").exists() + result = OrgLibraryGlobData(external_key=f"lib:{org_name}:*").exists() self.assertFalse(result) @@ -736,12 +735,12 @@ class TestOrgCourseGlobData(TestCase): """Tests for the OrgCourseGlobData scope.""" @data( - ("course-v1:OpenedX*", True), - ("course-v1:My-Org_1*", True), - ("course-v1:Org.with.dots*", True), - ("course-v1:Org:With:Plus*", False), + ("course-v1:OpenedX+*", True), + ("course-v1:My-Org_1+*", True), + ("course-v1:Org.with.dots+*", True), + ("course-v1:Org:With:Plus+*", False), ("course-v1:OpenedX", False), - ("other:OpenedX*", False), + ("other:OpenedX+*", False), ("course-v1:OpenedX**", False), ) @unpack @@ -750,10 +749,10 @@ def test_validate_external_key(self, external_key, expected_valid): self.assertEqual(OrgCourseGlobData.validate_external_key(external_key), expected_valid) @data( - ("course-v1:OpenedX*", "OpenedX"), - ("course-v1:My-Org_1*", "My-Org_1"), - ("course-v1:Org.with.dots*", "Org.with.dots"), - ("course-v1:Org:With:Plus*", None), + ("course-v1:OpenedX+*", "OpenedX"), + ("course-v1:My-Org_1+*", "My-Org_1"), + ("course-v1:Org.with.dots+*", "Org.with.dots"), + ("course-v1:Org:With:Plus+*", None), ("course-v1:*", None), ) @unpack @@ -767,7 +766,7 @@ def test_exists_true_when_org_has_courses(self): Organization.objects.create(short_name=org_name) CourseOverview.objects.create(org=org_name, display_name="Test Course") - result = OrgCourseGlobData(external_key=f"course-v1:{org_name}*").exists() + result = OrgCourseGlobData(external_key=f"course-v1:{org_name}+*").exists() self.assertTrue(result) @@ -775,7 +774,7 @@ def test_exists_false_when_org_does_not_exist(self): """exists() returns False when the org does not exist.""" org_name = "OpenedX" - result = OrgCourseGlobData(external_key=f"course-v1:{org_name}*").exists() + result = OrgCourseGlobData(external_key=f"course-v1:{org_name}+*").exists() self.assertFalse(result) @@ -784,6 +783,6 @@ def test_exists_false_when_org_exists_but_no_courses(self): org_name = "OpenedX" Organization.objects.create(short_name=org_name) - result = OrgCourseGlobData(external_key=f"course-v1:{org_name}*").exists() + result = OrgCourseGlobData(external_key=f"course-v1:{org_name}+*").exists() self.assertFalse(result) From 7c0d757acdc459b04c53274d96ca61003834b021 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 13:19:20 -0500 Subject: [PATCH 07/32] docs: update documentation for glob pattern format --- openedx_authz/api/data.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 92c019dc..e8d05372 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -529,7 +529,7 @@ class OrgLibraryGlobData(ContentLibraryData): """Organization-level glob pattern for content libraries. This class represents glob patterns that match multiple libraries within an organization. - Format: 'lib:ORG:*' where ORG is a valid organization identifier. + Format: ``lib:ORG:*`` where ORG is a valid organization identifier. The glob pattern allows granting permissions to all libraries within a specific organization without needing to specify each library individually. @@ -538,15 +538,15 @@ class OrgLibraryGlobData(ContentLibraryData): NAMESPACE (str): Inherited 'lib' from ContentLibraryData. ID_SEPARATOR (str): ':' for content library scopes. IS_GLOB (bool): True for organization-level glob patterns. - external_key (str): The glob pattern (e.g., 'lib:DemoX:*'). - namespaced_key (str): The pattern with namespace (e.g., 'lib^lib:DemoX:*'). + external_key (str): The glob pattern (e.g., ``lib:DemoX:*``). + namespaced_key (str): The pattern with namespace (e.g., ``lib^lib:DemoX:*``). Validation Rules: - - Must end with GLOBAL_SCOPE_WILDCARD (*) - - Must have format 'lib:ORG:*' (exactly one organization identifier) + - Must end with GLOBAL_SCOPE_WILDCARD (``*``) + - Must have format ``lib:ORG:*`` (exactly one organization identifier) - The organization must exist in at least one ContentLibrary - Wildcard can only appear at the end after org identifier - - Cannot have wildcards at slug level (lib:ORG:SLUG* is invalid) + - Cannot have wildcards at slug level (``lib:ORG:SLUG*`` is invalid) Examples: >>> glob = OrgLibraryGlobData(external_key='lib:DemoX:*') @@ -565,7 +565,7 @@ def org(self) -> str | None: """Get the organization identifier from the glob pattern. Returns: - str: The organization identifier (e.g., 'DemoX' from 'lib:DemoX:*'), None otherwise. + str: The organization identifier (e.g., ``DemoX`` from ``lib:DemoX:*``), None otherwise. """ return self.get_org(self.external_key) @@ -574,7 +574,7 @@ def validate_external_key(cls, external_key: str) -> bool: """Validate the external_key format for organization-level library globs. Args: - external_key (str): The external key to validate (e.g., 'lib:DemoX:*'). + external_key (str): The external key to validate (e.g., ``lib:DemoX:*``). Returns: bool: True if the format is valid, False otherwise. @@ -604,7 +604,7 @@ def get_org(cls, external_key: str) -> str | None: external_key (str): The external key to extract the organization identifier from. Returns: - str: The organization identifier (e.g., 'DemoX' from 'lib:DemoX:*'), None otherwise. + str: The organization identifier (e.g., ``DemoX`` from ``lib:DemoX:*``), None otherwise. """ suffix = cls.ID_SEPARATOR + GLOBAL_SCOPE_WILDCARD if not external_key.endswith(suffix): @@ -776,7 +776,7 @@ class OrgCourseGlobData(CourseOverviewData): namespaced_key (str): The pattern with namespace (e.g., 'course-v1^course-v1:OpenedX+*'). Validation Rules: - - Must end with GLOBAL_SCOPE_WILDCARD (*) + - Must end with GLOBAL_SCOPE_WILDCARD (``*``) - Must have format 'course-v1:ORG+*' (exactly one organization identifier) - The organization must exist in at least one CourseOverview - Wildcard can only appear at the end after org identifier From 377c24970755189dfd79ee2c0b189ee7e0e9848f Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 13:27:32 -0500 Subject: [PATCH 08/32] refactor: rename variable --- openedx_authz/api/data.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index e8d05372..4be371e2 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -817,8 +817,8 @@ def validate_external_key(cls, external_key: str) -> bool: return False # Enforce explicit org-level separator: 'course-v1:ORG+*' - glob_suffix = cls.ID_SEPARATOR + GLOBAL_SCOPE_WILDCARD - if not external_key.endswith(glob_suffix): + suffix = cls.ID_SEPARATOR + GLOBAL_SCOPE_WILDCARD + if not external_key.endswith(suffix): return False org = cls.get_org(external_key) From e18ae568041e6a3f596ccf7a1e8d69f651edb9a8 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 13:38:21 -0500 Subject: [PATCH 09/32] docs: correct glob pattern examples in ScopeMeta documentation --- openedx_authz/api/data.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 4be371e2..25a93dc0 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -177,11 +177,11 @@ def __call__(cls, *args, **kwargs): 1. external_key: Determines subclass from the key format. The namespace prefix before the first ':' is used to look up the appropriate subclass. Example: ScopeData(external_key='lib:DemoX:CSPROB') → ContentLibraryData - Example: ScopeData(external_key='lib:DemoX*') → OrgLibraryGlobData + Example: ScopeData(external_key='lib:DemoX:*') → OrgLibraryGlobData 2. namespaced_key: Determines subclass from the namespace prefix before '^'. Example: ScopeData(namespaced_key='lib^lib:DemoX:CSPROB') → ContentLibraryData - Example: ScopeData(namespaced_key='lib^lib:DemoX*') → OrgLibraryGlobData + Example: ScopeData(namespaced_key='lib^lib:DemoX:*') → OrgLibraryGlobData Usage patterns: - namespaced_key: Used when retrieving objects from the policy store @@ -193,7 +193,7 @@ def __call__(cls, *args, **kwargs): >>> isinstance(scope, ContentLibraryData) True >>> # From glob external key - >>> scope = ScopeData(external_key='lib:DemoX*') + >>> scope = ScopeData(external_key='lib:DemoX:*') >>> isinstance(scope, OrgLibraryGlobData) True >>> # From namespaced key (e.g., policy store) @@ -230,7 +230,7 @@ def get_subclass_by_namespaced_key(mcs, namespaced_key: str) -> Type["ScopeData" If the key contains a wildcard, returns the appropriate glob subclass. Args: - namespaced_key: The namespaced key (e.g., 'lib^lib:DemoX:CSPROB', 'lib^lib:DemoX*', 'global^generic'). + namespaced_key: The namespaced key (e.g., 'lib^lib:DemoX:CSPROB', 'lib^lib:DemoX:*', 'global^generic'). Returns: The ScopeData subclass for the namespace, or ScopeData if namespace not recognized. @@ -240,7 +240,9 @@ def get_subclass_by_namespaced_key(mcs, namespaced_key: str) -> Type["ScopeData" >>> ScopeMeta.get_subclass_by_namespaced_key('lib^lib:DemoX:CSPROB') - >>> ScopeMeta.get_subclass_by_namespaced_key('lib^lib:DemoX*') + >>> ScopeMeta.get_subclass_by_namespaced_key('course-v1^course-v1:WGU+*') + + >>> ScopeMeta.get_subclass_by_namespaced_key('lib^lib:DemoX:*') >>> ScopeMeta.get_subclass_by_namespaced_key('global^generic') @@ -272,7 +274,7 @@ def get_subclass_by_external_key(mcs, external_key: str) -> Type["ScopeData"]: the appropriate glob subclass instead of the standard scope class. Args: - external_key: The external key (e.g., 'lib:DemoX:CSPROB', 'lib:DemoX*', 'global:generic'). + external_key: The external key (e.g., 'lib:DemoX:CSPROB', 'lib:DemoX:*', 'global:generic'). Returns: The ScopeData subclass corresponding to the namespace. @@ -286,9 +288,9 @@ def get_subclass_by_external_key(mcs, external_key: str) -> Type["ScopeData"]: >>> ScopeMeta.get_subclass_by_external_key('course-v1:OpenedX+CS101+2024') - >>> ScopeMeta.get_subclass_by_external_key('lib:DemoX*') + >>> ScopeMeta.get_subclass_by_external_key('lib:DemoX:*') - >>> ScopeMeta.get_subclass_by_external_key('course-v1:OpenedX*') + >>> ScopeMeta.get_subclass_by_external_key('course-v1:OpenedX+*') Notes: @@ -537,7 +539,7 @@ class OrgLibraryGlobData(ContentLibraryData): Attributes: NAMESPACE (str): Inherited 'lib' from ContentLibraryData. ID_SEPARATOR (str): ':' for content library scopes. - IS_GLOB (bool): True for organization-level glob patterns. + IS_GLOB (bool): True for scope data that represents a glob pattern. external_key (str): The glob pattern (e.g., ``lib:DemoX:*``). namespaced_key (str): The pattern with namespace (e.g., ``lib^lib:DemoX:*``). @@ -662,7 +664,6 @@ class CourseOverviewData(ScopeData): Attributes: NAMESPACE (str): 'course-v1' for course scopes. ID_SEPARATOR (str): '+' for course scopes. - IS_GLOB (bool): False for course scopes. external_key (str): The course identifier (e.g., 'course-v1:TestOrg+TestCourse+2024_T1'). Must be a valid CourseKey format. namespaced_key (str): The course identifier with namespace @@ -771,7 +772,7 @@ class OrgCourseGlobData(CourseOverviewData): Attributes: NAMESPACE (str): Inherited 'course-v1' from CourseOverviewData. ID_SEPARATOR (str): '+' for course scopes. - IS_GLOB (bool): True for organization-level glob patterns. + IS_GLOB (bool): True for scope data that represents a glob pattern. external_key (str): The glob pattern (e.g., 'course-v1:OpenedX+*'). namespaced_key (str): The pattern with namespace (e.g., 'course-v1^course-v1:OpenedX+*'). From 129def1743050122244e1f16152249abd14ee671 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 14:51:36 -0500 Subject: [PATCH 10/32] test: add unit tests for handling unknown and invalid scope keys --- openedx_authz/tests/api/test_data.py | 75 ++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 3fd18413..21f96361 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -325,6 +325,43 @@ def test_scope_validate_external_key(self, external_key, expected_valid, expecte self.assertEqual(result, expected_valid) + def test_get_subclass_by_external_key_unknown_scope_raises_value_error(self): + """Unknown namespace should raise ValueError in get_subclass_by_external_key.""" + with self.assertRaises(ValueError): + ScopeMeta.get_subclass_by_external_key("unknown:DemoX") + + def test_get_subclass_by_external_key_invalid_format_raises_value_error(self): + """Invalid format (fails subclass.validate_external_key) should raise ValueError.""" + with self.assertRaises(ValueError): + ScopeMeta.get_subclass_by_external_key("lib:invalid_library_key") + + def test_scope_meta_initializes_registries_when_missing(self): + """ScopeMeta should create registries if they don't exist on initialization. + + This validates the defensive branch in ScopeMeta.__init__ that initializes + scope_registry and glob_registry when they are not present on the class. + """ + original_scope_registry = ScopeMeta.scope_registry + original_glob_registry = ScopeMeta.glob_registry + + try: + # Simulate an environment where the registries are not yet defined + del ScopeMeta.scope_registry + del ScopeMeta.glob_registry + + class TempScope(ScopeData): + NAMESPACE = "temp" + + # Metaclass should have recreated the registries on the class + self.assertTrue(hasattr(TempScope, "scope_registry")) + self.assertTrue(hasattr(TempScope, "glob_registry")) + # And the new scope should be registered under its namespace + self.assertIs(TempScope.scope_registry.get("temp"), TempScope) + finally: + # Restore original registries to avoid side effects on other tests + ScopeMeta.scope_registry = original_scope_registry + ScopeMeta.glob_registry = original_glob_registry + def test_direct_subclass_instantiation_bypasses_metaclass(self): """Test that direct subclass instantiation doesn't trigger metaclass logic. @@ -680,6 +717,17 @@ class TestOrgLibraryGlobData(TestCase): ("lib:DemoX:*", True), ("lib:Org-123:*", True), ("lib:Org.with.dots:*", True), + ("lib:Org With Space:*", False), + ("lib:Org/With/Slash:*", False), + ("lib:Org\\With\\Backslash:*", False), + ("lib:Org,With,Comma:*", False), + ("lib:Org;With;Semicolon:*", False), + ("lib:Org@WithAt:*", False), + ("lib:Org#WithHash:*", False), + ("lib:Org$WithDollar:*", False), + ("lib:Org&WithAmp:*", False), + ("lib:Org+WithPlus:*", False), + ("lib:(Org):*", False), ("lib:Org", False), ("other:DemoX:*", False), ("lib:DemoX:*:*", False), @@ -695,6 +743,8 @@ def test_validate_external_key(self, external_key, expected_valid): ("lib:Org.with.dots:*", "Org.with.dots"), ("lib:Org:With:Colon:*", None), ("lib:*", None), + ("lib:DemoX", None), + ("lib:DemoX:*:*", None), ) @unpack def test_get_org(self, external_key, expected_org): @@ -728,6 +778,13 @@ def test_exists_false_when_org_exists_but_no_libraries_in_db(self): self.assertFalse(result) + def test_exists_false_when_org_cannot_be_parsed(self): + """exists() returns False when org property is None (invalid pattern).""" + scope = OrgLibraryGlobData(external_key="lib:Org:With:Colon:*") + + self.assertIsNone(scope.org) + self.assertFalse(scope.exists()) + @ddt @override_settings(OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL="course_overviews.CourseOverview") @@ -738,6 +795,17 @@ class TestOrgCourseGlobData(TestCase): ("course-v1:OpenedX+*", True), ("course-v1:My-Org_1+*", True), ("course-v1:Org.with.dots+*", True), + ("course-v1:Org With Space+*", False), + ("course-v1:Org/With/Slash+*", False), + ("course-v1:Org\\With\\Backslash+*", False), + ("course-v1:Org,With,Comma+*", False), + ("course-v1:Org;With;Semicolon+*", False), + ("course-v1:Org@WithAt+*", False), + ("course-v1:Org#WithHash+*", False), + ("course-v1:Org$WithDollar+*", False), + ("course-v1:Org&WithAmp+*", False), + ("course-v1:Org+WithPlus+*", False), + ("course-v1:(Org)+*", False), ("course-v1:Org:With:Plus+*", False), ("course-v1:OpenedX", False), ("other:OpenedX+*", False), @@ -786,3 +854,10 @@ def test_exists_false_when_org_exists_but_no_courses(self): result = OrgCourseGlobData(external_key=f"course-v1:{org_name}+*").exists() self.assertFalse(result) + + def test_exists_false_when_org_cannot_be_parsed(self): + """exists() returns False when org property is None (invalid pattern).""" + scope = OrgCourseGlobData(external_key="course-v1:Org:With:Colon+*") + + self.assertIsNone(scope.org) + self.assertFalse(scope.exists()) From f4dfa7e608dfcd5525d0688656d7400c65b2e1b8 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 14:59:12 -0500 Subject: [PATCH 11/32] test: add missing methods to TempScope class --- openedx_authz/tests/api/test_data.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 21f96361..2dd57a70 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -350,8 +350,15 @@ def test_scope_meta_initializes_registries_when_missing(self): del ScopeMeta.glob_registry class TempScope(ScopeData): + """Temporary scope class for testing.""" NAMESPACE = "temp" + def get_object(self): + return None + + def exists(self) -> bool: + return False + # Metaclass should have recreated the registries on the class self.assertTrue(hasattr(TempScope, "scope_registry")) self.assertTrue(hasattr(TempScope, "glob_registry")) From 43d2112c8f22bfd690e7d205baecd19518655744 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 15:45:55 -0500 Subject: [PATCH 12/32] docs: clarify allowed characters pattern for scope external keys --- openedx_authz/api/data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 25a93dc0..6592d07e 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -38,7 +38,7 @@ GLOBAL_SCOPE_WILDCARD = "*" NAMESPACED_KEY_PATTERN = rf"^.+{re.escape(AUTHZ_POLICY_ATTRIBUTES_SEPARATOR)}.+$" -# Pattern for allowed characters in organization identifiers (from opaque-keys library) +# Pattern for allowed characters in the parts of a scope external key (from opaque-keys library) # Matches: word characters (letters, digits, underscore), hyphens, tildes, periods, colons # Reference: opaque_keys.edx.locator.Locator.ALLOWED_ID_CHARS ALLOWED_CHARS_PATTERN = re.compile(r"^[\w\-~.:]+$", re.UNICODE) From 9bd86eaa8d92b52ceeefe4101d5b0f91d7eaa8bb Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 15:46:07 -0500 Subject: [PATCH 13/32] refactor: handle glob scopes in ScopeManager --- openedx_authz/models/core.py | 5 +++++ openedx_authz/models/scopes.py | 10 ---------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/openedx_authz/models/core.py b/openedx_authz/models/core.py index 326ecbc3..a3208464 100644 --- a/openedx_authz/models/core.py +++ b/openedx_authz/models/core.py @@ -63,6 +63,11 @@ def get_or_create_for_external_key(self, scope_data): Raises: ValueError: If the namespace is not registered """ + # For glob scopes we don't create a Scope object since + # they don't represent a specific resource type + if scope_data.IS_GLOB: + return None + namespace = scope_data.NAMESPACE if namespace not in (scope_registry := Scope.get_registry()): raise ValueError(f"No Scope subclass registered for namespace '{namespace}'") diff --git a/openedx_authz/models/scopes.py b/openedx_authz/models/scopes.py index b9e7d951..ba28bda4 100644 --- a/openedx_authz/models/scopes.py +++ b/openedx_authz/models/scopes.py @@ -92,11 +92,6 @@ def get_or_create_for_external_key(cls, scope) -> "ContentLibraryScope": ContentLibraryScope: The Scope instance for the given ContentLibrary, or None if the scope is a glob pattern (contains wildcard). """ - # For glob scopes we don't create a Scope object since - # they don't represent a specific content library - if scope.IS_GLOB: - return None - library_key = LibraryLocatorV2.from_string(scope.external_key) content_library = ContentLibrary.objects.get_by_key(library_key) scope, _ = cls.objects.get_or_create(content_library=content_library) @@ -141,11 +136,6 @@ def get_or_create_for_external_key(cls, scope) -> "CourseScope": CourseScope: The Scope instance for the given CourseOverview, or None if the scope is a glob pattern (contains wildcard). """ - # For glob scopes we don't create a Scope object - # since they don't represent a specific course - if scope.IS_GLOB: - return None - course_key = CourseKey.from_string(scope.external_key) course_overview = CourseOverview.get_from_id(course_key) scope, _ = cls.objects.get_or_create(course_overview=course_overview) From 4f6bd6cff04167a21fdd0e5d46d10f76b2cb5abb Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 16 Mar 2026 18:16:12 -0500 Subject: [PATCH 14/32] test: enhance organization-level glob enforcement tests for courses and libraries --- openedx_authz/tests/test_enforcement.py | 129 ++++++++++++------------ openedx_authz/tests/test_utils.py | 95 ++++++++++++++++- 2 files changed, 161 insertions(+), 63 deletions(-) diff --git a/openedx_authz/tests/test_enforcement.py b/openedx_authz/tests/test_enforcement.py index d7a378ed..8afd7f2b 100644 --- a/openedx_authz/tests/test_enforcement.py +++ b/openedx_authz/tests/test_enforcement.py @@ -17,16 +17,25 @@ from openedx_authz import ROOT_DIRECTORY from openedx_authz.api.data import GLOBAL_SCOPE_WILDCARD, ContentLibraryData, CourseOverviewData -from openedx_authz.constants import permissions, roles +from openedx_authz.constants import roles +from openedx_authz.constants.permissions import ( + COURSES_CREATE_FILES, + COURSES_VIEW_COURSE, + MANAGE_LIBRARY_TEAM, + VIEW_LIBRARY, +) from openedx_authz.engine.matcher import is_admin_or_superuser_check from openedx_authz.tests.test_utils import ( make_action_key, - make_course_key, + make_course_assignment, + make_course_case, + make_library_assignment, + make_library_case, make_library_key, + make_policy, make_role_key, make_scope_key, make_user_key, - make_wildcard_key, ) User = get_user_model() @@ -588,79 +597,75 @@ def test_wildcard_library_access(self, scope: str, expected_result: bool): @ddt -class OrgGlobEnforcementTests(CasbinEnforcementTestCase): +class OrgGlobCourseEnforcementTests(CasbinEnforcementTestCase): """ - Tests for organization-level glob patterns in course and library scopes. + Tests for organization-level glob patterns in course scopes. This test class verifies that policies defined with org-level glob patterns - (e.g., "course-v1:OpenedX*" or "lib:DemoX*") are correctly enforced for - concrete course and library scopes that belong to those organizations. + (e.g., "course-v1:OpenedX+*") are correctly enforced for concrete course + scopes that belong to those organizations. """ - POLICY = [ - # Policies - [ - "p", - make_role_key(roles.COURSE_STAFF.external_key), - make_action_key("courses.view_course"), - make_wildcard_key(CourseOverviewData.NAMESPACE), - "allow", - ], - [ - "p", - make_role_key(roles.LIBRARY_ADMIN.external_key), - make_action_key("content_libraries.view_library"), - make_wildcard_key(ContentLibraryData.NAMESPACE), - "allow", - ], - # Role assignments - [ - "g", - make_user_key("user-1"), - make_role_key(roles.COURSE_STAFF.external_key), - make_course_key("course-v1:OpenedX*"), - ], - [ - "g", - make_user_key("user-2"), - make_role_key(roles.LIBRARY_ADMIN.external_key), - make_library_key("lib:DemoX*"), - ], + POLICIES = [ + make_policy(roles.COURSE_STAFF.external_key, COURSES_VIEW_COURSE.identifier, CourseOverviewData.NAMESPACE) + ] + + ASSIGNMENTS = [ + make_course_assignment("user1", roles.COURSE_STAFF.external_key, "course-v1:OpenedX+*"), ] CASES = [ # Permission granted - { - "subject": make_user_key("user-1"), - "action": make_action_key(permissions.COURSES_VIEW_COURSE.action.external_key), - "scope": make_course_key("course-v1:OpenedX+DemoCourse+2026_T1"), - "expected_result": True, - }, - { - "subject": make_user_key("user-2"), - "action": make_action_key(permissions.VIEW_LIBRARY.action.external_key), - "scope": make_library_key("lib:DemoX:OrgGlobLib"), - "expected_result": True, - }, + make_course_case("user1", COURSES_VIEW_COURSE.identifier, "course-v1:OpenedX+Python+2026", True), + make_course_case("user1", COURSES_VIEW_COURSE.identifier, "course-v1:OpenedX+K8S+2027", True), + make_course_case("user1", COURSES_VIEW_COURSE.identifier, "course-v1:OpenedX+JS+2028", True), # Permission denied - { - "subject": make_user_key("user-1"), - "action": make_action_key(permissions.COURSES_VIEW_COURSE.action.external_key), - "scope": make_course_key("course-v1:InexistentOrg+DemoCourse+2026_T1"), - "expected_result": False, - }, - { - "subject": make_user_key("user-2"), - "action": make_action_key(permissions.VIEW_LIBRARY.action.external_key), - "scope": make_library_key("lib:InexistentOrg:OrgGlobLib"), - "expected_result": False, - }, + make_course_case("user1", COURSES_CREATE_FILES.identifier, "course-v1:OpenedX+Python+2026", False), + make_course_case("user1", COURSES_VIEW_COURSE.identifier, "course-v1:OpenedXv2+Demo+2026", False), + make_course_case("user1", COURSES_VIEW_COURSE.identifier, "course-v1:InexistentOrg+Demo+2026", False), + make_course_case("user2", COURSES_VIEW_COURSE.identifier, "course-v1:OpenedX+Demo+2026", False), ] @data(*CASES) def test_org_level_glob_enforcement(self, request: AuthRequest): - """Test that org-level glob patterns in scopes are enforced correctly.""" - self._test_enforcement(self.POLICY, request) + """Test that org-level glob patterns in course scopes are enforced correctly.""" + self._test_enforcement(self.POLICIES + self.ASSIGNMENTS, request) + + +@ddt +class OrgGlobLibraryEnforcementTests(CasbinEnforcementTestCase): + """ + Tests for organization-level glob patterns in library scopes. + + This test class verifies that policies defined with org-level glob patterns + (e.g., "lib:DemoX:*") are correctly enforced for concrete library scopes + that belong to those organizations. + """ + + POLICIES = [ + make_policy(roles.LIBRARY_ADMIN.external_key, VIEW_LIBRARY.identifier, ContentLibraryData.NAMESPACE), + ] + + ASSIGNMENTS = [ + make_library_assignment("user1", roles.LIBRARY_ADMIN.external_key, "lib:DemoX:*"), + ] + + CASES = [ + # Permission granted + make_library_case("user1", VIEW_LIBRARY.identifier, "lib:DemoX:CS101", True), + make_library_case("user1", VIEW_LIBRARY.identifier, "lib:DemoX:CS102", True), + make_library_case("user1", VIEW_LIBRARY.identifier, "lib:DemoX:CS500", True), + # Permission denied + make_library_case("user1", MANAGE_LIBRARY_TEAM.identifier, "lib:DemoX:CS101", False), + make_library_case("user1", VIEW_LIBRARY.identifier, "lib:InexistentOrg:CS101", False), + make_library_case("user1", VIEW_LIBRARY.identifier, "lib:DemoX-similar:CS101", False), + make_library_case("user2", VIEW_LIBRARY.identifier, "lib:DemoX:CS101", False), + ] + + @data(*CASES) + def test_org_level_glob_enforcement(self, request: AuthRequest): + """Test that org-level glob patterns in library scopes are enforced correctly.""" + self._test_enforcement(self.POLICIES + self.ASSIGNMENTS, request) @pytest.mark.django_db diff --git a/openedx_authz/tests/test_utils.py b/openedx_authz/tests/test_utils.py index 52c29a3c..e3497143 100644 --- a/openedx_authz/tests/test_utils.py +++ b/openedx_authz/tests/test_utils.py @@ -11,6 +11,99 @@ ) +def make_policy(role_key: str, action_key: str, scope_key: str, effect: str = "allow") -> list[str]: + """Create a policy. + + Args: + role_key (str): The role key of the policy. + action_key (str): The action key of the policy. + scope_key (str): The scope key of the policy. + effect (str): The effect of the policy. + + Returns: + list[str]: The policy. + """ + return [ + "p", + make_role_key(role_key), + make_action_key(action_key), + make_wildcard_key(scope_key), + effect, + ] + + +def make_library_assignment(user_key: str, role_key: str, scope_key: str) -> list[str]: + """Create a library assignment. + + Args: + user_key (str): The user key of the assignment. + role_key (str): The role key of the assignment. + scope_key (str): The scope key of the assignment. + """ + return [ + "g", + make_user_key(user_key), + make_role_key(role_key), + make_library_key(scope_key), + ] + + +def make_course_assignment(user_key: str, role_key: str, scope_key: str) -> list[str]: + """Create a course assignment. + + Args: + user_key (str): The user key of the assignment. + role_key (str): The role key of the assignment. + scope_key (str): The scope key of the assignment. + + Returns: + list[str]: The course assignment. + """ + return [ + "g", + make_user_key(user_key), + make_role_key(role_key), + make_course_key(scope_key), + ] + + +def make_course_case(username: str, permission: str, scope: str, expected_result: bool) -> dict: + """Create a course case test data. + + Args: + username (str): The username of the user. + permission (str): The permission to test. + scope (str): The scope to test. + expected_result (bool): The expected result. + + Returns: + dict: The course case. + """ + return { + "subject": make_user_key(username), + "action": make_action_key(permission), + "scope": make_course_key(scope), + "expected_result": expected_result, + } + + +def make_library_case(username: str, permission: str, scope: str, expected_result: bool) -> dict: + """Create a library case test data. + + Args: + username (str): The username of the user. + permission (str): The permission to test. + scope (str): The scope to test. + expected_result (bool): The expected result. + """ + return { + "subject": make_user_key(username), + "action": make_action_key(permission), + "scope": make_library_key(scope), + "expected_result": expected_result, + } + + def make_user_key(key: str) -> str: """Create a namespaced user key. @@ -88,7 +181,7 @@ def make_wildcard_key(namespace: str) -> str: """Create a wildcard pattern for a given namespace. Args: - namespace: The namespace to create a wildcard for (e.g., 'lib', 'org', 'course') + namespace (str): The namespace to create a wildcard for (e.g., 'lib', 'org', 'course') Returns: str: Wildcard pattern (e.g., 'lib^*', 'org^*', 'course^*') From a22a9ff4f0ce226d776cf84a85bdaf789780997d Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 10:16:43 -0500 Subject: [PATCH 15/32] refactor: simplify external key validation --- openedx_authz/api/data.py | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 6592d07e..f7310493 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -41,7 +41,7 @@ # Pattern for allowed characters in the parts of a scope external key (from opaque-keys library) # Matches: word characters (letters, digits, underscore), hyphens, tildes, periods, colons # Reference: opaque_keys.edx.locator.Locator.ALLOWED_ID_CHARS -ALLOWED_CHARS_PATTERN = re.compile(r"^[\w\-~.:]+$", re.UNICODE) +ALLOWED_CHARS_PATTERN = re.compile(r"^[\w\-~.]+$", re.UNICODE) class GroupingPolicyIndex(Enum): @@ -584,11 +584,6 @@ def validate_external_key(cls, external_key: str) -> bool: if not external_key.startswith(cls.NAMESPACE + EXTERNAL_KEY_SEPARATOR): return False - # Enforce explicit org-level separator: 'lib:ORG:*' - suffix = cls.ID_SEPARATOR + GLOBAL_SCOPE_WILDCARD - if not external_key.endswith(suffix): - return False - org = cls.get_org(external_key) if org is None: return False @@ -612,13 +607,14 @@ def get_org(cls, external_key: str) -> str | None: if not external_key.endswith(suffix): return None + # Remove the suffix (:*) from the external key + # lib:DemoX:* -> lib:DemoX scope_prefix = external_key[: -len(suffix)] - parts = scope_prefix.split(EXTERNAL_KEY_SEPARATOR) - - if len(parts) != 2 or not parts[1]: - return None + # Split the scope prefix by the separator + # lib:DemoX -> ['lib', 'DemoX'] + _namespace, org = scope_prefix.split(EXTERNAL_KEY_SEPARATOR, 1) - return parts[1] + return org @classmethod def org_exists(cls, org: str) -> bool: @@ -817,11 +813,6 @@ def validate_external_key(cls, external_key: str) -> bool: if not external_key.startswith(cls.NAMESPACE + EXTERNAL_KEY_SEPARATOR): return False - # Enforce explicit org-level separator: 'course-v1:ORG+*' - suffix = cls.ID_SEPARATOR + GLOBAL_SCOPE_WILDCARD - if not external_key.endswith(suffix): - return False - org = cls.get_org(external_key) if org is None: return False @@ -845,13 +836,14 @@ def get_org(cls, external_key: str) -> str | None: if not external_key.endswith(suffix): return None + # Remove the suffix (+*) from the external key + # course-v1:ORG+* -> course-v1:ORG scope_prefix = external_key[: -len(suffix)] - parts = scope_prefix.split(EXTERNAL_KEY_SEPARATOR) - - if len(parts) != 2 or not parts[1]: - return None + # Split the scope prefix by the separator + # course-v1:ORG -> ['course-v1', 'ORG'] + _namespace, org = scope_prefix.split(EXTERNAL_KEY_SEPARATOR, 1) - return parts[1] + return org @classmethod def org_exists(cls, org: str) -> bool: From a7f46a425bc77e0af2f8f5b15fce5faa423b911e Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 10:25:10 -0500 Subject: [PATCH 16/32] test: update tests according get_org method changes --- openedx_authz/tests/api/test_data.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 2dd57a70..350c02cd 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -351,6 +351,7 @@ def test_scope_meta_initializes_registries_when_missing(self): class TempScope(ScopeData): """Temporary scope class for testing.""" + NAMESPACE = "temp" def get_object(self): @@ -748,10 +749,9 @@ def test_validate_external_key(self, external_key, expected_valid): ("lib:DemoX:*", "DemoX"), ("lib:Org-123:*", "Org-123"), ("lib:Org.with.dots:*", "Org.with.dots"), - ("lib:Org:With:Colon:*", None), - ("lib:*", None), + ("lib:Org:With:Colon:*", "Org:With:Colon"), ("lib:DemoX", None), - ("lib:DemoX:*:*", None), + ("lib:DemoX:+*", None), ) @unpack def test_get_org(self, external_key, expected_org): @@ -787,7 +787,7 @@ def test_exists_false_when_org_exists_but_no_libraries_in_db(self): def test_exists_false_when_org_cannot_be_parsed(self): """exists() returns False when org property is None (invalid pattern).""" - scope = OrgLibraryGlobData(external_key="lib:Org:With:Colon:*") + scope = OrgLibraryGlobData(external_key="lib:Invalid+*") self.assertIsNone(scope.org) self.assertFalse(scope.exists()) @@ -827,8 +827,7 @@ def test_validate_external_key(self, external_key, expected_valid): ("course-v1:OpenedX+*", "OpenedX"), ("course-v1:My-Org_1+*", "My-Org_1"), ("course-v1:Org.with.dots+*", "Org.with.dots"), - ("course-v1:Org:With:Plus+*", None), - ("course-v1:*", None), + ("course-v1:Org:With:Plus+*", "Org:With:Plus"), ) @unpack def test_get_org(self, external_key, expected_org): @@ -864,7 +863,7 @@ def test_exists_false_when_org_exists_but_no_courses(self): def test_exists_false_when_org_cannot_be_parsed(self): """exists() returns False when org property is None (invalid pattern).""" - scope = OrgCourseGlobData(external_key="course-v1:Org:With:Colon+*") + scope = OrgCourseGlobData(external_key="course-v1:Invalid:*") self.assertIsNone(scope.org) self.assertFalse(scope.exists()) From a50a59823a3aab9cb6df91415690e7093d752b3d Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 12:18:13 -0500 Subject: [PATCH 17/32] chore: add edx-organizations in base requirements --- requirements/base.in | 1 + requirements/base.txt | 17 ++++++++++++++++- requirements/dev.txt | 23 ++++++++++++++++++++++- requirements/doc.txt | 23 ++++++++++++++++++++++- requirements/pip.txt | 2 +- requirements/quality.txt | 23 ++++++++++++++++++++++- requirements/test.txt | 23 ++++++++++++++++++++++- 7 files changed, 106 insertions(+), 6 deletions(-) diff --git a/requirements/base.in b/requirements/base.in index d10ac489..b3cc7793 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -11,3 +11,4 @@ edx-opaque-keys # Opaque keys for resource identification edx-api-doc-tools # Tools for API documentation edx-django-utils # Used for RequestCache edx-drf-extensions # Extensions for Django Rest Framework used by Open edX +edx-organizations # Organizations library for Open edX diff --git a/requirements/base.txt b/requirements/base.txt index 7c9b6703..7fc673d2 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -28,6 +28,8 @@ django==4.2.24 # -r requirements/base.in # casbin-django-orm-adapter # django-crum + # django-model-utils + # django-simple-history # django-waffle # djangorestframework # drf-jwt @@ -35,8 +37,13 @@ django==4.2.24 # edx-api-doc-tools # edx-django-utils # edx-drf-extensions + # edx-organizations django-crum==0.7.9 # via edx-django-utils +django-model-utils==5.0.0 + # via edx-organizations +django-simple-history==3.11.0 + # via edx-organizations django-waffle==5.0.0 # via # edx-django-utils @@ -48,6 +55,7 @@ djangorestframework==3.16.1 # drf-yasg # edx-api-doc-tools # edx-drf-extensions + # edx-organizations dnspython==2.8.0 # via pymongo drf-jwt==1.19.2 @@ -61,11 +69,16 @@ edx-django-utils==8.0.1 # -r requirements/base.in # edx-drf-extensions edx-drf-extensions==10.6.0 - # via -r requirements/base.in + # via + # -r requirements/base.in + # edx-organizations edx-opaque-keys==3.0.0 # via # -r requirements/base.in # edx-drf-extensions + # edx-organizations +edx-organizations==7.3.0 + # via -r requirements/base.in idna==3.10 # via requests inflection==0.5.1 @@ -74,6 +87,8 @@ openedx-atlas==0.7.0 # via -r requirements/base.in packaging==26.0 # via drf-yasg +pillow==12.1.1 + # via edx-organizations psutil==7.1.0 # via edx-django-utils pycasbin==2.2.0 diff --git a/requirements/dev.txt b/requirements/dev.txt index 1bf6e34d..4c854827 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -90,6 +90,8 @@ django==4.2.24 # -r requirements/quality.txt # casbin-django-orm-adapter # django-crum + # django-model-utils + # django-simple-history # django-waffle # djangorestframework # drf-jwt @@ -98,10 +100,19 @@ django==4.2.24 # edx-django-utils # edx-drf-extensions # edx-i18n-tools + # edx-organizations django-crum==0.7.9 # via # -r requirements/quality.txt # edx-django-utils +django-model-utils==5.0.0 + # via + # -r requirements/quality.txt + # edx-organizations +django-simple-history==3.11.0 + # via + # -r requirements/quality.txt + # edx-organizations django-waffle==5.0.0 # via # -r requirements/quality.txt @@ -114,6 +125,7 @@ djangorestframework==3.16.1 # drf-yasg # edx-api-doc-tools # edx-drf-extensions + # edx-organizations dnspython==2.8.0 # via # -r requirements/quality.txt @@ -133,7 +145,9 @@ edx-django-utils==8.0.1 # -r requirements/quality.txt # edx-drf-extensions edx-drf-extensions==10.6.0 - # via -r requirements/quality.txt + # via + # -r requirements/quality.txt + # edx-organizations edx-i18n-tools==1.9.0 # via -r requirements/dev.in edx-lint==5.6.0 @@ -142,6 +156,9 @@ edx-opaque-keys==3.0.0 # via # -r requirements/quality.txt # edx-drf-extensions + # edx-organizations +edx-organizations==7.3.0 + # via -r requirements/quality.txt filelock==3.20.3 # via # -r requirements/ci.txt @@ -197,6 +214,10 @@ packaging==26.0 # wheel path==16.16.0 # via edx-i18n-tools +pillow==12.1.1 + # via + # -r requirements/quality.txt + # edx-organizations pip-tools==7.5.3 # via -r requirements/pip-tools.txt platformdirs==4.5.1 diff --git a/requirements/doc.txt b/requirements/doc.txt index 5c64f87f..c46333f7 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -61,6 +61,8 @@ django==4.2.24 # -r requirements/test.txt # casbin-django-orm-adapter # django-crum + # django-model-utils + # django-simple-history # django-waffle # djangorestframework # drf-jwt @@ -68,10 +70,19 @@ django==4.2.24 # edx-api-doc-tools # edx-django-utils # edx-drf-extensions + # edx-organizations django-crum==0.7.9 # via # -r requirements/test.txt # edx-django-utils +django-model-utils==5.0.0 + # via + # -r requirements/test.txt + # edx-organizations +django-simple-history==3.11.0 + # via + # -r requirements/test.txt + # edx-organizations django-waffle==5.0.0 # via # -r requirements/test.txt @@ -84,6 +95,7 @@ djangorestframework==3.16.1 # drf-yasg # edx-api-doc-tools # edx-drf-extensions + # edx-organizations dnspython==2.8.0 # via # -r requirements/test.txt @@ -112,11 +124,16 @@ edx-django-utils==8.0.1 # -r requirements/test.txt # edx-drf-extensions edx-drf-extensions==10.6.0 - # via -r requirements/test.txt + # via + # -r requirements/test.txt + # edx-organizations edx-opaque-keys==3.0.0 # via # -r requirements/test.txt # edx-drf-extensions + # edx-organizations +edx-organizations==7.3.0 + # via -r requirements/test.txt id==1.5.0 # via twine idna==3.10 @@ -175,6 +192,10 @@ packaging==26.0 # pytest # sphinx # twine +pillow==12.1.1 + # via + # -r requirements/test.txt + # edx-organizations pluggy==1.6.0 # via # -r requirements/test.txt diff --git a/requirements/pip.txt b/requirements/pip.txt index 2abaf834..b86ca0e0 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -12,5 +12,5 @@ wheel==0.46.3 # The following packages are considered to be unsafe in a requirements file: pip==26.0.1 # via -r requirements/pip.in -setuptools==82.0.0 +setuptools==82.0.1 # via -r requirements/pip.in diff --git a/requirements/quality.txt b/requirements/quality.txt index e94b9601..ac92c0df 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -60,6 +60,8 @@ django==4.2.24 # -r requirements/test.txt # casbin-django-orm-adapter # django-crum + # django-model-utils + # django-simple-history # django-waffle # djangorestframework # drf-jwt @@ -67,10 +69,19 @@ django==4.2.24 # edx-api-doc-tools # edx-django-utils # edx-drf-extensions + # edx-organizations django-crum==0.7.9 # via # -r requirements/test.txt # edx-django-utils +django-model-utils==5.0.0 + # via + # -r requirements/test.txt + # edx-organizations +django-simple-history==3.11.0 + # via + # -r requirements/test.txt + # edx-organizations django-waffle==5.0.0 # via # -r requirements/test.txt @@ -83,6 +94,7 @@ djangorestframework==3.16.1 # drf-yasg # edx-api-doc-tools # edx-drf-extensions + # edx-organizations dnspython==2.8.0 # via # -r requirements/test.txt @@ -102,13 +114,18 @@ edx-django-utils==8.0.1 # -r requirements/test.txt # edx-drf-extensions edx-drf-extensions==10.6.0 - # via -r requirements/test.txt + # via + # -r requirements/test.txt + # edx-organizations edx-lint==5.6.0 # via -r requirements/quality.in edx-opaque-keys==3.0.0 # via # -r requirements/test.txt # edx-drf-extensions + # edx-organizations +edx-organizations==7.3.0 + # via -r requirements/test.txt idna==3.10 # via # -r requirements/test.txt @@ -140,6 +157,10 @@ packaging==26.0 # -r requirements/test.txt # drf-yasg # pytest +pillow==12.1.1 + # via + # -r requirements/test.txt + # edx-organizations platformdirs==4.5.1 # via pylint pluggy==1.6.0 diff --git a/requirements/test.txt b/requirements/test.txt index a4b37e00..36a8ae2d 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -45,6 +45,8 @@ ddt==1.7.2 # -r requirements/base.txt # casbin-django-orm-adapter # django-crum + # django-model-utils + # django-simple-history # django-waffle # djangorestframework # drf-jwt @@ -52,10 +54,19 @@ ddt==1.7.2 # edx-api-doc-tools # edx-django-utils # edx-drf-extensions + # edx-organizations django-crum==0.7.9 # via # -r requirements/base.txt # edx-django-utils +django-model-utils==5.0.0 + # via + # -r requirements/base.txt + # edx-organizations +django-simple-history==3.11.0 + # via + # -r requirements/base.txt + # edx-organizations django-waffle==5.0.0 # via # -r requirements/base.txt @@ -68,6 +79,7 @@ djangorestframework==3.16.1 # drf-yasg # edx-api-doc-tools # edx-drf-extensions + # edx-organizations dnspython==2.8.0 # via # -r requirements/base.txt @@ -87,11 +99,16 @@ edx-django-utils==8.0.1 # -r requirements/base.txt # edx-drf-extensions edx-drf-extensions==10.6.0 - # via -r requirements/base.txt + # via + # -r requirements/base.txt + # edx-organizations edx-opaque-keys==3.0.0 # via # -r requirements/base.txt # edx-drf-extensions + # edx-organizations +edx-organizations==7.3.0 + # via -r requirements/base.txt idna==3.10 # via # -r requirements/base.txt @@ -113,6 +130,10 @@ packaging==26.0 # -r requirements/base.txt # drf-yasg # pytest +pillow==12.1.1 + # via + # -r requirements/base.txt + # edx-organizations pluggy==1.6.0 # via # pytest From 8790af91b282bbe0a834b197fe6c4dad0c92fec2 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 13:25:23 -0500 Subject: [PATCH 18/32] refactor: add organization-level glob base scope --- openedx_authz/api/data.py | 245 +++++++++++++++----------------------- 1 file changed, 93 insertions(+), 152 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index f7310493..6ba0f0e2 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -12,10 +12,15 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 -from openedx_authz.models.scopes import get_content_library_model, get_course_overview_model +from openedx_authz.models.scopes import ( + get_content_library_model, + get_course_overview_model, + get_organization_model, +) ContentLibrary = get_content_library_model() CourseOverview = get_course_overview_model() +Organization = get_organization_model() __all__ = [ "UserData", @@ -38,11 +43,6 @@ GLOBAL_SCOPE_WILDCARD = "*" NAMESPACED_KEY_PATTERN = rf"^.+{re.escape(AUTHZ_POLICY_ATTRIBUTES_SEPARATOR)}.+$" -# Pattern for allowed characters in the parts of a scope external key (from opaque-keys library) -# Matches: word characters (letters, digits, underscore), hyphens, tildes, periods, colons -# Reference: opaque_keys.edx.locator.Locator.ALLOWED_ID_CHARS -ALLOWED_CHARS_PATTERN = re.compile(r"^[\w\-~.]+$", re.UNICODE) - class GroupingPolicyIndex(Enum): """Index positions for fields in a Casbin grouping policy (g or g2). @@ -527,40 +527,34 @@ def __repr__(self): @define -class OrgLibraryGlobData(ContentLibraryData): - """Organization-level glob pattern for content libraries. +class OrgGlobData(ScopeData): + """Base class for organization-scoped *glob* scope keys. - This class represents glob patterns that match multiple libraries within an organization. - Format: ``lib:ORG:*`` where ORG is a valid organization identifier. + This represents an *organization-wide* pattern: it matches "all resources within an + organization" for a given namespace, rather than a single concrete object. The pattern is + stored in :attr:`external_key` and **must** end with the global wildcard (``*``). - The glob pattern allows granting permissions to all libraries within a specific organization - without needing to specify each library individually. + The expected shape is:: - Attributes: - NAMESPACE (str): Inherited 'lib' from ContentLibraryData. - ID_SEPARATOR (str): ':' for content library scopes. - IS_GLOB (bool): True for scope data that represents a glob pattern. - external_key (str): The glob pattern (e.g., ``lib:DemoX:*``). - namespaced_key (str): The pattern with namespace (e.g., ``lib^lib:DemoX:*``). + {NAMESPACE}{EXTERNAL_KEY_SEPARATOR}{org}{ID_SEPARATOR}* - Validation Rules: - - Must end with GLOBAL_SCOPE_WILDCARD (``*``) - - Must have format ``lib:ORG:*`` (exactly one organization identifier) - - The organization must exist in at least one ContentLibrary - - Wildcard can only appear at the end after org identifier - - Cannot have wildcards at slug level (``lib:ORG:SLUG*`` is invalid) + where ``{org}`` is the organization identifier (e.g., ``DemoX``) and ``ID_SEPARATOR`` is + namespace-specific (subclasses must define it to match their key format). - Examples: - >>> glob = OrgLibraryGlobData(external_key='lib:DemoX:*') - >>> glob.org - 'DemoX' + Attributes: + IS_GLOB (bool): Always True for organization-level glob patterns. + ID_SEPARATOR (str): Separator used right before the wildcard (e.g., ``:`` or ``+``). + ORG_NAME_VALID_PATTERN (re.Pattern | str): Regex used to validate the organization + identifier extracted from :attr:`external_key`. - Note: - This class is automatically instantiated by the ScopeMeta metaclass when - a library scope with a wildcard is created. + Examples: + - ``lib:DemoX:*`` (all libraries in org ``DemoX``) + - ``course-v1:DemoX+*`` (all courses in org ``DemoX``) """ IS_GLOB: ClassVar[bool] = True + ID_SEPARATOR: ClassVar[str] + ORG_NAME_VALID_PATTERN: ClassVar[re.Pattern] = r"^[a-zA-Z0-9._-]*$" @property def org(self) -> str | None: @@ -573,10 +567,10 @@ def org(self) -> str | None: @classmethod def validate_external_key(cls, external_key: str) -> bool: - """Validate the external_key format for organization-level library globs. + """Validate the external_key format for organization-level glob patterns. Args: - external_key (str): The external key to validate (e.g., ``lib:DemoX:*``). + external_key (str): The external key to validate (e.g., ``lib:DemoX:*`` or ``course-v1:DemoX+*``). Returns: bool: True if the format is valid, False otherwise. @@ -588,7 +582,7 @@ def validate_external_key(cls, external_key: str) -> bool: if org is None: return False - if not ALLOWED_CHARS_PATTERN.match(org): + if not re.match(cls.ORG_NAME_VALID_PATTERN, org): return False return True @@ -603,52 +597,84 @@ def get_org(cls, external_key: str) -> str | None: Returns: str: The organization identifier (e.g., ``DemoX`` from ``lib:DemoX:*``), None otherwise. """ + if not hasattr(cls, "ID_SEPARATOR"): + raise NotImplementedError("OrgGlobBaseClass requires subclasses to define ID_SEPARATOR.") + suffix = cls.ID_SEPARATOR + GLOBAL_SCOPE_WILDCARD if not external_key.endswith(suffix): return None - # Remove the suffix (:*) from the external key - # lib:DemoX:* -> lib:DemoX + # Remove the suffix from the external key + # lib:DemoX:* -> lib:DemoX | course-v1:DemoX+* -> course-v1:DemoX scope_prefix = external_key[: -len(suffix)] # Split the scope prefix by the separator - # lib:DemoX -> ['lib', 'DemoX'] + # lib:DemoX -> ['lib', 'DemoX'] | course-v1:DemoX -> ['course-v1', 'DemoX'] _namespace, org = scope_prefix.split(EXTERNAL_KEY_SEPARATOR, 1) return org - @classmethod - def org_exists(cls, org: str) -> bool: - """Check if at least one content library exists with the given organization. - - Args: - org (str): Organization identifier to check. + def get_object(self) -> Organization | None: + """Retrieve the Organization instance associated with this scope. Returns: - bool: True if at least one library with this org exists, False otherwise. + Organization | None: The Organization instance if found, + or None if the organization does not exist. """ - lib_obj = ContentLibrary.objects.filter(org__short_name=org).only("org").last() - return lib_obj is not None and lib_obj.org.short_name == org + try: + org_obj = Organization.objects.get(short_name=self.org) + if org_obj.short_name != self.org: + raise Organization.DoesNotExist + return org_obj + except Organization.DoesNotExist: + return None - def get_object(self) -> None: - """Glob patterns don't represent a single object. + def exists(self) -> bool: + """Check if the organization exists. Returns: - None: Glob patterns match multiple objects, not a single one. + bool: True if the organization exists, False otherwise. """ - return None + return self.get_object() is not None - def exists(self) -> bool: - """Check if the glob pattern is valid. - For glob patterns, existence means the organization exists, - not that a specific library exists. +@define +class OrgLibraryGlobData(OrgGlobData): + """Organization-level glob pattern for content libraries. - Returns: - bool: True if the organization exists, False otherwise. - """ - if self.org is None: - return False - return self.org_exists(self.org) + This class represents glob patterns that match multiple libraries within an organization. + Format: ``lib:ORG:*`` where ORG is a valid organization identifier. + + The glob pattern allows granting permissions to all libraries within a specific organization + without needing to specify each library individually. + + Attributes: + NAMESPACE (str): 'lib' for content library scopes. + ID_SEPARATOR (str): ':' for content library scopes. + IS_GLOB (bool): True for scope data that represents a glob pattern. + external_key (str): The glob pattern (e.g., ``lib:DemoX:*``). + namespaced_key (str): The pattern with namespace (e.g., ``lib^lib:DemoX:*``). + + Validation Rules: + - Must end with GLOBAL_SCOPE_WILDCARD (``*``) + - Must have format ``lib:ORG:*`` (exactly one organization identifier) + - The organization must exist + - Wildcard can only appear at the end after org identifier + - Cannot have wildcards at slug level (``lib:ORG:SLUG*`` is invalid) + + Examples: + >>> glob = OrgLibraryGlobData(external_key='lib:DemoX:*') + >>> glob.org + 'DemoX' + >>> glob.get_object() + + + Note: + This class is automatically instantiated by the ScopeMeta metaclass when + a library scope with a wildcard is created. + """ + + NAMESPACE: ClassVar[str] = "lib" + ID_SEPARATOR: ClassVar[str] = ":" @define @@ -756,7 +782,7 @@ def __repr__(self): @define -class OrgCourseGlobData(CourseOverviewData): +class OrgCourseGlobData(OrgGlobData): """Organization-level glob pattern for courses. This class represents glob patterns that match multiple courses within an organization. @@ -766,7 +792,7 @@ class OrgCourseGlobData(CourseOverviewData): without needing to specify each course individually. Attributes: - NAMESPACE (str): Inherited 'course-v1' from CourseOverviewData. + NAMESPACE (str): 'course-v1' for course scopes. ID_SEPARATOR (str): '+' for course scopes. IS_GLOB (bool): True for scope data that represents a glob pattern. external_key (str): The glob pattern (e.g., 'course-v1:OpenedX+*'). @@ -775,7 +801,7 @@ class OrgCourseGlobData(CourseOverviewData): Validation Rules: - Must end with GLOBAL_SCOPE_WILDCARD (``*``) - Must have format 'course-v1:ORG+*' (exactly one organization identifier) - - The organization must exist in at least one CourseOverview + - The organization must exist - Wildcard can only appear at the end after org identifier - Cannot have wildcards at course or run level (course-v1:ORG+COURSE* is invalid) @@ -783,101 +809,16 @@ class OrgCourseGlobData(CourseOverviewData): >>> glob = OrgCourseGlobData(external_key='course-v1:OpenedX+*') >>> glob.org 'OpenedX' + >>> glob.get_object() + Note: This class is automatically instantiated by the ScopeMeta metaclass when a course scope with a wildcard is created. """ - IS_GLOB: ClassVar[bool] = True - - @property - def org(self) -> str | None: - """Get the organization identifier from the glob pattern. - - Returns: - str | None: The organization identifier (e.g., 'OpenedX' from 'course-v1:OpenedX+*'), None otherwise. - """ - return self.get_org(self.external_key) - - @classmethod - def validate_external_key(cls, external_key: str) -> bool: - """Validate the external_key format for organization-level course globs. - - Args: - external_key (str): The external key to validate (e.g., 'course-v1:OpenedX+*'). - - Returns: - bool: True if the format is valid, False otherwise. - """ - if not external_key.startswith(cls.NAMESPACE + EXTERNAL_KEY_SEPARATOR): - return False - - org = cls.get_org(external_key) - if org is None: - return False - - if not ALLOWED_CHARS_PATTERN.match(org): - return False - - return True - - @classmethod - def get_org(cls, external_key: str) -> str | None: - """Extract the organization identifier from the glob pattern. - - Args: - external_key (str): The external key to extract the organization identifier from. - - Returns: - str | None: The organization identifier (e.g., 'OpenedX' from 'course-v1:OpenedX+*'), None otherwise. - """ - suffix = cls.ID_SEPARATOR + GLOBAL_SCOPE_WILDCARD - if not external_key.endswith(suffix): - return None - - # Remove the suffix (+*) from the external key - # course-v1:ORG+* -> course-v1:ORG - scope_prefix = external_key[: -len(suffix)] - # Split the scope prefix by the separator - # course-v1:ORG -> ['course-v1', 'ORG'] - _namespace, org = scope_prefix.split(EXTERNAL_KEY_SEPARATOR, 1) - - return org - - @classmethod - def org_exists(cls, org: str) -> bool: - """Check if at least one course exists with the given organization. - - Args: - org (str): Organization identifier to check. - - Returns: - bool: True if at least one course with this org exists, False otherwise. - """ - course_obj = CourseOverview.objects.filter(org=org).only("org").last() - return course_obj is not None and course_obj.org == org - - def get_object(self) -> None: - """Glob patterns don't represent a single object. - - Returns: - None: Glob patterns match multiple objects, not a single one. - """ - return None - - def exists(self) -> bool: - """Check if the glob pattern is valid. - - For glob patterns, existence means the organization exists, - not that a specific course exists. - - Returns: - bool: True if the organization exists, False otherwise. - """ - if self.org is None: - return False - return self.org_exists(self.org) + NAMESPACE: ClassVar[str] = "course-v1" + ID_SEPARATOR: ClassVar[str] = "+" class SubjectMeta(type): From 9fc3afad17c7441776b9a7eaf08aa63d25943e69 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 13:25:40 -0500 Subject: [PATCH 19/32] feat: add function to retrieve organization model from settings --- openedx_authz/models/scopes.py | 19 +++++++++++++++++++ openedx_authz/settings/test.py | 1 + 2 files changed, 20 insertions(+) diff --git a/openedx_authz/models/scopes.py b/openedx_authz/models/scopes.py index ba28bda4..6f063559 100644 --- a/openedx_authz/models/scopes.py +++ b/openedx_authz/models/scopes.py @@ -50,8 +50,27 @@ def get_course_overview_model(): return None +def get_organization_model(): + """Return the Organization model class specified by settings. + + The setting `OPENEDX_AUTHZ_ORGANIZATION_MODEL` should be an + app_label.ModelName string (e.g. 'organizations.Organization'). + """ + ORGANIZATION_MODEL = getattr( + settings, + "OPENEDX_AUTHZ_ORGANIZATION_MODEL", + "organizations.Organization", + ) + try: + app_label, model_name = ORGANIZATION_MODEL.split(".") + return apps.get_model(app_label, model_name, require_ready=False) + except LookupError: + return None + + ContentLibrary = get_content_library_model() CourseOverview = get_course_overview_model() +Organization = get_organization_model() class ContentLibraryScope(Scope): diff --git a/openedx_authz/settings/test.py b/openedx_authz/settings/test.py index cce6d3de..c9a82016 100644 --- a/openedx_authz/settings/test.py +++ b/openedx_authz/settings/test.py @@ -77,3 +77,4 @@ def plugin_settings(settings): # pylint: disable=unused-argument # Use stub model for testing instead of the real content_libraries app OPENEDX_AUTHZ_CONTENT_LIBRARY_MODEL = "stubs.ContentLibrary" OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL = "stubs.CourseOverview" +OPENEDX_AUTHZ_ORGANIZATION_MODEL = "stubs.Organization" From 87d55fe2830d4bc6fa50186cb702015398381107 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 13:27:06 -0500 Subject: [PATCH 20/32] test: remove tests no longer applicable --- openedx_authz/tests/api/test_data.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 350c02cd..9d5ffb64 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -776,15 +776,6 @@ def test_exists_false_when_org_does_not_exist_in_db(self): self.assertFalse(result) - def test_exists_false_when_org_exists_but_no_libraries_in_db(self): - """exists() returns False when the org exists but no libraries exist in the DB.""" - org_name = "DemoX" - Organization.objects.create(short_name=org_name) - - result = OrgLibraryGlobData(external_key=f"lib:{org_name}:*").exists() - - self.assertFalse(result) - def test_exists_false_when_org_cannot_be_parsed(self): """exists() returns False when org property is None (invalid pattern).""" scope = OrgLibraryGlobData(external_key="lib:Invalid+*") @@ -852,15 +843,6 @@ def test_exists_false_when_org_does_not_exist(self): self.assertFalse(result) - def test_exists_false_when_org_exists_but_no_courses(self): - """exists() returns False when the org exists but no courses exist.""" - org_name = "OpenedX" - Organization.objects.create(short_name=org_name) - - result = OrgCourseGlobData(external_key=f"course-v1:{org_name}+*").exists() - - self.assertFalse(result) - def test_exists_false_when_org_cannot_be_parsed(self): """exists() returns False when org property is None (invalid pattern).""" scope = OrgCourseGlobData(external_key="course-v1:Invalid:*") From ee870af91df239f89c57c78daeb29c9a7179ef4d Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 13:31:03 -0500 Subject: [PATCH 21/32] chore: remove pylint useless statement --- openedx_authz/tests/integration/test_models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx_authz/tests/integration/test_models.py b/openedx_authz/tests/integration/test_models.py index 9d879dc5..b7d5a4f3 100644 --- a/openedx_authz/tests/integration/test_models.py +++ b/openedx_authz/tests/integration/test_models.py @@ -26,8 +26,8 @@ from django.contrib.auth import get_user_model from django.db import IntegrityError from django.test import TestCase, override_settings -from organizations.api import ensure_organization # pylint: disable=import-error -from organizations.models import Organization # pylint: disable=import-error +from organizations.api import ensure_organization +from organizations.models import Organization from openedx_authz.api.data import ContentLibraryData, RoleData, SubjectData, UserData from openedx_authz.api.roles import assign_role_to_subject_in_scope From 606252db58170921f6af3e062456dc214e376b87 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 14:21:52 -0500 Subject: [PATCH 22/32] refactor: update exported data models in openedx_authz api --- openedx_authz/api/data.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 6ba0f0e2..0800ccc2 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -23,19 +23,20 @@ Organization = get_organization_model() __all__ = [ - "UserData", - "PermissionData", + "ActionData", + "ContentLibraryData", + "CourseOverviewData", "GroupingPolicyIndex", + "OrgCourseGlobData", + "OrgGlobData", + "OrgLibraryGlobData", + "PermissionData", "PolicyIndex", - "ActionData", "RoleAssignmentData", "RoleData", "ScopeData", "SubjectData", - "ContentLibraryData", - "CourseOverviewData", - "OrgLibraryGlobData", - "OrgCourseGlobData", + "UserData", ] AUTHZ_POLICY_ATTRIBUTES_SEPARATOR = "^" From ad4b55fb68143595ce861830b3dae4864e458aee Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 15:51:23 -0500 Subject: [PATCH 23/32] refactor: improve glob registration logic in ScopeMeta --- openedx_authz/api/data.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 0800ccc2..a342881e 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -162,8 +162,7 @@ def __init__(cls, name, bases, attrs): if not hasattr(cls, "glob_registry"): cls.glob_registry = {} - # Register glob classes (they have 'Glob' in their name) - if "Glob" in name and cls.NAMESPACE: + if cls.IS_GLOB and cls.NAMESPACE: cls.glob_registry[cls.NAMESPACE] = cls else: cls.scope_registry[cls.NAMESPACE] = cls From 04723d01f9fc966fb672057e4357d720f04bcb4a Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 16:08:21 -0500 Subject: [PATCH 24/32] refactor: rename OrgLibraryGlobData and OrgCourseGlobData --- openedx_authz/api/data.py | 26 +++++++-------- openedx_authz/tests/api/test_data.py | 48 ++++++++++++++-------------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index a342881e..eb6c52fb 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -27,9 +27,9 @@ "ContentLibraryData", "CourseOverviewData", "GroupingPolicyIndex", - "OrgCourseGlobData", + "OrgCourseOverviewGlobData", "OrgGlobData", - "OrgLibraryGlobData", + "OrgContentLibraryGlobData", "PermissionData", "PolicyIndex", "RoleAssignmentData", @@ -177,11 +177,11 @@ def __call__(cls, *args, **kwargs): 1. external_key: Determines subclass from the key format. The namespace prefix before the first ':' is used to look up the appropriate subclass. Example: ScopeData(external_key='lib:DemoX:CSPROB') → ContentLibraryData - Example: ScopeData(external_key='lib:DemoX:*') → OrgLibraryGlobData + Example: ScopeData(external_key='lib:DemoX:*') → OrgContentLibraryGlobData 2. namespaced_key: Determines subclass from the namespace prefix before '^'. Example: ScopeData(namespaced_key='lib^lib:DemoX:CSPROB') → ContentLibraryData - Example: ScopeData(namespaced_key='lib^lib:DemoX:*') → OrgLibraryGlobData + Example: ScopeData(namespaced_key='lib^lib:DemoX:*') → OrgContentLibraryGlobData Usage patterns: - namespaced_key: Used when retrieving objects from the policy store @@ -194,7 +194,7 @@ def __call__(cls, *args, **kwargs): True >>> # From glob external key >>> scope = ScopeData(external_key='lib:DemoX:*') - >>> isinstance(scope, OrgLibraryGlobData) + >>> isinstance(scope, OrgContentLibraryGlobData) True >>> # From namespaced key (e.g., policy store) >>> scope = ScopeData(namespaced_key='lib^lib:DemoX:CSPROB') @@ -241,9 +241,9 @@ def get_subclass_by_namespaced_key(mcs, namespaced_key: str) -> Type["ScopeData" >>> ScopeMeta.get_subclass_by_namespaced_key('lib^lib:DemoX:CSPROB') >>> ScopeMeta.get_subclass_by_namespaced_key('course-v1^course-v1:WGU+*') - + >>> ScopeMeta.get_subclass_by_namespaced_key('lib^lib:DemoX:*') - + >>> ScopeMeta.get_subclass_by_namespaced_key('global^generic') """ @@ -289,9 +289,9 @@ def get_subclass_by_external_key(mcs, external_key: str) -> Type["ScopeData"]: >>> ScopeMeta.get_subclass_by_external_key('course-v1:OpenedX+CS101+2024') >>> ScopeMeta.get_subclass_by_external_key('lib:DemoX:*') - + >>> ScopeMeta.get_subclass_by_external_key('course-v1:OpenedX+*') - + Notes: - The external_key format should be 'namespace:some-identifier' (e.g., 'lib:DemoX:CSPROB'). @@ -638,7 +638,7 @@ def exists(self) -> bool: @define -class OrgLibraryGlobData(OrgGlobData): +class OrgContentLibraryGlobData(OrgGlobData): """Organization-level glob pattern for content libraries. This class represents glob patterns that match multiple libraries within an organization. @@ -662,7 +662,7 @@ class OrgLibraryGlobData(OrgGlobData): - Cannot have wildcards at slug level (``lib:ORG:SLUG*`` is invalid) Examples: - >>> glob = OrgLibraryGlobData(external_key='lib:DemoX:*') + >>> glob = OrgContentLibraryGlobData(external_key='lib:DemoX:*') >>> glob.org 'DemoX' >>> glob.get_object() @@ -782,7 +782,7 @@ def __repr__(self): @define -class OrgCourseGlobData(OrgGlobData): +class OrgCourseOverviewGlobData(OrgGlobData): """Organization-level glob pattern for courses. This class represents glob patterns that match multiple courses within an organization. @@ -806,7 +806,7 @@ class OrgCourseGlobData(OrgGlobData): - Cannot have wildcards at course or run level (course-v1:ORG+COURSE* is invalid) Examples: - >>> glob = OrgCourseGlobData(external_key='course-v1:OpenedX+*') + >>> glob = OrgCourseOverviewGlobData(external_key='course-v1:OpenedX+*') >>> glob.org 'OpenedX' >>> glob.get_object() diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 9d5ffb64..ba106376 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -10,8 +10,8 @@ ActionData, ContentLibraryData, CourseOverviewData, - OrgCourseGlobData, - OrgLibraryGlobData, + OrgCourseOverviewGlobData, + OrgContentLibraryGlobData, PermissionData, RoleAssignmentData, RoleData, @@ -238,15 +238,15 @@ def test_scope_data_registration(self): # Glob registries for organization-level scopes self.assertIn("lib", ScopeMeta.glob_registry) - self.assertIs(ScopeMeta.glob_registry["lib"], OrgLibraryGlobData) + self.assertIs(ScopeMeta.glob_registry["lib"], OrgContentLibraryGlobData) self.assertIn("course-v1", ScopeMeta.glob_registry) - self.assertIs(ScopeMeta.glob_registry["course-v1"], OrgCourseGlobData) + self.assertIs(ScopeMeta.glob_registry["course-v1"], OrgCourseOverviewGlobData) @data( ("course-v1^course-v1:WGU+CS002+2025_T1", CourseOverviewData), ("lib^lib:DemoX:CSPROB", ContentLibraryData), - ("lib^lib:DemoX*", OrgLibraryGlobData), - ("course-v1^course-v1:OpenedX*", OrgCourseGlobData), + ("lib^lib:DemoX*", OrgContentLibraryGlobData), + ("course-v1^course-v1:OpenedX*", OrgCourseOverviewGlobData), ("global^generic_scope", ScopeData), ) @unpack @@ -265,8 +265,8 @@ def test_dynamic_instantiation_via_namespaced_key(self, namespaced_key, expected @data( ("course-v1^course-v1:WGU+CS002+2025_T1", CourseOverviewData), ("lib^lib:DemoX:CSPROB", ContentLibraryData), - ("lib^lib:DemoX:*", OrgLibraryGlobData), - ("course-v1^course-v1:OpenedX+*", OrgCourseGlobData), + ("lib^lib:DemoX:*", OrgContentLibraryGlobData), + ("course-v1^course-v1:OpenedX+*", OrgCourseOverviewGlobData), ("global^generic", ScopeData), ("unknown^something", ScopeData), ) @@ -286,8 +286,8 @@ def test_get_subclass_by_namespaced_key(self, namespaced_key, expected_class): @data( ("course-v1:WGU+CS002+2025_T1", CourseOverviewData), ("lib:DemoX:CSPROB", ContentLibraryData), - ("lib:DemoX:*", OrgLibraryGlobData), - ("course-v1:OpenedX+*", OrgCourseGlobData), + ("lib:DemoX:*", OrgContentLibraryGlobData), + ("course-v1:OpenedX+*", OrgCourseOverviewGlobData), ("lib:edX:Demo", ContentLibraryData), ("global:generic_scope", ScopeData), ) @@ -718,8 +718,8 @@ def test_exists_returns_false_when_library_does_not_exist(self, mock_content_lib @ddt @override_settings(OPENEDX_AUTHZ_CONTENT_LIBRARY_MODEL="content_libraries.ContentLibrary") -class TestOrgLibraryGlobData(TestCase): - """Tests for the OrgLibraryGlobData scope.""" +class TestOrgContentLibraryGlobData(TestCase): + """Tests for the OrgContentLibraryGlobData scope.""" @data( ("lib:DemoX:*", True), @@ -743,7 +743,7 @@ class TestOrgLibraryGlobData(TestCase): @unpack def test_validate_external_key(self, external_key, expected_valid): """Validate organization-level library glob external keys.""" - self.assertEqual(OrgLibraryGlobData.validate_external_key(external_key), expected_valid) + self.assertEqual(OrgContentLibraryGlobData.validate_external_key(external_key), expected_valid) @data( ("lib:DemoX:*", "DemoX"), @@ -756,7 +756,7 @@ def test_validate_external_key(self, external_key, expected_valid): @unpack def test_get_org(self, external_key, expected_org): """Test organization extraction from library glob pattern.""" - self.assertEqual(OrgLibraryGlobData.get_org(external_key), expected_org) + self.assertEqual(OrgContentLibraryGlobData.get_org(external_key), expected_org) def test_exists_true_when_org_has_libraries_in_db(self): """exists() returns True when at least one library with the org exists in the DB.""" @@ -764,7 +764,7 @@ def test_exists_true_when_org_has_libraries_in_db(self): organization = Organization.objects.create(short_name=org_name) ContentLibrary.objects.create(org=organization, slug="testlib", title="Test Library") - result = OrgLibraryGlobData(external_key=f"lib:{org_name}:*").exists() + result = OrgContentLibraryGlobData(external_key=f"lib:{org_name}:*").exists() self.assertTrue(result) @@ -772,13 +772,13 @@ def test_exists_false_when_org_does_not_exist_in_db(self): """exists() returns False when the org does not exist in the DB.""" org_name = "DemoX" - result = OrgLibraryGlobData(external_key=f"lib:{org_name}:*").exists() + result = OrgContentLibraryGlobData(external_key=f"lib:{org_name}:*").exists() self.assertFalse(result) def test_exists_false_when_org_cannot_be_parsed(self): """exists() returns False when org property is None (invalid pattern).""" - scope = OrgLibraryGlobData(external_key="lib:Invalid+*") + scope = OrgContentLibraryGlobData(external_key="lib:Invalid+*") self.assertIsNone(scope.org) self.assertFalse(scope.exists()) @@ -786,8 +786,8 @@ def test_exists_false_when_org_cannot_be_parsed(self): @ddt @override_settings(OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL="course_overviews.CourseOverview") -class TestOrgCourseGlobData(TestCase): - """Tests for the OrgCourseGlobData scope.""" +class TestOrgCourseOverviewGlobData(TestCase): + """Tests for the OrgCourseOverviewGlobData scope.""" @data( ("course-v1:OpenedX+*", True), @@ -812,7 +812,7 @@ class TestOrgCourseGlobData(TestCase): @unpack def test_validate_external_key(self, external_key, expected_valid): """Validate organization-level course glob external keys.""" - self.assertEqual(OrgCourseGlobData.validate_external_key(external_key), expected_valid) + self.assertEqual(OrgCourseOverviewGlobData.validate_external_key(external_key), expected_valid) @data( ("course-v1:OpenedX+*", "OpenedX"), @@ -823,7 +823,7 @@ def test_validate_external_key(self, external_key, expected_valid): @unpack def test_get_org(self, external_key, expected_org): """Test organization extraction from course glob pattern.""" - self.assertEqual(OrgCourseGlobData.get_org(external_key), expected_org) + self.assertEqual(OrgCourseOverviewGlobData.get_org(external_key), expected_org) def test_exists_true_when_org_has_courses(self): """exists() returns True when at least one course with the org exists.""" @@ -831,7 +831,7 @@ def test_exists_true_when_org_has_courses(self): Organization.objects.create(short_name=org_name) CourseOverview.objects.create(org=org_name, display_name="Test Course") - result = OrgCourseGlobData(external_key=f"course-v1:{org_name}+*").exists() + result = OrgCourseOverviewGlobData(external_key=f"course-v1:{org_name}+*").exists() self.assertTrue(result) @@ -839,13 +839,13 @@ def test_exists_false_when_org_does_not_exist(self): """exists() returns False when the org does not exist.""" org_name = "OpenedX" - result = OrgCourseGlobData(external_key=f"course-v1:{org_name}+*").exists() + result = OrgCourseOverviewGlobData(external_key=f"course-v1:{org_name}+*").exists() self.assertFalse(result) def test_exists_false_when_org_cannot_be_parsed(self): """exists() returns False when org property is None (invalid pattern).""" - scope = OrgCourseGlobData(external_key="course-v1:Invalid:*") + scope = OrgCourseOverviewGlobData(external_key="course-v1:Invalid:*") self.assertIsNone(scope.org) self.assertFalse(scope.exists()) From 4dee36704d344563ed3d115f86ceaf98ee66b6ac Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 16:10:23 -0500 Subject: [PATCH 25/32] chore: reorder import statements --- openedx_authz/tests/api/test_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index ba106376..1e9b77b3 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -10,8 +10,8 @@ ActionData, ContentLibraryData, CourseOverviewData, - OrgCourseOverviewGlobData, OrgContentLibraryGlobData, + OrgCourseOverviewGlobData, PermissionData, RoleAssignmentData, RoleData, From 8cad408c7e36de78ed8f5fd948f744c6bd6c15d2 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 16:35:56 -0500 Subject: [PATCH 26/32] chore: organize code --- openedx_authz/api/data.py | 208 +++++++++++++++++++------------------- 1 file changed, 104 insertions(+), 104 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index eb6c52fb..d85cf1c4 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -526,6 +526,110 @@ def __repr__(self): return self.namespaced_key +@define +class CourseOverviewData(ScopeData): + """A course scope for authorization in the Open edX platform. + + Courses uses the CourseKey format for identification. + + Attributes: + NAMESPACE (str): 'course-v1' for course scopes. + ID_SEPARATOR (str): '+' for course scopes. + external_key (str): The course identifier (e.g., 'course-v1:TestOrg+TestCourse+2024_T1'). + Must be a valid CourseKey format. + namespaced_key (str): The course identifier with namespace + (e.g., 'course-v1^course-v1:TestOrg+TestCourse+2024_T1'). + course_id (str): Property alias for external_key. + + Examples: + >>> course = CourseOverviewData(external_key='course-v1:TestOrg+TestCourse+2024_T1') + >>> course.namespaced_key + 'course-v1^course-v1:TestOrg+TestCourse+2024_T1' + >>> course.course_id + 'course-v1:TestOrg+TestCourse+2024_T1' + """ + + NAMESPACE: ClassVar[str] = "course-v1" + ID_SEPARATOR: ClassVar[str] = "+" + + @property + def course_id(self) -> str: + """The course identifier as used in Open edX (e.g., 'course-v1:TestOrg+TestCourse+2024_T1'). + + This is an alias for external_key that represents the course ID without the namespace prefix. + + Returns: + str: The course identifier without namespace. + """ + return self.external_key + + @property + def course_key(self) -> CourseKey: + """The CourseKey object for the course. + + Returns: + CourseKey: The course key object. + """ + return CourseKey.from_string(self.course_id) + + @classmethod + def validate_external_key(cls, external_key: str) -> bool: + """Validate the external_key format for CourseOverviewData. + + Args: + external_key: The external key to validate. + + Returns: + bool: True if valid, False otherwise. + """ + try: + CourseKey.from_string(external_key) + return True + except InvalidKeyError: + return False + + def get_object(self) -> CourseOverview | None: + """Retrieve the CourseOverview instance associated with this scope. + + This method converts the course_id to a CourseKey and queries the + database to fetch the corresponding CourseOverview object. + + Returns: + CourseOverview | None: The CourseOverview instance if found in the database, + or None if the course does not exist or has an invalid key format. + + Examples: + >>> course_scope = CourseOverviewData(external_key='course-v1:TestOrg+TestCourse+2024_T1') + >>> course_obj = course_scope.get_object() # CourseOverview object + """ + try: + course_obj = CourseOverview.get_from_id(self.course_key) + # Validate canonical key: get_by_key is case-insensitive, but we require exact match + # This ensures authorization uses canonical course IDs consistently + if course_obj.id != self.course_key: + raise CourseOverview.DoesNotExist + except (InvalidKeyError, CourseOverview.DoesNotExist): + return None + + return course_obj + + def exists(self) -> bool: + """Check if the course overview exists. + + Returns: + bool: True if the course overview exists, False otherwise. + """ + return self.get_object() is not None + + def __str__(self): + """Human readable string representation of the course overview.""" + return self.course_id + + def __repr__(self): + """Developer friendly string representation of the course overview.""" + return self.namespaced_key + + @define class OrgGlobData(ScopeData): """Base class for organization-scoped *glob* scope keys. @@ -677,110 +781,6 @@ class OrgContentLibraryGlobData(OrgGlobData): ID_SEPARATOR: ClassVar[str] = ":" -@define -class CourseOverviewData(ScopeData): - """A course scope for authorization in the Open edX platform. - - Courses uses the CourseKey format for identification. - - Attributes: - NAMESPACE (str): 'course-v1' for course scopes. - ID_SEPARATOR (str): '+' for course scopes. - external_key (str): The course identifier (e.g., 'course-v1:TestOrg+TestCourse+2024_T1'). - Must be a valid CourseKey format. - namespaced_key (str): The course identifier with namespace - (e.g., 'course-v1^course-v1:TestOrg+TestCourse+2024_T1'). - course_id (str): Property alias for external_key. - - Examples: - >>> course = CourseOverviewData(external_key='course-v1:TestOrg+TestCourse+2024_T1') - >>> course.namespaced_key - 'course-v1^course-v1:TestOrg+TestCourse+2024_T1' - >>> course.course_id - 'course-v1:TestOrg+TestCourse+2024_T1' - """ - - NAMESPACE: ClassVar[str] = "course-v1" - ID_SEPARATOR: ClassVar[str] = "+" - - @property - def course_id(self) -> str: - """The course identifier as used in Open edX (e.g., 'course-v1:TestOrg+TestCourse+2024_T1'). - - This is an alias for external_key that represents the course ID without the namespace prefix. - - Returns: - str: The course identifier without namespace. - """ - return self.external_key - - @property - def course_key(self) -> CourseKey: - """The CourseKey object for the course. - - Returns: - CourseKey: The course key object. - """ - return CourseKey.from_string(self.course_id) - - @classmethod - def validate_external_key(cls, external_key: str) -> bool: - """Validate the external_key format for CourseOverviewData. - - Args: - external_key: The external key to validate. - - Returns: - bool: True if valid, False otherwise. - """ - try: - CourseKey.from_string(external_key) - return True - except InvalidKeyError: - return False - - def get_object(self) -> CourseOverview | None: - """Retrieve the CourseOverview instance associated with this scope. - - This method converts the course_id to a CourseKey and queries the - database to fetch the corresponding CourseOverview object. - - Returns: - CourseOverview | None: The CourseOverview instance if found in the database, - or None if the course does not exist or has an invalid key format. - - Examples: - >>> course_scope = CourseOverviewData(external_key='course-v1:TestOrg+TestCourse+2024_T1') - >>> course_obj = course_scope.get_object() # CourseOverview object - """ - try: - course_obj = CourseOverview.get_from_id(self.course_key) - # Validate canonical key: get_by_key is case-insensitive, but we require exact match - # This ensures authorization uses canonical course IDs consistently - if course_obj.id != self.course_key: - raise CourseOverview.DoesNotExist - except (InvalidKeyError, CourseOverview.DoesNotExist): - return None - - return course_obj - - def exists(self) -> bool: - """Check if the course overview exists. - - Returns: - bool: True if the course overview exists, False otherwise. - """ - return self.get_object() is not None - - def __str__(self): - """Human readable string representation of the course overview.""" - return self.course_id - - def __repr__(self): - """Developer friendly string representation of the course overview.""" - return self.namespaced_key - - @define class OrgCourseOverviewGlobData(OrgGlobData): """Organization-level glob pattern for courses. From 03cfa1e0a1545695897ad1a6b54f038e755ae437 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 17 Mar 2026 18:13:57 -0500 Subject: [PATCH 27/32] chore: bump version to 1.1.0 --- CHANGELOG.rst | 16 ++++++++++++++++ openedx_authz/__init__.py | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 108ceab0..83bbefa8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,22 @@ Change Log Unreleased ********** +1.1.0 - 2026-03-17 +****************** + +Added +===== + +* Add support for organization global scopes. + +1.0.0 - 2026-03-13 +****************** + +Removed +======= + +* Dropped support for Python 3.11. + 0.23.0 - 2026-02-18 ******************** diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index ed41e993..08840f95 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "1.0.0" +__version__ = "1.1.0" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__)) From e4980151239c17f819ac02ae524af315b037ec23 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Thu, 19 Mar 2026 18:51:36 -0500 Subject: [PATCH 28/32] test: add more test cases --- openedx_authz/settings/common.py | 4 ++++ openedx_authz/tests/api/test_data.py | 25 ++++++++++++++++--------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/openedx_authz/settings/common.py b/openedx_authz/settings/common.py index 81e060b8..0d9f7445 100644 --- a/openedx_authz/settings/common.py +++ b/openedx_authz/settings/common.py @@ -50,6 +50,10 @@ def plugin_settings(settings): if not hasattr(settings, "OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL"): settings.OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL = "course_overviews.CourseOverview" + # Set default Organization model for swappable dependency + if not hasattr(settings, "OPENEDX_AUTHZ_ORGANIZATION_MODEL"): + settings.OPENEDX_AUTHZ_ORGANIZATION_MODEL = "organizations.Organization" + # Set default CASBIN_LOG_LEVEL if not already set. # This setting defines the logging level for the Casbin enforcer. if not hasattr(settings, "CASBIN_LOG_LEVEL"): diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 1e9b77b3..dc0b1a88 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -3,7 +3,7 @@ from unittest.mock import Mock, patch from ddt import data, ddt, unpack -from django.test import TestCase, override_settings +from django.test import TestCase from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_authz.api.data import ( @@ -717,7 +717,6 @@ def test_exists_returns_false_when_library_does_not_exist(self, mock_content_lib @ddt -@override_settings(OPENEDX_AUTHZ_CONTENT_LIBRARY_MODEL="content_libraries.ContentLibrary") class TestOrgContentLibraryGlobData(TestCase): """Tests for the OrgContentLibraryGlobData scope.""" @@ -737,6 +736,7 @@ class TestOrgContentLibraryGlobData(TestCase): ("lib:Org+WithPlus:*", False), ("lib:(Org):*", False), ("lib:Org", False), + ("lib:Org*", False), ("other:DemoX:*", False), ("lib:DemoX:*:*", False), ) @@ -752,14 +752,17 @@ def test_validate_external_key(self, external_key, expected_valid): ("lib:Org:With:Colon:*", "Org:With:Colon"), ("lib:DemoX", None), ("lib:DemoX:+*", None), + ("lib:DemoX*", None), + ("lib:DemoX:**", None), + ("lib:DemoX:suffix", None), ) @unpack def test_get_org(self, external_key, expected_org): """Test organization extraction from library glob pattern.""" self.assertEqual(OrgContentLibraryGlobData.get_org(external_key), expected_org) - def test_exists_true_when_org_has_libraries_in_db(self): - """exists() returns True when at least one library with the org exists in the DB.""" + def test_exists_true_when_org_exists(self): + """exists() returns True when the org exists.""" org_name = "DemoX" organization = Organization.objects.create(short_name=org_name) ContentLibrary.objects.create(org=organization, slug="testlib", title="Test Library") @@ -768,8 +771,8 @@ def test_exists_true_when_org_has_libraries_in_db(self): self.assertTrue(result) - def test_exists_false_when_org_does_not_exist_in_db(self): - """exists() returns False when the org does not exist in the DB.""" + def test_exists_false_when_org_does_not_exist(self): + """exists() returns False when the org does not exist.""" org_name = "DemoX" result = OrgContentLibraryGlobData(external_key=f"lib:{org_name}:*").exists() @@ -785,7 +788,6 @@ def test_exists_false_when_org_cannot_be_parsed(self): @ddt -@override_settings(OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL="course_overviews.CourseOverview") class TestOrgCourseOverviewGlobData(TestCase): """Tests for the OrgCourseOverviewGlobData scope.""" @@ -806,6 +808,7 @@ class TestOrgCourseOverviewGlobData(TestCase): ("course-v1:(Org)+*", False), ("course-v1:Org:With:Plus+*", False), ("course-v1:OpenedX", False), + ("course-v1:OpenedX*", False), ("other:OpenedX+*", False), ("course-v1:OpenedX**", False), ) @@ -819,14 +822,18 @@ def test_validate_external_key(self, external_key, expected_valid): ("course-v1:My-Org_1+*", "My-Org_1"), ("course-v1:Org.with.dots+*", "Org.with.dots"), ("course-v1:Org:With:Plus+*", "Org:With:Plus"), + ("course-v1:OpenedX", None), + ("course-v1:OpenedX*", None), + ("course-v1:OpenedX+**", None), + ("course-v1:OpenedX+suffix", None), ) @unpack def test_get_org(self, external_key, expected_org): """Test organization extraction from course glob pattern.""" self.assertEqual(OrgCourseOverviewGlobData.get_org(external_key), expected_org) - def test_exists_true_when_org_has_courses(self): - """exists() returns True when at least one course with the org exists.""" + def test_exists_true_when_org_exists(self): + """exists() returns True when the org exists.""" org_name = "OpenedX" Organization.objects.create(short_name=org_name) CourseOverview.objects.create(org=org_name, display_name="Test Course") From d3d4238146e5ca2804898bed8d1c2ec053246eeb Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 24 Mar 2026 08:49:33 -0500 Subject: [PATCH 29/32] refactor: remove organization model retrieval from settings --- .annotation_safe_list.yml | 4 ++++ openedx_authz/api/data.py | 8 ++------ openedx_authz/models/scopes.py | 19 ------------------- openedx_authz/settings/common.py | 4 ---- openedx_authz/settings/test.py | 2 +- openedx_authz/tests/api/test_data.py | 2 ++ openedx_authz/tests/stubs/models.py | 14 +------------- 7 files changed, 10 insertions(+), 43 deletions(-) diff --git a/.annotation_safe_list.yml b/.annotation_safe_list.yml index c2aa2fd5..01537c26 100644 --- a/.annotation_safe_list.yml +++ b/.annotation_safe_list.yml @@ -41,3 +41,7 @@ waffle.Switch: ".. no_pii:": "This model has no PII" casbin_adapter.CasbinRule: ".. no_pii:": "This model stores authorization policy rules and contains no PII" +organizations.HistoricalOrganization: + ".. no_pii:": "Historical snapshot of organization metadata and contains no PII" +organizations.HistoricalOrganizationCourse: + ".. no_pii:": "Historical relation between organizations and courses and contains no PII" diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index d85cf1c4..be0e60ea 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -11,16 +11,12 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 +from organizations.models import Organization -from openedx_authz.models.scopes import ( - get_content_library_model, - get_course_overview_model, - get_organization_model, -) +from openedx_authz.models.scopes import get_content_library_model, get_course_overview_model ContentLibrary = get_content_library_model() CourseOverview = get_course_overview_model() -Organization = get_organization_model() __all__ = [ "ActionData", diff --git a/openedx_authz/models/scopes.py b/openedx_authz/models/scopes.py index 6f063559..ba28bda4 100644 --- a/openedx_authz/models/scopes.py +++ b/openedx_authz/models/scopes.py @@ -50,27 +50,8 @@ def get_course_overview_model(): return None -def get_organization_model(): - """Return the Organization model class specified by settings. - - The setting `OPENEDX_AUTHZ_ORGANIZATION_MODEL` should be an - app_label.ModelName string (e.g. 'organizations.Organization'). - """ - ORGANIZATION_MODEL = getattr( - settings, - "OPENEDX_AUTHZ_ORGANIZATION_MODEL", - "organizations.Organization", - ) - try: - app_label, model_name = ORGANIZATION_MODEL.split(".") - return apps.get_model(app_label, model_name, require_ready=False) - except LookupError: - return None - - ContentLibrary = get_content_library_model() CourseOverview = get_course_overview_model() -Organization = get_organization_model() class ContentLibraryScope(Scope): diff --git a/openedx_authz/settings/common.py b/openedx_authz/settings/common.py index 0d9f7445..81e060b8 100644 --- a/openedx_authz/settings/common.py +++ b/openedx_authz/settings/common.py @@ -50,10 +50,6 @@ def plugin_settings(settings): if not hasattr(settings, "OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL"): settings.OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL = "course_overviews.CourseOverview" - # Set default Organization model for swappable dependency - if not hasattr(settings, "OPENEDX_AUTHZ_ORGANIZATION_MODEL"): - settings.OPENEDX_AUTHZ_ORGANIZATION_MODEL = "organizations.Organization" - # Set default CASBIN_LOG_LEVEL if not already set. # This setting defines the logging level for the Casbin enforcer. if not hasattr(settings, "CASBIN_LOG_LEVEL"): diff --git a/openedx_authz/settings/test.py b/openedx_authz/settings/test.py index c9a82016..ba449092 100644 --- a/openedx_authz/settings/test.py +++ b/openedx_authz/settings/test.py @@ -38,6 +38,7 @@ def plugin_settings(settings): # pylint: disable=unused-argument "openedx_authz.engine.apps.CasbinAdapterConfig", "openedx_authz.apps.OpenedxAuthzConfig", "openedx_authz.tests.stubs.apps.StubsConfig", + "organizations", ) MIDDLEWARE = [ @@ -77,4 +78,3 @@ def plugin_settings(settings): # pylint: disable=unused-argument # Use stub model for testing instead of the real content_libraries app OPENEDX_AUTHZ_CONTENT_LIBRARY_MODEL = "stubs.ContentLibrary" OPENEDX_AUTHZ_COURSE_OVERVIEW_MODEL = "stubs.CourseOverview" -OPENEDX_AUTHZ_ORGANIZATION_MODEL = "stubs.Organization" diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index dc0b1a88..91effab1 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -761,6 +761,7 @@ def test_get_org(self, external_key, expected_org): """Test organization extraction from library glob pattern.""" self.assertEqual(OrgContentLibraryGlobData.get_org(external_key), expected_org) + @patch("openedx_authz.api.data.Organization", Organization) def test_exists_true_when_org_exists(self): """exists() returns True when the org exists.""" org_name = "DemoX" @@ -832,6 +833,7 @@ def test_get_org(self, external_key, expected_org): """Test organization extraction from course glob pattern.""" self.assertEqual(OrgCourseOverviewGlobData.get_org(external_key), expected_org) + @patch("openedx_authz.api.data.Organization", Organization) def test_exists_true_when_org_exists(self): """exists() returns True when the org exists.""" org_name = "OpenedX" diff --git a/openedx_authz/tests/stubs/models.py b/openedx_authz/tests/stubs/models.py index 86c39079..20f53c67 100644 --- a/openedx_authz/tests/stubs/models.py +++ b/openedx_authz/tests/stubs/models.py @@ -10,19 +10,7 @@ from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 - - -class Organization(models.Model): - """Stub model representing an organization for testing purposes. - - .. no_pii: - """ - - name = models.CharField(max_length=255) - short_name = models.CharField(max_length=100) - - def __str__(self): - return str(self.name) +from organizations.models import Organization class ContentLibraryManager(models.Manager): From 23f340e9fe689899188583683fd9553562c6db56 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 24 Mar 2026 10:06:51 -0500 Subject: [PATCH 30/32] refactor: enhance error handling in ScopeMeta for glob scopes --- openedx_authz/api/data.py | 12 ++++++++---- openedx_authz/tests/api/test_data.py | 21 +++++++++++++++------ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index be0e60ea..c528a989 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -306,12 +306,16 @@ def get_subclass_by_external_key(mcs, external_key: str) -> Type["ScopeData"]: is_glob = GLOBAL_SCOPE_WILDCARD in external_key if is_glob: - # Try to get glob-specific class first glob_subclass = mcs.glob_registry.get(namespace) - if glob_subclass and glob_subclass.validate_external_key(external_key): - return glob_subclass - # Fall back to standard scope class + if not glob_subclass: + raise ValueError(f"Unknown glob scope: {namespace} for external_key: {external_key}") + + if not glob_subclass.validate_external_key(external_key): + raise ValueError(f"Invalid external_key format for glob scope: {external_key}") + + return glob_subclass + scope_subclass = mcs.scope_registry.get(namespace) if not scope_subclass: diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 91effab1..a2e6d79d 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -325,15 +325,24 @@ def test_scope_validate_external_key(self, external_key, expected_valid, expecte self.assertEqual(result, expected_valid) - def test_get_subclass_by_external_key_unknown_scope_raises_value_error(self): - """Unknown namespace should raise ValueError in get_subclass_by_external_key.""" + @data( + "unknown:DemoX", + "unknown:DemoX:*", + ) + def test_get_subclass_by_external_key_unknown_scope_raises_value_error(self, external_key): + """Unknown namespace should raise ValueError, including wildcard keys.""" with self.assertRaises(ValueError): - ScopeMeta.get_subclass_by_external_key("unknown:DemoX") + ScopeMeta.get_subclass_by_external_key(external_key) - def test_get_subclass_by_external_key_invalid_format_raises_value_error(self): - """Invalid format (fails subclass.validate_external_key) should raise ValueError.""" + @data( + "lib:invalid_library_key", + "lib:DemoX:slug*", + "course-v1:OpenedX+CS101+*", + ) + def test_get_subclass_by_external_key_invalid_format_raises_value_error(self, external_key): + """Invalid format should raise ValueError for regular and wildcard keys.""" with self.assertRaises(ValueError): - ScopeMeta.get_subclass_by_external_key("lib:invalid_library_key") + ScopeMeta.get_subclass_by_external_key(external_key) def test_scope_meta_initializes_registries_when_missing(self): """ScopeMeta should create registries if they don't exist on initialization. From e95b8b3cf705c586f84572d9558ac9ff8c50e2be Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 24 Mar 2026 17:02:33 -0500 Subject: [PATCH 31/32] refactor: improve error messages in RoleScopeValidationMixin --- openedx_authz/rest_api/v1/serializers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openedx_authz/rest_api/v1/serializers.py b/openedx_authz/rest_api/v1/serializers.py index df920368..58831c57 100644 --- a/openedx_authz/rest_api/v1/serializers.py +++ b/openedx_authz/rest_api/v1/serializers.py @@ -67,17 +67,17 @@ def validate(self, attrs) -> dict: try: scope = api.ScopeData(external_key=scope_value) except ValueError as exc: - raise serializers.ValidationError(exc) from exc + raise serializers.ValidationError({"scope": str(exc)}) from exc if not scope.exists(): - raise serializers.ValidationError(f"Scope '{scope_value}' does not exist") + raise serializers.ValidationError({"scope": f"Scope '{scope_value}' does not exist"}) role = api.RoleData(external_key=role_value) generic_scope = get_generic_scope(scope) role_definitions = api.get_role_definitions_in_scope(generic_scope) if role not in role_definitions: - raise serializers.ValidationError(f"Role '{role_value}' does not exist in scope '{scope_value}'") + raise serializers.ValidationError({"role": f"Role '{role_value}' does not exist in scope '{scope_value}'"}) return validated_data From ab17349625fa997504241d935f713d39e3843815 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 24 Mar 2026 17:02:50 -0500 Subject: [PATCH 32/32] test: add comprehensive tests for scope string validation in role assignment and removal --- openedx_authz/tests/rest_api/test_views.py | 137 +++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index 87dde6b7..3f8520b8 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -716,6 +716,143 @@ def test_remove_users_from_role_permissions(self, username: str, status_code: in self.assertEqual(response.status_code, status_code) +@ddt +class TestRoleUserAPIViewScopeStringValidation(ViewTestMixin): + """API tests for scope string validation on role assignment and removal (PUT/DELETE). + + These mirror security rules enforced by ``ScopeData(external_key=...)``: organization-level + globs must use ``lib:ORG:*`` or ``course-v1:ORG+*``. Malicious patterns must be rejected + before any assignment runs. + """ + + def setUp(self): + """Set up test fixtures.""" + super().setUp() + self.client.force_authenticate(user=self.admin_user) + self.url = reverse("openedx_authz:role-user-list") + + @data( + # Course: globs only after full org segment (ORG+*), not course-v1:ORG* or mid-key globs + "course-v1:OpenedX*", + "course-v1:OpenedX**", + "course-v1:c*", + "course-v1:Open*", + "course-v1:OpenedX+C*", + "course-v1:OpenedX+CS101+*", + "course-v1:OpenedX+CS101*", + # Library: org-level glob is lib:ORG:* — not slug-level or stray * + "lib:Org1:LIB*", + "lib:DemoX*", + "lib:DemoX:*:*", + "lib:DemoX:slug*", + # Wrong namespace or unparsable external keys + "other:OpenedX+*", + "unknown:DemoX:*", + "not-a-valid-external-key", + # Attempts to pass namespaced keys or Casbin-style keys as the external scope + "course-v1^course-v1:OpenedX+*", + "lib^lib:DemoX:*", + "course-v1^course-v1:OpenedX*", + ) + def test_put_rejects_malformed_or_overbroad_scope_strings(self, invalid_scope: str): + """PUT must return 400 when the scope is not a valid concrete key or org-level glob.""" + request_data = { + "role": roles.LIBRARY_ADMIN.external_key, + "scope": invalid_scope, + "users": ["regular_1"], + } + + response = self.client.put(self.url, data=request_data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + @data( + "course-v1:OpenedX*", + "course-v1:OpenedX+CS101+*", + "lib:DemoX*", + "unknown:DemoX:*", + "course-v1^course-v1:OpenedX+*", + ) + def test_delete_rejects_malformed_or_overbroad_scope_strings(self, invalid_scope: str): + """DELETE must return 400 for the same invalid scope strings as PUT.""" + query_params = { + "role": roles.LIBRARY_ADMIN.external_key, + "scope": invalid_scope, + "users": "regular_1", + } + + response = self.client.delete(f"{self.url}?{urlencode(query_params)}") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + @data( + # Empty org segment after validation (must not assign at "all orgs") + "lib::*", + "course-v1:+*", + # Valid shape but organization is not in the system + "lib:NonexistentOrgZ99:*", + "course-v1:NonexistentOrgZ99+*", + ) + def test_put_rejects_scope_that_does_not_exist(self, scope: str): + """Well-formed keys that do not resolve to an existing org/course must return 400.""" + request_data = { + "role": roles.LIBRARY_ADMIN.external_key, + "scope": scope, + "users": ["regular_1"], + } + + response = self.client.put(self.url, data=request_data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("scope", response.data) + self.assertIn("invalid", [error.code for error in response.data["scope"]]) + + @patch.object(api, "assign_role_to_user_in_scope", return_value=True) + @patch.object(api.OrgContentLibraryGlobData, "exists", return_value=True) + def test_put_accepts_valid_library_org_glob_scope(self, _mock_exists, _mock_assign): + """Valid library org glob passes serializer validation and reaches assignment.""" + request_data = { + "role": roles.LIBRARY_ADMIN.external_key, + "scope": "lib:Org1:*", + "users": ["regular_1"], + } + + response = self.client.put(self.url, data=request_data, format="json") + + self.assertEqual(response.status_code, status.HTTP_207_MULTI_STATUS) + self.assertEqual(len(response.data["completed"]), 1) + + @patch.object(api, "assign_role_to_user_in_scope", return_value=True) + @patch.object(api.OrgCourseOverviewGlobData, "exists", return_value=True) + def test_put_accepts_valid_course_org_glob_scope(self, _mock_exists, _mock_assign): + """Valid course org glob (course-v1:ORG+*) passes validation for a course role.""" + request_data = { + "role": roles.COURSE_STAFF.external_key, + "scope": "course-v1:OpenedX+*", + "users": ["regular_1"], + } + + response = self.client.put(self.url, data=request_data, format="json") + + self.assertEqual(response.status_code, status.HTTP_207_MULTI_STATUS) + self.assertEqual(len(response.data["completed"]), 1) + + @patch.object(api, "assign_role_to_user_in_scope", return_value=True) + @patch.object(api.CourseOverviewData, "exists", return_value=True) + def test_put_accepts_valid_full_course_key_scope(self, _mock_exists, _mock_assign): + """A full course run key is accepted for a course role when the course exists.""" + request_data = { + "role": roles.COURSE_STAFF.external_key, + "scope": "course-v1:OpenedX+DemoCourse+2026_T1", + "users": ["regular_1"], + } + + response = self.client.put(self.url, data=request_data, format="json") + + self.assertEqual(response.status_code, status.HTTP_207_MULTI_STATUS) + self.assertEqual(len(response.data["completed"]), 1) + + @ddt class TestRoleListView(ViewTestMixin): """Test suite for RoleListView."""