feat: enhance role assignment handling for users with org-wide scopes

Author: BryanttV

cms/djangoapps/contentstore/views/course.py

@@ -17,7 +17,7 @@
 from django.core.exceptions import FieldError, ImproperlyConfigured, PermissionDenied
 from django.core.exceptions import ValidationError as DjangoValidationError
 from django.db.models import QuerySet
-from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseNotFound
+from django.http import Http404, HttpRequest, HttpResponse, HttpResponseBadRequest, HttpResponseNotFound
 from django.shortcuts import redirect
 from django.urls import reverse
 from django.utils.translation import gettext as _
@@ -62,6 +62,7 @@
     has_studio_write_access,
     is_content_creator,
 )
+from common.djangoapps.student.models.user import CourseAccessRole
 from common.djangoapps.student.roles import (
     CourseInstructorRole,
     CourseStaffRole,
@@ -823,6 +824,7 @@ def _get_course_keys_for_org_scope(org_keys: set[str]):
 
     return CourseOverview.get_all_courses(orgs=org_keys).values_list('id', flat=True)
 
+
 def _get_course_keys_from_scopes(authz_scopes: list[ScopeData]):
     """
     Convert a set of Authz scopes into specific course keys.
@@ -842,6 +844,7 @@ def _get_course_keys_from_scopes(authz_scopes: list[ScopeData]):
         )
     return course_keys
 
+
 def _get_authz_accessible_courses_list(request):
     """
     List all courses available to the logged in user by
@@ -855,20 +858,39 @@ def _get_authz_accessible_courses_list(request):
 
     return _get_course_keys_from_scopes(authz_scopes)
 
-def _get_legacy_accessible_courses_list(request):
+
+def _get_legacy_accessible_courses_list(request: HttpRequest) -> set[CourseKey]:
     """
-    List all courses available to the logged in user by
-    evaluating legacy Django group roles and organization-level access.
+    Resolve candidate course keys from legacy ``CourseAccessRole`` records.
+
+    Only database-backed legacy roles are considered. AuthZ-managed access,
+    including org-wide scopes, is resolved separately by
+    ``_get_authz_accessible_courses_list``.
+
+    Course-level roles (``instructor``, ``staff``) are mapped directly to their
+    course keys. Org-wide roles expand to every course in that organization via
+    a single ``CourseOverview.get_all_courses(orgs=...)`` query. The ``staff``
+    role is matched exactly, so ``limited_staff`` assignments are excluded.
+
+    Args:
+        request: The incoming HTTP request; ``request.user`` determines which
+            legacy role records are evaluated.
+
+    Returns:
+        set[CourseKey]: Course keys the user may access through legacy roles.
+
+    Raises:
+        AccessListFallback: If a legacy role record has neither a course key nor
+            an organization
     """
     user = request.user
-    instructor_courses = UserBasedRole(user, CourseInstructorRole.ROLE).courses_with_role()
-
-    with strict_role_checking():
-        staff_courses = UserBasedRole(user, CourseStaffRole.ROLE).courses_with_role()
+    legacy_accesses = CourseAccessRole.objects.filter(
+        user=user,
+        role__in=[CourseInstructorRole.ROLE, CourseStaffRole.ROLE],
+    )
 
     group_keys = set()
     org_accesses = set()
-    legacy_accesses = instructor_courses | staff_courses
 
     for access in legacy_accesses:
         if access.course_id is not None:

common/djangoapps/student/roles.py

@@ -15,7 +15,7 @@
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import CourseLocator
 from openedx_authz.api import users as authz_api
-from openedx_authz.api.data import CourseOverviewData, RoleAssignmentData
+from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData, RoleAssignmentData
 from openedx_authz.constants import roles as authz_roles
 
 from common.djangoapps.student.models import CourseAccessRole
@@ -74,17 +74,6 @@ def authz_add_role(user: User, authz_role: str, course_key: str):
     legacy_role = get_legacy_role_from_authz_role(authz_role)
     emit_course_access_role_added(user, course_locator, course_locator.org, legacy_role)
 
-def authz_get_all_course_assignments_for_user(user: User) -> list[RoleAssignmentData]:
-    """
-    Get all course assignments for a user.
-    """
-    assignments = authz_api.get_user_role_assignments(user_external_key=user.username)
-    # filter courses only
-    filtered_assignments = [
-        assignment for assignment in assignments
-        if isinstance(assignment.scope, CourseOverviewData)
-    ]
-    return filtered_assignments
 
 def get_org_from_key(key: str) -> str:
     """
@@ -93,6 +82,7 @@ def get_org_from_key(key: str) -> str:
     parsed_key = CourseKey.from_string(key)
     return parsed_key.org
 
+
 def register_access_role(cls):
     """
     Decorator that allows access roles to be registered within the roles module and referenced by their
@@ -141,34 +131,108 @@ class AuthzCompatCourseAccessRole:
     """
     Generic data class for storing CourseAccessRole-compatible data
     to be used inside BulkRoleCache and RoleCache.
+
     This allows the cache to store both legacy and openedx-authz compatible roles
     """
     user_id: int
     username: str
     org: str
-    course_id: str  # Course key
+    course_id: str | None
     role: str
 
 
-def get_authz_compat_course_access_roles_for_user(user: User) -> set[AuthzCompatCourseAccessRole]:
+def authz_get_all_course_assignments_for_user(user: User) -> list[RoleAssignmentData]:
+    """
+    Return AuthZ role assignments for a user that apply to courses.
+
+    Includes assignments scoped to a specific course (``CourseOverviewData``) and
+    assignments scoped to all courses in an organization (``OrgCourseOverviewGlobData``).
+    Assignments for other resource types, such as content libraries, are excluded.
+
+    Args:
+        user (User): The user whose AuthZ role assignments should be retrieved.
+
+    Returns:
+        list[RoleAssignmentData]: Role assignments whose scope is course-level or org-wide
     """
-    Retrieve all CourseAccessRole objects for a given user and convert them to AuthzCompatCourseAccessRole objects.
+    assignments = authz_api.get_user_role_assignments(user_external_key=user.username)
+    return [
+        assignment
+        for assignment in assignments
+        if isinstance(assignment.scope, CourseOverviewData | OrgCourseOverviewGlobData)
+    ]
+
+
+def _compat_roles_from_authz_assignment(
+    user: User,
+    assignment: RoleAssignmentData,
+) -> set[AuthzCompatCourseAccessRole]:
     """
-    compat_role_assignments = set()
-    assignments = authz_get_all_course_assignments_for_user(user)
-    for assignment in assignments:
-        for role in assignment.roles:
-            legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key)
-            course_key = assignment.scope.external_key
-            org = get_org_from_key(course_key)
-            compat_role = AuthzCompatCourseAccessRole(
+    Convert an AuthZ role assignment into legacy-compatible course access roles.
+
+    Course-scoped assignments produce roles tied to a specific course key.
+    Org-wide assignments produce org-level roles with no course key (``course_id``
+    is ``None``), matching legacy ``OrgStaffRole`` / ``OrgInstructorRole`` behavior.
+    AuthZ roles without a legacy mapping are skipped.
+
+    Args:
+        user (User): The user associated with the assignment.
+        assignment (RoleAssignmentData): A single AuthZ role assignment, including
+            its scope and assigned roles.
+
+    Returns:
+        set[AuthzCompatCourseAccessRole]: Legacy-compatible role records for the
+            assignment. Returns an empty set if the scope is unsupported, the org
+            is missing for an org-wide assignment, or no roles could be mapped.
+    """
+    scope = assignment.scope
+    if isinstance(scope, CourseOverviewData):
+        course_id = scope.external_key
+        org = get_org_from_key(course_id)
+    elif isinstance(scope, OrgCourseOverviewGlobData):
+        org = scope.org
+        if not org:
+            return set()
+        course_id = None
+    else:
+        return set()
+
+    compat_roles = set()
+    for role in assignment.roles:
+        legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key)
+        if legacy_role is None:
+            continue
+        compat_roles.add(
+            AuthzCompatCourseAccessRole(
                 user_id=user.id,
                 username=user.username,
                 org=org,
-                course_id=course_key,
-                role=legacy_role
+                course_id=course_id,
+                role=legacy_role,
             )
-            compat_role_assignments.add(compat_role)
+        )
+    return compat_roles
+
+
+def get_authz_compat_course_access_roles_for_user(user: User) -> set[AuthzCompatCourseAccessRole]:
+    """
+    Retrieve AuthZ course and org role assignments for a user in legacy format.
+
+    Fetches all course-level and org-wide AuthZ assignments for the user and
+    converts each one into ``AuthzCompatCourseAccessRole`` records suitable for
+    ``RoleCache`` and other legacy permission checks.
+
+    Args:
+        user (User): The user whose AuthZ role assignments should be converted.
+
+    Returns:
+        set[AuthzCompatCourseAccessRole]: Legacy-compatible role records derived
+            from the user's AuthZ assignments. Returns an empty set if the user
+            has no applicable assignments.
+    """
+    compat_role_assignments = set()
+    for assignment in authz_get_all_course_assignments_for_user(user):
+        compat_role_assignments.update(_compat_roles_from_authz_assignment(user, assignment))
     return compat_role_assignments
 
 
