Skip to content

Commit ef6373b

Browse files
fixup! feat: check authz permissions for course tagging
1 parent 6656262 commit ef6373b

6 files changed

Lines changed: 42 additions & 45 deletions

File tree

cms/djangoapps/contentstore/views/block.py

Lines changed: 3 additions & 3 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

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: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@
77
from opaque_keys.edx.keys import CourseKey
88
from opaque_keys.edx.locator import LibraryLocatorV2
99
from openedx_authz import api as authz_api
10-
from openedx_authz.constants.permissions import COURSES_EXPORT_TAGS, COURSES_MANAGE_TAGS, COURSES_VIEW_COURSE
10+
from openedx_authz.constants.permissions import COURSES_EXPORT_TAGS
1111
from openedx_tagging import rules as oel_tagging_rules
1212

1313
from openedx.core import toggles as core_toggles
14+
1415
from .utils import get_context_key_from_key_string
1516

1617
log = logging.getLogger(__name__)
@@ -46,7 +47,7 @@ def has_view_object_tags_access(user, object_id):
4647
def should_use_authz_for_object(object_id) -> tuple[bool, CourseKey | None]:
4748
"""
4849
Check if openedx-authz should be used for the given object based on the context key and toggle.
49-
50+
5051
Returns (should_use_authz, course_key) where:
5152
- should_use_authz: True if authz should be used, False otherwise
5253
- course_key: The CourseKey if object is a course, None otherwise
@@ -62,6 +63,6 @@ def should_use_authz_for_object(object_id) -> tuple[bool, CourseKey | None]:
6263
# Check if toggle is active
6364
if not core_toggles.enable_authz_course_authoring(context_key):
6465
return False, context_key
65-
66+
6667
# Authz should be used for this course object
6768
return True, context_key

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,16 @@
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,
911
ObjectTagsByTaxonomySerializer,
10-
ObjectTagSerializer,
1112
TaxonomyListQueryParamsSerializer,
1213
TaxonomySerializer,
1314
)
1415
from organizations.models import Organization
1516
from rest_framework import fields, serializers
16-
from openedx_authz import api as authz_api
17-
from openedx_authz.constants.permissions import COURSES_MANAGE_TAGS
1817

1918
from ...auth import should_use_authz_for_object
2019
from ...models import TaxonomyOrg
@@ -118,7 +117,7 @@ def can_tag_object(self, obj_tag) -> bool | None:
118117
request.user.username, COURSES_MANAGE_TAGS.identifier, str(course_key)
119118
)
120119
return False
121-
120+
122121
# Fall back to parent implementation
123122
return super().can_tag_object(obj_tag)
124123

@@ -139,7 +138,7 @@ class Meta(ObjectTagMinimalSerializer.Meta):
139138
def can_delete_object_tag(self, instance) -> bool | None:
140139
"""
141140
Check if the user is authorized to delete the provided tag.
142-
141+
143142
Override to return `False` if the object tag is copied,
144143
and conditionally use openedx-authz for course objects with the toggle enabled.
145144
"""

openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2153,11 +2153,11 @@ def setUpClass(cls):
21532153

21542154
def setUp(self):
21552155
super().setUp()
2156-
2156+
21572157
# Create another course for cross-course scoping tests
21582158
self.other_course = self.store.create_course("OtherOrg", "OtherCourse", "Run", self.authorized_user.id)
21592159
self.other_course_key = self.other_course.id
2160-
2160+
21612161
# Create taxonomy
21622162
self.taxonomy = tagging_api.create_taxonomy(
21632163
name="Test Taxonomy",
@@ -2168,7 +2168,7 @@ def setUp(self):
21682168
org=None, # Global taxonomy not tied to any org
21692169
rel_type=TaxonomyOrg.RelType.OWNER,
21702170
)
2171-
2171+
21722172
# Create tags
21732173
self.tag1 = Tag.objects.create(
21742174
taxonomy=self.taxonomy,
@@ -2178,12 +2178,12 @@ def setUp(self):
21782178
taxonomy=self.taxonomy,
21792179
value="Tag 2",
21802180
)
2181-
2182-
# Create auditor user with view-only permissions
2181+
2182+
# Create auditor user with view-only permissions
21832183
self.auditor_user = UserFactory(password=self.password)
21842184
self.auditor_client = APIClient()
21852185
self.auditor_client.force_authenticate(user=self.auditor_user)
2186-
2186+
21872187
# Assign auditor role to auditor_user
21882188
self.add_user_to_role_in_course(
21892189
self.auditor_user,
@@ -2200,7 +2200,7 @@ def _update_tags_request(self, object_id, tags_data=None):
22002200
"tags": ["Tag 1", "Tag 2"]
22012201
}
22022202
]
2203-
2203+
22042204
url = OBJECT_TAG_UPDATE_URL.format(object_id=object_id)
22052205
return url, {"tagsData": tags_data}
22062206

@@ -2251,12 +2251,12 @@ def test_course_staff_sees_manage_permissions(self):
22512251
url, data = self._update_tags_request(str(self.course_key))
22522252
response = self.authorized_client.put(url, data, format='json')
22532253
assert response.status_code == status.HTTP_200_OK
2254-
2254+
22552255
# Now check permissions in GET response
22562256
url = self._get_tags_request(str(self.course_key))
22572257
response = self.authorized_client.get(url)
22582258
assert response.status_code == status.HTTP_200_OK
2259-
2259+
22602260
# Check serializer permissions in response data
22612261
taxonomies = response.data[str(self.course_key)]["taxonomies"]
22622262
assert len(taxonomies) == 1
@@ -2273,12 +2273,12 @@ def test_course_auditor_sees_view_only_permissions(self):
22732273
url, data = self._update_tags_request(str(self.course_key))
22742274
response = self.authorized_client.put(url, data, format='json')
22752275
assert response.status_code == status.HTTP_200_OK
2276-
2276+
22772277
# Now check permissions as auditor in GET response
22782278
url = self._get_tags_request(str(self.course_key))
22792279
response = self.auditor_client.get(url)
22802280
assert response.status_code == status.HTTP_200_OK
2281-
2281+
22822282
# Check serializer permissions in response data
22832283
taxonomies = response.data[str(self.course_key)]["taxonomies"]
22842284
assert len(taxonomies) == 1
@@ -2299,7 +2299,7 @@ def test_library_fallthrough_to_legacy(self):
22992299
"""Library object_id falls through to legacy permissions"""
23002300
# Create organization for library
23012301
org, created = Organization.objects.get_or_create(short_name="TestOrg")
2302-
2302+
23032303
# Create library
23042304
library = create_library(
23052305
org=org,
@@ -2308,14 +2308,14 @@ def test_library_fallthrough_to_legacy(self):
23082308
description="Test library for authz fallthrough",
23092309
)
23102310
library_key = library.key
2311-
2311+
23122312
# Grant library access to authorized_user
23132313
set_library_user_permissions(
23142314
library_key,
23152315
self.authorized_user,
23162316
AccessLevel.ADMIN_LEVEL
23172317
)
2318-
2318+
23192319
# Test that library requests fall through to legacy permissions
23202320
url = self._get_tags_request(str(library_key))
23212321
response = self.authorized_client.get(url)

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

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

66
from django.db.models import Count
77
from django.http import StreamingHttpResponse
8+
from openedx_authz import api as authz_api
9+
from openedx_authz.constants.permissions import COURSES_MANAGE_TAGS, COURSES_VIEW_COURSE
810
from openedx_events.content_authoring.data import ContentObjectChangedData, ContentObjectData
911
from openedx_events.content_authoring.signals import CONTENT_OBJECT_ASSOCIATIONS_CHANGED, CONTENT_OBJECT_TAGS_CHANGED
1012
from openedx_tagging import rules as oel_tagging_rules
1113
from openedx_tagging.rest_api.v1.views import ObjectTagView, TaxonomyView
12-
from openedx_authz.constants.permissions import COURSES_MANAGE_TAGS, COURSES_VIEW_COURSE
13-
from openedx_authz import api as authz_api
1414
from rest_framework import status
1515
from rest_framework.decorators import action
1616
from rest_framework.exceptions import PermissionDenied, ValidationError
@@ -174,14 +174,14 @@ def _should_use_authz(self) -> bool:
174174
def get_permissions(self):
175175
"""
176176
Override get_permissions when using openedx-authz.
177-
177+
178178
When the toggle is enabled for course objects, we need to change the default
179179
permission classes set by the parent ObjectTagView so that only openedx-authz
180180
permissions are used.
181181
"""
182182
if self._should_use_authz():
183183
return [IsAuthenticated()]
184-
184+
185185
return super().get_permissions()
186186

187187
def ensure_has_view_object_tag_permission(self, user, taxonomy, object_id):

0 commit comments

Comments
 (0)