Skip to content
Merged
31 changes: 22 additions & 9 deletions openedx_authz/api/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,14 @@ def __call__(cls, *args, **kwargs):
if cls is not ScopeData:
return super().__call__(*args, **kwargs)

# When working with global scopes, we can't determine subclass with an external_key since
# a global scope it's not attached to a specific resource type. So we only use * as
# an external_key to mean generic scope which maps to base ScopeData class.
# The only remaining issue is that internally the namespace key used in policies will be
# The global scope namespace (global^*), so we need to handle that case here.
if kwargs.get("external_key") == GENERIC_SCOPE_WILDCARD:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generic or global?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is a question on naming, I think global is clearer.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially used generic scope cause this scope can be specialized into other scopes: content libraries, courses... But this might be too ambiguous. It might make sense making it global when not specialized?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Global makes more sense to me 👍

return super().__call__(*args, **kwargs)

if "namespaced_key" in kwargs:
scope_cls = cls.get_subclass_by_namespaced_key(kwargs["namespaced_key"])
return super(ScopeMeta, scope_cls).__call__(*args, **kwargs)
Expand All @@ -198,15 +206,15 @@ def get_subclass_by_namespaced_key(mcs, namespaced_key: str) -> Type["ScopeData"
Extracts the namespace prefix (before '^') and returns the registered subclass.

Args:
namespaced_key: The namespaced key (e.g., 'lib^lib:DemoX:CSPROB', 'sc^generic').
namespaced_key: The namespaced key (e.g., 'lib^lib:DemoX:CSPROB', 'global^generic').

Returns:
The ScopeData subclass for the namespace, or ScopeData if namespace not recognized.

Examples:
>>> ScopeMeta.get_subclass_by_namespaced_key('lib^lib:DemoX:CSPROB')
<class 'ContentLibraryData'>
>>> ScopeMeta.get_subclass_by_namespaced_key('sc^generic')
>>> ScopeMeta.get_subclass_by_namespaced_key('global^generic')
<class 'ScopeData'>
"""
# TODO: Default separator, can't access directly from class so made it a constant
Expand All @@ -224,7 +232,7 @@ def get_subclass_by_external_key(mcs, external_key: str) -> Type["ScopeData"]:
the key format using the subclass's validate_external_key method.

Args:
external_key: The external key (e.g., 'lib:DemoX:CSPROB', 'sc:generic').
external_key: The external key (e.g., 'lib:DemoX:CSPROB', 'global:generic').

Returns:
The ScopeData subclass corresponding to the namespace.
Expand Down Expand Up @@ -263,11 +271,11 @@ def get_all_namespaces(mcs) -> dict[str, Type["ScopeData"]]:

Returns:
dict[str, Type["ScopeData"]]: A dictionary of all namespace prefixes registered in the scope registry.
Each namespace corresponds to a ScopeData subclass (e.g., 'lib', 'sc').
Each namespace corresponds to a ScopeData subclass (e.g., 'lib', 'global').

Examples:
>>> ScopeMeta.get_all_namespaces()
{'sc': ScopeData, 'lib': ContentLibraryData, 'org': OrganizationData}
{'global': ScopeData, 'lib': ContentLibraryData, 'org': OrganizationData}
"""
return mcs.scope_registry

Expand All @@ -293,17 +301,22 @@ class ScopeData(AuthZData, metaclass=ScopeMeta):
and not tied to any specific scope type, holding attributes common to all scopes.

Attributes:
NAMESPACE: 'sc' for generic scopes.
NAMESPACE: 'global' for generic scopes.
external_key: The scope identifier without namespace (e.g., 'generic_scope').
namespaced_key: The scope identifier with namespace (e.g., 'sc^generic_scope').
namespaced_key: The scope identifier with namespace (e.g., 'global^generic_scope').

Examples:
>>> scope = ScopeData(external_key='generic_scope')
>>> scope.namespaced_key
'sc^generic_scope'
'global^generic_scope'
"""

NAMESPACE: ClassVar[str] = "sc"
# The 'global' namespace is used for scopes that aren't tied to a specific resource type.
# This base class supports:
# 1. Global wildcard scopes (external_key='*') that apply across all resource types
# 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"

@classmethod
def validate_external_key(cls, _: str) -> bool:
Expand Down
12 changes: 6 additions & 6 deletions openedx_authz/rest_api/v1/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def get_permission_class(mcs, namespace: str) -> type["BaseScopePermission"]:
"""Retrieve the permission class for the given namespace.

Args:
namespace: The namespace identifier (e.g., 'lib', 'sc').
namespace: The namespace identifier (e.g., 'lib', 'global').

Returns:
type["BaseScopePermission"]: The permission class for the namespace,
Expand All @@ -54,8 +54,8 @@ class BaseScopePermission(BasePermission, metaclass=PermissionMeta):
specific authorization logic for their scope types.
"""

NAMESPACE: ClassVar[str] = "sc"
"""The namespace identifier for this permission class. Default ``sc`` for generic scopes."""
NAMESPACE: ClassVar[str] = "global"
"""The namespace identifier for this permission class. Default ``global`` for generic scopes."""

def get_scope_value(self, request) -> str | None:
"""Extract the scope value from the request.
Expand All @@ -78,15 +78,15 @@ def get_scope_namespace(self, request) -> str:
request: The Django REST framework request object.

Returns:
str: The scope namespace (e.g., 'lib', 'sc').
str: The scope namespace (e.g., 'lib', 'global').

Examples:
>>> request.data = {"scope": "lib:DemoX:CSPROB"}
>>> permission.get_scope_namespace(request)
'lib'
>>> request.data = {}
>>> permission.get_scope_namespace(request)
'sc'
'global'
"""
scope_value = self.get_scope_value(request)
if not scope_value:
Expand Down Expand Up @@ -137,7 +137,7 @@ class DynamicScopePermission(BaseScopePermission):
>>> request.data = {"scope": "lib:DemoX:CSPROB"}
>>> ContentLibraryPermission.has_permission(request, view)
>>> # For a generic scope request, this will delegate to BaseScopePermission
>>> request.data = {"scope": "sc:generic"}
>>> request.data = {"scope": "global:generic"}
>>> BaseScopePermission.has_permission(request, view)

Note:
Expand Down
2 changes: 1 addition & 1 deletion openedx_authz/rest_api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def validate_scope(self, value: str) -> api.ScopeData:
returns an instance of the appropriate ScopeData subclass.

Args:
value: The scope string to validate (e.g., 'lib', 'sc', 'org').
value: The scope string to validate (e.g., 'lib', 'global', 'org').

Returns:
ScopeData: An instance of the appropriate ScopeData subclass for the scope.
Expand Down
47 changes: 34 additions & 13 deletions openedx_authz/tests/api/test_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def test_scope_data_direct_instantiation_with_namespaced_key(self):
"""Test that ScopeData can be instantiated with namespaced_key.

Expected Result:
- ScopeData(namespaced_key='sc^generic') creates ScopeData instance
- ScopeData(namespaced_key='global^generic') creates ScopeData instance
"""
namespaced_key = f"{ScopeData.NAMESPACE}{ScopeData.SEPARATOR}generic"

Expand Down Expand Up @@ -222,25 +222,25 @@ def test_scope_data_registration(self):
"""Test that ScopeData and its subclasses are registered correctly.

Expected Result:
- 'sc' namespace maps to ScopeData class
- 'global' namespace maps to ScopeData class
- 'lib' namespace maps to ContentLibraryData class
"""
self.assertIn("sc", ScopeData.scope_registry)
self.assertIs(ScopeData.scope_registry["sc"], ScopeData)
self.assertIn("global", ScopeData.scope_registry)
self.assertIs(ScopeData.scope_registry["global"], ScopeData)
self.assertIn("lib", ScopeData.scope_registry)
self.assertIs(ScopeData.scope_registry["lib"], ContentLibraryData)

@data(
("lib^lib:DemoX:CSPROB", ContentLibraryData),
("sc^generic_scope", ScopeData),
("global^generic_scope", ScopeData),
)
@unpack
def test_dynamic_instantiation_via_namespaced_key(self, namespaced_key, expected_class):
"""Test that ScopeData dynamically instantiates the correct subclass.

Expected Result:
- ScopeData(namespaced_key='lib^...') returns ContentLibraryData instance
- ScopeData(namespaced_key='sc^...') returns ScopeData instance
- ScopeData(namespaced_key='global^...') returns ScopeData instance
"""
instance = ScopeData(namespaced_key=namespaced_key)

Expand All @@ -249,7 +249,7 @@ def test_dynamic_instantiation_via_namespaced_key(self, namespaced_key, expected

@data(
("lib^lib:DemoX:CSPROB", ContentLibraryData),
("sc^generic", ScopeData),
("global^generic", ScopeData),
("unknown^something", ScopeData),
)
@unpack
Expand All @@ -258,7 +258,7 @@ def test_get_subclass_by_namespaced_key(self, namespaced_key, expected_class):

Expected Result:
- 'lib^...' returns ContentLibraryData
- 'sc^...' returns ScopeData
- 'global^...' returns ScopeData
- 'unknown^...' returns ScopeData (fallback)
"""
subclass = ScopeMeta.get_subclass_by_namespaced_key(namespaced_key)
Expand All @@ -268,15 +268,15 @@ def test_get_subclass_by_namespaced_key(self, namespaced_key, expected_class):
@data(
("lib:DemoX:CSPROB", ContentLibraryData),
("lib:edX:Demo", ContentLibraryData),
("sc:generic_scope", ScopeData),
("global:generic_scope", ScopeData),
)
@unpack
def test_get_subclass_by_external_key(self, external_key, expected_class):
"""Test get_subclass_by_external_key returns correct subclass.

Expected Result:
- 'lib:...' returns ContentLibraryData
- 'sc:...' returns ScopeData
- 'global:...' returns ScopeData
"""
subclass = ScopeMeta.get_subclass_by_external_key(external_key)

Expand Down Expand Up @@ -319,12 +319,12 @@ def test_base_scope_data_with_external_key(self):
- ScopeData(external_key='...') creates ScopeData instance
- No dynamic subclass selection occurs
"""
scope = ScopeData(external_key="sc:generic_scope")
scope = ScopeData(external_key="global:generic_scope")

expected_namespaced = f"{ScopeData.NAMESPACE}{ScopeData.SEPARATOR}sc:generic_scope"
expected_namespaced = f"{ScopeData.NAMESPACE}{ScopeData.SEPARATOR}global:generic_scope"

self.assertIsInstance(scope, ScopeData)
self.assertEqual(scope.external_key, "sc:generic_scope")
self.assertEqual(scope.external_key, "global:generic_scope")
self.assertEqual(scope.namespaced_key, expected_namespaced)

def test_empty_namespaced_key_raises_value_error(self):
Expand All @@ -345,6 +345,27 @@ def test_empty_external_key_raises_value_error(self):
with self.assertRaises(ValueError):
SubjectData(external_key="")

def test_scope_data_with_wildcard_external_key(self):
"""Test that ScopeData instantiated with wildcard (*) returns base ScopeData.

When using the global scope wildcard '*', the metaclass should return a base
ScopeData instance rather than attempting subclass determination.

Expected Result:
- ScopeData(external_key='*') creates base ScopeData instance
- namespaced_key is 'global^*'
- No subclass determination occurs
"""
scope = ScopeData(external_key="*")

expected_namespaced = f"{ScopeData.NAMESPACE}{ScopeData.SEPARATOR}*"

self.assertIsInstance(scope, ScopeData)
# Ensure it's exactly ScopeData, not a subclass
self.assertEqual(type(scope), ScopeData)
self.assertEqual(scope.external_key, "*")
self.assertEqual(scope.namespaced_key, expected_namespaced)


@ddt
class TestDataRepresentation(TestCase):
Expand Down
16 changes: 8 additions & 8 deletions openedx_authz/tests/api/test_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,9 +500,9 @@ def test_get_all_role_assignments_scopes(self, subject_name, expected_roles):
(roles.LIBRARY_AUTHOR.external_key, "lib:Org6:project_beta", 1),
(roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org6:project_gamma", 1),
(roles.LIBRARY_USER.external_key, "lib:Org6:project_delta", 1),
("non_existent_role", "sc:any_library", 0),
(roles.LIBRARY_ADMIN.external_key, "sc:non_existent_scope", 0),
("non_existent_role", "sc:non_existent_scope", 0),
("non_existent_role", "global:any_library", 0),
(roles.LIBRARY_ADMIN.external_key, "global:non_existent_scope", 0),
("non_existent_role", "global:non_existent_scope", 0),
)
@unpack
def test_get_role_assignments_in_scope(self, role_name, scope_name, expected_count):
Expand Down Expand Up @@ -625,8 +625,8 @@ def test_get_scopes_for_subject_and_permission(self, subject_name, action_name,
(roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_201", {"liam"}),
(roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_301", {"liam"}),
("non_existent_role", "lib:Org4:art_101", set()),
(roles.LIBRARY_AUTHOR.external_key, "sc:non_existent_scope", set()),
("non_existent_role", "sc:non_existent_scope", set()),
(roles.LIBRARY_AUTHOR.external_key, "global:non_existent_scope", set()),
("non_existent_role", "global:non_existent_scope", set()),
)
@unpack
def test_get_subjects_for_role_in_scope(self, role_name: str, scope_name: str, expected_subjects: set[str]):
Expand Down Expand Up @@ -654,7 +654,7 @@ class TestRoleAssignmentAPI(RolesTestSetupMixin):
"""

@ddt_data(
(["mary", "john"], roles.LIBRARY_USER.external_key, "sc:batch_test", True),
(["mary", "john"], roles.LIBRARY_USER.external_key, "global:batch_test", True),
(
["paul", "diana", "lila"],
roles.LIBRARY_CONTRIBUTOR.external_key,
Expand Down Expand Up @@ -712,7 +712,7 @@ def test_batch_assign_role_to_subjects_in_scope(self, subject_names, role, scope
self.assertIn(role, role_names)

@ddt_data(
(["mary", "john"], roles.LIBRARY_USER.external_key, "sc:batch_test", True),
(["mary", "john"], roles.LIBRARY_USER.external_key, "global:batch_test", True),
(
["paul", "diana", "lila"],
roles.LIBRARY_CONTRIBUTOR.external_key,
Expand Down Expand Up @@ -827,7 +827,7 @@ def test_unassign_role_from_subject_in_scope(self, subject_names, role, scope_na
)
],
),
("sc:non_existent_scope", []),
("global:non_existent_scope", []),
)
@unpack
def test_get_all_role_assignments_in_scope(self, scope_name, expected_assignments):
Expand Down