Skip to content

Commit f0bad41

Browse files
authored
fix: add missing org scope support in instructor dashboard (#38721)
* feat: enhance role assignment handling for users with org-wide scopes * refactor: update role assertion methods * refactor: replace external_key initialization with build_external_key method * refactor: streamline role assignment by directly using build_external_key method * refactor: update role assignment scope initialization to use ScopeData * refactor: introduce helper function to extract org and course ID from AuthZ scope * refactor: replace has_access with administrative_accesses_to_course_for_user * docs: clarify legacy-only CourseAccessRole query in studio course list * refactor: simplify user role assignment retrieval by using scoped api method
1 parent bd2d3c1 commit f0bad41

5 files changed

Lines changed: 219 additions & 82 deletions

File tree

cms/djangoapps/contentstore/tests/test_course_listing.py

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -728,11 +728,10 @@ def test_course_listing_with_org_scope(self):
728728
the AuthZ course authoring toggle is enabled.
729729
"""
730730
_, _, authz_courses, legacy_courses = self._create_courses()
731-
org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*')
732731
assign_role_to_user_in_scope(
733732
self.authorized_user.username,
734733
COURSE_STAFF.external_key,
735-
org_scope.external_key,
734+
OrgCourseOverviewGlobData.build_external_key("Org1"),
736735
)
737736

738737
request = self._make_request(self.authorized_user)
@@ -761,11 +760,10 @@ def test_course_listing_with_org_scope_with_toggle(self):
761760
authz_keys, _, _, _ = self._create_courses()
762761
# enable only the first and third course keys
763762
enabled_keys = {str(authz_keys[0]), str(authz_keys[2])}
764-
org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*')
765763
assign_role_to_user_in_scope(
766764
self.authorized_user.username,
767765
COURSE_STAFF.external_key,
768-
org_scope.external_key,
766+
OrgCourseOverviewGlobData.build_external_key("Org1"),
769767
)
770768

771769
request = self._make_request(self.authorized_user)
@@ -788,11 +786,10 @@ def test_course_listing_with_org_scope_without_courses(self):
788786
courses, `get_courses_accessible_to_user` should return an empty
789787
list.
790788
"""
791-
org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org2+*')
792789
assign_role_to_user_in_scope(
793790
self.authorized_user.username,
794791
COURSE_STAFF.external_key,
795-
org_scope.external_key,
792+
OrgCourseOverviewGlobData.build_external_key("Org2"),
796793
)
797794

798795
request = self._make_request(self.authorized_user)
@@ -810,17 +807,15 @@ def test_course_listing_with_org_scope_fetched_once(self):
810807
"""
811808
Verify that course overviews are fetched once with all authorized orgs.
812809
"""
813-
org_scope1 = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*')
814-
org_scope2 = OrgCourseOverviewGlobData(external_key='course-v1:Org2+*')
815810
assign_role_to_user_in_scope(
816811
self.authorized_user.username,
817812
COURSE_STAFF.external_key,
818-
org_scope1.external_key,
813+
OrgCourseOverviewGlobData.build_external_key("Org1"),
819814
)
820815
assign_role_to_user_in_scope(
821816
self.authorized_user.username,
822817
COURSE_STAFF.external_key,
823-
org_scope2.external_key,
818+
OrgCourseOverviewGlobData.build_external_key("Org2"),
824819
)
825820

826821
request = self._make_request(self.authorized_user)

cms/djangoapps/contentstore/views/course.py

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from django.core.exceptions import FieldError, ImproperlyConfigured, PermissionDenied
1818
from django.core.exceptions import ValidationError as DjangoValidationError
1919
from django.db.models import QuerySet
20-
from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseNotFound
20+
from django.http import Http404, HttpRequest, HttpResponse, HttpResponseBadRequest, HttpResponseNotFound
2121
from django.shortcuts import redirect
2222
from django.urls import reverse
2323
from django.utils.translation import gettext as _
@@ -67,6 +67,7 @@
6767
has_studio_write_access,
6868
is_content_creator,
6969
)
70+
from common.djangoapps.student.models.user import CourseAccessRole
7071
from common.djangoapps.student.roles import (
7172
CourseInstructorRole,
7273
CourseStaffRole,
@@ -903,20 +904,43 @@ def _get_authz_accessible_courses_list(request):
903904

904905
return _get_course_keys_from_scopes(authz_scopes)
905906

906-
def _get_legacy_accessible_courses_list(request):
907+
908+
def _get_legacy_accessible_courses_list(request: HttpRequest) -> set[CourseKey]:
907909
"""
908-
List all courses available to the logged in user by
909-
evaluating legacy Django group roles and organization-level access.
910+
Resolve candidate course keys from legacy ``CourseAccessRole`` records.
911+
912+
Only database-backed legacy roles are considered. AuthZ-managed access,
913+
including org-wide scopes, is resolved separately by
914+
``_get_authz_accessible_courses_list``.
915+
916+
Course-level roles (``instructor``, ``staff``) are mapped directly to their
917+
course keys. Org-wide roles expand to every course in that organization via
918+
a single ``CourseOverview.get_all_courses(orgs=...)`` query. The ``staff``
919+
role is matched exactly, so ``limited_staff`` assignments are excluded.
920+
921+
Args:
922+
request: The incoming HTTP request; ``request.user`` determines which
923+
legacy role records are evaluated.
924+
925+
Returns:
926+
set[CourseKey]: Course keys the user may access through legacy roles.
927+
928+
Raises:
929+
AccessListFallback: If a legacy role record has neither a course key nor
930+
an organization
910931
"""
911932
user = request.user
912-
instructor_courses = UserBasedRole(user, CourseInstructorRole.ROLE).courses_with_role()
913-
914-
with strict_role_checking():
915-
staff_courses = UserBasedRole(user, CourseStaffRole.ROLE).courses_with_role()
933+
# Query CourseAccessRole directly instead of UserBasedRole.courses_with_role(),
934+
# which merges legacy DB records with AuthZ assignments. AuthZ access is resolved
935+
# separately in _get_authz_accessible_courses_list(). Exact role names (not
936+
# RoleCache inheritance) exclude limited_staff, matching strict_role_checking().
937+
legacy_accesses = CourseAccessRole.objects.filter(
938+
user=user,
939+
role__in=[CourseInstructorRole.ROLE, CourseStaffRole.ROLE],
940+
)
916941

917942
group_keys = set()
918943
org_accesses = set()
919-
legacy_accesses = instructor_courses | staff_courses
920944

921945
for access in legacy_accesses:
922946
if access.course_id is not None:

common/djangoapps/student/roles.py

Lines changed: 109 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from opaque_keys.edx.keys import CourseKey
1616
from opaque_keys.edx.locator import CourseLocator
1717
from openedx_authz.api import users as authz_api
18-
from openedx_authz.api.data import CourseOverviewData, RoleAssignmentData
18+
from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData, RoleAssignmentData
1919
from openedx_authz.constants import roles as authz_roles
2020

2121
from common.djangoapps.student.models import CourseAccessRole
@@ -74,17 +74,6 @@ def authz_add_role(user: User, authz_role: str, course_key: str):
7474
legacy_role = get_legacy_role_from_authz_role(authz_role)
7575
emit_course_access_role_added(user, course_locator, course_locator.org, legacy_role)
7676

77-
def authz_get_all_course_assignments_for_user(user: User) -> list[RoleAssignmentData]:
78-
"""
79-
Get all course assignments for a user.
80-
"""
81-
assignments = authz_api.get_user_role_assignments(user_external_key=user.username)
82-
# filter courses only
83-
filtered_assignments = [
84-
assignment for assignment in assignments
85-
if isinstance(assignment.scope, CourseOverviewData)
86-
]
87-
return filtered_assignments
8877

8978
def get_org_from_key(key: str) -> str:
9079
"""
@@ -93,6 +82,7 @@ def get_org_from_key(key: str) -> str:
9382
parsed_key = CourseKey.from_string(key)
9483
return parsed_key.org
9584

85+
9686
def register_access_role(cls):
9787
"""
9888
Decorator that allows access roles to be registered within the roles module and referenced by their
@@ -141,34 +131,120 @@ class AuthzCompatCourseAccessRole:
141131
"""
142132
Generic data class for storing CourseAccessRole-compatible data
143133
to be used inside BulkRoleCache and RoleCache.
134+
144135
This allows the cache to store both legacy and openedx-authz compatible roles
145136
"""
146137
user_id: int
147138
username: str
148139
org: str
149-
course_id: str # Course key
140+
course_id: str | None
150141
role: str
151142

152143

153-
def get_authz_compat_course_access_roles_for_user(user: User) -> set[AuthzCompatCourseAccessRole]:
144+
def _get_org_and_course_id_from_authz_scope(
145+
scope: CourseOverviewData | OrgCourseOverviewGlobData,
146+
) -> tuple[str, str | None] | None:
154147
"""
155-
Retrieve all CourseAccessRole objects for a given user and convert them to AuthzCompatCourseAccessRole objects.
148+
Extract the org and course key from an AuthZ course assignment scope.
149+
150+
Course-scoped assignments return ``(org, course_external_key)``.
151+
Org-wide assignments return ``(org, None)``.
152+
153+
Returns ``None`` when the org cannot be determined. For org-wide scopes,
154+
``OrgGlobData.org`` is typed as ``str | None`` because it is parsed from
155+
``external_key`` and returns ``None`` for malformed glob patterns.
156156
"""
157-
compat_role_assignments = set()
158-
assignments = authz_get_all_course_assignments_for_user(user)
159-
for assignment in assignments:
160-
for role in assignment.roles:
161-
legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key)
162-
course_key = assignment.scope.external_key
163-
org = get_org_from_key(course_key)
164-
compat_role = AuthzCompatCourseAccessRole(
157+
if isinstance(scope, CourseOverviewData):
158+
course_id = scope.external_key
159+
return get_org_from_key(course_id), course_id
160+
if isinstance(scope, OrgCourseOverviewGlobData):
161+
return scope.org, None
162+
return None
163+
164+
165+
def authz_get_all_course_assignments_for_user(user: User) -> list[RoleAssignmentData]:
166+
"""
167+
Return AuthZ role assignments for a user that apply to courses.
168+
169+
Includes assignments scoped to a specific course (``CourseOverviewData``) and
170+
assignments scoped to all courses in an organization (``OrgCourseOverviewGlobData``).
171+
Assignments for other resource types, such as content libraries, are excluded.
172+
173+
Args:
174+
user (User): The user whose AuthZ role assignments should be retrieved.
175+
176+
Returns:
177+
list[RoleAssignmentData]: Role assignments whose scope is course-level or org-wide
178+
"""
179+
return authz_api.get_user_role_assignments_per_scope_type(
180+
user_external_key=user.username,
181+
scope_types=(CourseOverviewData, OrgCourseOverviewGlobData),
182+
)
183+
184+
185+
def _compat_roles_from_authz_assignment(
186+
user: User,
187+
assignment: RoleAssignmentData,
188+
) -> set[AuthzCompatCourseAccessRole]:
189+
"""
190+
Convert an AuthZ role assignment into legacy-compatible course access roles.
191+
192+
Course-scoped assignments produce roles tied to a specific course key.
193+
Org-wide assignments produce org-level roles with no course key (``course_id``
194+
is ``None``), matching legacy ``OrgStaffRole`` / ``OrgInstructorRole`` behavior.
195+
AuthZ roles without a legacy mapping are skipped.
196+
197+
Args:
198+
user (User): The user associated with the assignment.
199+
assignment (RoleAssignmentData): A single AuthZ role assignment, including
200+
its scope and assigned roles.
201+
202+
Returns:
203+
set[AuthzCompatCourseAccessRole]: Legacy-compatible role records for the
204+
assignment. Returns an empty set if the org cannot be determined from
205+
the scope or no roles could be mapped.
206+
"""
207+
org_and_course_id = _get_org_and_course_id_from_authz_scope(assignment.scope)
208+
if org_and_course_id is None:
209+
return set()
210+
org, course_id = org_and_course_id
211+
212+
compat_roles = set()
213+
for role in assignment.roles:
214+
legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key)
215+
if legacy_role is None:
216+
continue
217+
compat_roles.add(
218+
AuthzCompatCourseAccessRole(
165219
user_id=user.id,
166220
username=user.username,
167221
org=org,
168-
course_id=course_key,
169-
role=legacy_role
222+
course_id=course_id,
223+
role=legacy_role,
170224
)
171-
compat_role_assignments.add(compat_role)
225+
)
226+
return compat_roles
227+
228+
229+
def get_authz_compat_course_access_roles_for_user(user: User) -> set[AuthzCompatCourseAccessRole]:
230+
"""
231+
Retrieve AuthZ course and org role assignments for a user in legacy format.
232+
233+
Fetches all course-level and org-wide AuthZ assignments for the user and
234+
converts each one into ``AuthzCompatCourseAccessRole`` records suitable for
235+
``RoleCache`` and other legacy permission checks.
236+
237+
Args:
238+
user (User): The user whose AuthZ role assignments should be converted.
239+
240+
Returns:
241+
set[AuthzCompatCourseAccessRole]: Legacy-compatible role records derived
242+
from the user's AuthZ assignments. Returns an empty set if the user
243+
has no applicable assignments.
244+
"""
245+
compat_role_assignments = set()
246+
for assignment in authz_get_all_course_assignments_for_user(user):
247+
compat_role_assignments.update(_compat_roles_from_authz_assignment(user, assignment))
172248
return compat_role_assignments
173249

