perf: reduce per-assignment Casbin calls in visible assignments fast path

docs/decisions/0014-bulk-assignment-queries-without-casbin-enforce.rst

@@ -0,0 +1,127 @@
+0014: Visible Role Assignment Queries Without Casbin Enforce Calls
+##################################################################
+
+Status
+******
+
+**Accepted** - *2026-05-28*
+
+Context
+*******
+
+``get_visible_role_assignments_for_user`` and ``get_visible_user_role_assignments_filtered_by_current_user``
+are the main entry points for listing role assignments in the admin console. Both answer two questions:
+
+1. Which assignments match the requested filters (org, scope, role)?
+2. Which of those assignments is the requesting user allowed to see?
+
+The original implementation called ``is_user_allowed`` once per candidate assignment.
+``is_user_allowed`` calls ``enforcer.enforce()``, one policy evaluation per call. With N
+assignments, that is N enforce calls per request.
+
+Profiling on a realistic dataset (muscat, ``GET /api/authz/v1/assignments/``) showed
+``_filter_allowed_assignments`` taking 16.74s for a non-admin user (see `pyinstrument report, main non-admin`_).
+Each ``enforce()`` call costs 20 to 80ms due to Casbin evaluating the full policy graph per
+(subject, action, object) triple, including role graph traversal via ``has_link`` and the
+custom matcher function. At scale, N enforce calls dominate the request time.
+
+The two questions were also answered in a different order: question 2 (authorization) ran first on
+the full assignment list, and question 1 (filtering) ran afterward on the grouped result.
+Assignments that would have been dropped by the filter were still evaluated by Casbin.
+
+Decision
+********
+
+Avoid Casbin ``enforce()`` in the visible-assignment hot path. Instead, retrieve the viewer's
+accessible scopes from the database and match assignment scopes in Python.
+
+1. Replace per-assignment enforce() with scope lookups
+=======================================================
+
+A new public function, ``filter_role_assignments_visible_to_subject`` (in
+``openedx_authz.api.roles``), replaces per-assignment ``enforce()`` calls. It:
+
+- calls ``get_scopes_for_subject_and_permission`` once per distinct permission type across all
+  candidates (one enforcer lookup per type, not one per assignment),
+- uses Casbin's own ``key_match_func`` to check whether each assignment's scope matches any of
+  the viewer's accessible scopes.
+
+``get_scopes_for_subject_and_permission`` is called once per distinct permission type (typically
+1-3 per request), not once per assignment. Matching is done in Python via ``key_match_func``.
+
+2. Filter by params before the authorization pass
+==================================================
+
+A new ``_filter_assignments_by_params`` function applies org, scope, and role filters on the
+flat assignment list before the authorization pass. Assignments that would be dropped by the
+filters are never evaluated for visibility.
+
+3. Cache role permission lookups within a call
+===============================================
+
+``get_role_assignments`` now uses a local ``_perm_cache`` dict to avoid calling
+``get_permissions_for_single_role`` more than once per role key per call.
+
+Consequences
+************
+
+#. **No Casbin enforce calls in the visible-assignment path** for the common case.
+   ``get_scopes_for_subject_and_permission`` is called once per distinct permission type, not
+   once per assignment.
+
+#. **The authorization pass and grouping step operate on a pre-filtered list.** Assignments
+   dropped by the filters are never evaluated for visibility.
+
+#. ``filter_role_assignments_visible_to_subject`` **is a public function** in
+   ``openedx_authz.api.roles``, available to callers who need visibility filtering outside of
+   the user-assignment endpoints.
+
+#. ``key_match_func`` **is used directly from** ``casbin.util``. This couples the visibility
+   filter to Casbin's matching semantics. If the model's matching behavior changes, this function
+   must change too.
+
+#. ``get_scopes_for_subject_and_permission`` **must return current data.** If the enforcer cache
+   is stale, the visibility filter produces wrong results silently. The per-assignment
+   ``enforce()`` approach had the same dependency, resolved per call rather than once upfront.
+
+Patterns for Bulk Authorization Paths
+**************************************
+
+While implementing this change, we identified some patterns for bulk authorization paths like this one:
+
+**Scope lookups for bulk visibility checks.**
+Query the viewer's accessible scopes once rather than calling enforce per item.
+``get_scopes_for_subject_and_permission`` does this.
+
+**batch_enforce to reduce per-call overhead**
+If per-item enforce calls are still needed, use Casbin's ``batch_enforce`` to reduce overhead getting the enforcer.
+This was implemented and tested but ultimately not used in this case since scope lookups were sufficient.
+
+**Use Casbin's own matching utilities.**
+``casbin.util.key_match_func`` implements the same glob-matching logic as the Casbin model's
+``keyMatch``. Use it rather than reimplementing the matching logic.
+
+**Filter before authorizing.**
+Apply cheap filters (field equality, etc.) before authorization. Casbin is not
+involved in the first pass.
+
+Alternatives Considered
+***********************
+
+``batch_enforce`` to replace the per-assignment loop
+=====================================================
+
+Replacing the ``enforce()`` loop with a single ``batch_enforce`` call was implemented first
+(see `528b129`_). It removed per-call overhead but kept N policy evaluations. Dropped in favor
+of the scope-based approach.
+
+References
+**********
+
+- `ADR 0005`_
+- `ADR 0012`_
+
+.. _ADR 0005: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0005-architecture-and-data-modeling.rst
+.. _ADR 0012: https://github.com/openedx/openedx-authz/blob/main/docs/decisions/0012-auditability.rst
+.. _528b129: https://github.com/openedx/openedx-authz/commit/528b129c829df13588e74965b1f8116d73320627
+.. _pyinstrument report, main non-admin: ../../../pyinstrument-reports/20260521-131048-main-muscat-nonadmin.html