@@ -843,11 +907,9 @@ def courses_with_role(self) -> set[AuthzCompatCourseAccessRole]:
         """
         Return a set of AuthzCompatCourseAccessRole for all of the courses with this user x (or derived from x) role.
         """
+        # Get all assignments for a user to a role
         roles = RoleCache.get_roles(self.role)
         legacy_assignments = CourseAccessRole.objects.filter(role__in=roles, user=self.user)
-
-        # Get all assignments for a user to a role
-        new_authz_roles = [get_authz_role_from_legacy_role(role) for role in roles]
         all_authz_user_assignments = authz_get_all_course_assignments_for_user(self.user)
 
         all_assignments = set()
@@ -863,19 +925,9 @@ def courses_with_role(self) -> set[AuthzCompatCourseAccessRole]:
                 ))
 
         for assignment in all_authz_user_assignments:
-            for role in assignment.roles:
-                if role.external_key not in new_authz_roles:
-                    continue
-                legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key)
-                course_key = assignment.scope.external_key
-                org = get_org_from_key(course_key)
-                all_assignments.add(AuthzCompatCourseAccessRole(
-                    user_id=self.user.id,
-                    username=self.user.username,
-                    org=org,
-                    course_id=course_key,
-                    role=legacy_role
-                ))
+            for compat_role in _compat_roles_from_authz_assignment(self.user, assignment):
+                if compat_role.role in roles:
+                    all_assignments.add(compat_role)
 
         return all_assignments
 
