squash!: Refactor to define admin view permissions in data classes

Author: rodmgwgu

openedx_authz/api/data.py

@@ -6,21 +6,25 @@
 from abc import abstractmethod
 from enum import Enum
 from functools import cached_property
-from typing import Any, ClassVar, Literal, Type
+from typing import Any, ClassVar, Type
 
 from attrs import define
 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.constants.permissions import COURSES_VIEW_COURSE_TEAM, VIEW_LIBRARY_TEAM
+from openedx_authz.data import AUTHZ_POLICY_ATTRIBUTES_SEPARATOR, ActionData, AuthzBaseClass, AuthZData, PermissionData
 from openedx_authz.models.scopes import get_content_library_model, get_course_overview_model
 
 ContentLibrary = get_content_library_model()
 CourseOverview = get_course_overview_model()
 
 __all__ = [
     "ActionData",
+    "AuthZData",
+    "AuthzBaseClass",
     "ContentLibraryData",
     "CourseOverviewData",
     "GroupingPolicyIndex",
@@ -36,7 +40,6 @@
     "UserData",
 ]
 
-AUTHZ_POLICY_ATTRIBUTES_SEPARATOR = "^"
 EXTERNAL_KEY_SEPARATOR = ":"
 GLOBAL_SCOPE_WILDCARD = "*"
 NAMESPACED_KEY_PATTERN = rf"^.+{re.escape(AUTHZ_POLICY_ATTRIBUTES_SEPARATOR)}.+$"
@@ -86,65 +89,6 @@ class PolicyIndex(Enum):
     # The rest of the fields are optional and can be ignored for now
 
 
-class AuthzBaseClass:
-    """Base class for all authz classes.
-
-    Attributes:
-        SEPARATOR: The separator between the namespace and the identifier (default: '^').
-        NAMESPACE: The namespace prefix for the data type (e.g., 'user', 'role', 'act', 'lib').
-    """
-
-    SEPARATOR: ClassVar[str] = AUTHZ_POLICY_ATTRIBUTES_SEPARATOR
-    NAMESPACE: ClassVar[str] = None
-
-
-@define
-class AuthZData(AuthzBaseClass):
-    """Base class for all authz data classes.
-
-    Attributes:
-        NAMESPACE: The namespace prefix for the data type (e.g., 'user', 'role', 'act', 'lib').
-        SEPARATOR: The separator between the namespace and the identifier (default: '^').
-        external_key: The ID for the object outside of the authz system (e.g., 'john_doe' for a user,
-            'instructor' for a role, 'lib:DemoX:CSPROB' for a content library).
-        namespaced_key: The ID for the object within the authz system, combining namespace and external_key
-            (e.g., 'user^john_doe', 'role^instructor', 'lib^lib:DemoX:CSPROB').
-
-    Examples:
-        >>> user = UserData(external_key='john_doe')
-        >>> user.namespaced_key
-        'user^john_doe'
-        >>> role = RoleData(namespaced_key='role^instructor')
-        >>> role.external_key
-        'instructor'
-    """
-
-    external_key: str = ""
-    namespaced_key: str = ""
-
-    def __attrs_post_init__(self):
-        """Post-initialization processing for attributes.
-
-        This method ensures that either external_key or namespaced_key is provided,
-        and derives the other attribute based on the NAMESPACE and SEPARATOR.
-        """
-        if not self.NAMESPACE:
-            # No namespace defined, nothing to do
-            return
-
-        if not self.external_key and not self.namespaced_key:
-            raise ValueError("Either external_key or namespaced_key must be provided.")
-
-        # Case 1: Initialized with external_key only, derive namespaced_key
-        if not self.namespaced_key:
-            self.namespaced_key = f"{self.NAMESPACE}{self.SEPARATOR}{self.external_key}"
-
-        # Case 2: Initialized with namespaced_key only, derive external_key. Assume valid format for
-        # namespaced_key at this point.
-        if not self.external_key:
-            self.external_key = self.namespaced_key.split(self.SEPARATOR, 1)[1]
-
-
 class ScopeMeta(type):
     """Metaclass for ScopeData to handle dynamic subclass instantiation based on namespace."""
 
@@ -397,6 +341,20 @@ def validate_external_key(cls, _: str) -> bool:
         """
         return True
 
+    @classmethod
+    @abstractmethod
+    def get_admin_view_permission(cls) -> PermissionData:
+        """Get the permission required to view this scope
+
+        This method should be implemented on every ScopeData subclass to define
+        which permission to check against when a user tries to see assignations
+        related to this scope in the Admin Console.
+
+        Returns:
+            PermissionData: The permission required to view this scope in the admin console.
+        """
+        raise NotImplementedError("Subclasses must implement get_admin_view_permission method.")
+
     @abstractmethod
     def get_object(self) -> Any | None:
         """Retrieve the underlying domain object that this scope represents.
