Skip to content

Commit f2624a4

Browse files
feat: check authz permissions for course tagging
1 parent b4a099f commit f2624a4

11 files changed

Lines changed: 356 additions & 39 deletions

File tree

cms/djangoapps/contentstore/views/block.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
from django.utils.translation import gettext as _
1212
from django.views.decorators.clickjacking import xframe_options_exempt
1313
from django.views.decorators.http import require_http_methods
14-
from openedx_authz.constants.permissions import COURSES_VIEW_COURSE
1514
from opaque_keys.edx.keys import CourseKey
15+
from openedx_authz.constants.permissions import COURSES_VIEW_COURSE
1616
from web_fragments.fragment import Fragment
1717

1818
from cms.djangoapps.contentstore.utils import load_services_for_studio
@@ -28,9 +28,9 @@
2828
from common.djangoapps.edxmako.shortcuts import render_to_response, render_to_string
2929
from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access
3030
from common.djangoapps.util.json_request import JsonResponse, expect_json
31-
from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled
32-
from openedx.core.djangoapps.authz.decorators import user_has_course_permission
3331
from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
32+
from openedx.core.djangoapps.authz.decorators import user_has_course_permission
33+
from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled
3434
from openedx.core.lib.xblock_utils import hash_resource, request_token, wrap_xblock, wrap_xblock_aside
3535
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
3636
from xmodule.x_module import ( # lint-amnesty, pylint: disable=wrong-import-order
@@ -332,7 +332,12 @@ def xblock_outline_handler(request, usage_key_string):
332332
a course.
333333
"""
334334
usage_key = usage_key_with_run(usage_key_string)
335-
if not user_has_course_permission(request.user, COURSES_VIEW_COURSE.identifier, usage_key.course_key, LegacyAuthoringPermission.READ):
335+
if not user_has_course_permission(
336+
request.user,
337+
COURSES_VIEW_COURSE.identifier,
338+
usage_key.course_key,
339+
LegacyAuthoringPermission.READ,
340+
):
336341
raise PermissionDenied()
337342

338343
response_format = request.GET.get("format", "html")

cms/djangoapps/contentstore/views/tests/test_block.py

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import ddt
1010
from bs4 import BeautifulSoup
1111
from django.conf import settings
12-
from django.core.exceptions import PermissionDenied
1312
from django.http import Http404
1413
from django.test import TestCase
1514
from django.test.client import RequestFactory
@@ -20,6 +19,7 @@
2019
from opaque_keys.edx.asides import AsideUsageKeyV2
2120
from opaque_keys.edx.keys import CourseKey, UsageKey
2221
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
22+
from openedx_authz.constants.roles import COURSE_STAFF
2323
from openedx_events.content_authoring.data import DuplicatedXBlockData
2424
from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED
2525
from openedx_events.testing import OpenEdxEventsTestMixin
@@ -55,13 +55,10 @@
5555
from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
5656
from common.test.utils import assert_dict_contains_subset
5757
from lms.djangoapps.lms_xblock.mixin import NONSENSICAL_ACCESS_RESTRICTION
58-
from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
5958
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin
6059
from openedx.core.djangoapps.content_tagging import api as tagging_api
6160
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration
6261
from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE
63-
from openedx_authz.constants.permissions import COURSES_VIEW_COURSE
64-
from openedx_authz.constants.roles import COURSE_STAFF
6562
from xmodule.course_block import DEFAULT_START_DATE
6663
from xmodule.modulestore import ModuleStoreEnum
6764
from xmodule.modulestore.django import modulestore
@@ -3529,10 +3526,10 @@ def test_authorized_user_gets_json_response(self):
35293526
Test that authorized user gets JSON response from xblock_outline_handler.
35303527
"""
35313528
outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
3532-
3529+
35333530
self.client.login(username=self.authorized_user.username, password=self.password)
35343531
resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
3535-
3532+
35363533
assert resp.status_code == 200
35373534
json_response = json.loads(resp.content.decode("utf-8"))
35383535
assert "id" in json_response
@@ -3544,21 +3541,21 @@ def test_unauthorized_user_gets_permission_denied(self):
35443541
Test that unauthorized user gets 403 response from xblock_outline_handler.
35453542
"""
35463543
outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
3547-
3544+
35483545
self.client.login(username=self.unauthorized_user.username, password=self.password)
35493546
resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
3550-
3547+
35513548
assert resp.status_code == 403
35523549

35533550
def test_superuser_gets_json_response(self):
35543551
"""
35553552
Test that superuser gets JSON response from xblock_outline_handler.
35563553
"""
35573554
outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
3558-
3555+
35593556
self.client.login(username=self.super_user.username, password=self.password)
35603557
resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
3561-
3558+
35623559
assert resp.status_code == 200
35633560
json_response = json.loads(resp.content.decode("utf-8"))
35643561
assert "id" in json_response
@@ -3570,10 +3567,10 @@ def test_staff_user_gets_json_response(self):
35703567
Test that staff user gets JSON response from xblock_outline_handler.
35713568
"""
35723569
outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
3573-
3570+
35743571
self.client.login(username=self.staff_user.username, password=self.password)
35753572
resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
3576-
3573+
35773574
assert resp.status_code == 200
35783575
json_response = json.loads(resp.content.decode("utf-8"))
35793576
assert "id" in json_response
@@ -3585,10 +3582,10 @@ def test_authorized_chapter_outline(self):
35853582
Test that authorized user can access chapter-level outline.
35863583
"""
35873584
outline_url = reverse_usage_url("xblock_outline_handler", self.chapter.location)
3588-
3585+
35893586
self.client.login(username=self.authorized_user.username, password=self.password)
35903587
resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
3591-
3588+
35923589
assert resp.status_code == 200
35933590
json_response = json.loads(resp.content.decode("utf-8"))
35943591
assert json_response["display_name"] == "Week 1"
@@ -3603,10 +3600,10 @@ def test_unauthorized_chapter_outline(self):
36033600
Test that unauthorized user cannot access chapter-level outline.
36043601
"""
36053602
outline_url = reverse_usage_url("xblock_outline_handler", self.chapter.location)
3606-
3603+
36073604
self.client.login(username=self.unauthorized_user.username, password=self.password)
36083605
resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
3609-
3606+
36103607
assert resp.status_code == 403
36113608

36123609

openedx/core/djangoapps/content_tagging/auth.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,15 @@
55

66
from opaque_keys import InvalidKeyError
77
from opaque_keys.edx.keys import CourseKey
8+
from opaque_keys.edx.locator import LibraryLocatorV2
89
from openedx_authz import api as authz_api
910
from openedx_authz.constants.permissions import COURSES_EXPORT_TAGS
1011
from openedx_tagging import rules as oel_tagging_rules
1112

1213
from openedx.core import toggles as core_toggles
1314

15+
from .utils import get_context_key_from_key_string
16+
1417
log = logging.getLogger(__name__)
1518

1619

@@ -39,3 +42,27 @@ def has_view_object_tags_access(user, object_id):
3942
# The obj arg expects a model, but we are passing an object
4043
oel_tagging_rules.ObjectTagPermissionItem(taxonomy=None, object_id=object_id), # type: ignore[arg-type]
4144
)
45+
46+
47+
def should_use_authz_for_object(object_id) -> tuple[bool, CourseKey | None]:
48+
"""
49+
Check if openedx-authz should be used for the given object based on the context key and toggle.
50+
51+
Returns (should_use_authz, course_key) where:
52+
- should_use_authz: True if authz should be used, False otherwise
53+
- course_key: The CourseKey if object is a course, None otherwise
54+
"""
55+
# Extract context_key and ensure it is a course_key
56+
try:
57+
context_key = get_context_key_from_key_string(object_id)
58+
if not isinstance(context_key, CourseKey) or isinstance(context_key, LibraryLocatorV2):
59+
return False, None
60+
except (ValueError, AttributeError):
61+
return False, None
62+
63+
# Check if toggle is active
64+
if not core_toggles.enable_authz_course_authoring(context_key):
65+
return False, context_key
66+
67+
# Authz should be used for this course object
68+
return True, context_key

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

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,18 @@
44

