Skip to content

Commit e3ab634

Browse files
authored
feat: add authz permission checks for the course group configuration endpoints (#38204)
1 parent e698efa commit e3ab634

4 files changed

Lines changed: 411 additions & 12 deletions

File tree

cms/djangoapps/contentstore/rest_api/v1/views/group_configurations.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@
55
from rest_framework.request import Request
66
from rest_framework.response import Response
77
from rest_framework.views import APIView
8+
from openedx_authz.constants.permissions import COURSES_MANAGE_GROUP_CONFIGURATIONS
89

910
from cms.djangoapps.contentstore.utils import get_group_configurations_context
1011
from cms.djangoapps.contentstore.rest_api.v1.serializers import (
1112
CourseGroupConfigurationsSerializer,
1213
)
13-
from common.djangoapps.student.auth import has_studio_read_access
14+
from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
15+
from openedx.core.djangoapps.authz.decorators import authz_permission_required
1416
from openedx.core.lib.api.view_utils import (
1517
DeveloperErrorViewMixin,
1618
verify_course_exists,
@@ -39,7 +41,11 @@ class CourseGroupConfigurationsView(DeveloperErrorViewMixin, APIView):
3941
},
4042
)
4143
@verify_course_exists()
42-
def get(self, request: Request, course_id: str):
44+
@authz_permission_required(
45+
authz_permission=COURSES_MANAGE_GROUP_CONFIGURATIONS.identifier,
46+
legacy_permission=LegacyAuthoringPermission.READ
47+
)
48+
def get(self, request: Request, course_key: CourseKey):
4349
"""
4450
Get an object containing course's settings group configurations.
4551
@@ -139,12 +145,8 @@ def get(self, request: Request, course_id: str):
139145
}
140146
```
141147
"""
142-
course_key = CourseKey.from_string(course_id)
143148
store = modulestore()
144149