@@ -494,6 +452,15 @@ def validate_external_key(cls, external_key: str) -> bool:
         except InvalidKeyError:
             return False
 
+    @classmethod
+    def get_admin_view_permission(cls) -> PermissionData:
+        """Get the permission required to view this scope
+
+        Returns:
+            PermissionData: The permission required to view this scope in the admin console.
+        """
+        return VIEW_LIBRARY_TEAM
+
     def get_object(self) -> ContentLibrary | None:
         """Retrieve the ContentLibrary instance associated with this scope.
 
@@ -607,6 +574,15 @@ def validate_external_key(cls, external_key: str) -> bool:
         except InvalidKeyError:
             return False
 
+    @classmethod
+    def get_admin_view_permission(cls) -> PermissionData:
+        """Get the permission required to view this scope
+
+        Returns:
+            PermissionData: The permission required to view this scope in the admin console.
+        """
+        return COURSES_VIEW_COURSE_TEAM
+
     def get_object(self) -> CourseOverview | None:
         """Retrieve the CourseOverview instance associated with this scope.
 
@@ -710,6 +686,15 @@ def validate_external_key(cls, external_key: str) -> bool:
 
         return True
 
+    @classmethod
+    def get_admin_view_permission(cls) -> PermissionData:
+        """Get the permission required to view this scope
+
+        Returns:
+            PermissionData: The permission required to view this scope in the admin console.
+        """
+        raise NotImplementedError("Subclasses must implement get_admin_view_permission method.")
+
     @classmethod
     def get_org(cls, external_key: str) -> str | None:
         """Extract the organization identifier from the glob pattern.
@@ -799,6 +784,15 @@ class OrgContentLibraryGlobData(OrgGlobData):
     NAMESPACE: ClassVar[str] = "lib"
     ID_SEPARATOR: ClassVar[str] = ":"
 
+    @classmethod
+    def get_admin_view_permission(cls) -> PermissionData:
+        """Get the permission required to view this scope
+
+        Returns:
+            PermissionData: The permission required to view this scope in the admin console.
+        """
+        return VIEW_LIBRARY_TEAM
+
 
 @define
 class OrgCourseOverviewGlobData(OrgGlobData):
@@ -839,6 +833,15 @@ class OrgCourseOverviewGlobData(OrgGlobData):
     NAMESPACE: ClassVar[str] = "course-v1"
     ID_SEPARATOR: ClassVar[str] = "+"
 
+    @classmethod
+    def get_admin_view_permission(cls) -> PermissionData:
+        """Get the permission required to view this scope
+
+        Returns:
+            PermissionData: The permission required to view this scope in the admin console.
+        """
+        return COURSES_VIEW_COURSE_TEAM
+
 
 class CCXCourseOverviewData(CourseOverviewData):
     """CCX course scope for authorization in the Open edX platform.
@@ -994,117 +997,6 @@ def __repr__(self):
         return self.namespaced_key
 
 
