refactor: address PR reviews

Author: mariajgrimaldi

openedx_authz/api/data.py

@@ -63,7 +63,7 @@ class AuthZData(AuthzBaseClass):
         SEPARATOR: The separator between the namespace and the identifier (e.g., ':', '@').
         external_key: The ID for the object outside of the authz system (e.g., username).
             Could also be used for human-readable names (e.g., role or action name).
-        namespaced_key: The ID for the object within the authz system (e.g., 'user@john_doe').
+        namespaced_key: The ID for the object within the authz system (e.g., 'user^john_doe').
     """
 
     external_key: str = ""
@@ -109,9 +109,9 @@ def __call__(cls, *args, **kwargs):
 
         There are two ways to instantiate:
         1. By providing external_key= and format for the external key determines the subclass
-        (e.g., 'lib^any-library' = ContentLibraryData).
+        (e.g., 'lib:DemoX:CSPROB' = ContentLibraryData).
         2. By providing namespaced_key= and the class is determined from the namespace prefix
-        in namespaced_key (e.g., 'lib@any-library' = ContentLibraryData).
+        in namespaced_key (e.g., 'lib^lib:DemoX:CSPROB' = ContentLibraryData).
 
         The namespaced key is usually used when getting objects from the policy store,
         while the external key is usually used when initializing from user input or API calls. For example,
@@ -138,7 +138,7 @@ def get_subclass_by_namespaced_key(mcs, namespaced_key: str) -> Type["ScopeData"
         """Get the appropriate subclass based on the namespace in namespaced_key.
 
         Args:
-            namespaced_key: The namespaced key (e.g., 'lib^any-library').
+            namespaced_key: The namespaced key (e.g., 'lib^lib:DemoX:CSPROB').
 
         Returns:
             The subclass of ScopeData corresponding to the namespace, or ScopeData if not found.
@@ -152,13 +152,13 @@ def get_subclass_by_external_key(mcs, external_key: str) -> Type["ScopeData"]:
         """Get the appropriate subclass based on the format of external_key.
 
         Args:
-            external_key: The external key (e.g., 'lib:any-library').
+            external_key: The external key (e.g., 'lib^lib:DemoX:CSPROB').
 
         Returns:
             The subclass of ScopeData corresponding to the namespace, or ScopeData if not found.
         """
         # Here we need to assume a couple of things:
-        # 1. The external_key is always in the format 'namespace...:other things'. E.g., 'lib:any-library',
+        # 1. The external_key is always in the format 'namespace...:other things'. E.g., 'lib:DemoX:CSPROB',
         # even 'course-v1:edX+DemoX+2021_T1'. This won't work for org scopes because they don't explicitly indicate
         # the namespace in the external key. TODO: We need to handle org scopes differently.
         # 2. The namespace is always the part before the first separator.
@@ -228,7 +228,7 @@ class ContentLibraryData(ScopeData):
     """A content library is a collection of content items.
 
     Attributes:
-        library_id: The content library identifier (e.g., 'library-v1:edX+DemoX+2021_T1').
+        library_id: The content library identifier (e.g., 'lib:DemoX:CSPROB').
         namespaced_key: Inherited from ScopeData, auto-generated from name if not provided.
 
     TODO: this class should live alongside library definitions and not here.
@@ -238,7 +238,7 @@ class ContentLibraryData(ScopeData):
 
     @property
     def library_id(self) -> str:
-        """The library identifier as used in Open edX (e.g., 'math_101', 'library-v1:edX+DemoX').
+        """The library identifier as used in Open edX (e.g., 'lib:DemoX:CSPROB').
 
         This is an alias for external_key that represents the library ID without the namespace prefix.
 
@@ -321,7 +321,7 @@ class SubjectData(AuthZData, metaclass=SubjectMeta):
     """A subject is an entity that can be assigned roles and permissions.
 
     Attributes:
