diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 569871b95bea..36cd5ff6367e 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 _ @@ -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: diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 6be650d05d4e..7e9e332b034b 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,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 diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 312676de1c92..21fed0d39336 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/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,54 @@ 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) + + 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") + 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 + 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..26fbda850ec4 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 has_access 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, + } ) + has_course_team_access = has_access(request.user, "staff", course_key).has_access + return has_course_team_access or has_discussion_privileges and request.method == "GET"