@@ -899,18 +951,12 @@ def has_courses_with_role(self, org: str | None = None) -> bool:
             return True
 
         # Then check for authz assignments
-        new_authz_roles = [get_authz_role_from_legacy_role(role) for role in roles]
         all_authz_user_assignments = authz_get_all_course_assignments_for_user(self.user)
 
         for assignment in all_authz_user_assignments:
-            for role in assignment.roles:
-                if role.external_key not in new_authz_roles:
+            for compat_role in _compat_roles_from_authz_assignment(self.user, assignment):
+                if compat_role.role not in roles:
                     continue
-                if org is None:
-                    # There is at least one assignment, short circuit
-                    return True
-                course_key = assignment.scope.external_key
-                parsed_org = get_org_from_key(course_key)
-                if org == parsed_org:
+                if org is None or org == compat_role.org:
                     return True
         return False

common/djangoapps/student/tests/test_roles.py

@@ -10,7 +10,14 @@
 from edx_toggles.toggles.testutils import override_waffle_flag
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import LibraryLocator
-from openedx_authz.api.data import ContentLibraryData, CourseOverviewData, RoleAssignmentData, RoleData, UserData
+from openedx_authz.api.data import (
+    ContentLibraryData,
+    CourseOverviewData,
+    OrgCourseOverviewGlobData,
+    RoleAssignmentData,
+    RoleData,
+    UserData,
+)
 from openedx_authz.constants.roles import COURSE_ADMIN, COURSE_STAFF
 from openedx_authz.engine.enforcer import AuthzEnforcer
 
@@ -39,6 +46,7 @@
     get_role_cache_key_for_course,
 )
 from common.djangoapps.student.tests.factories import AnonymousUserFactory, InstructorFactory, StaffFactory, UserFactory
+from lms.djangoapps.instructor import permissions as instructor_permissions
 from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
 from openedx.core.toggles import AUTHZ_COURSE_AUTHORING_FLAG
 
@@ -298,6 +306,50 @@ def test_get_authz_compat_course_access_roles_for_user(self):
             result = get_authz_compat_course_access_roles_for_user(self.student)
         assert result == set()
 
+    def test_get_authz_compat_course_access_roles_for_user_org_scope(self):
+        """
+        Org-wide AuthZ assignments should map to legacy org-level course access roles.
+        """
+        org_scope = OrgCourseOverviewGlobData(external_key="course-v1:OpenedX+*")
+        assignment = RoleAssignmentData(
+            subject=UserData(external_key=self.student.username),
+            roles=[RoleData(external_key=COURSE_ADMIN.external_key)],
+            scope=org_scope,
+        )
+        with patch("openedx_authz.api.users.get_user_role_assignments", return_value=[assignment]):
+            result = get_authz_compat_course_access_roles_for_user(self.student)
+
+        assert result == {
+            AuthzCompatCourseAccessRole(
+                user_id=self.student.id,
+                username=self.student.username,
+                org="OpenedX",
+                course_id=None,
+                role="instructor",
+            )
+        }
+
+    def test_org_scope_authz_role_grants_instructor_dashboard_permissions(self):
+        """
+        Org-wide AuthZ course_admin should grant legacy org instructor access used by the instructor dashboard.
+        """
+        course_key = CourseKey.from_string("course-v1:OpenedX+DemoX+DemoCourse")
+        org_scope = OrgCourseOverviewGlobData(external_key="course-v1:OpenedX+*")
+        assignment = RoleAssignmentData(
+            subject=UserData(external_key=self.student.username),
+            roles=[RoleData(external_key=COURSE_ADMIN.external_key)],
+            scope=org_scope,
+        )
+        with patch("openedx_authz.api.users.get_user_role_assignments", return_value=[assignment]):
+            if hasattr(self.student, "_roles"):
+                del self.student._roles
+            cache = RoleCache(self.student)
+
+        assert cache.has_role("instructor", None, "OpenedX")
+        assert OrgInstructorRole("OpenedX").has_user(self.student)
+        assert self.student.has_perm(instructor_permissions.VIEW_DASHBOARD, course_key)
+        assert self.student.has_perm(instructor_permissions.SHOW_TASKS, course_key)
+
 
 @ddt.ddt
 class RoleCacheTestCase(TestCase):  # pylint: disable=missing-class-docstring