-        namespaced_key: The subject identifier namespaced (e.g., 'sub@generic').
+        namespaced_key: The subject identifier namespaced (e.g., 'sub^generic').
     """
 
     NAMESPACE: ClassVar[str] = "sub"
@@ -335,7 +335,7 @@ class UserData(SubjectData):
         username: The username for the user (e.g., 'john_doe').
         namespaced_key: Inherited from SubjectData, auto-generated from username if not provided.
 
-    This class automatically adds the 'user@' namespace prefix to the subject ID.
+    This class automatically adds the 'user^' namespace prefix to the subject ID.
     Can be initialized with either external_key= or namespaced_key= parameter.
     """
 
@@ -366,7 +366,7 @@ class ActionData(AuthZData):
     """An action is an operation that can be performed in a specific scope.
 
     Attributes:
-        action: The action name. Automatically prefixed with 'act@' if not present.
+        action: The action name. Automatically prefixed with 'act^' if not present.
     """
 
     NAMESPACE: ClassVar[str] = "act"
@@ -417,7 +417,7 @@ class RoleData(AuthZData):
     """A role is a named group of permissions.
 
     Attributes:
-        name: The name of the role. Must have 'role@' namespace prefix.
+        name: The name of the role. Must have 'role^' namespace prefix.
         permissions: A list of permissions assigned to the role.
     """
 

openedx_authz/api/roles.py