openedx_authz/api/roles.py

@@ -8,8 +8,10 @@
 internally to manage the underlying policies and role assignments.
 """
 
+import logging
 from collections import defaultdict
 
+from casbin.util import key_match_func
 from crum import get_current_user
 from django.db import transaction
 from openedx_events.authz.data import RoleAssignmentData as RoleAssignmentEventData
@@ -51,6 +53,7 @@
     "get_all_role_assignments_per_scope_type",
     "unassign_role_from_subject_in_scope",
     "unassign_subject_from_all_roles",
+    "filter_role_assignments_visible_to_subject",
 ]
 
 # TODO: these are the concerns we still have to address:
@@ -61,6 +64,8 @@
 # I believe they assume the enforcer is already loaded with the relevant policies
 # in this case, ALL the policies, but that might not be the case
 
+log = logging.getLogger(__name__)
+
 
 def get_permissions_for_single_role(
     role: RoleData,
@@ -436,9 +441,12 @@ def get_role_assignments(
     field_index, field_values = _get_field_index_and_values(subject, role, scope)
     policies = enforcer.get_filtered_grouping_policy(field_index, *field_values)
 
+    _perm_cache: dict[str, list[PermissionData]] = {}
     for policy in policies:
         role = RoleData(namespaced_key=policy[GroupingPolicyIndex.ROLE.value])
-        role.permissions = get_permissions_for_single_role(role)
+        if role.namespaced_key not in _perm_cache:
+            _perm_cache[role.namespaced_key] = get_permissions_for_single_role(role)
+        role.permissions = _perm_cache[role.namespaced_key]
         role_assignments.append(
             RoleAssignmentData(
                 subject=SubjectData(namespaced_key=policy[GroupingPolicyIndex.SUBJECT.value]),
@@ -615,3 +623,42 @@ def get_all_role_assignments_per_scope_type(scope_types: tuple[type[ScopeData],
     return [
         role_assignment for role_assignment in get_role_assignments() if isinstance(role_assignment.scope, scope_types)
     ]
+
+
+def filter_role_assignments_visible_to_subject(
+    subject: SubjectData,
+    candidate_assignments: list[RoleAssignmentData],
+) -> list[RoleAssignmentData]:
+    """Return only the assignments the given subject has permission to view.
+
+    Looks up the scopes where the subject holds admin-view access (one DB query
+    per distinct permission type across all assignments), then filters using
+    key_match_func to support glob scope patterns. A viewer with admin-view
+    access (ex. VIEW_LIBRARY_TEAM) to ``lib:DemoX:*`` will see all assignments
+    whose scope matches that pattern.
+
+    Args:
+        subject: The viewer whose role assignments determine what is visible.
+        candidate_assignments: The candidate assignments to filter.
+
+    Returns:
+        The subset of assignments the subject is allowed to see.
+    """
+    viewer_scopes_by_permission: dict[str, list[ScopeData]] = {}
+    filtered_assignments = []
+    for assignment in candidate_assignments:
+        try:
+            perm = assignment.scope.get_admin_view_permission()
+        except NotImplementedError:
+            log.warning("Skipping assignment with unsupported scope %r", assignment.scope.external_key)
+            continue
+
+        if perm.identifier not in viewer_scopes_by_permission:
+            viewer_scopes_by_permission[perm.identifier] = get_scopes_for_subject_and_permission(subject, perm)
+        viewer_scopes = viewer_scopes_by_permission[perm.identifier]
+
+        # If any of the viewer's scopes match the assignment's scope, the assignment is visible to the viewer.
+        if any(key_match_func(assignment.scope.namespaced_key, vs.namespaced_key) for vs in viewer_scopes):
+            filtered_assignments.append(assignment)
+
+    return filtered_assignments

openedx_authz/api/users.py

@@ -22,14 +22,14 @@
     ScopeData,
     SuperAdminAssignmentData,
     UserAssignments,
-    UserAssignmentsFilter,
     UserData,
 )
 from openedx_authz.api.permissions import is_subject_allowed
 from openedx_authz.api.roles import (
     assign_role_to_subject_in_scope,
     batch_assign_role_to_subjects_in_scope,
     batch_unassign_role_from_subjects_in_scope,
+    filter_role_assignments_visible_to_subject,
     get_all_subject_role_assignments_in_scope,
     get_role_assignments,
     get_scopes_for_subject_and_permission,
@@ -40,8 +40,8 @@
     unassign_role_from_subject_in_scope,
     unassign_subject_from_all_roles,
 )
-from openedx_authz.api.utils import filter_user_assignments, get_user_assignment_map
-from openedx_authz.utils import get_user_by_username_or_email
+from openedx_authz.api.utils import get_user_assignment_map
+from openedx_authz.utils import get_user_by_username_or_email, is_user_staff_or_superuser
 
 log = logging.getLogger(__name__)
 
@@ -203,11 +203,12 @@ def get_visible_user_role_assignments_filtered_by_current_user(
         list[RoleAssignmentData]: A list of role assignments for the user, filtered by orgs/roles and permissions.
     """
     user_role_assignments = get_user_role_assignments(user_external_key=user_external_key)
