diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 40e4ebe970eb..3ae766dffc89 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -16,6 +16,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 RoleAssignmentData, CourseOverviewData from openedx_authz.constants import roles as authz_roles from common.djangoapps.student.models import CourseAccessRole @@ -73,6 +74,24 @@ 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: + """ + Get the org from a course key. + """ + parsed_key = CourseKey.from_string(key) + return parsed_key.org def register_access_role(cls): """ @@ -136,13 +155,12 @@ 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) 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, @@ -825,9 +843,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() @@ -847,8 +863,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, @@ -880,9 +895,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: @@ -892,7 +905,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 diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 48e4e36ed0ea..9485a7ca84d1 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 @@ -11,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 @@ -32,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 ) @@ -236,6 +239,23 @@ 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. + """ + 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