From ea19e48a461edfc2df3b726e99ca4cf00bf74c95 Mon Sep 17 00:00:00 2001 From: Kevyn Suarez Date: Mon, 28 Sep 2026 16:47:19 -0500 Subject: [PATCH 1/9] fix: make UserOrgFilterBackend recognize openedx-authz manage_tags roles TaxonomyOrgView filtered taxonomies using only legacy role resolution (get_admin_orgs/get_user_orgs), so a user holding only an openedx-authz role like course_editor, with no legacy CourseAccessRole, got an empty org list and hit queryset.none(), blocking them from seeing even global taxonomies. Add get_authz_manage_tags_orgs, which resolves the orgs where the user holds courses.manage_tags via get_scopes_for_user_and_permission, and OR it into UserOrgFilterBackend's existing user_orgs set alongside the legacy roles, the same additive pattern already used for object tags in ObjectTagTaxonomyOrgFilterBackend. A scope only counts once its course has actually switched to openedx-authz, so a policy assignment ahead of that course's own toggle doesn't grant early access. Closes openedx-authz#448 --- .../content_tagging/rest_api/v1/filters.py | 7 +- .../rest_api/v1/tests/test_views.py | 87 +++++++++++++++++-- .../core/djangoapps/content_tagging/rules.py | 27 ++++++ 3 files changed, 113 insertions(+), 8 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py index a51bf6c99812..7372d7630712 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py @@ -7,7 +7,7 @@ from rest_framework.filters import BaseFilterBackend from ...models import TaxonomyOrg -from ...rules import get_admin_orgs, get_user_orgs +from ...rules import get_admin_orgs, get_authz_manage_tags_orgs, get_user_orgs from ...utils import rules_cache @@ -25,7 +25,10 @@ def filter_queryset(self, request, queryset, _): return queryset user_admin_orgs = get_admin_orgs(request.user) - user_orgs = get_user_orgs(request.user) # Orgs that the user is a content creator or instructor + # Orgs that the user is a content creator or instructor in, either through a legacy + # role or, additively, through an openedx-authz role like course_editor that grants + # courses.manage_tags with no legacy equivalent (see get_authz_manage_tags_orgs). + user_orgs = list(set(get_user_orgs(request.user)) | set(get_authz_manage_tags_orgs(request.user))) if len(user_orgs) == 0 and len(user_admin_orgs) == 0: return queryset.none() diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 81cbb440b032..9aff989bf41b 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -36,6 +36,7 @@ OrgStaffRole, ) from common.djangoapps.student.tests.factories import StaffFactory, UserFactory +from openedx.core import toggles as core_toggles from openedx.core.djangoapps.authz.tests.mixins import CourseAuthzTestMixin from openedx.core.djangoapps.content_libraries.api import AccessLevel, create_library, set_library_user_permissions from openedx.core.djangoapps.content_tagging import api as tagging_api @@ -586,12 +587,14 @@ def test_create_taxonomy_rejects_invalid_type(self) -> None: @ddt.data( ('staff', 10), - ("content_creatorA", 22), - ("library_staffA", 22), - ("library_userA", 22), - ("instructorA", 22), - ("course_instructorA", 22), - ("course_staffA", 22), + # Non-admin users now also cost one extra query to check for orgs granted through + # openedx-authz's courses.manage_tags (get_authz_manage_tags_orgs). + ("content_creatorA", 23), + ("library_staffA", 23), + ("library_userA", 23), + ("instructorA", 23), + ("course_instructorA", 23), + ("course_staffA", 23), ) @ddt.unpack def test_list_taxonomy_query_count(self, user_attr: str, expected_queries: int): @@ -2447,6 +2450,78 @@ def test_library_fallthrough_to_legacy(self): assert response.status_code == status.HTTP_200_OK +class TestTaxonomyOrgViewWithAuthz(CourseAuthzTestMixin, SharedModuleStoreTestCase, APITestCase): + """ + Test TaxonomyOrgView (GET /taxonomies/?org=X) with authz permissions. + + Regression tests for openedx-authz#448: a user who only holds an openedx-authz role + (e.g. course_editor) with no legacy org role used to get an empty list, even for + global taxonomies, because UserOrgFilterBackend only knew about legacy roles. + """ + + authz_roles_to_assign = [COURSE_EDITOR.external_key] + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + cls.course_key = cls.course.id + + def setUp(self): + super().setUp() + self.course_org, _ = Organization.objects.get_or_create(short_name=self.course_key.org) + + self.org_taxonomy = tagging_api.create_taxonomy( + name="Org Taxonomy", description="Taxonomy scoped to the course org", orgs=[self.course_org], + ) + self.disabled_org_taxonomy = tagging_api.create_taxonomy( + name="Disabled Org Taxonomy", description="Disabled taxonomy scoped to the course org", + enabled=False, orgs=[self.course_org], + ) + + def _list_url(self, org=None): + url = TAXONOMY_ORG_LIST_URL + return f"{url}?org={org}" if org else url + + def test_course_editor_sees_org_taxonomies(self): + """A course editor (authz-only, no legacy org role) sees enabled taxonomies for their course's org.""" + response = self.authorized_client.get(self._list_url(self.course_org.short_name)) + assert response.status_code == status.HTTP_200_OK + taxonomy_ids = {t["id"] for t in response.data["results"]} + assert self.org_taxonomy.pk in taxonomy_ids + assert self.disabled_org_taxonomy.pk not in taxonomy_ids + + def test_course_editor_doesnt_see_other_org_taxonomies(self): + """A course editor doesn't get access to a different org's taxonomies through this permission.""" + other_org_taxonomy = tagging_api.create_taxonomy( + name="Other Org Taxonomy", description="Taxonomy scoped to an unrelated org", + ) + other_org, _ = Organization.objects.get_or_create(short_name="OtherOrgForTaxonomy") + set_taxonomy_orgs(other_org_taxonomy, orgs=[other_org]) + + response = self.authorized_client.get(self._list_url(other_org.short_name)) + assert response.status_code == status.HTTP_200_OK + taxonomy_ids = {t["id"] for t in response.data["results"]} + assert other_org_taxonomy.pk not in taxonomy_ids + + def test_no_role_user_still_denied(self): + """A user with no role at all, legacy or authz, still gets nothing for the org.""" + response = self.unauthorized_client.get(self._list_url(self.course_org.short_name)) + assert response.status_code == status.HTTP_200_OK + assert response.data["results"] == [] + + def test_authz_role_ignored_when_course_toggle_disabled(self): + """ + A course_editor assignment doesn't grant access ahead of that course's own authz + toggle being enabled: legacy access stays authoritative until it flips. + """ + with patch.object(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, "is_enabled", return_value=False): + response = self.authorized_client.get(self._list_url(self.course_org.short_name)) + assert response.status_code == status.HTTP_200_OK + taxonomy_ids = {t["id"] for t in response.data["results"]} + assert self.org_taxonomy.pk not in taxonomy_ids + + @skip_unless_cms @ddt.ddt class TestDownloadTemplateView(APITestCase): diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index fe1064f22b5e..ee6ae839abc8 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -9,6 +9,7 @@ import rules from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_authz import api as authz_api +from openedx_authz.api.data import CourseOverviewData from openedx_authz.constants import permissions as authz_permissions from organizations.models import Organization @@ -21,6 +22,7 @@ OrgInstructorRole, OrgLibraryUserRole, OrgStaffRole, + enable_authz_course_authoring, ) from .auth import should_use_course_authz_for_object @@ -127,6 +129,31 @@ def get_user_orgs(user: UserType, orgs: list[Organization] | None = None) -> lis return user_orgs +def get_authz_manage_tags_orgs(user: UserType) -> list[Organization]: + """ + Return the orgs where the user holds courses.manage_tags through openedx-authz. + + This is the authz equivalent of _get_course_user_orgs: roles like course_editor, + course_staff, and course_admin only exist in openedx-authz, with no legacy + CourseAccessRole for get_user_orgs to find, so callers that need to recognize them + have to check authz separately and add the result on top of get_user_orgs. + + A scope is only counted if the course has actually switched to openedx-authz: a + policy assignment can exist ahead of that course's own toggle, and until it flips, + legacy access stays authoritative for it (see should_use_course_authz_for_object). + """ + scopes = authz_api.get_scopes_for_user_and_permission( + user.username, authz_permissions.COURSES_MANAGE_TAGS.identifier + ) + + org_names = set() + for scope in scopes: + if isinstance(scope, CourseOverviewData) and enable_authz_course_authoring(scope.course_key): + org_names.add(scope.org) + + return rules_cache.get_orgs(list(org_names)) if org_names else [] + + @rules.predicate def can_create_taxonomy(user: UserType) -> bool: """ From 7e5ac713023ecc1cbb22e0bfe2ad18da6279f0a9 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 5 Oct 2026 18:57:52 -0500 Subject: [PATCH 2/9] fix: apply openedx-authz manage_tags orgs consistently across tagging rules The previous commit only widened UserOrgFilterBackend, so a user with an authz-only role (e.g. course_editor) could list an org's taxonomies but got 403 opening them, because can_view_taxonomy still resolved orgs through legacy roles alone. Fold the authz lookup into get_user_orgs via _get_authz_manage_tags_orgs, so the list filters, is_org_user and can_view_taxonomy share one source, and drop the duplicated set-union in UserOrgFilterBackend. - Count org-wide and platform-wide course glob scopes, which were silently dropped; they are checked against the global authz flag as they have no single course to toggle. - Skip the per-course flag lookup once an org is already resolved. - Ignore legacy course roles in courses that have switched to openedx-authz, so visibility matches can_change_object_tag_objectid, which requires courses.manage_tags there. Co-Authored-By: Claude Sonnet 5.5 --- .../content_tagging/rest_api/v1/filters.py | 7 +- .../rest_api/v1/tests/test_views.py | 2 +- .../core/djangoapps/content_tagging/rules.py | 75 ++++++++++++++----- 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py index 7372d7630712..3bfc7e166536 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py @@ -7,7 +7,7 @@ from rest_framework.filters import BaseFilterBackend from ...models import TaxonomyOrg -from ...rules import get_admin_orgs, get_authz_manage_tags_orgs, get_user_orgs +from ...rules import get_admin_orgs, get_user_orgs from ...utils import rules_cache @@ -25,10 +25,7 @@ def filter_queryset(self, request, queryset, _): return queryset user_admin_orgs = get_admin_orgs(request.user) - # Orgs that the user is a content creator or instructor in, either through a legacy - # role or, additively, through an openedx-authz role like course_editor that grants - # courses.manage_tags with no legacy equivalent (see get_authz_manage_tags_orgs). - user_orgs = list(set(get_user_orgs(request.user)) | set(get_authz_manage_tags_orgs(request.user))) + user_orgs = get_user_orgs(request.user) if len(user_orgs) == 0 and len(user_admin_orgs) == 0: return queryset.none() diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 9aff989bf41b..3e920cbe2436 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -588,7 +588,7 @@ def test_create_taxonomy_rejects_invalid_type(self) -> None: @ddt.data( ('staff', 10), # Non-admin users now also cost one extra query to check for orgs granted through - # openedx-authz's courses.manage_tags (get_authz_manage_tags_orgs). + # openedx-authz's courses.manage_tags (_get_authz_manage_tags_orgs, via get_user_orgs). ("content_creatorA", 23), ("library_staffA", 23), ("library_userA", 23), diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index ee6ae839abc8..962e3d3bb7d3 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -7,9 +7,11 @@ import django.contrib.auth.models import openedx_tagging.rules as oel_tagging import rules +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_authz import api as authz_api -from openedx_authz.api.data import CourseOverviewData +from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData, PlatformCourseOverviewGlobData from openedx_authz.constants import permissions as authz_permissions from organizations.models import Organization @@ -81,16 +83,34 @@ def _get_course_user_orgs(user: UserType, orgs: list[Organization]) -> list[Orga if not orgs: return [] + authz_course_cache: dict[str, bool] = {} + + def course_uses_authz(course_id) -> bool: + """ + True if the course has switched to openedx-authz, where legacy course roles are no longer + authoritative: tagging there requires courses.manage_tags (see can_change_object_tag_objectid), + so those users are picked up by _get_authz_manage_tags_orgs instead. + """ + key = str(course_id) + if key not in authz_course_cache: + try: + authz_course_cache[key] = enable_authz_course_authoring(CourseKey.from_string(key)) + except InvalidKeyError: + authz_course_cache[key] = False + return authz_course_cache[key] + def user_has_role_ignore_course_id(user, role_name, org_name) -> bool: """ - Returns True if the given user has the given role for the given org, OR for any courses in this org. + Returns True if the given user has the given role for the given org, OR for any courses in this org + that have not switched to openedx-authz. """ # We use the user's RoleCache here to avoid re-querying. roles_cache = get_role_cache(user) course_roles = get_course_roles(user) return any( access_role.role in roles_cache.get_roles(role_name) and - access_role.org == org_name + access_role.org == org_name and + not (access_role.course_id and course_uses_authz(access_role.course_id)) for access_role in course_roles ) @@ -119,39 +139,60 @@ def get_user_orgs(user: UserType, orgs: list[Organization] | None = None) -> lis """ Return a list of orgs that the given user is a member of (instructor or content creator), from the given list of orgs. + + Membership comes from legacy roles and, additively, from openedx-authz roles that grant + courses.manage_tags (see _get_authz_manage_tags_orgs). """ org_list = rules_cache.get_orgs() if orgs is None else orgs content_creator_orgs = _get_content_creator_orgs(user, org_list) course_user_orgs = _get_course_user_orgs(user, org_list) library_user_orgs = _get_library_user_orgs(user, org_list) - user_orgs = list(set(content_creator_orgs) | set(course_user_orgs) | set(library_user_orgs)) + authz_orgs = _get_authz_manage_tags_orgs(user, org_list) + return list(set(content_creator_orgs) | set(course_user_orgs) | set(library_user_orgs) | set(authz_orgs)) - return user_orgs - -def get_authz_manage_tags_orgs(user: UserType) -> list[Organization]: +def _get_authz_manage_tags_orgs(user: UserType, orgs: list[Organization]) -> list[Organization]: """ - Return the orgs where the user holds courses.manage_tags through openedx-authz. + Return the orgs, from the given list, where the user holds courses.manage_tags through openedx-authz. This is the authz equivalent of _get_course_user_orgs: roles like course_editor, course_staff, and course_admin only exist in openedx-authz, with no legacy - CourseAccessRole for get_user_orgs to find, so callers that need to recognize them - have to check authz separately and add the result on top of get_user_orgs. + CourseAccessRole for the legacy helpers to find. - A scope is only counted if the course has actually switched to openedx-authz: a - policy assignment can exist ahead of that course's own toggle, and until it flips, - legacy access stays authoritative for it (see should_use_course_authz_for_object). + Course scopes only count if that course has switched to openedx-authz: a policy assignment + can exist ahead of the course's own toggle, and until it flips, legacy access stays + authoritative for it (see should_use_course_authz_for_object). Org-wide and platform-wide + glob scopes have no single course to check, so they count when the flag is on globally. """ + if not orgs: + return [] + scopes = authz_api.get_scopes_for_user_and_permission( user.username, authz_permissions.COURSES_MANAGE_TAGS.identifier ) org_names = set() + all_orgs = False + glob_authz_enabled = None for scope in scopes: - if isinstance(scope, CourseOverviewData) and enable_authz_course_authoring(scope.course_key): - org_names.add(scope.org) - - return rules_cache.get_orgs(list(org_names)) if org_names else [] + if isinstance(scope, CourseOverviewData): + # Skip the per-course flag lookup when the org is already known. + if scope.org not in org_names and enable_authz_course_authoring(scope.course_key): + org_names.add(scope.org) + elif isinstance(scope, OrgCourseOverviewGlobData | PlatformCourseOverviewGlobData): + if glob_authz_enabled is None: + glob_authz_enabled = enable_authz_course_authoring() + if not glob_authz_enabled: + continue + if isinstance(scope, PlatformCourseOverviewGlobData): + all_orgs = True + break + if scope.org: + org_names.add(scope.org) + + if all_orgs: + return list(orgs) + return [org for org in orgs if org.short_name in org_names] @rules.predicate From 25c0693bec5f9101c1f447f0cd0204cdefe438d3 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 5 Oct 2026 18:59:00 -0500 Subject: [PATCH 3/9] test: cover openedx-authz manage_tags org resolution in tagging rules Add TestRulesAuthzManageTagsOrgs for get_user_orgs and can_view_taxonomy: course, org-glob and platform-glob scopes, the course toggle gating, a single flag lookup per org, authz-only users viewing their org's taxonomies, and legacy course roles being ignored once a course has switched to openedx-authz. Co-Authored-By: Claude Sonnet 5.5 --- .../content_tagging/tests/test_rules.py | 97 ++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_rules.py b/openedx/core/djangoapps/content_tagging/tests/test_rules.py index 7fb2274c8238..e45cdb3de5f0 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_rules.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_rules.py @@ -7,6 +7,7 @@ from django.test import TestCase from edx_toggles.toggles.testutils import override_waffle_flag from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator, LibraryLocatorV2 +from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData, PlatformCourseOverviewGlobData from openedx_authz.constants import permissions as authz_permissions from openedx_tagging.models import Tag from openedx_tagging.rules import ObjectTagPermissionItem @@ -16,7 +17,7 @@ from openedx.core.toggles import AUTHZ_COURSE_AUTHORING_FLAG from .. import api -from ..rules import can_change_object_tag_objectid, can_remove_object_tag_objectid +from ..rules import can_change_object_tag_objectid, can_remove_object_tag_objectid, can_view_taxonomy, get_user_orgs from .test_api import TestTaxonomyMixin User = get_user_model() @@ -850,3 +851,97 @@ def test_library_object_unchanged(self, mock_is_user_allowed): authz_permissions.MANAGE_LIBRARY_TAGS.identifier, str(library_key), ) + + +@patch("openedx_authz.api.get_scopes_for_user_and_permission") +class TestRulesAuthzManageTagsOrgs(TestTaxonomyMixin, TestCase): + """ + Tests for how get_user_orgs and can_view_taxonomy resolve orgs granted through openedx-authz + courses.manage_tags, which have no legacy CourseAccessRole equivalent. + """ + + def setUp(self): + super().setUp() + self.superuser = User.objects.create(username="superuser", email="superuser@example.com", is_superuser=True) + self.authz_user = User.objects.create(username="authz_user", email="authz_user@example.com") + self.legacy_user = User.objects.create(username="legacy_user", email="legacy_user@example.com") + # org1's short_name ("OeX") matches this course's org. + self.course_key = CourseLocator.from_string("course-v1:OeX+DemoX+Demo_Course") + self.org2_only_taxonomy = api.create_taxonomy(name="Axim only") + api.set_taxonomy_orgs(self.org2_only_taxonomy, orgs=[self.org2]) + + @staticmethod + def _course_scope(course_key): + return CourseOverviewData(external_key=str(course_key)) + + @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_course_scope_grants_its_org(self, mock_get_scopes): + mock_get_scopes.return_value = [self._course_scope(self.course_key)] + assert get_user_orgs(self.authz_user) == [self.org1] + + @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=False) + def test_course_scope_ignored_when_course_not_switched(self, mock_get_scopes): + mock_get_scopes.return_value = [self._course_scope(self.course_key)] + assert not get_user_orgs(self.authz_user) + + @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_org_glob_scope_grants_its_org(self, mock_get_scopes): + mock_get_scopes.return_value = [OrgCourseOverviewGlobData(external_key="course-v1:OeX+*")] + assert get_user_orgs(self.authz_user) == [self.org1] + + @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=False) + def test_org_glob_scope_ignored_when_flag_off(self, mock_get_scopes): + mock_get_scopes.return_value = [OrgCourseOverviewGlobData(external_key="course-v1:OeX+*")] + assert not get_user_orgs(self.authz_user) + + @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_platform_glob_scope_grants_all_orgs(self, mock_get_scopes): + mock_get_scopes.return_value = [PlatformCourseOverviewGlobData(external_key="course-v1:*")] + assert set(get_user_orgs(self.authz_user)) == {self.org1, self.org2} + + def test_flag_checked_once_per_org(self, mock_get_scopes): + mock_get_scopes.return_value = [ + self._course_scope(CourseLocator.from_string("course-v1:OeX+A+1")), + self._course_scope(CourseLocator.from_string("course-v1:OeX+B+2")), + self._course_scope(CourseLocator.from_string("course-v1:OeX+C+3")), + ] + with patch("openedx.core.djangoapps.content_tagging.rules.enable_authz_course_authoring") as mock_flag: + mock_flag.return_value = True + assert get_user_orgs(self.authz_user) == [self.org1] + assert mock_flag.call_count == 1 + + @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_can_view_taxonomy_for_authz_only_user(self, mock_get_scopes): + """The per-taxonomy check agrees with the list filter: authz-only users can open their org's taxonomies.""" + mock_get_scopes.return_value = [self._course_scope(self.course_key)] + assert can_view_taxonomy(self.authz_user, self.taxonomy_one_org) + assert can_view_taxonomy(self.authz_user, self.taxonomy_both_orgs) + assert not can_view_taxonomy(self.authz_user, self.org2_only_taxonomy) + + @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=False) + def test_legacy_course_role_counts_before_course_switches(self, mock_get_scopes): + mock_get_scopes.return_value = [] + add_users(self.superuser, CourseStaffRole(self.course_key), self.legacy_user) + assert get_user_orgs(self.legacy_user) == [self.org1] + + def test_legacy_course_role_ignored_once_course_switches(self, mock_get_scopes): + """ + Legacy course staff with no authz role in a switched course don't see the org's taxonomies, matching + can_change_object_tag_objectid, which requires courses.manage_tags there. + """ + mock_get_scopes.return_value = [] + with override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=False): + add_users(self.superuser, CourseStaffRole(self.course_key), self.legacy_user) + with override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True): + legacy_user = User.objects.get(pk=self.legacy_user.pk) # fresh instance, no cached role cache + assert not get_user_orgs(legacy_user) + assert not can_view_taxonomy(legacy_user, self.taxonomy_one_org) + + def test_legacy_course_role_kept_with_authz_manage_tags(self, mock_get_scopes): + """A switched-course staff member who also holds manage_tags through authz still resolves the org.""" + mock_get_scopes.return_value = [self._course_scope(self.course_key)] + with override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=False): + add_users(self.superuser, CourseStaffRole(self.course_key), self.legacy_user) + with override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True): + legacy_user = User.objects.get(pk=self.legacy_user.pk) + assert get_user_orgs(legacy_user) == [self.org1] From 45c6207084a23c76615e7cbfa872a71e70d2b637 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 6 Oct 2026 17:50:16 -0500 Subject: [PATCH 4/9] perf: cache openedx-authz manage_tags scopes per request in tagging rules get_user_orgs is evaluated once per taxonomy, and each call re-queried the authz policy store for the user's courses.manage_tags scopes. Cache the scopes in the tagging rules request cache and update the query counts asserted in the content_tagging view tests. Co-Authored-By: Claude Sonnet 5.5 --- .../rest_api/v1/tests/test_views.py | 24 +++++++++---------- .../core/djangoapps/content_tagging/rules.py | 4 +--- .../content_tagging/tests/test_rules.py | 2 ++ .../core/djangoapps/content_tagging/utils.py | 22 +++++++++++++++++ 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 3e920cbe2436..91494a696b7e 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -593,8 +593,8 @@ def test_create_taxonomy_rejects_invalid_type(self) -> None: ("library_staffA", 23), ("library_userA", 23), ("instructorA", 23), - ("course_instructorA", 23), - ("course_staffA", 23), + ("course_instructorA", 25), + ("course_staffA", 25), ) @ddt.unpack def test_list_taxonomy_query_count(self, user_attr: str, expected_queries: int): @@ -2069,16 +2069,16 @@ def test_get_copied_tags(self): ('staff', 'courseA', 10), ('staff', 'libraryA', 13), ('staff', 'collection_key', 13), - ("content_creatorA", 'courseA', 14, False), - ("content_creatorA", 'libraryA', 17, False), - ("content_creatorA", 'collection_key', 17, False), - ("library_staffA", 'libraryA', 17, False), # Library users can only view objecttags, not change them? - ("library_staffA", 'collection_key', 17, False), - ("library_userA", 'libraryA', 17, False), - ("library_userA", 'collection_key', 17, False), - ("instructorA", 'courseA', 14), - ("course_instructorA", 'courseA', 14), - ("course_staffA", 'courseA', 14), + ("content_creatorA", 'courseA', 15, False), + ("content_creatorA", 'libraryA', 18, False), + ("content_creatorA", 'collection_key', 18, False), + ("library_staffA", 'libraryA', 18, False), # Library users can only view objecttags, not change them? + ("library_staffA", 'collection_key', 18, False), + ("library_userA", 'libraryA', 18, False), + ("library_userA", 'collection_key', 18, False), + ("instructorA", 'courseA', 15), + ("course_instructorA", 'courseA', 15), + ("course_staffA", 'courseA', 15), ) @ddt.unpack def test_object_tags_query_count( diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index 962e3d3bb7d3..90fad55c720d 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -167,9 +167,7 @@ def _get_authz_manage_tags_orgs(user: UserType, orgs: list[Organization]) -> lis if not orgs: return [] - scopes = authz_api.get_scopes_for_user_and_permission( - user.username, authz_permissions.COURSES_MANAGE_TAGS.identifier - ) + scopes = rules_cache.get_authz_manage_tags_scopes(user) org_names = set() all_orgs = False diff --git a/openedx/core/djangoapps/content_tagging/tests/test_rules.py b/openedx/core/djangoapps/content_tagging/tests/test_rules.py index e45cdb3de5f0..ea9e2923790a 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_rules.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_rules.py @@ -18,6 +18,7 @@ from .. import api from ..rules import can_change_object_tag_objectid, can_remove_object_tag_objectid, can_view_taxonomy, get_user_orgs +from ..utils import rules_cache from .test_api import TestTaxonomyMixin User = get_user_model() @@ -862,6 +863,7 @@ class TestRulesAuthzManageTagsOrgs(TestTaxonomyMixin, TestCase): def setUp(self): super().setUp() + rules_cache.clear() # authz scopes are cached per request self.superuser = User.objects.create(username="superuser", email="superuser@example.com", is_superuser=True) self.authz_user = User.objects.create(username="authz_user", email="authz_user@example.com") self.legacy_user = User.objects.create(username="legacy_user", email="legacy_user@example.com") diff --git a/openedx/core/djangoapps/content_tagging/utils.py b/openedx/core/djangoapps/content_tagging/utils.py index 658f23d16b5d..d5cb149a5223 100644 --- a/openedx/core/djangoapps/content_tagging/utils.py +++ b/openedx/core/djangoapps/content_tagging/utils.py @@ -131,5 +131,27 @@ def get_library_orgs(self, user, org_names: list[str]) -> list[Organization]: library_orgs[org_name] for org_name in org_names if org_name in library_orgs ] + def get_authz_manage_tags_scopes(self, user) -> list: + """ + Returns the openedx-authz scopes where the given user holds courses.manage_tags. + + Cached for the duration of the request: org membership is evaluated once per taxonomy, + and each lookup would otherwise hit the authz policy store again. + """ + # Import here to keep openedx-authz out of this module's import-time dependencies. + from openedx_authz import api as authz_api + from openedx_authz.constants import permissions as authz_permissions + + cache_key = f'authz_manage_tags_scopes:{user.username}' + scopes = self.request_cache.data.get(cache_key) + if scopes is None: + scopes = list( + authz_api.get_scopes_for_user_and_permission( + user.username, authz_permissions.COURSES_MANAGE_TAGS.identifier + ) + ) + self.request_cache.set(cache_key, scopes) + return scopes + rules_cache = TaggingRulesCache() From 546b7a596d655ade135fcf0583d0f60200e7148e Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Wed, 14 Oct 2026 18:35:04 -0500 Subject: [PATCH 5/9] test: add docstrings to openedx-authz manage_tags org tests Co-Authored-By: Claude Sonnet 5.5 --- .../core/djangoapps/content_tagging/tests/test_rules.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_rules.py b/openedx/core/djangoapps/content_tagging/tests/test_rules.py index ea9e2923790a..9dcd3ceaa816 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_rules.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_rules.py @@ -874,34 +874,41 @@ def setUp(self): @staticmethod def _course_scope(course_key): + """Build the authz course scope for the given course key.""" return CourseOverviewData(external_key=str(course_key)) @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) def test_course_scope_grants_its_org(self, mock_get_scopes): + """A course-level manage_tags grant resolves to that course's org once the course has switched to authz.""" mock_get_scopes.return_value = [self._course_scope(self.course_key)] assert get_user_orgs(self.authz_user) == [self.org1] @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=False) def test_course_scope_ignored_when_course_not_switched(self, mock_get_scopes): + """A course-level grant is ignored while the course still uses legacy access.""" mock_get_scopes.return_value = [self._course_scope(self.course_key)] assert not get_user_orgs(self.authz_user) @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) def test_org_glob_scope_grants_its_org(self, mock_get_scopes): + """An org-wide manage_tags grant resolves to that org when authz is enabled.""" mock_get_scopes.return_value = [OrgCourseOverviewGlobData(external_key="course-v1:OeX+*")] assert get_user_orgs(self.authz_user) == [self.org1] @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=False) def test_org_glob_scope_ignored_when_flag_off(self, mock_get_scopes): + """An org-wide grant is ignored when authz is not enabled.""" mock_get_scopes.return_value = [OrgCourseOverviewGlobData(external_key="course-v1:OeX+*")] assert not get_user_orgs(self.authz_user) @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) def test_platform_glob_scope_grants_all_orgs(self, mock_get_scopes): + """A platform-wide manage_tags grant resolves to every org when authz is enabled.""" mock_get_scopes.return_value = [PlatformCourseOverviewGlobData(external_key="course-v1:*")] assert set(get_user_orgs(self.authz_user)) == {self.org1, self.org2} def test_flag_checked_once_per_org(self, mock_get_scopes): + """The authz flag is looked up once per org, not once per course scope in that org.""" mock_get_scopes.return_value = [ self._course_scope(CourseLocator.from_string("course-v1:OeX+A+1")), self._course_scope(CourseLocator.from_string("course-v1:OeX+B+2")), @@ -922,6 +929,7 @@ def test_can_view_taxonomy_for_authz_only_user(self, mock_get_scopes): @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=False) def test_legacy_course_role_counts_before_course_switches(self, mock_get_scopes): + """Legacy course staff still resolve the org while the course has not switched to authz.""" mock_get_scopes.return_value = [] add_users(self.superuser, CourseStaffRole(self.course_key), self.legacy_user) assert get_user_orgs(self.legacy_user) == [self.org1] From 261814883d2419c0dc8e5c2da302fb4d5ee42cc6 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Wed, 14 Oct 2026 18:35:52 -0500 Subject: [PATCH 6/9] refactor: drop redundant per-call authz flag cache in tagging rules Course waffle flag overrides are already request-cached, so the local dict in _get_course_user_orgs added nothing. Co-Authored-By: Claude Sonnet 5.5 --- openedx/core/djangoapps/content_tagging/rules.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index 90fad55c720d..83503976e87c 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -83,21 +83,16 @@ def _get_course_user_orgs(user: UserType, orgs: list[Organization]) -> list[Orga if not orgs: return [] - authz_course_cache: dict[str, bool] = {} - def course_uses_authz(course_id) -> bool: """ True if the course has switched to openedx-authz, where legacy course roles are no longer authoritative: tagging there requires courses.manage_tags (see can_change_object_tag_objectid), so those users are picked up by _get_authz_manage_tags_orgs instead. """ - key = str(course_id) - if key not in authz_course_cache: - try: - authz_course_cache[key] = enable_authz_course_authoring(CourseKey.from_string(key)) - except InvalidKeyError: - authz_course_cache[key] = False - return authz_course_cache[key] + try: + return enable_authz_course_authoring(CourseKey.from_string(str(course_id))) + except InvalidKeyError: + return False def user_has_role_ignore_course_id(user, role_name, org_name) -> bool: """ From e5bf6cdbb3a4f2db79a9c07f74e97520c791d088 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Wed, 14 Oct 2026 18:37:30 -0500 Subject: [PATCH 7/9] refactor: move authz scope-to-course-key expansion to openedx.core.djangoapps.authz Make the logic of _get_course_keys_from_scopes reusable outside of cms so other apps (e.g. content_tagging) can share the same scope semantics. Co-Authored-By: Claude Sonnet 5.5 --- .../contentstore/tests/test_course_listing.py | 8 +- cms/djangoapps/contentstore/views/course.py | 79 +----------------- openedx/core/djangoapps/authz/scopes.py | 81 +++++++++++++++++++ 3 files changed, 87 insertions(+), 81 deletions(-) create mode 100644 openedx/core/djangoapps/authz/scopes.py diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index e209be33ff30..4c7e3a8d947a 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -23,7 +23,6 @@ _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 @@ -39,6 +38,7 @@ ) from common.djangoapps.student.tests.factories import UserFactory from openedx.core import toggles as core_toggles +from openedx.core.djangoapps.authz.scopes import get_course_keys_from_scopes 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 @@ -976,7 +976,7 @@ 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([ + course_keys = get_course_keys_from_scopes([ PlatformCourseOverviewGlobData(external_key=PlatformCourseOverviewGlobData.build_external_key()) ]) @@ -995,7 +995,7 @@ 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([ + course_keys = get_course_keys_from_scopes([ PlatformCourseOverviewGlobData(external_key=PlatformCourseOverviewGlobData.build_external_key()) ]) @@ -1014,7 +1014,7 @@ def test_get_course_keys_from_scopes_platform_scope_short_circuits(self): "is_enabled", side_effect=self._mock_authz_toggle(enabled_keys), ): - course_keys = _get_course_keys_from_scopes( + course_keys = get_course_keys_from_scopes( [ OrgCourseOverviewGlobData(external_key=OrgCourseOverviewGlobData.build_external_key("Org1")), PlatformCourseOverviewGlobData(external_key=PlatformCourseOverviewGlobData.build_external_key()), diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 300bed93ef5a..9cb5fbfaf351 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -30,12 +30,6 @@ 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, - PlatformCourseOverviewGlobData, - ScopeData, -) from openedx_authz.constants.permissions import ( COURSES_MANAGE_COURSE_UPDATES, COURSES_MANAGE_GROUP_CONFIGURATIONS, @@ -76,7 +70,6 @@ GlobalStaff, OrgStaffRole, UserBasedRole, - enable_authz_course_authoring, strict_role_checking, ) from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json @@ -84,6 +77,7 @@ 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.authz.scopes import get_course_keys_from_scopes from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements from openedx.core.djangoapps.models.course_details import CourseDetails @@ -847,75 +841,6 @@ def filter_course(course): return filter(filter_course, filtered_courses) -def _get_course_keys_for_org_scope(org_keys: set[str]): - """ - Convert a set of organization keys into specific course keys. - """ - - return CourseOverview.get_all_courses(orgs=org_keys).values_list('id', flat=True) - - -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 enable_authz_course_authoring(course_key)} - - -def _get_course_keys_from_scopes(authz_scopes: list[ScopeData]) -> set[CourseKey]: - """ - 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 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 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 @@ -927,7 +852,7 @@ def _get_authz_accessible_courses_list(request): COURSES_VIEW_COURSE.identifier ) - return _get_course_keys_from_scopes(authz_scopes) + return get_course_keys_from_scopes(authz_scopes) def _get_legacy_accessible_courses_list(request: HttpRequest) -> set[CourseKey]: diff --git a/openedx/core/djangoapps/authz/scopes.py b/openedx/core/djangoapps/authz/scopes.py new file mode 100644 index 000000000000..57835bb8d357 --- /dev/null +++ b/openedx/core/djangoapps/authz/scopes.py @@ -0,0 +1,81 @@ +"""Helpers for resolving openedx-authz scopes into concrete course keys.""" +from opaque_keys.edx.keys import CourseKey +from openedx_authz.api.data import ( + CourseOverviewData, + OrgCourseOverviewGlobData, + PlatformCourseOverviewGlobData, + ScopeData, +) + +from common.djangoapps.student.roles import enable_authz_course_authoring +from openedx.core import toggles as core_toggles +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview + + +def _get_course_keys_for_org_scope(org_keys: set[str]): + """ + Convert a set of organization keys into specific course keys. + """ + + return CourseOverview.get_all_courses(orgs=org_keys).values_list('id', flat=True) + + +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 enable_authz_course_authoring(course_key)} + + +def get_course_keys_from_scopes(authz_scopes: list[ScopeData]) -> set[CourseKey]: + """ + 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 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 enable_authz_course_authoring(key) + ) + + return course_keys From 0f43c717b1b1913ea6910c75c9b0c562bff58276 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Wed, 14 Oct 2026 18:38:02 -0500 Subject: [PATCH 8/9] refactor: reuse shared authz scope expansion for tagging org membership Resolve courses.manage_tags scopes with get_course_keys_from_scopes instead of a bespoke expansion, so tagging follows the same course, org and platform scope semantics as the rest of the authz-aware code. Co-Authored-By: Claude Sonnet 5.5 --- .../core/djangoapps/content_tagging/rules.py | 34 +++---------------- .../content_tagging/tests/test_rules.py | 20 ++++------- .../core/djangoapps/content_tagging/utils.py | 29 +++++++++------- 3 files changed, 29 insertions(+), 54 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index 83503976e87c..8b0e11ba549f 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -11,7 +11,6 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_authz import api as authz_api -from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData, PlatformCourseOverviewGlobData from openedx_authz.constants import permissions as authz_permissions from organizations.models import Organization @@ -154,41 +153,18 @@ def _get_authz_manage_tags_orgs(user: UserType, orgs: list[Organization]) -> lis course_staff, and course_admin only exist in openedx-authz, with no legacy CourseAccessRole for the legacy helpers to find. - Course scopes only count if that course has switched to openedx-authz: a policy assignment - can exist ahead of the course's own toggle, and until it flips, legacy access stays - authoritative for it (see should_use_course_authz_for_object). Org-wide and platform-wide - glob scopes have no single course to check, so they count when the flag is on globally. + The user's scopes (course, org-wide and platform-wide) are expanded into course keys, keeping only + the courses that have switched to openedx-authz: a policy assignment can exist ahead of the course's + own toggle, and until it flips, legacy access stays authoritative for it (see + should_use_course_authz_for_object). """ if not orgs: return [] - scopes = rules_cache.get_authz_manage_tags_scopes(user) - - org_names = set() - all_orgs = False - glob_authz_enabled = None - for scope in scopes: - if isinstance(scope, CourseOverviewData): - # Skip the per-course flag lookup when the org is already known. - if scope.org not in org_names and enable_authz_course_authoring(scope.course_key): - org_names.add(scope.org) - elif isinstance(scope, OrgCourseOverviewGlobData | PlatformCourseOverviewGlobData): - if glob_authz_enabled is None: - glob_authz_enabled = enable_authz_course_authoring() - if not glob_authz_enabled: - continue - if isinstance(scope, PlatformCourseOverviewGlobData): - all_orgs = True - break - if scope.org: - org_names.add(scope.org) - - if all_orgs: - return list(orgs) + org_names = rules_cache.get_authz_manage_tags_org_names(user) return [org for org in orgs if org.short_name in org_names] -@rules.predicate def can_create_taxonomy(user: UserType) -> bool: """ Returns True if the given user can create a taxonomy. diff --git a/openedx/core/djangoapps/content_tagging/tests/test_rules.py b/openedx/core/djangoapps/content_tagging/tests/test_rules.py index 9dcd3ceaa816..1a563ab8879d 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_rules.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_rules.py @@ -14,6 +14,7 @@ from common.djangoapps.student.auth import add_users, update_org_role from common.djangoapps.student.roles import CourseStaffRole, OrgStaffRole +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.toggles import AUTHZ_COURSE_AUTHORING_FLAG from .. import api @@ -892,33 +893,26 @@ def test_course_scope_ignored_when_course_not_switched(self, mock_get_scopes): @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) def test_org_glob_scope_grants_its_org(self, mock_get_scopes): """An org-wide manage_tags grant resolves to that org when authz is enabled.""" + CourseOverviewFactory(id=self.course_key) + CourseOverviewFactory(id=CourseLocator(self.org2.short_name, "DemoX", "Demo_Course")) mock_get_scopes.return_value = [OrgCourseOverviewGlobData(external_key="course-v1:OeX+*")] assert get_user_orgs(self.authz_user) == [self.org1] @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=False) def test_org_glob_scope_ignored_when_flag_off(self, mock_get_scopes): """An org-wide grant is ignored when authz is not enabled.""" + CourseOverviewFactory(id=self.course_key) mock_get_scopes.return_value = [OrgCourseOverviewGlobData(external_key="course-v1:OeX+*")] assert not get_user_orgs(self.authz_user) @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) def test_platform_glob_scope_grants_all_orgs(self, mock_get_scopes): - """A platform-wide manage_tags grant resolves to every org when authz is enabled.""" + """A platform-wide manage_tags grant resolves to the orgs of all courses when authz is enabled.""" + CourseOverviewFactory(id=self.course_key) + CourseOverviewFactory(id=CourseLocator(self.org2.short_name, "DemoX", "Demo_Course")) mock_get_scopes.return_value = [PlatformCourseOverviewGlobData(external_key="course-v1:*")] assert set(get_user_orgs(self.authz_user)) == {self.org1, self.org2} - def test_flag_checked_once_per_org(self, mock_get_scopes): - """The authz flag is looked up once per org, not once per course scope in that org.""" - mock_get_scopes.return_value = [ - self._course_scope(CourseLocator.from_string("course-v1:OeX+A+1")), - self._course_scope(CourseLocator.from_string("course-v1:OeX+B+2")), - self._course_scope(CourseLocator.from_string("course-v1:OeX+C+3")), - ] - with patch("openedx.core.djangoapps.content_tagging.rules.enable_authz_course_authoring") as mock_flag: - mock_flag.return_value = True - assert get_user_orgs(self.authz_user) == [self.org1] - assert mock_flag.call_count == 1 - @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) def test_can_view_taxonomy_for_authz_only_user(self, mock_get_scopes): """The per-taxonomy check agrees with the list filter: authz-only users can open their org's taxonomies.""" diff --git a/openedx/core/djangoapps/content_tagging/utils.py b/openedx/core/djangoapps/content_tagging/utils.py index d5cb149a5223..8de70c71c990 100644 --- a/openedx/core/djangoapps/content_tagging/utils.py +++ b/openedx/core/djangoapps/content_tagging/utils.py @@ -131,27 +131,32 @@ def get_library_orgs(self, user, org_names: list[str]) -> list[Organization]: library_orgs[org_name] for org_name in org_names if org_name in library_orgs ] - def get_authz_manage_tags_scopes(self, user) -> list: + def get_authz_manage_tags_org_names(self, user) -> set[str]: """ - Returns the openedx-authz scopes where the given user holds courses.manage_tags. + Returns the short names of the orgs of the courses where the given user holds courses.manage_tags + through openedx-authz. + + Scopes are expanded with get_course_keys_from_scopes, the same way other authz-aware course + listings do, so only courses that have switched to openedx-authz are taken into account. Cached for the duration of the request: org membership is evaluated once per taxonomy, - and each lookup would otherwise hit the authz policy store again. + and each lookup would otherwise hit the authz policy store and the course overviews again. """ # Import here to keep openedx-authz out of this module's import-time dependencies. from openedx_authz import api as authz_api from openedx_authz.constants import permissions as authz_permissions - cache_key = f'authz_manage_tags_scopes:{user.username}' - scopes = self.request_cache.data.get(cache_key) - if scopes is None: - scopes = list( - authz_api.get_scopes_for_user_and_permission( - user.username, authz_permissions.COURSES_MANAGE_TAGS.identifier - ) + from openedx.core.djangoapps.authz.scopes import get_course_keys_from_scopes + + cache_key = f'authz_manage_tags_org_names:{user.username}' + org_names = self.request_cache.data.get(cache_key) + if org_names is None: + scopes = authz_api.get_scopes_for_user_and_permission( + user.username, authz_permissions.COURSES_MANAGE_TAGS.identifier ) - self.request_cache.set(cache_key, scopes) - return scopes + org_names = {course_key.org for course_key in get_course_keys_from_scopes(list(scopes))} + self.request_cache.set(cache_key, org_names) + return org_names rules_cache = TaggingRulesCache() From cd51cf99481bb9aeb942024572f6d8f0efdcb6db Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Thu, 15 Oct 2026 13:56:54 -0500 Subject: [PATCH 9/9] test: set CourseOverview org explicitly in authz org glob tests CourseOverviewFactory defaults the org column to "edX" regardless of the id, so org-wide scope expansion, which filters on that column, found no courses. Co-Authored-By: Claude Sonnet 5.5 --- .../djangoapps/content_tagging/tests/test_rules.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_rules.py b/openedx/core/djangoapps/content_tagging/tests/test_rules.py index 1a563ab8879d..1567648dcfec 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_rules.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_rules.py @@ -893,23 +893,27 @@ def test_course_scope_ignored_when_course_not_switched(self, mock_get_scopes): @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) def test_org_glob_scope_grants_its_org(self, mock_get_scopes): """An org-wide manage_tags grant resolves to that org when authz is enabled.""" - CourseOverviewFactory(id=self.course_key) - CourseOverviewFactory(id=CourseLocator(self.org2.short_name, "DemoX", "Demo_Course")) + CourseOverviewFactory(id=self.course_key, org=self.course_key.org) + CourseOverviewFactory( + id=CourseLocator(self.org2.short_name, "DemoX", "Demo_Course"), org=self.org2.short_name + ) mock_get_scopes.return_value = [OrgCourseOverviewGlobData(external_key="course-v1:OeX+*")] assert get_user_orgs(self.authz_user) == [self.org1] @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=False) def test_org_glob_scope_ignored_when_flag_off(self, mock_get_scopes): """An org-wide grant is ignored when authz is not enabled.""" - CourseOverviewFactory(id=self.course_key) + CourseOverviewFactory(id=self.course_key, org=self.course_key.org) mock_get_scopes.return_value = [OrgCourseOverviewGlobData(external_key="course-v1:OeX+*")] assert not get_user_orgs(self.authz_user) @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True) def test_platform_glob_scope_grants_all_orgs(self, mock_get_scopes): """A platform-wide manage_tags grant resolves to the orgs of all courses when authz is enabled.""" - CourseOverviewFactory(id=self.course_key) - CourseOverviewFactory(id=CourseLocator(self.org2.short_name, "DemoX", "Demo_Course")) + CourseOverviewFactory(id=self.course_key, org=self.course_key.org) + CourseOverviewFactory( + id=CourseLocator(self.org2.short_name, "DemoX", "Demo_Course"), org=self.org2.short_name + ) mock_get_scopes.return_value = [PlatformCourseOverviewGlobData(external_key="course-v1:*")] assert set(get_user_orgs(self.authz_user)) == {self.org1, self.org2}