diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index 9a56b8ec7c76..061fc895e23c 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 @@ -832,3 +833,100 @@ 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 + } + + self.assertEqual(result_ids, expected_ids) # noqa: PT009 + + def test_course_listing_with_platform_scope_with_toggle(self): + """ + If the authz toggle is enabled only for a subset of courses, 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]} + self.assertEqual(result_ids, expected) # noqa: PT009 + + def test_get_course_keys_from_scopes_with_platform_scope(self): + """ + Platform-wide scopes should resolve to all courses with AuthZ enabled. + """ + 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:*")]) + + self.assertEqual(course_keys, set(authz_keys) | set(legacy_keys)) # noqa: PT009 + + 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. + """ + 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:*"), + ] + ) + + self.assertEqual(course_keys, {authz_keys[0]}) # noqa: PT009 diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 569871b95bea..d5530a7792d3 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, @@ -823,25 +828,61 @@ 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. + + Returns: + set[CourseKey]: Course keys on the platform where the AuthZ course + authoring feature flag is enabled. + """ + return { + course_key + for course_key in CourseOverview.get_all_courses().values_list("id", flat=True) + 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 + 2. Course-specific access (CourseOverviewData): Returns individual course keys + 3. Organization-wide access (OrgCourseOverviewGlobData): Returns all courses in specified orgs + + 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 d7fdb0bcae3c..f7c3a20e0753 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 305d396d4619..f8573eadd473 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 903e7e43a23e..070c2d6a163b 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -1935,9 +1935,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() @@ -2061,6 +2062,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() + + self.assertEqual(filtered.count(), 2) # noqa: PT009 + + 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() + + self.assertEqual(filtered.count(), 2) # noqa: PT009 + slugs = set(filtered.values_list("slug", flat=True)) + self.assertEqual(slugs, {"org-glob-lib1", "org-glob-lib2"}) # noqa: PT009 + def test_authz_scope_q_object_has_correct_structure(self): """ Test that HasPermissionInContentLibraryScope.query() generates Q object @@ -2078,14 +2132,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) @@ -2180,8 +2230,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) @@ -2295,8 +2345,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 10338b04b8b9..1c87f0054c79 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.16.0 +openedx-authz==1.18.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 7913481ff4a9..f6be00b6ec41 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.16.0 +openedx-authz==1.18.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 af8e942ef81c..c9728358ae3f 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.16.0 +openedx-authz==1.18.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 11aeae10d01b..5945f9a30fa8 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.16.0 +openedx-authz==1.18.0 # via -r requirements/edx/base.txt openedx-calc==5.0.0 # via