145-
if not has_studio_read_access(request.user, course_key):
146-
self.permission_denied(request)
147-
148150
with store.bulk_operations(course_key):
149151
course = modulestore().get_course(course_key)
150152
group_configurations_context = get_group_configurations_context(course, store)

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_group_configurations.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,16 @@
33
"""
44
from django.urls import reverse
55
from rest_framework import status
6+
from rest_framework.test import APIClient
67

8+
from cms.djangoapps.contentstore.api.tests.base import BaseCourseViewTest
79
from cms.djangoapps.contentstore.course_group_config import (
810
CONTENT_GROUP_CONFIGURATION_NAME,
911
)
1012
from cms.djangoapps.contentstore.tests.utils import CourseTestCase
13+
from common.djangoapps.student.tests.factories import UserFactory
14+
from openedx_authz.constants.roles import COURSE_DATA_RESEARCHER, COURSE_STAFF
15+
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthzTestMixin
1116
from xmodule.partitions.partitions import (
1217
Group,
1318
UserPartition,
@@ -53,3 +58,61 @@ def test_success_response(self):
5358
self.assertContains(response, "Group C")
5459
self.assertContains(response, CONTENT_GROUP_CONFIGURATION_NAME)
5560
self.assertEqual(response.status_code, status.HTTP_200_OK)
61+
62+
class CourseGroupConfigurationsAuthzTest(CourseAuthzTestMixin, BaseCourseViewTest):
63+
"""
64+
Tests Course Group Configuration API authorization using openedx-authz.
65+
The endpoint uses COURSES_MANAGE_GROUP_CONFIGURATIONS permission.
66+
"""
67+
68+
view_name = "cms.djangoapps.contentstore:v1:group_configurations"
69+
authz_roles_to_assign = [COURSE_STAFF.external_key]
70+
71+
def test_authorized_user_can_access(self):
72+
"""User with COURSE_STAFF role can access."""
73+
resp = self.authorized_client.get(self.get_url(self.course_key))
74+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
75+
76+
def test_unauthorized_user_cannot_access(self):
77+
"""User without role cannot access."""
78+
resp = self.unauthorized_client.get(self.get_url(self.course_key))
79+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
80+
81+
def test_role_scoped_to_course(self):
82+
"""Authorization should only apply to the assigned course."""
83+
other_course = self.store.create_course("OtherOrg", "OtherCourse", "Run", self.staff.id)
84+
85+
resp = self.authorized_client.get(self.get_url(other_course.id))
86+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
87+
88+
def test_staff_user_allowed_via_legacy(self):
89+
"""
90+
Staff users should still pass through legacy fallback.
91+
"""
92+
self.client.login(username=self.staff.username, password=self.password)
93+
94+
resp = self.client.get(self.get_url(self.course_key))
95+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
96+
97+
def test_superuser_allowed(self):
98+
"""Superusers should always be allowed."""
99+
superuser = UserFactory(is_superuser=True)
100+
101+
client = APIClient()
102+
client.force_authenticate(user=superuser)
103+
104+
resp = client.get(self.get_url(self.course_key))
105+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
106+
107+
def test_non_staff_user_cannot_access(self):
108+
"""
109+
User without required permissions should be denied.
110+
This case validates that a non-staff user doesn't get access.
111+
"""
112+
non_staff_user = UserFactory()
113+
non_staff_client = APIClient()
114+
self.add_user_to_role(non_staff_user, COURSE_DATA_RESEARCHER.external_key)
115+
non_staff_client.force_authenticate(user=non_staff_user)
116+
117+
resp = non_staff_client.get(self.get_url(self.course_key))
118+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)

cms/djangoapps/contentstore/views/course.py

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@
5858
)
5959
from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
6060
from openedx.core.djangoapps.authz.decorators import user_has_course_permission
61-
from openedx_authz.constants.permissions import COURSES_MANAGE_COURSE_UPDATES, COURSES_VIEW_COURSE_UPDATES
61+
from openedx_authz.constants.permissions import (
62+
COURSES_MANAGE_COURSE_UPDATES,
63+
COURSES_VIEW_COURSE_UPDATES,
64+
COURSES_MANAGE_GROUP_CONFIGURATIONS,
65+
)
6266
from common.djangoapps.student.roles import (
6367
CourseInstructorRole,
6468
CourseStaffRole,
@@ -150,16 +154,37 @@ class AccessListFallback(Exception):
150154
pass # lint-amnesty, pylint: disable=unnecessary-pass
151155

152156

153-
def get_course_and_check_access(course_key, user, depth=0):
157+
def _get_course_block(course_key, depth=0):
154158
"""
155159
Function used to calculate and return the locator and course block
156160
for the view functions in this file.
157161
"""
158-
if not has_studio_read_access(user, course_key):
159-
raise PermissionDenied()
160162
course_block = modulestore().get_course(course_key, depth=depth)
161163
return course_block
162164

165+
def get_course_and_check_access(course_key, user, depth=0):
166+
"""
167+
Function used to validate permission and return a course block
168+
for the view functions in this file.
169+
"""
170+
if not has_studio_read_access(user, course_key):
171+
raise PermissionDenied()
172+
return _get_course_block(course_key, depth)
173+
174+
def get_course_and_check_manage_group_configurations_access(course_key, user, depth=0):
175+
"""
176+
Function used to validate permission and return a course block
177+
for the view functions for group configurations in this file.
178+
"""
179+
if not user_has_course_permission(
180+
user=user,
181+
authz_permission=COURSES_MANAGE_GROUP_CONFIGURATIONS.identifier,
182+
course_key=course_key,
183+
legacy_permission=LegacyAuthoringPermission.READ
184+
):
185+
raise PermissionDenied()
186+
return _get_course_block(course_key, depth)
187+
163188

164189
def reindex_course_and_check_access(course_key, user):
165190
"""
@@ -1628,7 +1653,7 @@ def group_configurations_list_handler(request, course_key_string):
16281653
course_key = CourseKey.from_string(course_key_string)
16291654
store = modulestore()
16301655
with store.bulk_operations(course_key):
1631-
course = get_course_and_check_access(course_key, request.user)
1656+
course = get_course_and_check_manage_group_configurations_access(course_key, request.user)
16321657

16331658
if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'):
16341659
if use_new_group_configurations_page(course_key):
@@ -1671,7 +1696,7 @@ def group_configurations_detail_handler(request, course_key_string, group_config
16711696
course_key = CourseKey.from_string(course_key_string)
16721697
store = modulestore()
16731698
with store.bulk_operations(course_key):
1674-
course = get_course_and_check_access(course_key, request.user)
1699+
course = get_course_and_check_manage_group_configurations_access(course_key, request.user)
16751700
matching_id = [p for p in course.user_partitions
16761701
if str(p.id) == str(group_configuration_id)]
16771702
if matching_id:

0 commit comments

Comments
 (0)