@@ -94,12 +94,12 @@ def get_permissions_for_active_roles_in_scope(
 
     Role Definition vs Role Assignment:
 
-    - Policy roles define potential permissions with namespace patterns (e.g., 'lib@*')
+    - Policy roles define potential permissions with namespace patterns (e.g., 'lib^*')
     - Actual permissions are granted only when roles are assigned to subjects with
-      concrete scopes (e.g., 'lib@123')
-    - The namespace pattern in the policy ('lib@*') indicates the role is designed
+      concrete scopes (e.g., 'lib^lib:DemoX:CSPROB')
+    - The namespace pattern in the policy ('lib^*') indicates the role is designed
       for resources in that namespace, but doesn't grant blanket access
-    - The specific scope at assignment time ('lib@123') determines the exact
+    - The specific scope at assignment time ('lib^lib:DemoX:CSPROB') determines the exact
       resource the permissions apply to
 
     Behavior:
@@ -140,7 +140,7 @@ def get_role_definitions_in_scope(scope: ScopeData) -> list[RoleData]:
     definitions vs assignments.
 
     Args:
-        scope: The scope to filter roles (e.g., 'lib@*' or '*' for global).
+        scope: The scope to filter roles (e.g., 'lib^*' or '*' for global).
 
     Returns:
         list[Role]: A list of roles.
@@ -185,7 +185,7 @@ def get_all_roles_in_scope(scope: ScopeData) -> list[list[str]]:
     """Get all the available role grouping policies in a specific scope.
 
     Args:
-        scope: The scope to filter roles (e.g., 'lib@*' or '*' for global).
+        scope: The scope to filter roles (e.g., 'lib^*' or '*' for global).
 
     Returns:
         list[list[str]]: A list of policies in the specified scope.
@@ -260,7 +260,7 @@ def get_subject_role_assignments(subject: SubjectData) -> list[RoleAssignmentDat
         subject: The ID of the subject namespaced (e.g., 'subject^john_doe').
 
     Returns:
-        list[Role]: A list of role names and all their metadata assigned to the subject.
+        list[RoleAssignmentData]: A list of role assignments for the subject.
     """
     role_assignments = []
     for policy in enforcer.get_filtered_grouping_policy(
@@ -289,7 +289,7 @@ def get_subject_role_assignments_in_scope(
         scope: The scope to filter roles (e.g., 'library:123').
 
     Returns:
-        list[RoleAssignment]: A list of role assignments for the subject in the scope.
+        list[RoleAssignmentData]: A list of role assignments for the subject in the scope.
     """
     # TODO: we still need to get the remaining data for the role like email, etc
     role_assignments = []
@@ -322,7 +322,7 @@ def get_subject_role_assignments_for_role_in_scope(
         scope: The scope to filter subjects (e.g., 'library:123' or '*' for global).
 
     Returns:
-        list[RoleAssignment]: A list of subjects assigned to the specified role in the specified scope.
+        list[RoleAssignmentData]: A list of subjects assigned to the specified role in the specified scope.
     """
     role_assignments = []
     for subject in enforcer.get_users_for_role_in_domain(
@@ -357,7 +357,7 @@ def get_all_subject_role_assignments_in_scope(
         scope: The scope to filter subjects (e.g., 'library:123' or '*' for global).
 
     Returns:
-        list[RoleAssignment]: A list of role assignments for all subjects in the specified scope.
+        list[RoleAssignmentData]: A list of role assignments for all subjects in the specified scope.
     """
     role_assignments_per_subject = {}
     roles_in_scope = get_all_roles_in_scope(scope)

openedx_authz/api/users.py

@@ -6,7 +6,7 @@
 
 These methods internally namespace user identifiers to ensure consistency
 with the role management system, which uses namespaced subjects
-(e.g., 'user@john_doe').
+(e.g., 'user^john_doe').
 """
 
 from openedx_authz.api.data import (

openedx_authz/engine/config/model.conf

@@ -6,33 +6,36 @@
 # - Action grouping (manage → read/write/edit/delete to reduce duplication)
 # - System-wide roles (global scope "*" applies everywhere)
 # - Negative rules (deny overrides allow for exceptions)
-# - Namespace support (course:*, lib:*, org:*, etc.)
+# - Namespace support (user^, role^, act^, lib^, org^, course^, etc.)
 # - Extensibility (new resource types just need new namespaces)
+# - Separator: ^ for AuthZ policy attributes, : for external keys
 ############################################
 
 [request_definition]
 # Request format: subject (user), action, scope (specific resource being accessed)
 #
-# sub   = subject/principal with namespace (e.g., "user:alice", "service:lms")
-# act   = action with namespace (e.g., "act:read", "act:manage", "act:edit-courses")
-# scope = authorization scope context (e.g., "org:OpenedX", "course-v1:...", "*" for global)
+# sub   = subject/principal with namespace (e.g., "user^alice", "service^lms")
+# act   = action with namespace (e.g., "act^read", "act^manage", "act^edit_courses")
+# scope = authorization scope context (e.g., "org^OpenedX", "lib^lib:...", "*" for global)
 #
 # SCOPE SEMANTICS:
 # Scope determines the authorization context and which role assignments apply
-# - "*"             = global scope (system-wide roles apply everywhere)
-# - "org:..."       = organization-scoped roles (apply within specific organization)
-# - "course-v1:..." = course-scoped roles (apply within specific course)
-# - "lib:..."       = library-scoped roles (apply within specific library)
+# - "*"                    = global scope (system-wide roles apply everywhere)
+# - "org^..."          = organization-scoped roles (namespaced external org key)
+# - "course^course-v1:..." = course-scoped roles (namespaced external course key)
+# - "lib^lib:..."          = library-scoped roles (namespaced external library key)
 #
+# Note: AuthZ policy attributes use ^ separator for namespace prefix (e.g., user^alice, role^admin),
+# while external keys (course-v1:..., lib:...) retain their original : separator format.
 # Application must provide appropriate scope based on business logic.
 r = sub, act, scope
 
 [policy_definition]
 # Policy format: subject (role), action, scope (pattern), effect
 #
-# sub   = role or user with namespace (e.g., "role:org_admin", "user:bob")
-# act   = action identifier (e.g., "act:manage", "act:read", "act:edit-courses")
-# scope = scope where policy applies (e.g., "*", "org:*", "course-v1:*", "lib:*")
+# sub   = role or user with namespace (e.g., "role^org_admin", "user^bob")
+# act   = action identifier (e.g., "act^manage", "act^read", "act^edit_courses")
+# scope = scope where policy applies (e.g., "*", "org^*", "lib^*")
 # eft   = "allow" or "deny" (deny overrides allow for exceptions)
 p = sub, act, scope, eft
 
@@ -41,22 +44,22 @@ p = sub, act, scope, eft
 # Format: user/subject, role, scope
 #
 # Examples:
-# g, user:alice, role:org_admin, org:OpenedX         # Alice is org admin for OpenedX
-# g, user:bob, role:course_instructor, course-v1:... # Bob is instructor for specific course
-# g, user:carol, role:library_admin, *               # Carol is global library admin
+# g, user^alice, role^org_admin, org^OpenedX         # Alice is org admin for OpenedX
+# g, user^bob, role^course_instructor, course^course-v1:... # Bob is instructor for specific course
+# g, user^carol, role^library_admin, *                   # Carol is global library admin
 #
 # Role hierarchy (optional):
-# g, role:org_admin, role:org_editor, org:OpenedX    # org_admin inherits org_editor permissions
+# g, role^org_admin, role^org_editor, org^OpenedX    # org_admin inherits org_editor permissions
 g = _, _, _
 
 # g2: Action grouping and implications
 # Maps high-level actions to specific actions to reduce policy duplication
 #
 # Examples:
-# g2, act:manage, act:edit        # manage implies edit
-# g2, act:manage, act:delete      # manage implies delete
-# g2, act:edit-courses, act:read  # edit-courses implies read (for resource access)
-# g2, act:edit-courses, act:write # edit-courses implies write (for resource modification)
+# g2, act^manage, act^edit        # manage implies edit
+# g2, act^manage, act^delete      # manage implies delete
+# g2, act^edit_courses, act^read  # edit_courses implies read (for resource access)
+# g2, act^edit_courses, act^write # edit_courses implies write (for resource modification)
 g2 = _, _
 
 [policy_effect]

openedx_authz/management/commands/load_policies.py

@@ -4,9 +4,6 @@
 - Specifying the path to the Casbin policy file. Default is 'openedx_authz/engine/config/authz.policy'.
 - Specifying the Casbin model configuration file. Default is 'openedx_authz/engine/config/model.conf'.
 - Optionally clearing existing policies in the database before loading new ones.
-
-Example Usage:
-    python manage.py load_policies --policy-file-path /path/to/policy.csv
 """
 
 import os
@@ -27,8 +24,8 @@ class Command(BaseCommand):
     and persistence of authorization policies within the Django application.
 
     Example Usage:
-        python manage.py load_policies --policy-file-path /path/to/policy.csv
-        python manage.py load_policies --policy-file-path /path/to/policy.csv --clear-existing
+        python manage.py load_policies --policy-file-path /path/to/authz.policy
+        python manage.py load_policies --policy-file-path /path/to/authz.policy --model-file-path /path/to/model.conf
         python manage.py load_policies
     """
 

openedx_authz/tests/api/test_roles.py

@@ -67,7 +67,7 @@ def _assign_roles_to_users(
 
         Args:
             assignments (list of dict): List of assignment dictionaries, each containing:
-                - subject (str): ID of the user namespaced (e.g., 'user:john_doe').
+                - subject (str): ID of the user namespaced (e.g., 'user^john_doe').
                 - role_id (str): Name of the role to assign.
                 - scope (str): Scope in which to assign the role.
         """

openedx_authz/tests/api/test_users.py

@@ -39,7 +39,7 @@ def _assign_roles_to_users(
 
         Args:
             assignments (list of dict): List of assignment dictionaries, each containing:
-                - subject (str): ID of the user namespaced (e.g., 'user:john_doe').
+                - subject (str): ID of the user namespaced (e.g., 'user^john_doe').
                 - role_id (str): Name of the role to assign.
                 - scope (str): Scope in which to assign the role.
         """

openedx_authz/tests/test_enforcement.py

@@ -145,7 +145,7 @@ class SystemWideRoleTests(CasbinEnforcementTestCase):
         {
             "subject": make_user_key("user-1"),
             "action": make_action_key("manage"),
-            "scope": make_library_key("lib@any-org@any-library"),
+            "scope": make_library_key("lib:DemoX:CSPROB"),
             "expected_result": True,
         },
     ]
@@ -235,10 +235,10 @@ class RoleAssignmentTests(CasbinEnforcementTestCase):
             make_role_key("course_admin"),
             make_scope_key("course", "course-v1:any-org+any-course+any-course-run"),
         ],
-        ["g", make_user_key("user-6"), make_role_key("library_admin"), make_library_key("lib@any-org@any-library")],
-        ["g", make_user_key("user-7"), make_role_key("library_editor"), make_library_key("lib@any-org@any-library")],
-        ["g", make_user_key("user-8"), make_role_key("library_reviewer"), make_library_key("lib@any-org@any-library")],
-        ["g", make_user_key("user-9"), make_role_key("library_author"), make_library_key("lib@any-org@any-library")],
+        ["g", make_user_key("user-6"), make_role_key("library_admin"), make_library_key("lib:DemoX:CSPROB")],
+        ["g", make_user_key("user-7"), make_role_key("library_editor"), make_library_key("lib:DemoX:CSPROB")],
+        ["g", make_user_key("user-8"), make_role_key("library_reviewer"), make_library_key("lib:DemoX:CSPROB")],
+        ["g", make_user_key("user-9"), make_role_key("library_author"), make_library_key("lib:DemoX:CSPROB")],
     ] + COMMON_ACTION_GROUPING
 
     CASES = [
@@ -275,25 +275,25 @@ class RoleAssignmentTests(CasbinEnforcementTestCase):
         {
             "subject": make_user_key("user-6"),
             "action": make_action_key("manage"),
-            "scope": make_library_key("lib@any-org@any-library"),
+            "scope": make_library_key("lib:DemoX:CSPROB"),
             "expected_result": True,
         },
         {
             "subject": make_user_key("user-7"),
             "action": make_action_key("edit"),
-            "scope": make_library_key("lib@any-org@any-library"),
+            "scope": make_library_key("lib:DemoX:CSPROB"),
             "expected_result": True,
         },
         {
             "subject": make_user_key("user-8"),
             "action": make_action_key("read"),
-            "scope": make_library_key("lib@any-org@any-library"),
+            "scope": make_library_key("lib:DemoX:CSPROB"),
             "expected_result": True,
         },
         {
             "subject": make_user_key("user-9"),
             "action": make_action_key("write"),
-            "scope": make_library_key("lib@any-org@any-library"),
+            "scope": make_library_key("lib:DemoX:CSPROB"),
             "expected_result": True,
         },
     ]
@@ -395,7 +395,7 @@ class WildcardScopeTests(CasbinEnforcementTestCase):
         ("*", True),
         (make_scope_key("org", "MIT"), True),
         (make_scope_key("course", "course-v1:OpenedX+DemoX+CS101"), True),
-        (make_library_key("lib@OpenedX:math-basics"), True),
+        (make_library_key("lib:OpenedX:math-basics"), True),
     )
     @unpack
     def test_wildcard_global_access(self, scope: str, expected_result: bool):
@@ -412,7 +412,7 @@ def test_wildcard_global_access(self, scope: str, expected_result: bool):
         ("*", False),
         (make_scope_key("org", "MIT"), True),
         (make_scope_key("course", "course-v1:OpenedX+DemoX+CS101"), False),
-        (make_library_key("lib@OpenedX:math-basics"), False),
+        (make_library_key("lib:OpenedX:math-basics"), False),
     )
     @unpack
     def test_wildcard_org_access(self, scope: str, expected_result: bool):
@@ -429,7 +429,7 @@ def test_wildcard_org_access(self, scope: str, expected_result: bool):
         ("*", False),
         (make_scope_key("org", "MIT"), False),
         (make_scope_key("course", "course-v1:OpenedX+DemoX+CS101"), True),
-        (make_library_key("lib@OpenedX:math-basics"), False),
+        (make_library_key("lib:OpenedX:math-basics"), False),
     )
     @unpack
     def test_wildcard_course_access(self, scope: str, expected_result: bool):
@@ -446,7 +446,7 @@ def test_wildcard_course_access(self, scope: str, expected_result: bool):
         ("*", False),
         (make_scope_key("org", "MIT"), False),
         (make_scope_key("course", "course-v1:OpenedX+DemoX+CS101"), False),
-        (make_library_key("lib@OpenedX:math-basics"), True),
+        (make_library_key("lib:OpenedX:math-basics"), True),
     )
     @unpack
     def test_wildcard_library_access(self, scope: str, expected_result: bool):