174250

@@ -843,11 +919,9 @@ def courses_with_role(self) -> set[AuthzCompatCourseAccessRole]:
843919
"""
844920
Return a set of AuthzCompatCourseAccessRole for all of the courses with this user x (or derived from x) role.
845921
"""
922+
# Get all assignments for a user to a role
846923
roles = RoleCache.get_roles(self.role)
847924
legacy_assignments = CourseAccessRole.objects.filter(role__in=roles, user=self.user)
848-
849-
# Get all assignments for a user to a role
850-
new_authz_roles = [get_authz_role_from_legacy_role(role) for role in roles]
851925
all_authz_user_assignments = authz_get_all_course_assignments_for_user(self.user)
852926

853927
all_assignments = set()
@@ -863,19 +937,9 @@ def courses_with_role(self) -> set[AuthzCompatCourseAccessRole]:
863937
))
864938

865939
for assignment in all_authz_user_assignments:
866-
for role in assignment.roles:
867-
if role.external_key not in new_authz_roles:
868-
continue
869-
legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key)
870-
course_key = assignment.scope.external_key
871-
org = get_org_from_key(course_key)
872-
all_assignments.add(AuthzCompatCourseAccessRole(
873-
user_id=self.user.id,
874-
username=self.user.username,
875-
org=org,
876-
course_id=course_key,
877-
role=legacy_role
878-
))
940+
for compat_role in _compat_roles_from_authz_assignment(self.user, assignment):
941+
if compat_role.role in roles:
942+
all_assignments.add(compat_role)
879943

880944
return all_assignments
881945

@@ -899,18 +963,12 @@ def has_courses_with_role(self, org: str | None = None) -> bool:
899963
return True
900964

901965
# Then check for authz assignments
902-
new_authz_roles = [get_authz_role_from_legacy_role(role) for role in roles]
903966
all_authz_user_assignments = authz_get_all_course_assignments_for_user(self.user)
904967

905968
for assignment in all_authz_user_assignments:
906-
for role in assignment.roles:
907-
if role.external_key not in new_authz_roles:
969+
for compat_role in _compat_roles_from_authz_assignment(self.user, assignment):
970+
if compat_role.role not in roles:
908971
continue
909-
if org is None:
910-
# There is at least one assignment, short circuit
911-
return True
912-
course_key = assignment.scope.external_key
913-
parsed_org = get_org_from_key(course_key)
914-
if org == parsed_org:
972+
if org is None or org == compat_role.org:
915973
return True
916974
return False

0 commit comments

Comments
 (0)