55
from __future__ import annotations
66

7+
from openedx_authz import api as authz_api
8+
from openedx_authz.constants.permissions import COURSES_MANAGE_TAGS
79
from openedx_tagging.rest_api.v1.serializers import (
810
ObjectTagMinimalSerializer,
11+
ObjectTagsByTaxonomySerializer,
912
TaxonomyListQueryParamsSerializer,
1013
TaxonomySerializer,
1114
)
1215
from organizations.models import Organization
1316
from rest_framework import fields, serializers
1417

18+
from ...auth import should_use_authz_for_object
1519
from ...models import TaxonomyOrg
1620

1721

@@ -95,27 +99,61 @@ class Meta:
9599
read_only_fields = ["orgs", "all_orgs"]
96100

97101

102+
class ObjectTagOrgByTaxonomySerializer(ObjectTagsByTaxonomySerializer):
103+
"""
104+
Extend ObjectTagsByTaxonomySerializer to conditionally use openedx-authz for can_tag_object.
105+
"""
106+
107+
def can_tag_object(self, obj_tag) -> bool | None:
108+
"""
109+
Check if the user is authorized to tag the provided object.
110+
Conditionally use openedx-authz for course objects with the toggle enabled.
111+
"""
112+
should_use_authz, course_key = should_use_authz_for_object(obj_tag.object_id)
113+
if should_use_authz:
114+
request = self.context.get('request')
115+
if request and hasattr(request, 'user'):
116+
return authz_api.is_user_allowed(
117+
request.user.username, COURSES_MANAGE_TAGS.identifier, str(course_key)
118+
)
119+
return False
120+
121+
# Fall back to parent implementation
122+
return super().can_tag_object(obj_tag)
123+
124+
98125
class ObjectTagCopiedMinimalSerializer(ObjectTagMinimalSerializer):
99126
"""
100127
Serializer for Object Tags.
101128
102-
This override `get_can_delete_objecttag` to avoid delete
103-
object tags if is copied.
129+
This overrides `can_delete_object_tag` to avoid deleting
130+
object tags if they are copied and to conditionally use openedx-authz.
104131
"""
105132

