From 2e46a2227dcb602b56291f3f7d0ce3bba6126162 Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Mon, 6 Apr 2026 18:08:30 -0600 Subject: [PATCH 1/6] feat: enforce openedx-authz permissions on object tag and xblock outline endpoints When AUTHZ_COURSE_AUTHORING_FLAG is enabled for a course, legacy permission checks are fully bypassed in favor of openedx-authz: - PUT /object_tags/{object_id}/: enforces courses.manage_tags - GET /object_tags/{object_id}/: patches can_tag_object in response - GET /xblock/outline/{usage_key}: enforces courses.view_course For object tag endpoints, the parent ObjectTagView legacy checks (ObjectTagObjectPermissions, per-taxonomy can_tag_object, and view_objecttag in get_queryset) are all bypassed via get_permissions, get_queryset, and _update_tags overrides. Course key is extracted from any content key type via get_context_key_from_key_string. Library content is unaffected (falls through to legacy). When the flag is off, all endpoints use existing legacy behavior. Co-authored-by: Kiro --- cms/djangoapps/contentstore/views/block.py | 10 +- .../contentstore/views/tests/test_block.py | 67 +++++++++- .../rest_api/v1/tests/test_views.py | 116 +++++++++++++++- .../content_tagging/rest_api/v1/views.py | 126 +++++++++++++++++- 4 files changed, 308 insertions(+), 11 deletions(-) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index e606477ee222..4d60640cb2cf 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -12,6 +12,7 @@ from django.views.decorators.clickjacking import xframe_options_exempt from django.views.decorators.http import require_http_methods from opaque_keys.edx.keys import CourseKey +from openedx_authz.constants.permissions import COURSES_VIEW_COURSE from web_fragments.fragment import Fragment from cms.djangoapps.contentstore.utils import load_services_for_studio @@ -27,6 +28,8 @@ from common.djangoapps.edxmako.shortcuts import render_to_response, render_to_string from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access from common.djangoapps.util.json_request import JsonResponse, expect_json +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission +from openedx.core.djangoapps.authz.decorators import user_has_course_permission from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled from openedx.core.lib.xblock_utils import hash_resource, request_token, wrap_xblock, wrap_xblock_aside from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -329,7 +332,12 @@ def xblock_outline_handler(request, usage_key_string): a course. """ usage_key = usage_key_with_run(usage_key_string) - if not has_studio_read_access(request.user, usage_key.course_key): + if not user_has_course_permission( + request.user, + COURSES_VIEW_COURSE.identifier, + usage_key.course_key, + LegacyAuthoringPermission.READ, + ): raise PermissionDenied() response_format = request.GET.get("format", "html") diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index bb1206169189..dbaa91861a2d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -19,10 +19,12 @@ from opaque_keys.edx.asides import AsideUsageKeyV2 from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator +from openedx_authz.constants.roles import COURSE_STAFF from openedx_events.content_authoring.data import DuplicatedXBlockData from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED from openedx_events.testing import OpenEdxEventsTestMixin from pytz import UTC +from rest_framework.test import APITestCase from web_fragments.fragment import Fragment from webob import Response from xblock.core import XBlockAside @@ -54,14 +56,20 @@ from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from common.test.utils import assert_dict_contains_subset from lms.djangoapps.lms_xblock.mixin import NONSENSICAL_ACCESS_RESTRICTION +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthzTestMixin from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE +from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.course_block import DEFAULT_START_DATE from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError -from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import ( + TEST_DATA_SPLIT_MODULESTORE, + ModuleStoreTestCase, + SharedModuleStoreTestCase, +) from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, LibraryFactory, check_mongo_calls from xmodule.partitions.partitions import ( ENROLLMENT_TRACK_PARTITION_ID, @@ -4615,3 +4623,60 @@ def test_xblock_edit_view_contains_resources(self): self.assertGreater(len(resource_links), 0, f"No CSS resources found in HTML. Found: {resource_links}") # noqa: PT009 # pylint: disable=line-too-long self.assertGreater(len(script_sources), 0, f"No JS resources found in HTML. Found: {script_sources}") # noqa: PT009 # pylint: disable=line-too-long + + + +@skip_unless_cms +class XBlockOutlineHandlerAuthzTest(CourseAuthzTestMixin, SharedModuleStoreTestCase, APITestCase): + """ + Tests xblock_outline_handler authorization via openedx-authz. + + When the AUTHZ_COURSE_AUTHORING_FLAG is enabled, the endpoint should + enforce courses.view_course via openedx-authz instead of legacy + has_studio_read_access. + """ + + authz_roles_to_assign = [COURSE_STAFF.external_key] + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.password = 'test' + cls.course = CourseFactory.create() + cls.course_key = cls.course.id + cls.staff = StaffFactory(course_key=cls.course_key, password=cls.password) + cls.chapter = BlockFactory.create( + parent_location=cls.course.location, + category="chapter", + display_name="Week 1", + user_id=cls.staff.id, + ) + + def _get_outline_url(self): + return reverse_usage_url("xblock_outline_handler", self.chapter.location) + + def test_authorized_user_can_access(self): + """User with COURSE_STAFF role can access.""" + self.authorized_client.login(username=self.authorized_user.username, password=self.password) + resp = self.authorized_client.get(self._get_outline_url(), HTTP_ACCEPT="application/json") + assert resp.status_code == 200 + + def test_unauthorized_user_cannot_access(self): + """User without role cannot access.""" + self.unauthorized_client.login(username=self.unauthorized_user.username, password=self.password) + resp = self.unauthorized_client.get(self._get_outline_url(), HTTP_ACCEPT="application/json") + assert resp.status_code == 403 + + def test_role_scoped_to_course(self): + """Authorization should only apply to the assigned course.""" + other_course = self.store.create_course("OtherOrg", "OtherCourse", "Run", self.staff.id) + other_chapter = BlockFactory.create( + parent_location=other_course.location, + category="chapter", + display_name="Other Week", + user_id=self.staff.id, + ) + url = reverse_usage_url("xblock_outline_handler", other_chapter.location) + self.authorized_client.login(username=self.authorized_user.username, password=self.password) + resp = self.authorized_client.get(url, HTTP_ACCEPT="application/json") + assert resp.status_code == 403 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 35f209f8a7e1..696530ec139c 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 @@ -1979,19 +1979,19 @@ def test_get_copied_tags(self): assert response.data[str(object_id_2)]["taxonomies"] == expected_tags @ddt.data( - ('staff', 'courseA', 8), + ('staff', 'courseA', 10), ('staff', 'libraryA', 17), ('staff', 'collection_key', 17), - ("content_creatorA", 'courseA', 18, False), + ("content_creatorA", 'courseA', 20, False), ("content_creatorA", 'libraryA', 23, False), ("content_creatorA", 'collection_key', 23, False), ("library_staffA", 'libraryA', 23, False), # Library users can only view objecttags, not change them? ("library_staffA", 'collection_key', 23, False), ("library_userA", 'libraryA', 23, False), ("library_userA", 'collection_key', 23, False), - ("instructorA", 'courseA', 18), - ("course_instructorA", 'courseA', 18), - ("course_staffA", 'courseA', 18), + ("instructorA", 'courseA', 20), + ("course_instructorA", 'courseA', 20), + ("course_staffA", 'courseA', 20), ) @ddt.unpack def test_object_tags_query_count( @@ -2136,6 +2136,112 @@ def test_superuser_allowed(self): resp = client.get(self.get_url(self.course_key)) self.assertEqual(resp.status_code, status.HTTP_200_OK) # noqa: PT009 +@skip_unless_cms +class TestObjectTagUpdateWithAuthz(CourseAuthzTestMixin, SharedModuleStoreTestCase, APITestCase): + """ + Tests object tag endpoints with openedx-authz. + + When the AUTHZ_COURSE_AUTHORING_FLAG is enabled for a course, + PUT /object_tags/{object_id}/ should enforce courses.manage_tags, + and GET /object_tags/{object_id}/ should return authz-aware + can_tag_object values. + + When authz is active, the parent's legacy permission checks + (ObjectTagObjectPermissions and per-taxonomy can_tag_object) are + bypassed entirely — permissions are enforced solely via openedx-authz. + """ + + authz_roles_to_assign = [COURSE_STAFF.external_key] + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.password = 'test' + cls.course = CourseFactory.create() + cls.course_key = cls.course.id + cls.staff = StaffFactory(course_key=cls.course_key, password=cls.password) + + def setUp(self): + super().setUp() + self.taxonomy = tagging_api.create_taxonomy(name="Test Taxonomy") + tagging_api.set_taxonomy_orgs(self.taxonomy, all_orgs=True, orgs=[]) + Tag.objects.create(taxonomy=self.taxonomy, value="Tag 1") + + def _put_tags(self, client): + """Helper to PUT tags on the course.""" + url = OBJECT_TAG_UPDATE_URL.format(object_id=self.course_key) + return client.put( + url, + {"tagsData": [{"taxonomy": self.taxonomy.id, "tags": ["Tag 1"]}]}, + format="json", + ) + + def _make_auditor_client(self, course_key=None): + """ + Create a user with authz course_auditor role (no manage_tags permission). + Since get_permissions() replaces legacy checks with IsAuthenticated when + authz is active, no legacy role is needed to pass dispatch. + """ + from openedx_authz.constants.roles import COURSE_AUDITOR # pylint: disable=import-outside-toplevel + + course_key = course_key or self.course_key + auditor = UserFactory(password=self.password) + self.add_user_to_role_in_course(auditor, COURSE_AUDITOR.external_key, course_key) + client = APIClient() + client.force_authenticate(user=auditor) + return client + + def test_update_object_tags_authorized(self): + """Authorized user can update object tags.""" + assert self._put_tags(self.authorized_client).status_code == status.HTTP_200_OK + + def test_update_object_tags_denied_by_authz(self): + """User with legacy access but no authz manage_tags is denied.""" + auditor_client = self._make_auditor_client() + assert self._put_tags(auditor_client).status_code == status.HTTP_403_FORBIDDEN + + def test_update_object_tags_scoped_to_course(self): + """Authorized user for one course cannot tag objects in another course.""" + other_course = self.store.create_course("OtherOrg", "OtherCourse", "Run", self.staff.id) + url = OBJECT_TAG_UPDATE_URL.format(object_id=other_course.id) + resp = self.authorized_client.put( + url, + {"tagsData": [{"taxonomy": self.taxonomy.id, "tags": ["Tag 1"]}]}, + format="json", + ) + assert resp.status_code == status.HTTP_403_FORBIDDEN + + def test_superuser_allowed(self): + """Superusers should always be allowed.""" + superuser = UserFactory(is_superuser=True) + client = APIClient() + client.force_authenticate(user=superuser) + assert self._put_tags(client).status_code == status.HTTP_200_OK + + def test_retrieve_can_tag_object_authorized(self): + """Authorized user sees can_tag_object=true and can_delete_objecttag=true.""" + self._put_tags(self.authorized_client) # ensure tags exist + url = OBJECT_TAGS_URL.format(object_id=self.course_key) + resp = self.authorized_client.get(url) + assert resp.status_code == status.HTTP_200_OK + for taxonomy_entry in resp.data[str(self.course_key)]["taxonomies"]: + assert taxonomy_entry["can_tag_object"] is True + for tag in taxonomy_entry["tags"]: + assert tag["can_delete_objecttag"] is True + + def test_retrieve_can_tag_object_denied(self): + """User sees can_tag_object=false and can_delete_objecttag=false when authz denies manage_tags.""" + auditor_client = self._make_auditor_client() + self._put_tags(self.authorized_client) # ensure tags exist + url = OBJECT_TAGS_URL.format(object_id=self.course_key) + resp = auditor_client.get(url) + assert resp.status_code == status.HTTP_200_OK + for taxonomy_entry in resp.data[str(self.course_key)]["taxonomies"]: + assert taxonomy_entry["can_tag_object"] is False + for tag in taxonomy_entry["tags"]: + assert tag["can_delete_objecttag"] is False + + @skip_unless_cms @ddt.ddt class TestDownloadTemplateView(APITestCase): diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index a52a8810b547..3ac64a34b18d 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -5,16 +5,23 @@ from django.db.models import Count from django.http import StreamingHttpResponse +from opaque_keys.edx.keys import CourseKey +from openedx_authz import api as authz_api +from openedx_authz.constants.permissions import COURSES_MANAGE_TAGS from openedx_events.content_authoring.data import ContentObjectChangedData, ContentObjectData from openedx_events.content_authoring.signals import CONTENT_OBJECT_ASSOCIATIONS_CHANGED, CONTENT_OBJECT_TAGS_CHANGED from openedx_tagging import rules as oel_tagging_rules +from openedx_tagging.api import TagDoesNotExist, get_object_tags, tag_object +from openedx_tagging.rest_api.v1.serializers import ObjectTagUpdateBodySerializer from openedx_tagging.rest_api.v1.views import ObjectTagView, TaxonomyView from rest_framework import status from rest_framework.decorators import action from rest_framework.exceptions import PermissionDenied, ValidationError +from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView +from openedx.core import toggles as core_toggles from openedx.core.types.http import RestRequest from ...api import ( @@ -28,6 +35,7 @@ ) from ...auth import has_view_object_tags_access from ...rules import get_admin_orgs +from ...utils import get_context_key_from_key_string from .filters import ObjectTagTaxonomyOrgFilterBackend, UserOrgFilterBackend from .serializers import ( ObjectTagCopiedMinimalSerializer, @@ -154,14 +162,99 @@ class ObjectTagOrgView(ObjectTagView): minimal_serializer_class = ObjectTagCopiedMinimalSerializer filter_backends = [ObjectTagTaxonomyOrgFilterBackend] + def _get_course_key(self, object_id): + """ + Extract the course key from any content key string. + Returns None if the object_id is not course-related (e.g., library content). + """ + try: + context_key = get_context_key_from_key_string(object_id) + if isinstance(context_key, CourseKey): + return context_key + except ValueError: + pass + return None + + def _is_authz_active(self, object_id): + """ + Returns (course_key, True) if authz is enabled for this object's course, + (None, False) otherwise. + """ + course_key = self._get_course_key(object_id) + if course_key and core_toggles.enable_authz_course_authoring(course_key): + return course_key, True + return course_key, False + + def get_permissions(self): + """ + When authz is enabled for the course, skip the parent's legacy + DjangoObjectPermissions. Our update/retrieve methods handle + permissions via openedx-authz directly. + """ + object_id = self.kwargs.get('object_id', '') + _, authz_active = self._is_authz_active(object_id) + if authz_active: + return [IsAuthenticated()] + return super().get_permissions() + + def get_queryset(self): + """ + When authz is active, skip the parent's legacy view_objecttag permission + check inside get_queryset. Any authenticated user can view tags; the + can_tag_object field controls edit access. + """ + object_id = self.kwargs["object_id"] + _, authz_active = self._is_authz_active(object_id) + if authz_active: + return get_object_tags(object_id) + return super().get_queryset() + + def retrieve(self, request, *args, **kwargs): + """ + Override retrieve to update can_tag_object permission fields when authz is enabled. + + The parent serializer computes can_tag_object via legacy django-rules. When authz + is enabled for the course, we override those values so the frontend correctly + shows/hides tagging controls. + """ + response = super().retrieve(request, *args, **kwargs) + object_id = kwargs.get('object_id', '') + course_key, authz_active = self._is_authz_active(object_id) + + if authz_active: + can_tag = authz_api.is_user_allowed( + request.user.username, COURSES_MANAGE_TAGS.identifier, str(course_key) + ) + for obj_data in response.data.values(): + for taxonomy_entry in obj_data.get("taxonomies", []): + taxonomy_entry["can_tag_object"] = can_tag + for tag in taxonomy_entry.get("tags", []): + tag["can_delete_objecttag"] = can_tag + + return response + def update(self, request, *args, **kwargs) -> Response: """ - Extend the update method to fire CONTENT_OBJECT_ASSOCIATIONS_CHANGED event + Update object tags. When authz is enabled for the course, permissions are + enforced entirely via openedx-authz (courses.manage_tags), bypassing the + parent's legacy per-taxonomy permission checks. When authz is not enabled, + delegates to the parent's update which uses legacy django-rules checks. """ - response = super().update(request, *args, **kwargs) - if response.status_code == 200: - object_id = kwargs.get('object_id') + object_id = kwargs.get('object_id', '') + course_key, authz_active = self._is_authz_active(object_id) + + if authz_active: + if not authz_api.is_user_allowed( + request.user.username, COURSES_MANAGE_TAGS.identifier, str(course_key) + ): + raise PermissionDenied( + "You do not have permission to manage tags for this course." + ) + response = self._update_tags(request, object_id) + else: + response = super().update(request, *args, **kwargs) + if response.status_code == 200: # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( @@ -180,6 +273,31 @@ def update(self, request, *args, **kwargs) -> Response: return response + def _update_tags(self, request, object_id): + """ + Apply tag updates without legacy permission checks. + Replicates the parent's ObjectTagView.update() logic for tag saving + but skips the per-taxonomy oel_tagging.can_tag_object check. + """ + body = ObjectTagUpdateBodySerializer(data=request.data) + body.is_valid(raise_exception=True) + + data = body.validated_data.get("tagsData", []) + if not data: + return self.retrieve(request, object_id) + + for tags_data in data: + taxonomy = tags_data.get("taxonomy") + tags = tags_data.get("tags", []) + try: + tag_object(object_id, taxonomy, tags) + except TagDoesNotExist as e: + raise ValidationError from e + except ValueError as e: + raise ValidationError from e + + return self.retrieve(request, object_id) + class ObjectTagExportView(APIView): """" From 1240f913cafa535d008875768f9c3d39f40e54e4 Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Tue, 14 Apr 2026 14:09:09 -0600 Subject: [PATCH 2/6] fixup! feat: enforce openedx-authz permissions on object tag and xblock outline endpoints --- .../content_tagging/rest_api/v1/views.py | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index 3ac64a34b18d..a6f0d6599d22 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -12,11 +12,11 @@ from openedx_events.content_authoring.signals import CONTENT_OBJECT_ASSOCIATIONS_CHANGED, CONTENT_OBJECT_TAGS_CHANGED from openedx_tagging import rules as oel_tagging_rules from openedx_tagging.api import TagDoesNotExist, get_object_tags, tag_object -from openedx_tagging.rest_api.v1.serializers import ObjectTagUpdateBodySerializer +from openedx_tagging.rest_api.v1.serializers import ObjectTagListQueryParamsSerializer, ObjectTagUpdateBodySerializer from openedx_tagging.rest_api.v1.views import ObjectTagView, TaxonomyView from rest_framework import status from rest_framework.decorators import action -from rest_framework.exceptions import PermissionDenied, ValidationError +from rest_framework.exceptions import MethodNotAllowed, PermissionDenied, ValidationError from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView @@ -206,7 +206,17 @@ def get_queryset(self): object_id = self.kwargs["object_id"] _, authz_active = self._is_authz_active(object_id) if authz_active: - return get_object_tags(object_id) + query_params = ObjectTagListQueryParamsSerializer( + data=self.request.query_params.dict() + ) + query_params.is_valid(raise_exception=True) + taxonomy = query_params.validated_data.get("taxonomy", None) + taxonomy_id = taxonomy.cast().id if taxonomy else None + + if object_id.endswith("*") or "," in object_id: + raise ValidationError("Retrieving tags from multiple objects is not yet supported.") + + return get_object_tags(object_id, taxonomy_id) return super().get_queryset() def retrieve(self, request, *args, **kwargs): @@ -240,7 +250,7 @@ def update(self, request, *args, **kwargs) -> Response: parent's legacy per-taxonomy permission checks. When authz is not enabled, delegates to the parent's update which uses legacy django-rules checks. """ - object_id = kwargs.get('object_id', '') + object_id = kwargs.pop('object_id', '') course_key, authz_active = self._is_authz_active(object_id) if authz_active: @@ -250,7 +260,7 @@ def update(self, request, *args, **kwargs) -> Response: raise PermissionDenied( "You do not have permission to manage tags for this course." ) - response = self._update_tags(request, object_id) + response = self._update_tags(request, object_id, **kwargs) else: response = super().update(request, *args, **kwargs) @@ -273,12 +283,16 @@ def update(self, request, *args, **kwargs) -> Response: return response - def _update_tags(self, request, object_id): + def _update_tags(self, request, object_id, **kwargs): """ Apply tag updates without legacy permission checks. Replicates the parent's ObjectTagView.update() logic for tag saving but skips the per-taxonomy oel_tagging.can_tag_object check. """ + partial = kwargs.pop('partial', False) + if partial: + raise MethodNotAllowed("PATCH", detail="PATCH not allowed") + body = ObjectTagUpdateBodySerializer(data=request.data) body.is_valid(raise_exception=True) From dbbfc5ed8ae497d05cdf32979c510adb59bf527d Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Tue, 14 Apr 2026 14:38:55 -0600 Subject: [PATCH 3/6] fixup! feat: enforce openedx-authz permissions on object tag and xblock outline endpoints --- openedx/core/djangoapps/content_tagging/rest_api/v1/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index a6f0d6599d22..60aea6acda1b 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -250,7 +250,7 @@ def update(self, request, *args, **kwargs) -> Response: parent's legacy per-taxonomy permission checks. When authz is not enabled, delegates to the parent's update which uses legacy django-rules checks. """ - object_id = kwargs.pop('object_id', '') + object_id = kwargs.get('object_id', '') course_key, authz_active = self._is_authz_active(object_id) if authz_active: @@ -260,6 +260,7 @@ def update(self, request, *args, **kwargs) -> Response: raise PermissionDenied( "You do not have permission to manage tags for this course." ) + kwargs.pop('object_id', None) response = self._update_tags(request, object_id, **kwargs) else: response = super().update(request, *args, **kwargs) From b7b17dc46a2597f173056830473c02d301ce3c96 Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Tue, 14 Apr 2026 16:53:54 -0600 Subject: [PATCH 4/6] fixup! feat: enforce openedx-authz permissions on object tag and xblock outline endpoints --- .../content_tagging/rest_api/v1/views.py | 125 ++++++------------ 1 file changed, 40 insertions(+), 85 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index 60aea6acda1b..b46a3845387f 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -11,12 +11,10 @@ from openedx_events.content_authoring.data import ContentObjectChangedData, ContentObjectData from openedx_events.content_authoring.signals import CONTENT_OBJECT_ASSOCIATIONS_CHANGED, CONTENT_OBJECT_TAGS_CHANGED from openedx_tagging import rules as oel_tagging_rules -from openedx_tagging.api import TagDoesNotExist, get_object_tags, tag_object -from openedx_tagging.rest_api.v1.serializers import ObjectTagListQueryParamsSerializer, ObjectTagUpdateBodySerializer from openedx_tagging.rest_api.v1.views import ObjectTagView, TaxonomyView from rest_framework import status from rest_framework.decorators import action -from rest_framework.exceptions import MethodNotAllowed, PermissionDenied, ValidationError +from rest_framework.exceptions import PermissionDenied, ValidationError from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView @@ -188,8 +186,8 @@ def _is_authz_active(self, object_id): def get_permissions(self): """ When authz is enabled for the course, skip the parent's legacy - DjangoObjectPermissions. Our update/retrieve methods handle - permissions via openedx-authz directly. + DjangoObjectPermissions since permission checks are handled by + the overridden check_* methods below. """ object_id = self.kwargs.get('object_id', '') _, authz_active = self._is_authz_active(object_id) @@ -197,75 +195,61 @@ def get_permissions(self): return [IsAuthenticated()] return super().get_permissions() - def get_queryset(self): + def check_view_object_tags_permission(self, object_id, taxonomy=None): """ - When authz is active, skip the parent's legacy view_objecttag permission - check inside get_queryset. Any authenticated user can view tags; the - can_tag_object field controls edit access. + When authz is active, any authenticated user can view tags. + The can_tag_object field controls edit access in the response. """ - object_id = self.kwargs["object_id"] _, authz_active = self._is_authz_active(object_id) if authz_active: - query_params = ObjectTagListQueryParamsSerializer( - data=self.request.query_params.dict() - ) - query_params.is_valid(raise_exception=True) - taxonomy = query_params.validated_data.get("taxonomy", None) - taxonomy_id = taxonomy.cast().id if taxonomy else None - - if object_id.endswith("*") or "," in object_id: - raise ValidationError("Retrieving tags from multiple objects is not yet supported.") + return + super().check_view_object_tags_permission(object_id, taxonomy) - return get_object_tags(object_id, taxonomy_id) - return super().get_queryset() - - def retrieve(self, request, *args, **kwargs): + def check_can_tag_object_permission(self, object_id, taxonomy): """ - Override retrieve to update can_tag_object permission fields when authz is enabled. - - The parent serializer computes can_tag_object via legacy django-rules. When authz - is enabled for the course, we override those values so the frontend correctly - shows/hides tagging controls. + When authz is active, enforce courses.manage_tags via openedx-authz. """ - response = super().retrieve(request, *args, **kwargs) - object_id = kwargs.get('object_id', '') course_key, authz_active = self._is_authz_active(object_id) + if authz_active: + if not authz_api.is_user_allowed( + self.request.user.username, COURSES_MANAGE_TAGS.identifier, str(course_key) + ): + raise PermissionDenied( + "You do not have permission to manage tags for this course." + ) + return + super().check_can_tag_object_permission(object_id, taxonomy) + def get_can_tag_object_value(self, user, obj_tag): + """ + When authz is active, return whether the user has courses.manage_tags. + """ + course_key, authz_active = self._is_authz_active(obj_tag.object_id) if authz_active: - can_tag = authz_api.is_user_allowed( - request.user.username, COURSES_MANAGE_TAGS.identifier, str(course_key) + return authz_api.is_user_allowed( + user.username, COURSES_MANAGE_TAGS.identifier, str(course_key) ) - for obj_data in response.data.values(): - for taxonomy_entry in obj_data.get("taxonomies", []): - taxonomy_entry["can_tag_object"] = can_tag - for tag in taxonomy_entry.get("tags", []): - tag["can_delete_objecttag"] = can_tag + return super().get_can_tag_object_value(user, obj_tag) - return response - - def update(self, request, *args, **kwargs) -> Response: + def get_can_delete_objecttag_value(self, user, object_id): """ - Update object tags. When authz is enabled for the course, permissions are - enforced entirely via openedx-authz (courses.manage_tags), bypassing the - parent's legacy per-taxonomy permission checks. When authz is not enabled, - delegates to the parent's update which uses legacy django-rules checks. + When authz is active, return whether the user has courses.manage_tags. """ - object_id = kwargs.get('object_id', '') course_key, authz_active = self._is_authz_active(object_id) - if authz_active: - if not authz_api.is_user_allowed( - request.user.username, COURSES_MANAGE_TAGS.identifier, str(course_key) - ): - raise PermissionDenied( - "You do not have permission to manage tags for this course." - ) - kwargs.pop('object_id', None) - response = self._update_tags(request, object_id, **kwargs) - else: - response = super().update(request, *args, **kwargs) + return authz_api.is_user_allowed( + user.username, COURSES_MANAGE_TAGS.identifier, str(course_key) + ) + return super().get_can_delete_objecttag_value(user, object_id) + def update(self, request, *args, **kwargs) -> Response: + """ + Extend the update method to fire CONTENT_OBJECT_ASSOCIATIONS_CHANGED event. + """ + response = super().update(request, *args, **kwargs) if response.status_code == 200: + object_id = self.kwargs.get('object_id', '') + # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( @@ -284,35 +268,6 @@ def update(self, request, *args, **kwargs) -> Response: return response - def _update_tags(self, request, object_id, **kwargs): - """ - Apply tag updates without legacy permission checks. - Replicates the parent's ObjectTagView.update() logic for tag saving - but skips the per-taxonomy oel_tagging.can_tag_object check. - """ - partial = kwargs.pop('partial', False) - if partial: - raise MethodNotAllowed("PATCH", detail="PATCH not allowed") - - body = ObjectTagUpdateBodySerializer(data=request.data) - body.is_valid(raise_exception=True) - - data = body.validated_data.get("tagsData", []) - if not data: - return self.retrieve(request, object_id) - - for tags_data in data: - taxonomy = tags_data.get("taxonomy") - tags = tags_data.get("tags", []) - try: - tag_object(object_id, taxonomy, tags) - except TagDoesNotExist as e: - raise ValidationError from e - except ValueError as e: - raise ValidationError from e - - return self.retrieve(request, object_id) - class ObjectTagExportView(APIView): """" From 3466d86567a0d177379a0a30691372849f83fdfe Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Wed, 15 Apr 2026 11:21:24 -0600 Subject: [PATCH 5/6] temp: point openedx-core to authz refactor branch --- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 9 +++++---- requirements/edx/development.txt | 10 +++++----- requirements/edx/doc.txt | 8 ++++---- requirements/edx/github.in | 3 ++- requirements/edx/testing.txt | 8 ++++---- 6 files changed, 21 insertions(+), 19 deletions(-) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 1b2ab4b757ed..6bea45b48790 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -65,7 +65,7 @@ numpy<2.0.0 # breaking changes which openedx-core devs want to roll out manually. New patch versions # are OK to accept automatically. # Issue for unpinning: https://github.com/openedx/edx-platform/issues/35269 -openedx-core<0.40 +openedx-core<0.41 # Date: 2023-11-29 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 869acdb1bcbd..801468a25482 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -4,6 +4,11 @@ # # make upgrade # +-e git+https://github.com/WGU-Open-edX/openedx-core.git@tpayne/object-tag-view-authz-refactor#egg=openedx-core + # via + # -c requirements/constraints.txt + # -r requirements/edx/github.in + # -r requirements/edx/kernel.in acid-xblock==0.4.1 # via -r requirements/edx/kernel.in aiohappyeyeballs==2.6.1 @@ -833,10 +838,6 @@ openedx-calc==5.0.0 # via # -r requirements/edx/kernel.in # xblocks-contrib -openedx-core==0.39.0 - # via - # -c requirements/constraints.txt - # -r requirements/edx/kernel.in openedx-django-pyfs==4.0.0 # via xblock openedx-django-require==3.0.0 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 905a754e88a9..8acbc8a155d7 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -4,6 +4,11 @@ # # make upgrade # +-e git+https://github.com/WGU-Open-edX/openedx-core.git@tpayne/object-tag-view-authz-refactor#egg=openedx-core + # via + # -c requirements/constraints.txt + # -r requirements/edx/doc.txt + # -r requirements/edx/testing.txt accessible-pygments==0.0.5 # via # -r requirements/edx/doc.txt @@ -1385,11 +1390,6 @@ openedx-calc==5.0.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # xblocks-contrib -openedx-core==0.39.0 - # via - # -c requirements/constraints.txt - # -r requirements/edx/doc.txt - # -r requirements/edx/testing.txt openedx-django-pyfs==4.0.0 # via # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 29dbcb3f074d..f7609cb8ac0b 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -4,6 +4,10 @@ # # make upgrade # +-e git+https://github.com/WGU-Open-edX/openedx-core.git@tpayne/object-tag-view-authz-refactor#egg=openedx-core + # via + # -c requirements/constraints.txt + # -r requirements/edx/base.txt accessible-pygments==0.0.5 # via pydata-sphinx-theme acid-xblock==0.4.1 @@ -1010,10 +1014,6 @@ openedx-calc==5.0.0 # via # -r requirements/edx/base.txt # xblocks-contrib -openedx-core==0.39.0 - # via - # -c requirements/constraints.txt - # -r requirements/edx/base.txt openedx-django-pyfs==4.0.0 # via # -r requirements/edx/base.txt diff --git a/requirements/edx/github.in b/requirements/edx/github.in index 7fdb2c051ce8..f2ed3688da4e 100644 --- a/requirements/edx/github.in +++ b/requirements/edx/github.in @@ -80,7 +80,8 @@ # Release candidates being tested. ############################################################################## -# ... add dependencies here +# https://github.com/openedx/openedx-platform/pull/38292 +-e git+https://github.com/WGU-Open-edX/openedx-core.git@tpayne/object-tag-view-authz-refactor#egg=openedx-core ############################################################################## # Critical fixes for packages that are not yet available in a PyPI release. diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 88e137db23cb..79103ef78ade 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -4,6 +4,10 @@ # # make upgrade # +-e git+https://github.com/WGU-Open-edX/openedx-core.git@tpayne/object-tag-view-authz-refactor#egg=openedx-core + # via + # -c requirements/constraints.txt + # -r requirements/edx/base.txt acid-xblock==0.4.1 # via -r requirements/edx/base.txt aiohappyeyeballs==2.6.1 @@ -1057,10 +1061,6 @@ openedx-calc==5.0.0 # via # -r requirements/edx/base.txt # xblocks-contrib -openedx-core==0.39.0 - # via - # -c requirements/constraints.txt - # -r requirements/edx/base.txt openedx-django-pyfs==4.0.0 # via # -r requirements/edx/base.txt From b12c37017b8ae7f8a190464a8c74791cbed49bc6 Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Wed, 15 Apr 2026 12:39:05 -0600 Subject: [PATCH 6/6] fixup! feat: enforce openedx-authz permissions on object tag and xblock outline endpoints --- .../rest_api/v1/serializers.py | 49 ++++++++++++++++++- .../content_tagging/rest_api/v1/views.py | 24 +-------- 2 files changed, 49 insertions(+), 24 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py index e15f04504a3e..7a916b00c574 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py @@ -4,15 +4,22 @@ from __future__ import annotations +from opaque_keys.edx.keys import CourseKey +from openedx_authz import api as authz_api +from openedx_authz.constants.permissions import COURSES_MANAGE_TAGS from openedx_tagging.rest_api.v1.serializers import ( ObjectTagMinimalSerializer, + ObjectTagsByTaxonomySerializer, TaxonomyListQueryParamsSerializer, TaxonomySerializer, ) from organizations.models import Organization from rest_framework import fields, serializers +from openedx.core import toggles as core_toggles + from ...models import TaxonomyOrg +from ...utils import get_context_key_from_key_string class TaxonomyOrgListQueryParamsSerializer(TaxonomyListQueryParamsSerializer): @@ -95,6 +102,25 @@ class Meta: read_only_fields = ["orgs", "all_orgs"] +def _check_authz_manage_tags(context, object_id): + """ + Check courses.manage_tags via openedx-authz if the flag is active for this object's course. + Returns (can_manage: bool, authz_active: True) if authz handled it, + (None, False) if legacy should be used. + """ + try: + context_key = get_context_key_from_key_string(object_id) + if isinstance(context_key, CourseKey) and core_toggles.enable_authz_course_authoring(context_key): + request = context.get("request") + if request: + return authz_api.is_user_allowed( + request.user.username, COURSES_MANAGE_TAGS.identifier, str(context_key) + ), True + except ValueError: + pass + return None, False + + class ObjectTagCopiedMinimalSerializer(ObjectTagMinimalSerializer): """ Serializer for Object Tags. @@ -112,10 +138,29 @@ def get_can_delete_objecttag(self, instance): """ Verify if the user can delete the object tag. - Override to return `False` if the object tag is copied. + Override to return `False` if the object tag is copied, + and to use authz when active. """ if instance.is_copied: - # The user can't delete copied tags. return False + can_manage, authz_active = _check_authz_manage_tags(self.context, instance.object_id) + if authz_active: + return can_manage + return super().get_can_delete_objecttag(instance) + + +class ObjectTagsByTaxonomyOrgSerializer(ObjectTagsByTaxonomySerializer): + """ + Extends ObjectTagsByTaxonomySerializer to use authz for can_tag_object when active. + """ + + def get_can_tag_object(self, obj_tag): + """ + When authz is active, return whether the user has courses.manage_tags. + """ + can_manage, authz_active = _check_authz_manage_tags(self.context, obj_tag.object_id) + if authz_active: + return can_manage + return super().get_can_tag_object(obj_tag) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index b46a3845387f..f8e55beeb4d5 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -37,6 +37,7 @@ from .filters import ObjectTagTaxonomyOrgFilterBackend, UserOrgFilterBackend from .serializers import ( ObjectTagCopiedMinimalSerializer, + ObjectTagsByTaxonomyOrgSerializer, TaxonomyOrgListQueryParamsSerializer, TaxonomyOrgSerializer, TaxonomyUpdateOrgBodySerializer, @@ -158,6 +159,7 @@ class ObjectTagOrgView(ObjectTagView): Refer to ObjectTagView docstring for usage details. """ minimal_serializer_class = ObjectTagCopiedMinimalSerializer + taxonomy_serializer_class = ObjectTagsByTaxonomyOrgSerializer filter_backends = [ObjectTagTaxonomyOrgFilterBackend] def _get_course_key(self, object_id): @@ -220,28 +222,6 @@ def check_can_tag_object_permission(self, object_id, taxonomy): return super().check_can_tag_object_permission(object_id, taxonomy) - def get_can_tag_object_value(self, user, obj_tag): - """ - When authz is active, return whether the user has courses.manage_tags. - """ - course_key, authz_active = self._is_authz_active(obj_tag.object_id) - if authz_active: - return authz_api.is_user_allowed( - user.username, COURSES_MANAGE_TAGS.identifier, str(course_key) - ) - return super().get_can_tag_object_value(user, obj_tag) - - def get_can_delete_objecttag_value(self, user, object_id): - """ - When authz is active, return whether the user has courses.manage_tags. - """ - course_key, authz_active = self._is_authz_active(object_id) - if authz_active: - return authz_api.is_user_allowed( - user.username, COURSES_MANAGE_TAGS.identifier, str(course_key) - ) - return super().get_can_delete_objecttag_value(user, object_id) - def update(self, request, *args, **kwargs) -> Response: """ Extend the update method to fire CONTENT_OBJECT_ASSOCIATIONS_CHANGED event.