diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index e37649dbdec9..8d3bd47f79c4 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -728,11 +728,10 @@ def test_course_listing_with_org_scope(self): the AuthZ course authoring toggle is enabled. """ _, _, authz_courses, legacy_courses = self._create_courses() - org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*') assign_role_to_user_in_scope( self.authorized_user.username, COURSE_STAFF.external_key, - org_scope.external_key, + OrgCourseOverviewGlobData.build_external_key("Org1"), ) request = self._make_request(self.authorized_user) @@ -761,11 +760,10 @@ def test_course_listing_with_org_scope_with_toggle(self): authz_keys, _, _, _ = self._create_courses() # enable only the first and third course keys enabled_keys = {str(authz_keys[0]), str(authz_keys[2])} - org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*') assign_role_to_user_in_scope( self.authorized_user.username, COURSE_STAFF.external_key, - org_scope.external_key, + OrgCourseOverviewGlobData.build_external_key("Org1"), ) request = self._make_request(self.authorized_user) @@ -788,11 +786,10 @@ def test_course_listing_with_org_scope_without_courses(self): courses, `get_courses_accessible_to_user` should return an empty list. """ - org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org2+*') assign_role_to_user_in_scope( self.authorized_user.username, COURSE_STAFF.external_key, - org_scope.external_key, + OrgCourseOverviewGlobData.build_external_key("Org2"), ) request = self._make_request(self.authorized_user) @@ -810,17 +807,15 @@ def test_course_listing_with_org_scope_fetched_once(self): """ Verify that course overviews are fetched once with all authorized orgs. """ - org_scope1 = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*') - org_scope2 = OrgCourseOverviewGlobData(external_key='course-v1:Org2+*') assign_role_to_user_in_scope( self.authorized_user.username, COURSE_STAFF.external_key, - org_scope1.external_key, + OrgCourseOverviewGlobData.build_external_key("Org1"), ) assign_role_to_user_in_scope( self.authorized_user.username, COURSE_STAFF.external_key, - org_scope2.external_key, + OrgCourseOverviewGlobData.build_external_key("Org2"), ) request = self._make_request(self.authorized_user) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index fd8858bc2273..d3da7b9ec064 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/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 _ @@ -67,6 +67,7 @@ has_studio_write_access, is_content_creator, ) +from common.djangoapps.student.models.user import CourseAccessRole from common.djangoapps.student.roles import ( CourseInstructorRole, CourseStaffRole, @@ -903,20 +904,43 @@ 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() + # Query CourseAccessRole directly instead of UserBasedRole.courses_with_role(), + # which merges legacy DB records with AuthZ assignments. AuthZ access is resolved + # separately in _get_authz_accessible_courses_list(). Exact role names (not + # RoleCache inheritance) exclude limited_staff, matching strict_role_checking(). + 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: diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 6be650d05d4e..fc12d4a86e8f 100644 --- a/common/djangoapps/student/roles.py +++ b/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,120 @@ 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 _get_org_and_course_id_from_authz_scope( + scope: CourseOverviewData | OrgCourseOverviewGlobData, +) -> tuple[str, str | None] | None: """ - Retrieve all CourseAccessRole objects for a given user and convert them to AuthzCompatCourseAccessRole objects. + Extract the org and course key from an AuthZ course assignment scope. + + Course-scoped assignments return ``(org, course_external_key)``. + Org-wide assignments return ``(org, None)``. + + Returns ``None`` when the org cannot be determined. For org-wide scopes, + ``OrgGlobData.org`` is typed as ``str | None`` because it is parsed from + ``external_key`` and returns ``None`` for malformed glob patterns. """ - 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( + if isinstance(scope, CourseOverviewData): + course_id = scope.external_key + return get_org_from_key(course_id), course_id + if isinstance(scope, OrgCourseOverviewGlobData): + return scope.org, None + return None + + +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 + """ + return authz_api.get_user_role_assignments_per_scope_type( + user_external_key=user.username, + scope_types=(CourseOverviewData, OrgCourseOverviewGlobData), + ) + + +def _compat_roles_from_authz_assignment( + user: User, + assignment: RoleAssignmentData, +) -> set[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 org cannot be determined from + the scope or no roles could be mapped. + """ + org_and_course_id = _get_org_and_course_id_from_authz_scope(assignment.scope) + if org_and_course_id is None: + return set() + org, course_id = org_and_course_id + + 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 +919,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 +937,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 +963,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 diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 312676de1c92..dd5381b45473 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -10,7 +10,15 @@ 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, + ScopeData, + UserData, +) from openedx_authz.constants.roles import COURSE_ADMIN, COURSE_STAFF from openedx_authz.engine.enforcer import AuthzEnforcer @@ -39,6 +47,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 +307,56 @@ 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. + """ + assignment = RoleAssignmentData( + subject=UserData(external_key=self.student.username), + roles=[RoleData(external_key=COURSE_ADMIN.external_key)], + scope=ScopeData( + external_key=OrgCourseOverviewGlobData.build_external_key("OpenedX"), + ), + ) + with patch("openedx_authz.api.users.get_user_role_assignments", return_value=[assignment]): + result = get_authz_compat_course_access_roles_for_user(self.student) + + self.assertCountEqual( # noqa: PT009 + 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. + """ + # pylint: disable=protected-access + course_key = CourseKey.from_string("course-v1:OpenedX+DemoX+DemoCourse") + assignment = RoleAssignmentData( + subject=UserData(external_key=self.student.username), + roles=[RoleData(external_key=COURSE_ADMIN.external_key)], + scope=ScopeData( + external_key=OrgCourseOverviewGlobData.build_external_key("OpenedX"), + ), + ) + with patch("openedx_authz.api.users.get_user_role_assignments", return_value=[assignment]): + if hasattr(self.student, "_roles"): + del self.student._roles + self.student._roles = RoleCache(self.student) + + self.assertTrue(self.student._roles.has_role("instructor", None, "OpenedX")) # noqa: PT009 + self.assertTrue(OrgInstructorRole("OpenedX").has_user(self.student)) # noqa: PT009 + self.assertTrue(self.student.has_perm(instructor_permissions.VIEW_DASHBOARD, course_key)) # noqa: PT009 + self.assertTrue(self.student.has_perm(instructor_permissions.SHOW_TASKS, course_key)) # noqa: PT009 + @ddt.ddt class RoleCacheTestCase(TestCase): # pylint: disable=missing-class-docstring diff --git a/openedx/core/djangoapps/course_groups/permissions.py b/openedx/core/djangoapps/course_groups/permissions.py index cfc3aefb2f8c..29e953321b1a 100644 --- a/openedx/core/djangoapps/course_groups/permissions.py +++ b/openedx/core/djangoapps/course_groups/permissions.py @@ -5,7 +5,8 @@ from opaque_keys.edx.keys import CourseKey from rest_framework import permissions -from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff +from common.djangoapps.student.roles import GlobalStaff +from lms.djangoapps.courseware.access import administrative_accesses_to_course_for_user from lms.djangoapps.discussion.django_comment_client.utils import get_user_role_names from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_ADMINISTRATOR, @@ -23,15 +24,15 @@ def has_permission(self, request, view): """Returns true if the user is admin or staff and request method is GET.""" if GlobalStaff().has_user(request.user) or request.user.is_superuser: return True - course_key = CourseKey.from_string(view.kwargs.get('course_key_string')) + course_key = CourseKey.from_string(view.kwargs.get("course_key_string")) user_roles = get_user_role_names(request.user, course_key) - has_discussion_privileges = bool(user_roles & { - FORUM_ROLE_ADMINISTRATOR, - FORUM_ROLE_MODERATOR, - FORUM_ROLE_COMMUNITY_TA, - }) - return ( - CourseInstructorRole(course_key).has_user(request.user) or - CourseStaffRole(course_key).has_user(request.user) or - has_discussion_privileges and request.method == "GET" + has_discussion_privileges = bool( + user_roles + & { + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, + FORUM_ROLE_COMMUNITY_TA, + } ) + _, staff_access, instructor_access = administrative_accesses_to_course_for_user(request.user, course_key) + return staff_access or instructor_access or (has_discussion_privileges and request.method == "GET")