From 65b57d9fac1d6abdd16ea475de52ced8ad6dd8ba Mon Sep 17 00:00:00 2001 From: Kevyn Suarez Date: Wed, 12 Aug 2026 11:45:27 -0500 Subject: [PATCH 1/5] fix: platform-wide glob role assignments crash get_orgs_for_user/has_org_for_user RoleBase._authz_get_orgs_for_user() read assignment.scope.org for every AuthZ assignment, but PlatformGlobData (course-v1:*, lib:*) has no .org attribute at all, unlike CourseOverviewData/OrgGlobData where it's a real (possibly None) field. Any user with a platform-wide role crashed get_orgs_for_user()/has_org_for_user() with an AttributeError. Special-case platform-wide assignments: since they cover every org, return all registered org short names instead of deriving them from individual assignments. Fixes https://github.com/openedx/openedx-authz/issues/380 --- common/djangoapps/student/roles.py | 6 ++++ common/djangoapps/student/tests/test_roles.py | 29 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 81a28773950f..58e91bf96b68 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -17,6 +17,7 @@ from openedx_authz.api import users as authz_api from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData, RoleAssignmentData from openedx_authz.constants import roles as authz_roles +from organizations.api import get_organizations from common.djangoapps.student.models import CourseAccessRole from common.djangoapps.student.signals.signals import emit_course_access_role_added, emit_course_access_role_removed @@ -632,6 +633,11 @@ def _authz_get_orgs_for_user(self, user) -> list[str]: user_external_key=user.username, role_external_key=role, ) + # A platform-wide grant (course-v1:*, lib:*) covers every org, not just the ones + # with a concrete assignment. Platform-glob scopes have no .org attribute at all + # (unlike org-glob/course/library scopes, where it's a real field that can be None). + if any(assignment.scope.IS_PLATFORM_GLOB for assignment in assignments): + return [org["short_name"] for org in get_organizations()] orgs = {assignment.scope.org for assignment in assignments if assignment.scope.org is not None} return list(orgs) diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 1979bc6687fd..975339ea67dd 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -14,6 +14,7 @@ ContentLibraryData, CourseOverviewData, OrgCourseOverviewGlobData, + PlatformCourseOverviewGlobData, RoleAssignmentData, RoleData, ScopeData, @@ -21,6 +22,7 @@ ) from openedx_authz.constants.roles import COURSE_ADMIN, COURSE_STAFF from openedx_authz.engine.enforcer import AuthzEnforcer +from organizations.api import add_organization from common.djangoapps.student.admin import CourseAccessRoleHistoryAdmin from common.djangoapps.student.models import CourseAccessRoleHistory, User @@ -313,6 +315,33 @@ def test_get_orgs_for_user_authz(self): result = role.get_orgs_for_user(self.student) self.assertCountEqual(result, [self.course_key.org, other_org]) # noqa: PT009 + @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_get_orgs_for_user_authz_platform_glob(self): + """ + A platform-wide glob assignment (course-v1:*) has no `.org` attribute, unlike + course/org-glob scopes. get_orgs_for_user must special-case it and return every + registered org instead of crashing with an AttributeError. + """ + role = CourseStaffRole(self.course_key) + + for org in self.orgs: + add_organization({"name": org, "short_name": org, "description": ""}) + + staff_authz_role = RoleData(external_key=COURSE_STAFF) + assignments = [ + RoleAssignmentData( + subject=UserData(external_key=self.student.username), + roles=[staff_authz_role], + scope=PlatformCourseOverviewGlobData(external_key="course-v1:*"), + ), + ] + + with patch("openedx_authz.api.users.get_user_role_assignments_filtered", return_value=assignments): + result = role.get_orgs_for_user(self.student) + self.assertCountEqual(result, self.orgs) # noqa: PT009 + assert role.has_org_for_user(self.student) + assert role.has_org_for_user(self.student, org=self.orgs[0]) + def test_get_authz_compat_course_access_roles_for_user(self): """ Test that get_authz_compat_course_access_roles_for_user doesn't crash when the user From 526ea04052985778b71933d0ba70e1de09b553fb Mon Sep 17 00:00:00 2001 From: Kevyn Suarez Date: Wed, 12 Aug 2026 12:29:46 -0500 Subject: [PATCH 2/5] chore: retrigger CI (flaky JS video-volume-control test) From ace8a06a7dd693007166dae0bfe9db12ac80565c Mon Sep 17 00:00:00 2001 From: Kevyn Suarez Date: Fri, 14 Aug 2026 16:22:13 -0500 Subject: [PATCH 3/5] refactor: use build_external_key() instead of hardcoded glob key strings Per review feedback from @BryanttV on #38980, also applied to test_course_listing.py which had the same pattern in unrelated pre-existing tests. --- .../contentstore/tests/test_course_listing.py | 12 ++++++++---- common/djangoapps/student/tests/test_roles.py | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index 8d3bd47f79c4..fb4e66c8c5c2 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -936,7 +936,9 @@ def test_get_course_keys_from_scopes_with_platform_scope(self): "is_enabled", side_effect=self._mock_authz_toggle(enabled_keys), ): - course_keys = _get_course_keys_from_scopes([PlatformCourseOverviewGlobData(external_key="course-v1:*")]) + course_keys = _get_course_keys_from_scopes([ + PlatformCourseOverviewGlobData(external_key=PlatformCourseOverviewGlobData.build_external_key()) + ]) assert course_keys == set(authz_keys) | set(legacy_keys) @@ -953,7 +955,9 @@ def test_get_course_keys_from_scopes_with_platform_scope_global_flag_enabled(sel "is_enabled", side_effect=self._mock_authz_toggle(enabled_keys, global_enabled=True), ): - course_keys = _get_course_keys_from_scopes([PlatformCourseOverviewGlobData(external_key="course-v1:*")]) + course_keys = _get_course_keys_from_scopes([ + PlatformCourseOverviewGlobData(external_key=PlatformCourseOverviewGlobData.build_external_key()) + ]) assert course_keys == set(CourseOverview.get_all_courses().values_list("id", flat=True)) @@ -972,8 +976,8 @@ def test_get_course_keys_from_scopes_platform_scope_short_circuits(self): ): course_keys = _get_course_keys_from_scopes( [ - OrgCourseOverviewGlobData(external_key="course-v1:Org1+*"), - PlatformCourseOverviewGlobData(external_key="course-v1:*"), + OrgCourseOverviewGlobData(external_key=OrgCourseOverviewGlobData.build_external_key("Org1")), + PlatformCourseOverviewGlobData(external_key=PlatformCourseOverviewGlobData.build_external_key()), ] ) diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 975339ea67dd..07326fd85ebf 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -332,7 +332,7 @@ def test_get_orgs_for_user_authz_platform_glob(self): RoleAssignmentData( subject=UserData(external_key=self.student.username), roles=[staff_authz_role], - scope=PlatformCourseOverviewGlobData(external_key="course-v1:*"), + scope=PlatformCourseOverviewGlobData(external_key=PlatformCourseOverviewGlobData.build_external_key()), ), ] From 795de56f943b8ad0ddd33e0710dbcd6cbb922c3e Mon Sep 17 00:00:00 2001 From: Kevyn Suarez Date: Tue, 18 Aug 2026 18:31:07 -0500 Subject: [PATCH 4/5] refactor: use assign_role_to_user_in_scope instead of mocking assignments Per review feedback from @BryanttV on #38980: exercising the real Casbin policy assignment + get_orgs_for_user path is more faithful than mocking get_user_role_assignments_filtered directly, and it catches issues the mock papered over (the mocked RoleData had external_key=COURSE_STAFF, the RoleData object itself, instead of COURSE_STAFF.external_key). --- common/djangoapps/student/tests/test_roles.py | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 07326fd85ebf..224447edc32a 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -20,6 +20,7 @@ ScopeData, UserData, ) +from openedx_authz.api.users import assign_role_to_user_in_scope from openedx_authz.constants.roles import COURSE_ADMIN, COURSE_STAFF from openedx_authz.engine.enforcer import AuthzEnforcer from organizations.api import add_organization @@ -327,20 +328,17 @@ def test_get_orgs_for_user_authz_platform_glob(self): for org in self.orgs: add_organization({"name": org, "short_name": org, "description": ""}) - staff_authz_role = RoleData(external_key=COURSE_STAFF) - assignments = [ - RoleAssignmentData( - subject=UserData(external_key=self.student.username), - roles=[staff_authz_role], - scope=PlatformCourseOverviewGlobData(external_key=PlatformCourseOverviewGlobData.build_external_key()), - ), - ] + assign_role_to_user_in_scope( + self.student.username, + COURSE_STAFF.external_key, + PlatformCourseOverviewGlobData.build_external_key(), + ) + AuthzEnforcer.get_enforcer().load_policy() - with patch("openedx_authz.api.users.get_user_role_assignments_filtered", return_value=assignments): - result = role.get_orgs_for_user(self.student) - self.assertCountEqual(result, self.orgs) # noqa: PT009 - assert role.has_org_for_user(self.student) - assert role.has_org_for_user(self.student, org=self.orgs[0]) + result = role.get_orgs_for_user(self.student) + self.assertCountEqual(result, self.orgs) # noqa: PT009 + assert role.has_org_for_user(self.student) + assert role.has_org_for_user(self.student, org=self.orgs[0]) def test_get_authz_compat_course_access_roles_for_user(self): """ From fbb2726e7593d7f27cbbe57552ba4ddaa1a06007 Mon Sep 17 00:00:00 2001 From: Kevyn Suarez Date: Wed, 19 Aug 2026 15:28:06 -0500 Subject: [PATCH 5/5] test: add subset-vs-all-orgs coverage for _authz_get_orgs_for_user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review feedback from @mariajgrimaldi on #38980: registers a third org that's never assigned, then asserts an org-scoped grant returns only its own org while a platform-wide grant returns all three, against the same pool of registered orgs — so the two branches (roles.py L640 vs L641, both list[str]) are actually distinguished by the test instead of coincidentally matching. --- common/djangoapps/student/tests/test_roles.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 224447edc32a..8993bfaf9e63 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -340,6 +340,37 @@ def test_get_orgs_for_user_authz_platform_glob(self): assert role.has_org_for_user(self.student) assert role.has_org_for_user(self.student, org=self.orgs[0]) + @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_get_orgs_for_user_authz_platform_glob_vs_org_scoped(self): + """ + Side-by-side check that the platform-glob branch (return every registered org) + and the regular branch (return only the orgs with a concrete assignment) produce + the same list[str] shape, over the same pool of registered orgs: an org-scoped + grant returns a subset, a platform-wide grant returns all of them. + """ + role = CourseStaffRole(self.course_key) + third_org = "Universal" + all_orgs = [*self.orgs, third_org] + + for org in all_orgs: + add_organization({"name": org, "short_name": org, "description": ""}) + + subset_user = UserFactory() + assign_role_to_user_in_scope( + subset_user.username, + COURSE_STAFF.external_key, + OrgCourseOverviewGlobData.build_external_key(self.orgs[0]), + ) + assign_role_to_user_in_scope( + self.student.username, + COURSE_STAFF.external_key, + PlatformCourseOverviewGlobData.build_external_key(), + ) + AuthzEnforcer.get_enforcer().load_policy() + + self.assertCountEqual(role.get_orgs_for_user(subset_user), [self.orgs[0]]) # noqa: PT009 + self.assertCountEqual(role.get_orgs_for_user(self.student), all_orgs) # noqa: PT009 + def test_get_authz_compat_course_access_roles_for_user(self): """ Test that get_authz_compat_course_access_roles_for_user doesn't crash when the user