feat: make can_tag_object's course check authorization-service aware [verawood]

openedx/core/djangoapps/content_tagging/rest_api/v1/views.py

@@ -9,7 +9,7 @@
 from django.db.models import Count
 from django.http import StreamingHttpResponse
 from openedx_authz import api as authz_api
-from openedx_authz.constants.permissions import COURSES_MANAGE_TAGS, COURSES_VIEW_COURSE
+from openedx_authz.constants.permissions import COURSES_VIEW_COURSE
 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
@@ -207,24 +207,6 @@ def ensure_has_view_object_tag_permission(self, user, taxonomy, object_id):
             # Fall back to parent implementation
             super().ensure_has_view_object_tag_permission(user, taxonomy, object_id)
 
-    def ensure_user_has_can_tag_object_permissions(self, user, tags_data, object_id):
-        """
-        Check if user has permission to tag object for each taxonomy in tags_data.
-
-        This method is overridden to conditionally use openedx-authz when the toggle is enabled.
-
-        When using openedx-authz, if the user has manage tags permission for the course,
-        they can tag the object regardless of the taxonomy.
-        """
-        should_use_authz, course_key = self._authz_check
-        if should_use_authz and not authz_api.is_user_allowed(
-            user.username, COURSES_MANAGE_TAGS.identifier, str(course_key)
-        ):
-            raise PermissionDenied("You do not have permission to manage object tags.")
-        if not should_use_authz:
-            # Fall back to parent implementation
-            super().ensure_user_has_can_tag_object_permissions(user, tags_data, object_id)
-
     def update(self, request, *args, **kwargs) -> Response:
         """
         Extend the update method to fire CONTENT_OBJECT_ASSOCIATIONS_CHANGED event

openedx/core/djangoapps/content_tagging/rules.py

@@ -23,6 +23,7 @@
     OrgStaffRole,
 )
 
+from .auth import should_use_course_authz_for_object
 from .models import TaxonomyOrg
 from .utils import check_taxonomy_context_key_org, get_context_key_from_key_string, rules_cache
 
@@ -225,8 +226,10 @@ def can_change_object_tag_objectid(user: UserType, object_id: str) -> bool:
     For Content Libraries V2, this requires either explicit library tagging permission
     (MANAGE_LIBRARY_TAGS) or org-level admin access for the library's org.
 
-    For other contexts (courses, xblocks, etc.), this requires studio write access or
-    org-level admin access for the object's org.
+    For a course (or object within one) switched to openedx-authz, this requires
+    courses.manage_tags through that service alone, with no fallback to studio write or
+    org-admin access; for any other course, xblock, etc. not yet switched, those checks
+    still apply exactly as before.
     """
     if not object_id:
         return True
@@ -246,6 +249,12 @@ def can_change_object_tag_objectid(user: UserType, object_id: str) -> bool:
     ):
         return True
 
+    should_use_authz, course_key = should_use_course_authz_for_object(object_id)
+    if should_use_authz:
+        return authz_api.is_user_allowed(
+            user.username, authz_permissions.COURSES_MANAGE_TAGS.identifier, str(course_key)
+        )
+
     # For other contexts (courses, xblocks, etc.), use general write or org-admin access
     if has_studio_write_access(user, context_key):
         return True

openedx/core/djangoapps/content_tagging/tests/test_rules.py

@@ -5,13 +5,15 @@
 import ddt
 from django.contrib.auth import get_user_model
 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.constants import permissions as authz_permissions
 from openedx_tagging.models import Tag, UserSystemDefinedTaxonomy
 from openedx_tagging.rules import ObjectTagPermissionItem
 
 from common.djangoapps.student.auth import add_users, update_org_role
 from common.djangoapps.student.roles import CourseStaffRole, OrgStaffRole
+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
@@ -743,3 +745,127 @@ def test_empty_object_id(self):
 
         with self.assertRaises(ValueError):  # noqa: PT027
             can_remove_object_tag_objectid(self.library_user, "")