106133
is_copied = serializers.BooleanField(read_only=True)
107134

108135
class Meta(ObjectTagMinimalSerializer.Meta):
109136
fields = ObjectTagMinimalSerializer.Meta.fields + ["is_copied"]
110137

111-
def get_can_delete_objecttag(self, instance):
138+
def can_delete_object_tag(self, instance) -> bool | None:
112139
"""
113-
Verify if the user can delete the object tag.
140+
Check if the user is authorized to delete the provided tag.
114141
115-
Override to return `False` if the object tag is copied.
142+
Override to return `False` if the object tag is copied,
143+
and conditionally use openedx-authz for course objects with the toggle enabled.
116144
"""
117145
if instance.is_copied:
118146
# The user can't delete copied tags.
119147
return False
120148

121-
return super().get_can_delete_objecttag(instance)
149+
should_use_authz, course_key = should_use_authz_for_object(instance.object_id)
150+
if should_use_authz:
151+
request = self.context.get('request')
152+
if request and hasattr(request, 'user'):
153+
return authz_api.is_user_allowed(
154+
request.user.username, COURSES_MANAGE_TAGS.identifier, str(course_key)
155+
)
156+
return False
157+
158+
# Fall back to parent implementation
159+
return super().can_delete_object_tag(instance)

0 commit comments

Comments
 (0)