From fd19bacf67b76957ad5b08a176659185b65611ea Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 19 Jun 2026 11:53:56 -0500 Subject: [PATCH 1/3] feat: add platform glob scope support (#38660) * feat: add platform-wide authz scope support * feat: enhance course listing authorization with global toggle support * chore: upgrade openedx-authz to 1.19.0 * docs: add docstring for mock authorization toggle in course listing tests --- .../contentstore/tests/test_course_listing.py | 155 +++++++++++++++++- cms/djangoapps/contentstore/views/course.py | 54 +++++- common/djangoapps/student/auth.py | 18 +- .../content_libraries/api/libraries.py | 9 +- .../content_libraries/permissions.py | 27 ++- .../tests/test_content_libraries.py | 80 +++++++-- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 10 files changed, 307 insertions(+), 44 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index 31c9be4c2741..eb0bed25524d 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -10,7 +10,7 @@ from ccx_keys.locator import CCXLocator from django.test import RequestFactory from opaque_keys.edx.locations import CourseLocator -from openedx_authz.api.data import OrgCourseOverviewGlobData +from openedx_authz.api.data import OrgCourseOverviewGlobData, PlatformCourseOverviewGlobData from openedx_authz.api.users import assign_role_to_user_in_scope from openedx_authz.constants.roles import COURSE_DATA_RESEARCHER, COURSE_EDITOR, COURSE_STAFF @@ -21,6 +21,7 @@ _accessible_courses_iter_for_tests, _accessible_courses_list_from_groups, _accessible_courses_summary_iter, + _get_course_keys_from_scopes, get_courses_accessible_to_user, ) from common.djangoapps.course_action_state.models import CourseRerunState @@ -434,8 +435,11 @@ def _create_course(self, course_key): return CourseOverviewFactory.create(id=course.id, org=course_key.org) - def _mock_authz_toggle(self, enabled_keys): + def _mock_authz_toggle(self, enabled_keys, global_enabled=False): + """Return a mock is_enabled side effect for AUTHZ_COURSE_AUTHORING_FLAG.""" def _is_enabled(course_key=None, **_): + if course_key is None: + return global_enabled return str(course_key) in enabled_keys return _is_enabled @@ -832,3 +836,150 @@ def test_course_listing_with_org_scope_fetched_once(self): courses, _ = get_courses_accessible_to_user(request) mock_get_all_courses.assert_called_once_with(orgs={"Org1", "Org2"}) + + def test_course_listing_with_platform_scope(self): + """ + Verify that a platform-wide scope (`course-v1:*`) grants access to all + courses across orgs when the AuthZ course authoring toggle is enabled. + """ + _, _, authz_courses, legacy_courses = self._create_courses() + org2_course_key = CourseLocator("Org2", "Course1", "AuthzRun") + org2_course = self._create_course(org2_course_key) + assign_role_to_user_in_scope( + self.authorized_user.username, + COURSE_STAFF.external_key, + PlatformCourseOverviewGlobData.build_external_key(), + ) + + request = self._make_request(self.authorized_user) + + with patch.object( + core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, + "is_enabled", + return_value=True, + ): + courses, _ = get_courses_accessible_to_user(request) + + result_ids = {c.id for c in courses} + expected_ids = { + *(c.id for c in authz_courses), + *(c.id for c in legacy_courses), + org2_course.id + } + + assert result_ids == expected_ids + + def test_course_listing_with_platform_scope_with_toggle(self): + """ + If the global authz toggle is disabled and only a subset of courses have + the per-course toggle enabled, only those course keys should appear when + resolving a platform-wide scope. + """ + authz_keys, _, _, _ = self._create_courses() + org2_course_key = CourseLocator("Org2", "Course1", "AuthzRun") + self._create_course(org2_course_key) + enabled_keys = {str(authz_keys[0]), str(authz_keys[2])} + assign_role_to_user_in_scope( + self.authorized_user.username, + COURSE_STAFF.external_key, + PlatformCourseOverviewGlobData.build_external_key(), + ) + + request = self._make_request(self.authorized_user) + + with patch.object( + core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, + "is_enabled", + side_effect=self._mock_authz_toggle(enabled_keys), + ): + courses, _ = get_courses_accessible_to_user(request) + + result_ids = {c.id for c in courses} + expected = {authz_keys[0], authz_keys[2]} + + assert result_ids == expected + + def test_course_listing_with_platform_scope_global_flag_enabled(self): + """ + When the global AuthZ toggle is enabled, platform scope should return all + courses without validating the per-course toggle. + """ + authz_keys, legacy_keys, authz_courses, legacy_courses = self._create_courses() + org2_course_key = CourseLocator("Org2", "Course1", "AuthzRun") + org2_course = self._create_course(org2_course_key) + enabled_keys = {str(authz_keys[0])} + assign_role_to_user_in_scope( + self.authorized_user.username, + COURSE_STAFF.external_key, + PlatformCourseOverviewGlobData.build_external_key(), + ) + + request = self._make_request(self.authorized_user) + + with patch.object( + core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, + "is_enabled", + side_effect=self._mock_authz_toggle(enabled_keys, global_enabled=True), + ): + courses, _ = get_courses_accessible_to_user(request) + + result_ids = {c.id for c in courses} + expected_ids = {*(c.id for c in authz_courses), *(c.id for c in legacy_courses), org2_course.id} + + assert result_ids == expected_ids + + def test_get_course_keys_from_scopes_with_platform_scope(self): + """ + Platform-wide scopes should resolve to all courses with AuthZ enabled + when the global toggle is disabled. + """ + authz_keys, legacy_keys, _, _ = self._create_courses() + enabled_keys = {str(key) for key in authz_keys + legacy_keys} + + with patch.object( + core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, + "is_enabled", + side_effect=self._mock_authz_toggle(enabled_keys), + ): + course_keys = _get_course_keys_from_scopes([PlatformCourseOverviewGlobData(external_key="course-v1:*")]) + + assert course_keys == set(authz_keys) | set(legacy_keys) + + def test_get_course_keys_from_scopes_with_platform_scope_global_flag_enabled(self): + """ + Platform-wide scopes should return all courses when the global AuthZ toggle + is enabled, regardless of per-course toggle state. + """ + authz_keys, legacy_keys, _, _ = self._create_courses() + enabled_keys = {str(authz_keys[0])} + + with patch.object( + core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, + "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:*")]) + + assert course_keys == set(CourseOverview.get_all_courses().values_list("id", flat=True)) + + def test_get_course_keys_from_scopes_platform_scope_short_circuits(self): + """ + When a platform-wide scope is present, org and course scopes should be + ignored and only the platform scope resolution should apply. + """ + authz_keys, _, _, _ = self._create_courses() + enabled_keys = {str(authz_keys[0])} + + with patch.object( + core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, + "is_enabled", + side_effect=self._mock_authz_toggle(enabled_keys), + ): + course_keys = _get_course_keys_from_scopes( + [ + OrgCourseOverviewGlobData(external_key="course-v1:Org1+*"), + PlatformCourseOverviewGlobData(external_key="course-v1:*"), + ] + ) + + assert course_keys == {authz_keys[0]} diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 3d248944bdd5..910e78465fbf 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -29,7 +29,12 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import BlockUsageLocator from openedx_authz.api import get_scopes_for_user_and_permission -from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData, ScopeData +from openedx_authz.api.data import ( + CourseOverviewData, + OrgCourseOverviewGlobData, + PlatformCourseOverviewGlobData, + ScopeData, +) from openedx_authz.constants.permissions import ( COURSES_MANAGE_COURSE_UPDATES, COURSES_MANAGE_GROUP_CONFIGURATIONS, @@ -832,25 +837,68 @@ def _get_course_keys_for_org_scope(org_keys: set[str]): return CourseOverview.get_all_courses(orgs=org_keys).values_list('id', flat=True) -def _get_course_keys_from_scopes(authz_scopes: list[ScopeData]): + +def _get_course_keys_from_platform_scope() -> set[CourseKey]: + """ + Resolve course keys for a platform-wide Authz scope. + + When the AuthZ course authoring feature flag is globally enabled, all courses + are returned without per-course validation. Otherwise, only courses with the + per-course toggle enabled are included. + + Returns: + set[CourseKey]: Course keys accessible on the platform. + """ + course_keys = CourseOverview.get_all_courses().values_list("id", flat=True) + + if core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.is_enabled(): + return set(course_keys) + + return {course_key for course_key in course_keys if core_toggles.enable_authz_course_authoring(course_key)} + + +def _get_course_keys_from_scopes(authz_scopes: list[ScopeData]) -> set[CourseKey]: """ - Convert a set of Authz scopes into specific course keys. + Convert authorization scopes into a set of accessible course keys. + + This function processes authorization scopes with the following precedence: + 1. Platform-wide access (PlatformCourseOverviewGlobData): Returns all courses + when the AuthZ course authoring toggle is globally enabled; otherwise only + courses with the per-course toggle enabled + 2. Course-specific access (CourseOverviewData): Returns individual course keys + 3. Organization-wide access (OrgCourseOverviewGlobData): Returns all courses in specified orgs + + For non-platform scopes, only courses with the authz course authoring toggle + enabled are included. + + Args: + authz_scopes: List of authorization scope data objects from the authz system. + + Returns: + set[CourseKey]: Set of course keys the user has access to based on their scopes. """ + if any(isinstance(access, PlatformCourseOverviewGlobData) for access in authz_scopes): + return _get_course_keys_from_platform_scope() + course_keys = set() org_keys = set() + for access in authz_scopes: if isinstance(access, CourseOverviewData) and access.course_key: if core_toggles.enable_authz_course_authoring(access.course_key): course_keys.add(access.course_key) elif isinstance(access, OrgCourseOverviewGlobData) and access.org: org_keys.add(access.org) + if org_keys: course_keys.update( key for key in _get_course_keys_for_org_scope(org_keys) if core_toggles.enable_authz_course_authoring(key) ) + return course_keys + def _get_authz_accessible_courses_list(request): """ List all courses available to the logged in user by diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index 4071b2119e9a..0c24bfe6f3b5 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -253,16 +253,20 @@ def is_content_creator(user, org): def _has_content_creator_access(user, org): """ Check if the user has content creator access based on AuthZ permissions. + + Returns: + bool: True if the user has platform-wide or org-scoped course creation permission. """ - if settings.FEATURES.get('DISABLE_COURSE_CREATION', False): + if settings.FEATURES.get("DISABLE_COURSE_CREATION", False): return False - # Using Org scope. e.g. "course-v1:{org}+*" - org_scope_key = authz_api.OrgCourseOverviewGlobData.build_external_key(org) - return authz_api.is_user_allowed( - user.username, - COURSES_CREATE_COURSE.identifier, - org_scope_key + scope_keys = ( + authz_api.PlatformCourseOverviewGlobData.build_external_key(), + authz_api.OrgCourseOverviewGlobData.build_external_key(org), + ) + return any( + authz_api.is_user_allowed(user.username, COURSES_CREATE_COURSE.identifier, scope_key) + for scope_key in scope_keys ) diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index 1d326d448cc8..cd99c14e923f 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -249,14 +249,13 @@ def user_can_create_library(user: AbstractUser) -> bool: """ library_permission = permissions.CAN_CREATE_CONTENT_LIBRARY lib_permission_in_authz = _transform_legacy_lib_permission_to_authz_permission(library_permission) - # The authz_api.is_user_allowed check only validates permissions within a specific library context. Since - # creating a library is not tied to an existing one, we use user.has_perm (via Bridgekeeper) to check if the user - # can create libraries, meaning they have the course creator role. In the future, this should rely on a global (*) - # role defined in the Authorization Framework for instance-level resource creation. + # The authz_api.is_user_allowed check validates platform-wide library creation via + # PlatformContentLibraryGlobData. We also use user.has_perm (via Bridgekeeper) as a fallback + # for users with the course creator role until library creation is fully modeled in authz. has_perms = user.has_perm(library_permission) or authz_api.is_user_allowed( user, lib_permission_in_authz, - authz_api.data.GLOBAL_SCOPE_WILDCARD, + authz_api.PlatformContentLibraryGlobData.build_external_key(), ) return has_perms diff --git a/openedx/core/djangoapps/content_libraries/permissions.py b/openedx/core/djangoapps/content_libraries/permissions.py index 1d0c7f9c0544..d4503aa121d1 100644 --- a/openedx/core/djangoapps/content_libraries/permissions.py +++ b/openedx/core/djangoapps/content_libraries/permissions.py @@ -5,7 +5,7 @@ are deprecated in favor of openedx-authz. See https://github.com/openedx/openedx-platform/issues/37409. """ from bridgekeeper import perms, rules -from bridgekeeper.rules import Attribute, ManyRelation, Relation, Rule, blanket_rule, in_current_groups +from bridgekeeper.rules import UNIVERSAL, Attribute, ManyRelation, Relation, Rule, blanket_rule, in_current_groups from django.conf import settings from django.db.models import Q from openedx_authz import api as authz_api @@ -159,6 +159,8 @@ def query(self, user): >>> rule = HasPermissionInContentLibraryScope('view', filter_keys=['org', 'slug']) >>> q = rule.query(user) >>> # Results in: Q(org__short_name='OrgA', slug='lib-a') | Q(org__short_name='OrgB', slug='lib-b') + >>> # Platform-wide scope 'lib:*' returns UNIVERSAL (all libraries) + >>> # Org scope 'lib:OrgA:*' returns Q(org__short_name__in=['OrgA']) >>> >>> # Apply to queryset >>> libraries = ContentLibrary.objects.filter(q) @@ -171,15 +173,24 @@ def query(self, user): self.permission.identifier ) - library_keys = [scope.library_key for scope in scopes] + if any(isinstance(access, authz_api.PlatformContentLibraryGlobData) for access in scopes): + return UNIVERSAL - if not library_keys: - return Q(pk__in=[]) # No access, return Q that matches nothing - - # Build Q object: OR together (org AND slug) conditions for each library + org_keys = set() query = Q() - for library_key in library_keys: - query |= Q(org__short_name=library_key.org, slug=library_key.slug) + + for access in scopes: + if isinstance(access, authz_api.ContentLibraryData): + library_key = access.library_key + query |= Q(org__short_name=library_key.org, slug=library_key.slug) + elif isinstance(access, authz_api.OrgContentLibraryGlobData) and access.org: + org_keys.add(access.org) + + if org_keys: + query |= Q(org__short_name__in=org_keys) + + if not query: + return Q(pk__in=[]) # No access, return Q that matches nothing return query diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 6d947aa786d4..a2599331889c 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -1910,9 +1910,10 @@ def test_authz_scope_filters_by_authorized_libraries(self): 'openedx_authz.api.get_scopes_for_user_and_permission' ) as mock_get_scopes: # Mock: User authorized for lib1 (org1:lib1) and lib2 (org2:lib2) only, NOT lib3 - mock_scope1 = type('Scope', (), {'library_key': LibraryLocatorV2.from_string(lib1['id'])})() - mock_scope2 = type('Scope', (), {'library_key': LibraryLocatorV2.from_string(lib2['id'])})() - mock_get_scopes.return_value = [mock_scope1, mock_scope2] + mock_get_scopes.return_value = [ + authz_api.ContentLibraryData(external_key=str(LibraryLocatorV2.from_string(lib1['id']))), + authz_api.ContentLibraryData(external_key=str(LibraryLocatorV2.from_string(lib2['id']))), + ] all_libs = ContentLibrary.objects.filter(slug__in=['lib1', 'lib2', 'lib3']) filtered = perms[CAN_VIEW_THIS_CONTENT_LIBRARY].filter(user, all_libs).distinct() @@ -2036,6 +2037,59 @@ def test_authz_scope_handles_empty_scopes(self): "Library should exist in database", ) + def test_authz_scope_platform_glob_returns_all_libraries(self): + """ + Test that PlatformContentLibraryGlobData grants access to all libraries. + """ + user = UserFactory.create(username="platform_glob_user", is_staff=False) + + Organization.objects.get_or_create(short_name="glob-org1", defaults={"name": "Glob Org 1"}) + Organization.objects.get_or_create(short_name="glob-org2", defaults={"name": "Glob Org 2"}) + + with self.as_user(self.admin_user): + self._create_library(slug="glob-lib1", org="glob-org1", title="Glob Library 1") + self._create_library(slug="glob-lib2", org="glob-org2", title="Glob Library 2") + + ContentLibraryPermission.objects.filter(user=user).delete() + + with patch( + 'openedx_authz.api.get_scopes_for_user_and_permission', + return_value=[authz_api.PlatformContentLibraryGlobData(external_key="lib:*")], + ): + all_libs = ContentLibrary.objects.filter(slug__in=["glob-lib1", "glob-lib2"]) + filtered = perms[CAN_VIEW_THIS_CONTENT_LIBRARY].filter(user, all_libs).distinct() + + assert filtered.count() == 2 + + def test_authz_scope_org_glob_filters_by_org(self): + """ + Test that OrgContentLibraryGlobData grants access to all libraries in the org. + """ + user = UserFactory.create(username="org_glob_user", is_staff=False) + + Organization.objects.get_or_create(short_name="org-glob1", defaults={"name": "Org Glob 1"}) + Organization.objects.get_or_create(short_name="org-glob2", defaults={"name": "Org Glob 2"}) + + with self.as_user(self.admin_user): + self._create_library(slug="org-glob-lib1", org="org-glob1", title="Org Glob Library 1") + self._create_library(slug="org-glob-lib2", org="org-glob1", title="Org Glob Library 2") + self._create_library(slug="org-glob-lib3", org="org-glob2", title="Org Glob Library 3") + + ContentLibraryPermission.objects.filter(user=user).delete() + + with patch( + 'openedx_authz.api.get_scopes_for_user_and_permission', + return_value=[authz_api.OrgContentLibraryGlobData(external_key="lib:org-glob1:*")], + ): + all_libs = ContentLibrary.objects.filter( + slug__in=["org-glob-lib1", "org-glob-lib2", "org-glob-lib3"] + ) + filtered = perms[CAN_VIEW_THIS_CONTENT_LIBRARY].filter(user, all_libs).distinct() + + assert filtered.count() == 2 + slugs = set(filtered.values_list("slug", flat=True)) + assert slugs == {"org-glob-lib1", "org-glob-lib2"} + def test_authz_scope_q_object_has_correct_structure(self): """ Test that HasPermissionInContentLibraryScope.query() generates Q object @@ -2053,14 +2107,10 @@ def test_authz_scope_q_object_has_correct_structure(self): with patch( "openedx_authz.api.get_scopes_for_user_and_permission" ) as mock_get_scopes: - # Create scopes with specific org/slug values we can verify - mock_scope1 = type("Scope", (), { - "library_key": type("Key", (), {"org": "specific-org1", "slug": "specific-slug1"})() - })() - mock_scope2 = type("Scope", (), { - "library_key": type("Key", (), {"org": "specific-org2", "slug": "specific-slug2"})() - })() - mock_get_scopes.return_value = [mock_scope1, mock_scope2] + mock_get_scopes.return_value = [ + authz_api.ContentLibraryData(external_key="lib:specific-org1:specific-slug1"), + authz_api.ContentLibraryData(external_key="lib:specific-org2:specific-slug2"), + ] q_obj = rule.query(user) @@ -2155,8 +2205,8 @@ def test_authz_scope_q_object_matches_exact_org_slug_pairs(self): lib2_key = LibraryLocatorV2.from_string(lib2['id']) mock_get_scopes.return_value = [ - type('Scope', (), {'library_key': lib1_key})(), - type('Scope', (), {'library_key': lib2_key})(), + authz_api.ContentLibraryData(external_key=str(lib1_key)), + authz_api.ContentLibraryData(external_key=str(lib2_key)), ] q_obj = rule.query(user) @@ -2270,8 +2320,8 @@ def test_authz_scope_with_combined_authz_and_legacy_permissions(self): lib3_key = LibraryLocatorV2.from_string(lib3['id']) mock_get_scopes.return_value = [ - type('Scope', (), {'library_key': lib1_key})(), - type('Scope', (), {'library_key': lib3_key})(), + authz_api.ContentLibraryData(external_key=str(lib1_key)), + authz_api.ContentLibraryData(external_key=str(lib3_key)), ] all_libs = ContentLibrary.objects.filter(slug__in=['comb-lib1', 'comb-lib2', 'comb-lib3', 'comb-lib4']) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 5c168692d864..979f2f2b00e9 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -834,7 +834,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==1.15.0 +openedx-authz==1.19.0 # via -r requirements/edx/kernel.in openedx-calc==5.0.0 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 21861b40692b..c2285e88323e 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1374,7 +1374,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==1.15.0 +openedx-authz==1.19.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 18113cd479f5..208bfca6e57e 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -1012,7 +1012,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==1.15.0 +openedx-authz==1.19.0 # via -r requirements/edx/base.txt openedx-calc==5.0.0 # via diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index e0358192c8f3..1a6ba8ddd9d4 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1052,7 +1052,7 @@ openedx-atlas==0.7.0 # enterprise-integrated-channels # openedx-authz # openedx-forum -openedx-authz==1.15.0 +openedx-authz==1.19.0 # via -r requirements/edx/base.txt openedx-calc==5.0.0 # via From d06d2d3f0f1ae22cb80ba158f9c9c1c72500e7b0 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 19 Jun 2026 13:44:11 -0500 Subject: [PATCH 2/3] 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 --- .../contentstore/tests/test_course_listing.py | 15 +- cms/djangoapps/contentstore/views/course.py | 42 ++++- common/djangoapps/student/roles.py | 160 ++++++++++++------ common/djangoapps/student/tests/test_roles.py | 61 ++++++- .../djangoapps/course_groups/permissions.py | 23 +-- 5 files changed, 219 insertions(+), 82 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index eb0bed25524d..47865d326cbd 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -728,11 +728,10 @@ def test_course_listing_with_org_scope(self): the AuthZ course authoring toggle is enabled. """ _, _, authz_courses, legacy_courses = self._create_courses() - org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*') assign_role_to_user_in_scope( self.authorized_user.username, COURSE_STAFF.external_key, - org_scope.external_key, + OrgCourseOverviewGlobData.build_external_key("Org1"), ) request = self._make_request(self.authorized_user) @@ -761,11 +760,10 @@ def test_course_listing_with_org_scope_with_toggle(self): authz_keys, _, _, _ = self._create_courses() # enable only the first and third course keys enabled_keys = {str(authz_keys[0]), str(authz_keys[2])} - org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*') assign_role_to_user_in_scope( self.authorized_user.username, COURSE_STAFF.external_key, - org_scope.external_key, + OrgCourseOverviewGlobData.build_external_key("Org1"), ) request = self._make_request(self.authorized_user) @@ -788,11 +786,10 @@ def test_course_listing_with_org_scope_without_courses(self): courses, `get_courses_accessible_to_user` should return an empty list. """ - org_scope = OrgCourseOverviewGlobData(external_key='course-v1:Org2+*') assign_role_to_user_in_scope( self.authorized_user.username, COURSE_STAFF.external_key, - org_scope.external_key, + OrgCourseOverviewGlobData.build_external_key("Org2"), ) request = self._make_request(self.authorized_user) @@ -810,17 +807,15 @@ def test_course_listing_with_org_scope_fetched_once(self): """ Verify that course overviews are fetched once with all authorized orgs. """ - org_scope1 = OrgCourseOverviewGlobData(external_key='course-v1:Org1+*') - org_scope2 = OrgCourseOverviewGlobData(external_key='course-v1:Org2+*') assign_role_to_user_in_scope( self.authorized_user.username, COURSE_STAFF.external_key, - org_scope1.external_key, + OrgCourseOverviewGlobData.build_external_key("Org1"), ) assign_role_to_user_in_scope( self.authorized_user.username, COURSE_STAFF.external_key, - org_scope2.external_key, + OrgCourseOverviewGlobData.build_external_key("Org2"), ) request = self._make_request(self.authorized_user) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 910e78465fbf..b2aed14123fd 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -17,7 +17,7 @@ from django.core.exceptions import FieldError, ImproperlyConfigured, PermissionDenied from django.core.exceptions import ValidationError as DjangoValidationError from django.db.models import QuerySet -from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseNotFound +from django.http import Http404, HttpRequest, HttpResponse, HttpResponseBadRequest, HttpResponseNotFound from django.shortcuts import redirect from django.urls import reverse from django.utils.translation import gettext as _ @@ -67,6 +67,7 @@ has_studio_write_access, is_content_creator, ) +from common.djangoapps.student.models.user import CourseAccessRole from common.djangoapps.student.roles import ( CourseInstructorRole, CourseStaffRole, @@ -912,20 +913,43 @@ def _get_authz_accessible_courses_list(request): return _get_course_keys_from_scopes(authz_scopes) -def _get_legacy_accessible_courses_list(request): + +def _get_legacy_accessible_courses_list(request: HttpRequest) -> set[CourseKey]: """ - List all courses available to the logged in user by - evaluating legacy Django group roles and organization-level access. + Resolve candidate course keys from legacy ``CourseAccessRole`` records. + + Only database-backed legacy roles are considered. AuthZ-managed access, + including org-wide scopes, is resolved separately by + ``_get_authz_accessible_courses_list``. + + Course-level roles (``instructor``, ``staff``) are mapped directly to their + course keys. Org-wide roles expand to every course in that organization via + a single ``CourseOverview.get_all_courses(orgs=...)`` query. The ``staff`` + role is matched exactly, so ``limited_staff`` assignments are excluded. + + Args: + request: The incoming HTTP request; ``request.user`` determines which + legacy role records are evaluated. + + Returns: + set[CourseKey]: Course keys the user may access through legacy roles. + + Raises: + AccessListFallback: If a legacy role record has neither a course key nor + an organization """ user = request.user - instructor_courses = UserBasedRole(user, CourseInstructorRole.ROLE).courses_with_role() - - with strict_role_checking(): - staff_courses = UserBasedRole(user, CourseStaffRole.ROLE).courses_with_role() + # Query CourseAccessRole directly instead of UserBasedRole.courses_with_role(), + # which merges legacy DB records with AuthZ assignments. AuthZ access is resolved + # separately in _get_authz_accessible_courses_list(). Exact role names (not + # RoleCache inheritance) exclude limited_staff, matching strict_role_checking(). + legacy_accesses = CourseAccessRole.objects.filter( + user=user, + role__in=[CourseInstructorRole.ROLE, CourseStaffRole.ROLE], + ) group_keys = set() org_accesses = set() - legacy_accesses = instructor_courses | staff_courses for access in legacy_accesses: if access.course_id is not None: diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index c8c0b5f09441..9a3d143e8573 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -15,7 +15,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 CourseOverviewData, RoleAssignmentData +from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData, RoleAssignmentData from openedx_authz.constants import roles as authz_roles from common.djangoapps.student.models import CourseAccessRole @@ -74,17 +74,6 @@ 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: """ @@ -93,6 +82,7 @@ def get_org_from_key(key: str) -> str: parsed_key = CourseKey.from_string(key) return parsed_key.org + def register_access_role(cls): """ Decorator that allows access roles to be registered within the roles module and referenced by their @@ -141,34 +131,120 @@ class AuthzCompatCourseAccessRole: """ Generic data class for storing CourseAccessRole-compatible data to be used inside BulkRoleCache and RoleCache. + This allows the cache to store both legacy and openedx-authz compatible roles """ user_id: int username: str org: str - course_id: str # Course key + course_id: str | None role: str -def get_authz_compat_course_access_roles_for_user(user: User) -> set[AuthzCompatCourseAccessRole]: +def _get_org_and_course_id_from_authz_scope( + scope: CourseOverviewData | OrgCourseOverviewGlobData, +) -> tuple[str, str | None] | None: """ - Retrieve all CourseAccessRole objects for a given user and convert them to AuthzCompatCourseAccessRole objects. + Extract the org and course key from an AuthZ course assignment scope. + + Course-scoped assignments return ``(org, course_external_key)``. + Org-wide assignments return ``(org, None)``. + + Returns ``None`` when the org cannot be determined. For org-wide scopes, + ``OrgGlobData.org`` is typed as ``str | None`` because it is parsed from + ``external_key`` and returns ``None`` for malformed glob patterns. """ - compat_role_assignments = set() - 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 - org = get_org_from_key(course_key) - compat_role = AuthzCompatCourseAccessRole( + if isinstance(scope, CourseOverviewData): + course_id = scope.external_key + return get_org_from_key(course_id), course_id + if isinstance(scope, OrgCourseOverviewGlobData): + return scope.org, None + return None + + +def authz_get_all_course_assignments_for_user(user: User) -> list[RoleAssignmentData]: + """ + Return AuthZ role assignments for a user that apply to courses. + + Includes assignments scoped to a specific course (``CourseOverviewData``) and + assignments scoped to all courses in an organization (``OrgCourseOverviewGlobData``). + Assignments for other resource types, such as content libraries, are excluded. + + Args: + user (User): The user whose AuthZ role assignments should be retrieved. + + Returns: + list[RoleAssignmentData]: Role assignments whose scope is course-level or org-wide + """ + return authz_api.get_user_role_assignments_per_scope_type( + user_external_key=user.username, + scope_types=(CourseOverviewData, OrgCourseOverviewGlobData), + ) + + +def _compat_roles_from_authz_assignment( + user: User, + assignment: RoleAssignmentData, +) -> set[AuthzCompatCourseAccessRole]: + """ + Convert an AuthZ role assignment into legacy-compatible course access roles. + + Course-scoped assignments produce roles tied to a specific course key. + Org-wide assignments produce org-level roles with no course key (``course_id`` + is ``None``), matching legacy ``OrgStaffRole`` / ``OrgInstructorRole`` behavior. + AuthZ roles without a legacy mapping are skipped. + + Args: + user (User): The user associated with the assignment. + assignment (RoleAssignmentData): A single AuthZ role assignment, including + its scope and assigned roles. + + Returns: + set[AuthzCompatCourseAccessRole]: Legacy-compatible role records for the + assignment. Returns an empty set if the org cannot be determined from + the scope or no roles could be mapped. + """ + org_and_course_id = _get_org_and_course_id_from_authz_scope(assignment.scope) + if org_and_course_id is None: + return set() + org, course_id = org_and_course_id + + compat_roles = set() + for role in assignment.roles: + legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key) + if legacy_role is None: + continue + compat_roles.add( + AuthzCompatCourseAccessRole( user_id=user.id, username=user.username, org=org, - course_id=course_key, - role=legacy_role + course_id=course_id, + role=legacy_role, ) - compat_role_assignments.add(compat_role) + ) + return compat_roles + + +def get_authz_compat_course_access_roles_for_user(user: User) -> set[AuthzCompatCourseAccessRole]: + """ + Retrieve AuthZ course and org role assignments for a user in legacy format. + + Fetches all course-level and org-wide AuthZ assignments for the user and + converts each one into ``AuthzCompatCourseAccessRole`` records suitable for + ``RoleCache`` and other legacy permission checks. + + Args: + user (User): The user whose AuthZ role assignments should be converted. + + Returns: + set[AuthzCompatCourseAccessRole]: Legacy-compatible role records derived + from the user's AuthZ assignments. Returns an empty set if the user + has no applicable assignments. + """ + compat_role_assignments = set() + for assignment in authz_get_all_course_assignments_for_user(user): + compat_role_assignments.update(_compat_roles_from_authz_assignment(user, assignment)) return compat_role_assignments @@ -843,11 +919,9 @@ def courses_with_role(self) -> set[AuthzCompatCourseAccessRole]: """ Return a set of AuthzCompatCourseAccessRole for all of the courses with this user x (or derived from x) role. """ + # Get all assignments for a user to a role roles = RoleCache.get_roles(self.role) legacy_assignments = CourseAccessRole.objects.filter(role__in=roles, user=self.user) - - # 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_get_all_course_assignments_for_user(self.user) all_assignments = set() @@ -863,19 +937,9 @@ def courses_with_role(self) -> set[AuthzCompatCourseAccessRole]: )) for assignment in all_authz_user_assignments: - for role in assignment.roles: - if role.external_key not in new_authz_roles: - continue - legacy_role = get_legacy_role_from_authz_role(authz_role=role.external_key) - course_key = assignment.scope.external_key - org = get_org_from_key(course_key) - all_assignments.add(AuthzCompatCourseAccessRole( - user_id=self.user.id, - username=self.user.username, - org=org, - course_id=course_key, - role=legacy_role - )) + for compat_role in _compat_roles_from_authz_assignment(self.user, assignment): + if compat_role.role in roles: + all_assignments.add(compat_role) return all_assignments @@ -899,18 +963,12 @@ def has_courses_with_role(self, org: str | None = None) -> bool: return True # Then check for authz assignments - new_authz_roles = [get_authz_role_from_legacy_role(role) for role in roles] 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: - if role.external_key not in new_authz_roles: + for compat_role in _compat_roles_from_authz_assignment(self.user, assignment): + if compat_role.role not in roles: continue - if org is None: - # There is at least one assignment, short circuit - return True - course_key = assignment.scope.external_key - parsed_org = get_org_from_key(course_key) - if org == parsed_org: + if org is None or org == compat_role.org: return True return False diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index 44d711926fdd..332bfe35085b 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -10,7 +10,15 @@ from edx_toggles.toggles.testutils import override_waffle_flag from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocator -from openedx_authz.api.data import ContentLibraryData, CourseOverviewData, RoleAssignmentData, RoleData, UserData +from openedx_authz.api.data import ( + ContentLibraryData, + CourseOverviewData, + OrgCourseOverviewGlobData, + RoleAssignmentData, + RoleData, + ScopeData, + UserData, +) from openedx_authz.constants.roles import COURSE_ADMIN, COURSE_STAFF from openedx_authz.engine.enforcer import AuthzEnforcer @@ -39,6 +47,7 @@ get_role_cache_key_for_course, ) from common.djangoapps.student.tests.factories import AnonymousUserFactory, InstructorFactory, StaffFactory, UserFactory +from lms.djangoapps.instructor import permissions as instructor_permissions from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.toggles import AUTHZ_COURSE_AUTHORING_FLAG @@ -298,6 +307,56 @@ def test_get_authz_compat_course_access_roles_for_user(self): result = get_authz_compat_course_access_roles_for_user(self.student) assert result == set() + def test_get_authz_compat_course_access_roles_for_user_org_scope(self): + """ + Org-wide AuthZ assignments should map to legacy org-level course access roles. + """ + assignment = RoleAssignmentData( + subject=UserData(external_key=self.student.username), + roles=[RoleData(external_key=COURSE_ADMIN.external_key)], + scope=ScopeData( + external_key=OrgCourseOverviewGlobData.build_external_key("OpenedX"), + ), + ) + with patch("openedx_authz.api.users.get_user_role_assignments", return_value=[assignment]): + result = get_authz_compat_course_access_roles_for_user(self.student) + + self.assertCountEqual( # noqa: PT009 + result, + { + AuthzCompatCourseAccessRole( + user_id=self.student.id, + username=self.student.username, + org="OpenedX", + course_id=None, + role="instructor", + ) + }, + ) + + def test_org_scope_authz_role_grants_instructor_dashboard_permissions(self): + """ + Org-wide AuthZ course_admin should grant legacy org instructor access used by the instructor dashboard. + """ + # pylint: disable=protected-access + course_key = CourseKey.from_string("course-v1:OpenedX+DemoX+DemoCourse") + assignment = RoleAssignmentData( + subject=UserData(external_key=self.student.username), + roles=[RoleData(external_key=COURSE_ADMIN.external_key)], + scope=ScopeData( + external_key=OrgCourseOverviewGlobData.build_external_key("OpenedX"), + ), + ) + with patch("openedx_authz.api.users.get_user_role_assignments", return_value=[assignment]): + if hasattr(self.student, "_roles"): + del self.student._roles + self.student._roles = RoleCache(self.student) + + self.assertTrue(self.student._roles.has_role("instructor", None, "OpenedX")) # noqa: PT009 + self.assertTrue(OrgInstructorRole("OpenedX").has_user(self.student)) # noqa: PT009 + self.assertTrue(self.student.has_perm(instructor_permissions.VIEW_DASHBOARD, course_key)) # noqa: PT009 + self.assertTrue(self.student.has_perm(instructor_permissions.SHOW_TASKS, course_key)) # noqa: PT009 + @ddt.ddt class RoleCacheTestCase(TestCase): # lint-amnesty, pylint: disable=missing-class-docstring diff --git a/openedx/core/djangoapps/course_groups/permissions.py b/openedx/core/djangoapps/course_groups/permissions.py index cfc3aefb2f8c..29e953321b1a 100644 --- a/openedx/core/djangoapps/course_groups/permissions.py +++ b/openedx/core/djangoapps/course_groups/permissions.py @@ -5,7 +5,8 @@ from opaque_keys.edx.keys import CourseKey from rest_framework import permissions -from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff +from common.djangoapps.student.roles import GlobalStaff +from lms.djangoapps.courseware.access import administrative_accesses_to_course_for_user from lms.djangoapps.discussion.django_comment_client.utils import get_user_role_names from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_ADMINISTRATOR, @@ -23,15 +24,15 @@ def has_permission(self, request, view): """Returns true if the user is admin or staff and request method is GET.""" if GlobalStaff().has_user(request.user) or request.user.is_superuser: return True - course_key = CourseKey.from_string(view.kwargs.get('course_key_string')) + course_key = CourseKey.from_string(view.kwargs.get("course_key_string")) user_roles = get_user_role_names(request.user, course_key) - has_discussion_privileges = bool(user_roles & { - FORUM_ROLE_ADMINISTRATOR, - FORUM_ROLE_MODERATOR, - FORUM_ROLE_COMMUNITY_TA, - }) - return ( - CourseInstructorRole(course_key).has_user(request.user) or - CourseStaffRole(course_key).has_user(request.user) or - has_discussion_privileges and request.method == "GET" + has_discussion_privileges = bool( + user_roles + & { + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, + FORUM_ROLE_COMMUNITY_TA, + } ) + _, staff_access, instructor_access = administrative_accesses_to_course_for_user(request.user, course_key) + return staff_access or instructor_access or (has_discussion_privileges and request.method == "GET") From 1efb8dacb8fef17cbe19c2930c059c8968a23938 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 22 Jun 2026 15:45:17 -0500 Subject: [PATCH 3/3] fix: preserve catalog and staff checks for authZ about-page access (#38736) * fix: preserve catalog and staff checks for authZ about-page access * refactor: remove about page catalog visibility error function and return CatalogVisibilityError directly --- lms/djangoapps/course_api/tests/test_api.py | 123 +++++++++--------- lms/djangoapps/courseware/access.py | 64 ++++++--- lms/djangoapps/courseware/tests/test_about.py | 37 ++++++ .../courseware/tests/test_access.py | 69 ++++++++++ 4 files changed, 215 insertions(+), 78 deletions(-) diff --git a/lms/djangoapps/course_api/tests/test_api.py b/lms/djangoapps/course_api/tests/test_api.py index b84b920c4a47..a3500a0d59b7 100644 --- a/lms/djangoapps/course_api/tests/test_api.py +++ b/lms/djangoapps/course_api/tests/test_api.py @@ -16,11 +16,14 @@ from rest_framework.request import Request from rest_framework.test import APIRequestFactory +from common.djangoapps.student.roles import CourseLimitedStaffRole +from common.djangoapps.student.tests.factories import StaffFactory, UserFactory from lms.djangoapps.courseware.exceptions import CourseAccessRedirect from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.django_utils import ( # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.course_block import CATALOG_VISIBILITY_ABOUT, CATALOG_VISIBILITY_NONE +from xmodule.modulestore.exceptions import ItemNotFoundError # pylint: disable=wrong-import-order +from xmodule.modulestore.tests.django_utils import ( # pylint: disable=wrong-import-order ModuleStoreTestCase, SharedModuleStoreTestCase, ) @@ -146,58 +149,69 @@ def setUpClass(cls): cls.course = cls.create_course() cls.hidden_course = cls.create_course( - course='hidden', - visible_to_staff_only=True + course="hidden", + visible_to_staff_only=True, + catalog_visibility=CATALOG_VISIBILITY_NONE, + ) + cls.about_only_course = cls.create_course( + course="aboutonly", + catalog_visibility=CATALOG_VISIBILITY_ABOUT, ) def test_get_existing_course_as_authorized_user(self): """User with COURSE_EDITOR role can access course.""" - self.add_user_to_role_in_course( - self.authorized_user, - COURSE_EDITOR.external_key, - self.course.id - ) + self.add_user_to_role_in_course(self.authorized_user, COURSE_EDITOR.external_key, self.course.id) - course = self._make_api_call( - self.authorized_user, - self.authorized_user, - self.course.id - ) + course = self._make_api_call(self.authorized_user, self.authorized_user, self.course.id) + + self.verify_course(course) + + def test_get_existing_course_without_authz_role_when_catalog_visible(self): + """User without AuthZ role can still access when catalog visibility allows.""" + course = self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.course.id) self.verify_course(course) - def test_get_existing_course_as_unauthorized_user(self): - """User without role should be denied.""" + def test_about_only_catalog_visibility_without_authz_role(self): + """User without AuthZ role can access when catalog visibility is about-only.""" + course = self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.about_only_course.id) + + self.verify_course(course, course_id=str(self.about_only_course.id)) + + def test_hidden_course_denied_without_authz_role(self): + """User without AuthZ role is denied when catalog visibility is none.""" with pytest.raises(CourseAccessRedirect): - self._make_api_call( - self.unauthorized_user, - self.unauthorized_user, - self.course.id - ) + self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.hidden_course.id) def test_get_nonexistent_course(self): """Nonexistent course should raise 404.""" - course_key = CourseKey.from_string('edX/toy/nope') + course_key = CourseKey.from_string("edX/toy/nope") with pytest.raises(Http404): - self._make_api_call( - self.authorized_user, - self.authorized_user, - course_key - ) + self._make_api_call(self.authorized_user, self.authorized_user, course_key) + + def test_course_staff_bypasses_authz_on_hidden_course(self): + """Course staff can access a hidden course without an AuthZ role.""" + course_staff = StaffFactory.create(course_key=self.hidden_course.id) + + course = self._make_api_call(course_staff, course_staff, self.hidden_course.id) + + self.verify_course(course, course_id=str(self.hidden_course.id)) + + def test_limited_staff_bypasses_authz_on_hidden_course(self): + """Limited course staff can access a hidden course without an AuthZ role.""" + limited_staff = UserFactory(password=self.password) + CourseLimitedStaffRole(self.hidden_course.id).add_users(limited_staff) + + course = self._make_api_call(limited_staff, limited_staff, self.hidden_course.id) + + self.verify_course(course, course_id=str(self.hidden_course.id)) def test_hidden_course_for_staff(self): """Staff can access hidden course.""" - course = self._make_api_call( - self.staff_user, - self.staff_user, - self.hidden_course.id - ) + course = self._make_api_call(self.staff_user, self.staff_user, self.hidden_course.id) - self.verify_course( - course, - course_id='course-v1:edX+hidden+2012_Fall' - ) + self.verify_course(course, course_id=str(self.hidden_course.id)) def test_hidden_course_for_staff_as_unauthorized_user(self): """ @@ -205,39 +219,22 @@ def test_hidden_course_for_staff_as_unauthorized_user(self): should not bypass visibility rules. """ with pytest.raises(CourseAccessRedirect): - self._make_api_call( - self.staff_user, - self.unauthorized_user, - self.hidden_course.id - ) + self._make_api_call(self.staff_user, self.unauthorized_user, self.hidden_course.id) def test_user_gains_access_after_role_assignment(self): - """User initially denied, then allowed after role assignment.""" + """User denied when catalog is hidden, then allowed after role assignment.""" with pytest.raises(CourseAccessRedirect): - self._make_api_call( - self.unauthorized_user, - self.unauthorized_user, - self.course.id - ) - self.add_user_to_role_in_course( - self.unauthorized_user, - COURSE_EDITOR.external_key, - self.course.id - ) - course = self._make_api_call( - self.unauthorized_user, - self.unauthorized_user, - self.course.id - ) - self.verify_course(course) + self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.hidden_course.id) + + self.add_user_to_role_in_course(self.unauthorized_user, COURSE_EDITOR.external_key, self.hidden_course.id) + + course = self._make_api_call(self.unauthorized_user, self.unauthorized_user, self.hidden_course.id) + + self.verify_course(course, course_id=str(self.hidden_course.id)) def test_staff_access_without_authz_role(self): """Staff bypasses AuthZ roles.""" - course = self._make_api_call( - self.staff_user, - self.staff_user, - self.course.id - ) + course = self._make_api_call(self.staff_user, self.staff_user, self.course.id) self.verify_course(course) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index a99949c5fdbb..9669cd8bdcf8 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -45,6 +45,7 @@ from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException from lms.djangoapps.ccx.models import CustomCourseForEdX from lms.djangoapps.courseware.access_response import ( + AccessResponse, CatalogVisibilityError, IncorrectPartitionGroupError, MilestoneAccessError, @@ -64,6 +65,7 @@ from lms.djangoapps.courseware.toggles import course_is_invitation_only from lms.djangoapps.mobile_api.models import IgnoreMobileAvailableFlagConfig from openedx.core import toggles as core_toggles +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission from openedx.core.djangoapps.authz.decorators import user_has_course_permission from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.features.course_duration_limits.access import check_course_expired @@ -434,30 +436,62 @@ def can_see_in_catalog(): # can provide a meaningful error message instead of a generic 404. return catalog_response - def legacy_can_see_about_page(): + def _about_page_catalog_visibility_access() -> AccessResponse | None: + """ + Shared catalog-visibility gate for about-page access. + + Grants access when catalog_visibility is CATALOG_AND_ABOUT or ABOUT only. + + Returns ACCESS_GRANTED when either applies, or None when catalog visibility + does not allow the about page (callers fall through to staff/AuthZ checks). + """ both_response = _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT) if both_response: return ACCESS_GRANTED about_response = _has_catalog_visibility(courselike, CATALOG_VISIBILITY_ABOUT) if about_response: return ACCESS_GRANTED - if _has_staff_access_to_block(user, courselike, courselike.id): - return ACCESS_GRANTED - # Return the typed CatalogVisibilityError so downstream handlers - # can provide a meaningful error message instead of a generic 404. - return both_response + return None - @function_trace('can_see_about_page') - def can_see_about_page(): + @function_trace("can_see_about_page") + def can_see_about_page() -> AccessResponse | CatalogVisibilityError: """ - Implements the "can see course about page" logic if a course about page should be visible - In this case we use the catalog_visibility property on the course block - but also allow course staff to see this. + Entry point for about-page visibility checks. + + Grants access when any of the following is true: + - the course catalog_visibility allows the about page, or + - the user has course staff access (including limited staff via role inheritance), or + - the user is authenticated, AuthZ course authoring is enabled for the course, + and the user has COURSES_VIEW_COURSE (including legacy Studio read access + as a fallback during RBAC migration). + + Learners, beta testers, and other course-team roles without staff access rely on + catalog visibility only; they are not checked explicitly here. + + AuthZ must not replace catalog visibility or staff bypass; those checks run + first so enrolled learners and beta testers are not blocked by authoring + permissions they do not hold. + + Returns CatalogVisibilityError when all checks fail. """ - if user and not user.is_anonymous and core_toggles.enable_authz_course_authoring(courselike.id): - is_authz_allowed = user_has_course_permission(user, COURSES_VIEW_COURSE.identifier, courselike.id) - return ACCESS_GRANTED if is_authz_allowed else CatalogVisibilityError() - return legacy_can_see_about_page() + catalog_visibility_access = _about_page_catalog_visibility_access() + if catalog_visibility_access: + return catalog_visibility_access + + if _has_staff_access_to_block(user, courselike, courselike.id): + return ACCESS_GRANTED + + if ( + user + and not user.is_anonymous + and core_toggles.enable_authz_course_authoring(courselike.id) + and user_has_course_permission( + user, COURSES_VIEW_COURSE.identifier, courselike.id, LegacyAuthoringPermission.READ + ) + ): + return ACCESS_GRANTED + + return CatalogVisibilityError() checkers = { 'load': can_load, diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index 067318d797ce..bf7c3366ee11 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -9,6 +9,7 @@ import ddt import pytz +from crum import set_current_request from django.conf import settings from django.test.utils import override_settings from django.urls import reverse @@ -19,6 +20,7 @@ from common.djangoapps.student.tests.factories import CourseEnrollmentAllowedFactory, UserFactory from common.djangoapps.track.tests import EventTrackingTestCase from common.djangoapps.util.milestones_helpers import get_prerequisite_courses_display, set_prerequisite_courses +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin from openedx.core.djangoapps.models.course_details import CourseDetails from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, course_home_url from openedx.features.course_experience.waffle import ENABLE_COURSE_ABOUT_SIDEBAR_HTML @@ -223,6 +225,41 @@ def test_about_page_public_view(self, course_visibility): self.assertContains(resp, "Enroll Now") +class AuthzAboutPageTestCase( + CourseAuthoringAuthzTestMixin, + LoginEnrollmentTestCase, + SharedModuleStoreTestCase, + EventTrackingTestCase, +): + """ + About page HTTP access when AuthZ course authoring is enabled. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course_without_about = CourseFactory.create(catalog_visibility=CATALOG_VISIBILITY_NONE) + cls.course_with_about = CourseFactory.create(catalog_visibility=CATALOG_VISIBILITY_ABOUT) + CourseDetails.update_about_item(cls.course_without_about, "overview", "WITHOUT ABOUT", None) + CourseDetails.update_about_item(cls.course_with_about, "overview", "WITH ABOUT", None) + + def setUp(self): + super().setUp() + self.addCleanup(set_current_request, None) + assert self.client.login(username=self.unauthorized_user.username, password=self.password) + + @override_settings(COURSE_ABOUT_VISIBILITY_PERMISSION="see_about_page") + def test_about_page_honors_catalog_visibility_without_authz_role(self): + """A learner without AuthZ roles can view catalog-visible about pages.""" + url = reverse("about_course", args=[str(self.course_with_about.id)]) + resp = self.client.get(url) + self.assertContains(resp, "WITH ABOUT") + + url = reverse("about_course", args=[str(self.course_without_about.id)]) + resp = self.client.get(url) + self.assertRedirects(resp, reverse("dashboard"), fetch_redirect_response=False) + + class AboutTestCaseXML(LoginEnrollmentTestCase, ModuleStoreTestCase): """ Tests for the course about page diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index b00cca78f2e1..e9e8ae7e2382 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -43,6 +43,7 @@ from lms.djangoapps.courseware.masquerade import CourseMasquerade from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase, masquerade_as_group_member from lms.djangoapps.courseware.toggles import course_is_invitation_only +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES @@ -1019,3 +1020,71 @@ def test_course_catalog_access_num_queries_enterprise(self, user_attr_name, cour course_overview = CourseOverview.get_from_id(course.id) with self.assertNumQueries(num_queries, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST): bool(access.has_access(user, 'see_exists', course_overview, course_key=course.id)) + + +class AuthzSeeAboutPageAccessTestCase(CourseAuthoringAuthzTestMixin, SharedModuleStoreTestCase): + """ + AuthZ-specific see_about_page edge cases not covered elsewhere. + + Catalog visibility grants, staff bypass, AuthZ role grants, and learner + denials are tested in test__catalog_visibility*, TestGetCourseDetailAuthz, + and AuthzAboutPageTestCase. + """ + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course_public = CourseFactory.create( + catalog_visibility=CATALOG_VISIBILITY_CATALOG_AND_ABOUT, + course="authzpublic", + ) + cls.course_about_only = CourseFactory.create( + catalog_visibility=CATALOG_VISIBILITY_ABOUT, + course="authzabout", + ) + cls.course_hidden = CourseFactory.create( + catalog_visibility=CATALOG_VISIBILITY_NONE, + course="authzhidden", + ) + + def _see_about_page_response(self, user, course): + course_overview = CourseOverview.get_from_id(course.id) + return access.has_access(user, "see_about_page", course_overview, course_key=course.id) + + def test_enrolled_learner_denied_when_catalog_hidden(self): + """Enrollment alone does not grant about-page access when catalog is hidden.""" + CourseEnrollmentFactory(user=self.unauthorized_user, course_id=self.course_hidden.id) + + response = self._see_about_page_response(self.unauthorized_user, self.course_hidden) + + assert not response + assert isinstance(response, access_response.CatalogVisibilityError) + + def test_beta_tester_granted_via_catalog_about(self): + """Beta testers rely on catalog visibility, not AuthZ authoring permissions.""" + beta_tester = BetaTesterFactory.create(course_key=self.course_about_only.id) + + response = self._see_about_page_response(beta_tester, self.course_about_only) + + assert response + + def test_anonymous_user_uses_legacy_path(self): + """ + Anonymous users skip the AuthZ path even when course authoring AuthZ is enabled. + + user_has_course_permission is only reached on the AuthZ path, so it must not + be called for anonymous users on a catalog-hidden course. + """ + anonymous_user = AnonymousUserFactory.create() + + with patch( + "lms.djangoapps.courseware.access.user_has_course_permission", + ) as mock_authz_permission: + hidden_response = self._see_about_page_response(anonymous_user, self.course_hidden) + + mock_authz_permission.assert_not_called() + assert not hidden_response + assert isinstance(hidden_response, access_response.CatalogVisibilityError) + + public_response = self._see_about_page_response(anonymous_user, self.course_public) + assert public_response