-@define
-class ActionData(AuthZData):
-    """An action represents an operation that can be performed in the authorization system.
-
-    Actions are the operations that can be allowed or denied in authorization policies.
-
-    Attributes:
-        NAMESPACE: 'act' for actions.
-        external_key: The action identifier (e.g., 'content_libraries.view_library').
-        namespaced_key: The action identifier with namespace (e.g., 'act^content_libraries.view_library').
-        name: Property that returns a human-readable action name (e.g., 'Content Libraries > View Library').
-
-    Examples:
-        >>> action = ActionData(external_key='content_libraries.delete_library')
-        >>> action.namespaced_key
-        'act^content_libraries.delete_library'
-        >>> action.name
-        'Content Libraries > Delete Library'
-    """
-
-    NAMESPACE: ClassVar[str] = "act"
-
-    @property
-    def name(self) -> str:
-        """The human-readable name of the action (e.g., 'Content Libraries > Delete Library').
-
-        This property transforms the external_key into a human-readable display name
-        by replacing dots with ' > ' and capitalizing each word.
-
-        Returns:
-            str: The human-readable action name (e.g., 'Content Libraries > Delete Library').
-        """
-        parts = self.external_key.split(".")
-        return " > ".join(part.replace("_", " ").title() for part in parts)
-
-    def __str__(self):
-        """Human readable string representation of the action."""
-        return self.name
-
-    def __repr__(self):
-        """Developer friendly string representation of the action."""
-        return self.namespaced_key
-
-
-@define
-class PermissionData:
-    """A permission combines an action with an effect (allow or deny).
-
-    Permissions define whether a specific action should be allowed or denied.
-    They are typically associated with roles in the authorization system.
-
-    Attributes:
-        action: The action being permitted or denied (ActionData instance).
-        effect: The effect of the permission, either 'allow' or 'deny' (default: 'allow').
-
-    Examples:
-        >>> read_action = ActionData(external_key='read')
-        >>> permission = PermissionData(action=read_action, effect='allow')
-        >>> str(permission)
-        'Read - allow'
-        >>> write_action = ActionData(external_key='write')
-        >>> deny_perm = PermissionData(action=write_action, effect='deny')
-        >>> str(deny_perm)
-        'Write - deny'
-    """
-
-    action: ActionData = None
-    effect: Literal["allow", "deny"] = "allow"
-
-    @property
-    def identifier(self) -> str:
-        """Get the permission identifier.
-
-        Returns:
-            str: The permission identifier (e.g., 'content_libraries.delete_library').
-        """
-        return self.action.external_key
-
-    def __eq__(self, other: "PermissionData") -> bool:
-        """Compare permissions based on their action identifier.
-
-        Two PermissionData instances are considered equal if they have the same action's
-        external_key and effect.
-
-        Args:
-            other: Another PermissionData instance or any object.
-
-        Returns:
-            bool: True if the actions match, False otherwise.
-
-        Example:
-            >>> perm1 = PermissionData(action=ActionData(external_key='view'), effect='allow')
-            >>> perm2 = PermissionData(action=ActionData(external_key='view'), effect='allow')
-            >>> perm1 == perm2  # True - same action and effect
-            True
-            >>> perm1 in [perm2]  # Uses __eq__
-            True
-        """
-        if self.action is None or other.action is None:
-            return False
-        return self.action.external_key == other.action.external_key and self.effect == other.effect
-
-    def __str__(self):
-        """Human readable string representation of the permission and its effect."""
-        return f"{self.action} - {self.effect}"
-
-    def __repr__(self):
-        """Developer friendly string representation of the permission."""
-        return f"{self.action.namespaced_key} => {self.effect}"
-
-
 @define(eq=False)
 class RoleData(AuthZData):
     """A role is a named collection of permissions that can be assigned to subjects.

openedx_authz/api/users.py

@@ -11,10 +11,6 @@
 
 from openedx_authz.api.data import (
     ActionData,
-    ContentLibraryData,
-    CourseOverviewData,
-    OrgContentLibraryGlobData,
-    OrgCourseOverviewGlobData,
     PermissionData,
     RoleAssignmentData,
     RoleData,
@@ -39,7 +35,6 @@
     unassign_subject_from_all_roles,
 )
 from openedx_authz.api.utils import filter_user_assignments, get_user_assignment_map
-from openedx_authz.constants.permissions import COURSES_MANAGE_COURSE_TEAM, MANAGE_LIBRARY_TEAM
 
 __all__ = [
     "assign_role_to_user_in_scope",
@@ -224,11 +219,8 @@ def _filter_allowed_assignments(
     for assignment in assignments:
         permission = None
 
-        # For CourseOverviewData and ContentLibraryData, check for the view permission
-        if isinstance(assignment.scope, (CourseOverviewData, OrgCourseOverviewGlobData)):
-            permission = COURSES_MANAGE_COURSE_TEAM.identifier
-        elif isinstance(assignment.scope, (ContentLibraryData, OrgContentLibraryGlobData)):
-            permission = MANAGE_LIBRARY_TEAM.identifier
+        # Get the permission needed to view the specific scope in the admin console
+        permission = assignment.scope.get_admin_view_permission().identifier
 
         if permission and is_user_allowed(
             user_external_key=user_external_key,

openedx_authz/api/utils.py

@@ -54,6 +54,17 @@ def filter_user_assignments(
 ) -> list[UserAssignments]:
     """
     Filter user assignments by orgs or scopes.
+
+    Returns a list of users that have at least one assignment matching the filters,
+    with only the matching assignments for each matching user.
+
+    Args:
+        users_with_assignments (list[UserAssignments]): The list of users with their role assignments.
+        by (UserAssignmentsFilter): The filter type (by orgs or scopes).
+        values (list[str]): The list of orgs or scopes to filter by.
+
+    Returns:
+        list[UserAssignments]: The filtered list of users with their role assignments.
     """
     if not values:
         return users_with_assignments
@@ -62,7 +73,7 @@ def _get_value_to_filter(assignment: RoleAssignmentData) -> str:
         if by == UserAssignmentsFilter.SCOPES:
             return assignment.scope.external_key
         elif by == UserAssignmentsFilter.ORGS:
-            return assignment.scope.org
+            return getattr(assignment.scope, "org", None)
         else:
             raise ValueError(f"Invalid filter: '{by}'. Must be one of {[f.value for f in UserAssignmentsFilter]}")
 

openedx_authz/constants/permissions.py

@@ -2,7 +2,7 @@
 Default permission constants.
 """
 
-from openedx_authz.api.data import ActionData, PermissionData
+from openedx_authz.data import ActionData, PermissionData
 
 # Content Library Permissions