-    # Filter assignments based on the user's permissions
-    user_role_assignments = _filter_allowed_assignments(
-        user_external_key=allowed_for_user_external_key,
-        assignments=user_role_assignments,
-    )
+
+    if allowed_for_user_external_key:
+        user_role_assignments = _filter_allowed_assignments(
+            user_external_key=allowed_for_user_external_key,
+            assignments=user_role_assignments,
+        )
 
     # Only include assignments whose subject corresponds to an active user,
     # consistent with get_superadmin_assignments which filters by is_active=True.
@@ -271,34 +272,68 @@ def get_all_user_role_assignments_in_scope(
     return get_all_subject_role_assignments_in_scope(ScopeData(external_key=scope_external_key))
 
 
+def _filter_candidate_assignments_by_params(
+    assignments: list[RoleAssignmentData],
+    orgs: list[str] | None,
+    scopes: list[str] | None,
+    roles: list[str] | None,
+) -> list[RoleAssignmentData]:
+    """Reduce the candidate assignment set using cheap Python filters before authorization.
+
+    Filters by org, scope external key, and role external key. All filters accept lists
+    and are applied only when provided. This runs before the scope-based authorization
+    pass to avoid paying the DB cost for assignments that would be dropped anyway.
+
+    Args:
+        assignments: The full assignment list to filter. Each entry has exactly one role
+            (one policy line = one RoleAssignmentData), as produced by get_role_assignments.
+        orgs: Optional list of org identifiers to keep (e.g., ['edX', 'MITx']), matched against scope.org.
+        scopes: Optional list of scope external keys to keep (e.g., ['lib:DemoX:CSPROB']).
+        roles: Optional list of role external keys to keep (e.g., ['library_admin', 'instructor']).
+
+    Returns:
+        The filtered assignment list.
+    """
+    if scopes:
+        assignments = [a for a in assignments if a.scope.external_key in scopes]
+    if orgs:
+        assignments = [a for a in assignments if getattr(a.scope, "org", None) in orgs]
+    if roles:
+        # get_role_assignments always produces one role per assignment, so use
+        # a.roles[0] check to avoid iterating over the list of roles for every assignment
+        assignments = [a for a in assignments if a.roles and a.roles[0].external_key in roles]
+    return assignments
+
+
 def _filter_allowed_assignments(
     assignments: list[RoleAssignmentData], user_external_key: str = None
 ) -> list[RoleAssignmentData]:
-    """
-    Filter the given role assignments to only include those that the user has permission to view.
+    """Return only the assignments the given viewer is authorized to see.
+
+    Delegates to filter_role_assignments_visible_to_subject, which uses the viewer's
+    own role assignments to determine scope visibility without calling Casbin enforce().
+    Returns all assignments unchanged when no viewer is specified.
+
+    Args:
+        assignments: The candidate assignments to filter.
+        user_external_key: Username of the viewer. If None, no filtering is applied.
 
-    Assignments whose scope does not implement ``get_admin_view_permission``
-    are skipped with a warning
+    Returns:
+        The subset of assignments the viewer is allowed to see.
     """
     if not user_external_key:
         # If no user is specified, return all assignments
         return assignments
-    allowed_assignments: list[RoleAssignmentData] = []
-    for assignment in assignments:
-        try:
-            permission = assignment.scope.get_admin_view_permission().identifier
-        except NotImplementedError:
-            log.warning("Skipping assignment with unsupported scope %r", assignment.scope.external_key)
-            continue
 
-        if permission and is_user_allowed(
-            user_external_key=user_external_key,
-            action_external_key=permission,
-            scope_external_key=assignment.scope.external_key,
-        ):
-            allowed_assignments.append(assignment)
+    # Temporary: the scope-based filter in filter_role_assignments_visible_to_subject uses
+    # key_match instead of enforce(), so the enforcer never runs is_admin_or_superuser_check.
+    # Staff and superusers must return early here or they lose the access that check would have granted them.
+    if is_user_staff_or_superuser(user_external_key):
+        return assignments
 
-    return allowed_assignments
+    return filter_role_assignments_visible_to_subject(
+        UserData(external_key=user_external_key), assignments
+    )
 
 
 def get_visible_role_assignments_for_user(
@@ -320,30 +355,18 @@ def get_visible_role_assignments_for_user(
         list[UserAssignments]: A list of users with their role assignments, filtered by orgs/scopes and permissions.
     """
     user_role_assignments = get_user_role_assignments_filtered()
-    # Filter assignments based on the user's permissions
-    user_role_assignments = _filter_allowed_assignments(
-        user_external_key=allowed_for_user_external_key,
-        assignments=user_role_assignments,
-    )
-    # Group assignments by user
-    users_with_assignments = get_user_assignment_map(user_role_assignments)
 
-    users_with_assignments = filter_user_assignments(
-        users_with_assignments=users_with_assignments,
-        by=UserAssignmentsFilter.SCOPES,
-        values=scopes,
-    )
-    users_with_assignments = filter_user_assignments(
-        users_with_assignments=users_with_assignments,
-        by=UserAssignmentsFilter.ORGS,
-        values=orgs,
-    )
-    users_with_assignments = filter_user_assignments(
-        users_with_assignments=users_with_assignments,
-        by=UserAssignmentsFilter.ROLES,
-        values=roles,
+    user_role_assignments = _filter_candidate_assignments_by_params(
+        user_role_assignments, orgs=orgs, scopes=scopes, roles=roles
     )
-    return users_with_assignments
+
+    if allowed_for_user_external_key:
+        user_role_assignments = _filter_allowed_assignments(
+            user_external_key=allowed_for_user_external_key,
+            assignments=user_role_assignments,
+        )
+
+    return get_user_assignment_map(user_role_assignments)
 
 
 def is_user_allowed(