From 308a6e6f47c4e222a9f74a5c87d5f2a7aec6aad0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Mon, 9 Mar 2026 13:30:12 -0600 Subject: [PATCH 1/6] fix: authz compat layer was failing on libraries v2 keys --- common/djangoapps/student/roles.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 40e4ebe970eb..979d658bd808 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -13,8 +13,9 @@ from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from common.djangoapps.student.signals.signals import emit_course_access_role_added, emit_course_access_role_removed from opaque_keys.edx.django.models import CourseKeyField +from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey -from opaque_keys.edx.locator import CourseLocator +from opaque_keys.edx.locator import CourseLocator, LibraryLocatorV2 from openedx_authz.api import users as authz_api from openedx_authz.constants import roles as authz_roles @@ -73,6 +74,15 @@ 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 get_org_from_key(key: str) -> str: + """ + Get the org from a course or library key. + """ + try: + parsed_key = CourseKey.from_string(key) + except InvalidKeyError: + parsed_key = LibraryLocatorV2.from_string(key) + return parsed_key.org def register_access_role(cls): """ @@ -141,8 +151,7 @@ def get_authz_compat_course_access_roles_for_user(user: User) -> set[AuthzCompat for role in assignment.roles: legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key) course_key = assignment.scope.external_key - parsed_key = CourseKey.from_string(course_key) - org = parsed_key.org + org = get_org_from_key(course_key) compat_role = AuthzCompatCourseAccessRole( user_id=user.id, username=user.username, @@ -847,8 +856,7 @@ def courses_with_role(self) -> set[AuthzCompatCourseAccessRole]: continue legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key) course_key = assignment.scope.external_key - parsed_key = CourseKey.from_string(course_key) - org = parsed_key.org + org = get_org_from_key(course_key) all_assignments.add(AuthzCompatCourseAccessRole( user_id=self.user.id, username=self.user.username, @@ -892,7 +900,7 @@ def has_courses_with_role(self, org: str | None = None) -> bool: # There is at least one assignment, short circuit return True course_key = assignment.scope.external_key - parsed_key = CourseKey.from_string(course_key) - if org == parsed_key.org: + parsed_org = get_org_from_key(course_key) + if org == parsed_org: return True return False From 7de4b5b7ebf7c7dd86c35cd078b80dd2f7a3c543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Mon, 9 Mar 2026 15:37:08 -0600 Subject: [PATCH 2/6] squash!: Filter only courses --- common/djangoapps/student/roles.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 979d658bd808..abbbd17c2c65 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -18,6 +18,7 @@ from opaque_keys.edx.locator import CourseLocator, LibraryLocatorV2 from openedx_authz.api import users as authz_api from openedx_authz.constants import roles as authz_roles +from openedx_authz.models import RoleAssignmentData from common.djangoapps.student.models import CourseAccessRole from openedx.core.lib.cache_utils import get_cache @@ -74,6 +75,15 @@ 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 assignment.scope.NAMESPACE == 'course-v1'] + return filtered_assignments + def get_org_from_key(key: str) -> str: """ Get the org from a course or library key. @@ -146,7 +156,7 @@ def get_authz_compat_course_access_roles_for_user(user: User) -> set[AuthzCompat Retrieve all CourseAccessRole objects for a given user and convert them to AuthzCompatCourseAccessRole objects. """ compat_role_assignments = set() - assignments = authz_api.get_user_role_assignments(user_external_key=user.username) + 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) @@ -834,9 +844,7 @@ def courses_with_role(self) -> set[AuthzCompatCourseAccessRole]: # 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_api.get_user_role_assignments( - user_external_key=self.user.username - ) + all_authz_user_assignments = authz_get_all_course_assignments_for_user(self.user) all_assignments = set() @@ -888,9 +896,7 @@ def has_courses_with_role(self, org: str | None = None) -> bool: # Then check for authz assignments new_authz_roles = [get_authz_role_from_legacy_role(role) for role in roles] - all_authz_user_assignments = authz_api.get_user_role_assignments( - user_external_key=self.user.username - ) + 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: From 3327d8249fad4792a9883a65c26a4862bea62e5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Mon, 9 Mar 2026 15:40:35 -0600 Subject: [PATCH 3/6] squash!: Fix import --- common/djangoapps/student/roles.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index abbbd17c2c65..56d024bc17b6 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -17,8 +17,8 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import CourseLocator, LibraryLocatorV2 from openedx_authz.api import users as authz_api +from openedx_authz.api.data import RoleAssignmentData from openedx_authz.constants import roles as authz_roles -from openedx_authz.models import RoleAssignmentData from common.djangoapps.student.models import CourseAccessRole from openedx.core.lib.cache_utils import get_cache From 13e0b7bfaf545109aae03b9fa949848f3bf7285a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Wed, 11 Mar 2026 10:05:36 -0600 Subject: [PATCH 4/6] squash!: Check for instance type instead of namespace string, remove trying to parse libv2 keys as its no longer required --- common/djangoapps/student/roles.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 56d024bc17b6..53ecaa2cf034 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -13,11 +13,10 @@ from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from common.djangoapps.student.signals.signals import emit_course_access_role_added, emit_course_access_role_removed from opaque_keys.edx.django.models import CourseKeyField -from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey -from opaque_keys.edx.locator import CourseLocator, LibraryLocatorV2 +from opaque_keys.edx.locator import CourseLocator from openedx_authz.api import users as authz_api -from openedx_authz.api.data import RoleAssignmentData +from openedx_authz.api.data import RoleAssignmentData, CourseOverviewData from openedx_authz.constants import roles as authz_roles from common.djangoapps.student.models import CourseAccessRole @@ -81,17 +80,17 @@ def authz_get_all_course_assignments_for_user(user: User) -> list[RoleAssignment """ assignments = authz_api.get_user_role_assignments(user_external_key=user.username) # filter courses only - filtered_assignments = [assignment for assignment in assignments if assignment.scope.NAMESPACE == 'course-v1'] + filtered_assignments = [ + assignment for assignment in assignments + if isinstance(assignment.scope, CourseOverviewData) + ] return filtered_assignments def get_org_from_key(key: str) -> str: """ Get the org from a course or library key. """ - try: - parsed_key = CourseKey.from_string(key) - except InvalidKeyError: - parsed_key = LibraryLocatorV2.from_string(key) + parsed_key = CourseKey.from_string(key) return parsed_key.org def register_access_role(cls): From be9bc274823db485281be028cc5e08327aa73fa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Wed, 11 Mar 2026 12:46:59 -0600 Subject: [PATCH 5/6] squash!: Add test for get_authz_compat_course_access_roles_for_user --- common/djangoapps/student/roles.py | 2 +- common/djangoapps/student/tests/test_roles.py | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 53ecaa2cf034..3ae766dffc89 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -88,7 +88,7 @@ def authz_get_all_course_assignments_for_user(user: User) -> list[RoleAssignment def get_org_from_key(key: str) -> str: """ - Get the org from a course or library key. + Get the org from a course key. """ parsed_key = CourseKey.from_string(key) return parsed_key.org diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 48e4e36ed0ea..6a3a3783e764 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -4,6 +4,7 @@ import ddt +from unittest.mock import patch from django.contrib.auth.models import Permission from django.test import TestCase from edx_toggles.toggles.testutils import override_waffle_flag @@ -236,6 +237,26 @@ def test_get_orgs_for_user(self): role_second_org.add_users(self.student) assert len(role.get_orgs_for_user(self.student)) == 2 + def test_get_authz_compat_course_access_roles_for_user(self): + """ + Thest that get_authz_compat_course_access_roles_for_user doesn't crash when the user + has Libraries V2 or other non-course roles in their assignments. + """ + from openedx_authz.api.data import ContentLibraryData, RoleAssignmentData, RoleData, UserData + from common.djangoapps.student.roles import get_authz_compat_course_access_roles_for_user + + lib_assignment = RoleAssignmentData( + subject=UserData(external_key=self.student.username), + roles=[RoleData(external_key='test-role')], + scope=ContentLibraryData(external_key='lib:edX:test-lib'), + ) + with patch( + 'openedx_authz.api.users.get_subject_role_assignments', + return_value=[lib_assignment], + ): + result = get_authz_compat_course_access_roles_for_user(self.student) + assert result == set() + @ddt.ddt class RoleCacheTestCase(TestCase): # lint-amnesty, pylint: disable=missing-class-docstring From 561a5ea7224fe775933c6a557e36d0f503fdeb31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20M=C3=A9ndez?= Date: Wed, 11 Mar 2026 12:57:14 -0600 Subject: [PATCH 6/6] squash!: Take import out of test --- common/djangoapps/student/tests/test_roles.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 6a3a3783e764..9485a7ca84d1 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -12,6 +12,7 @@ from opaque_keys.edx.locator import LibraryLocator from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory +from openedx_authz.api.data import ContentLibraryData, RoleAssignmentData, RoleData, UserData from openedx_authz.engine.enforcer import AuthzEnforcer from common.djangoapps.student.admin import CourseAccessRoleHistoryAdmin @@ -33,6 +34,7 @@ OrgInstructorRole, OrgStaffRole, RoleCache, + get_authz_compat_course_access_roles_for_user, get_role_cache_key_for_course, ROLE_CACHE_UNGROUPED_ROLES__KEY ) @@ -242,9 +244,6 @@ def test_get_authz_compat_course_access_roles_for_user(self): Thest that get_authz_compat_course_access_roles_for_user doesn't crash when the user has Libraries V2 or other non-course roles in their assignments. """ - from openedx_authz.api.data import ContentLibraryData, RoleAssignmentData, RoleData, UserData - from common.djangoapps.student.roles import get_authz_compat_course_access_roles_for_user - lib_assignment = RoleAssignmentData( subject=UserData(external_key=self.student.username), roles=[RoleData(external_key='test-role')],