Skip to content

fix: authz compat layer was failing on libraries v2 keys #38131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wgu-taylor-payne merged 6 commits into from
Mar 11, 2026
+46 −13
Merged

fix: authz compat layer was failing on libraries v2 keys #38131

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
308a6e6
fix: authz compat layer was failing on libraries v2 keys
rodmgwgu Mar 9, 2026
7de4b5b
squash!: Filter only courses
rodmgwgu Mar 9, 2026
3327d82
squash!: Fix import
rodmgwgu Mar 9, 2026
13e0b7b
squash!: Check for instance type instead of namespace string, remove …
rodmgwgu Mar 11, 2026
be9bc27
squash!: Add test for get_authz_compat_course_access_roles_for_user
rodmgwgu Mar 11, 2026
561a5ea
squash!: Take import out of test
rodmgwgu Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 26 additions & 13 deletions common/djangoapps/student/roles.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Copy link
Contributor

@wgu-taylor-payne wgu-taylor-payne Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we filter by the course-v1 namespace before calling this function in each case, is it worth keeping?

Copy link
Contributor Author

@rodmgwgu rodmgwgu Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the Libraries V2 key parsing as it's not needed, but I've kept the function as I think it improves legibility on the implementation, what do you think?

"""
Get the org from a course or library key.
"""
parsed_key = CourseKey.from_string(key)
return parsed_key.org

def register_access_role(cls):
"""
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()

Expand All @@ -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,
Expand Down Expand Up @@ -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:
Expand All @@ -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
Loading