+
+
+class TestRulesCourseAuthzPermissions(TestTaxonomyMixin, TestCase):
+    """
+    Tests for can_change_object_tag_objectid's course-authz-aware branch.
+    """
+
+    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",
+        )
+        self.org_admin_user = User.objects.create(
+            username="org_admin_user",
+            email="org_admin_user@example.com",
+        )
+
+        self.course_key = CourseLocator.from_string("course-v1:OeX+DemoX+Demo_Course")
+        self.xblock_key = BlockUsageLocator(
+            course_key=self.course_key,
+            block_type='problem',
+            block_id='block_id',
+        )
+        # Granted while the waffle flag is off, so this goes through the legacy role path.
+        add_users(self.superuser, CourseStaffRole(self.course_key), self.legacy_user)
+        # org1's short_name ("OeX") matches self.course_key's org, so this is an org-level
+        # admin for the course's org, not merely a course-level role.
+        update_org_role(self.superuser, OrgStaffRole, self.org_admin_user, [self.org1.short_name])
+
+    @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True)
+    @patch("openedx_authz.api.is_user_allowed")
+    def test_course_switched_authz_only_role_allowed(self, mock_is_user_allowed):
+        """
+        A course switched to openedx-authz grants access via authz_api.is_user_allowed alone.
+        """
+        mock_is_user_allowed.return_value = True
+
+        result = can_change_object_tag_objectid(self.authz_user, str(self.course_key))
+
+        self.assertTrue(result)  # noqa: PT009
+        mock_is_user_allowed.assert_called_once_with(
+            self.authz_user.username,
+            authz_permissions.COURSES_MANAGE_TAGS.identifier,
+            str(self.course_key),
+        )
+
+    @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True)
+    @patch("openedx_authz.api.is_user_allowed")
+    def test_course_switched_legacy_only_role_denied(self, mock_is_user_allowed):
+        """
+        A switched course does not fall back to a legacy role: the switch is exclusive, not
+        an OR with the legacy check.
+        """
+        mock_is_user_allowed.return_value = False
+
+        result = can_change_object_tag_objectid(self.legacy_user, str(self.course_key))
+
+        self.assertFalse(result)  # noqa: PT009
+        mock_is_user_allowed.assert_called_once_with(
+            self.legacy_user.username,
+            authz_permissions.COURSES_MANAGE_TAGS.identifier,
+            str(self.course_key),
+        )
+
+    @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True)
+    @patch("openedx_authz.api.is_user_allowed")
+    def test_xblock_in_switched_course_authz_only_role_allowed(self, mock_is_user_allowed):
+        """
+        An xblock inside a switched course resolves through the same authz-only branch as
+        the course itself: should_use_course_authz_for_object resolves the xblock's usage
+        key down to its course key.
+        """
+        mock_is_user_allowed.return_value = True
+
+        result = can_change_object_tag_objectid(self.authz_user, str(self.xblock_key))
+
+        self.assertTrue(result)  # noqa: PT009
+        mock_is_user_allowed.assert_called_once_with(
+            self.authz_user.username,
+            authz_permissions.COURSES_MANAGE_TAGS.identifier,
+            str(self.course_key),
+        )
+
+    @patch("openedx_authz.api.is_user_allowed")
+    def test_course_not_switched_legacy_role_allowed(self, mock_is_user_allowed):
+        """
+        A course that hasn't been switched to openedx-authz keeps resolving through the
+        legacy role check, unaffected by this change.
+        """
+        result = can_change_object_tag_objectid(self.legacy_user, str(self.course_key))
+
+        self.assertTrue(result)  # noqa: PT009
+        mock_is_user_allowed.assert_not_called()
+
+    @override_waffle_flag(AUTHZ_COURSE_AUTHORING_FLAG, active=True)
+    @patch("openedx_authz.api.is_user_allowed")
+    def test_library_object_unchanged(self, mock_is_user_allowed):
+        """
+        Library objects are unaffected by this branch: should_use_course_authz_for_object
+        only recognizes CourseKeys, so the library check still resolves through
+        MANAGE_LIBRARY_TAGS even with the course-authz flag on.
+        """
+        mock_is_user_allowed.return_value = True
+        library_key = LibraryLocatorV2.from_string(f"lib:{self.org1.short_name}:test_library")
+
+        result = can_change_object_tag_objectid(self.legacy_user, str(library_key))
+
+        self.assertTrue(result)  # noqa: PT009
+        mock_is_user_allowed.assert_called_once_with(
+            self.legacy_user.username,
+            authz_permissions.MANAGE_LIBRARY_TAGS.identifier,
+            str(library_key),
+        )