Skip to content

Commit e698efa

Browse files
rodmgwgurodmgwgu
authored
feat: add authz permission checks for course grading configuration endpoints (#38208)
1 parent 4629823 commit e698efa

4 files changed

Lines changed: 286 additions & 21 deletions

File tree

‎cms/djangoapps/contentstore/rest_api/v0/tests/test_authoring_grading.py‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
"""
2+
Unit tests for authoring grading views.
3+
"""
4+
import json
5+
6+
from openedx_authz.constants.roles import COURSE_DATA_RESEARCHER, COURSE_STAFF
7+
from rest_framework import status
8+
from rest_framework.test import APIClient
9+
10+
from cms.djangoapps.contentstore.api.tests.base import BaseCourseViewTest
11+
from common.djangoapps.student.tests.factories import UserFactory
12+
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthzTestMixin
13+
14+
class AuthoringGradingViewAuthzTest(CourseAuthzTestMixin, BaseCourseViewTest):
15+
"""
16+
Tests Authoring Grading configuration API authorization using openedx-authz.
17+
The endpoint uses the COURSES_EDIT_GRADING_SETTINGS permission.
18+
"""
19+
20+
view_name = "cms.djangoapps.contentstore:v0:cms_api_update_grading"
21+
authz_roles_to_assign = [COURSE_STAFF.external_key]
22+
post_data = json.dumps({
23+
"graders": [
24+
{
25+
"type": "Homework",
26+
"min_count": 1,
27+
"drop_count": 0,
28+
"short_label": "",
29+
"weight": 100,
30+
"id": 0
31+
}
32+
],
33+
"grade_cutoffs": {
34+
"A": 0.75,
35+
"B": 0.63,
36+
"C": 0.57,
37+
"D": 0.5
38+
},
39+
"grace_period": {
40+
"hours": 12,
41+
"minutes": 0
42+
},
43+
"minimum_grade_credit": 0.7,
44+
"is_credit_course": True
45+
})
46+
47+
def test_authorized_user_can_access_post(self):
48+
"""User with COURSE_STAFF role can access."""
49+
resp = self.authorized_client.post(
50+
self.get_url(self.course_key),
51+
data=self.post_data,
52+
content_type="application/json"
53+
)
54+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
55+
56+
def test_unauthorized_user_cannot_access_post(self):
57+
"""User without role cannot access."""
58+
resp = self.unauthorized_client.post(
59+
self.get_url(self.course_key),
60+
data=self.post_data,
61+
content_type="application/json"
62+
)
63+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
64+
65+
def test_role_scoped_to_course_post(self):
66+
"""Authorization should only apply to the assigned course."""
67+
other_course = self.store.create_course("OtherOrg", "OtherCourse", "Run", self.staff.id)
68+
69+
resp = self.authorized_client.post(
70+
self.get_url(other_course.id),
71+
data=self.post_data,
72+
content_type="application/json"
73+
)
74+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
75+
76+
def test_staff_user_allowed_via_legacy_post(self):
77+
"""
78+
Staff users should still pass through legacy fallback.
79+
"""
80+
self.client.login(username=self.staff.username, password=self.password)
81+
82+
resp = self.client.post(
83+
self.get_url(self.course_key),
84+
data=self.post_data,
85+
content_type="application/json"
86+
)
87+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
88+
89+
def test_superuser_allowed_post(self):
90+
"""Superusers should always be allowed."""
91+
superuser = UserFactory(is_superuser=True)
92+
93+
client = APIClient()
94+
client.force_authenticate(user=superuser)
95+
96+
resp = client.post(
97+
self.get_url(self.course_key),
98+
data=self.post_data,
99+
content_type="application/json"
100+
)
101+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
102+
103+
def test_non_staff_user_cannot_access_post(self):
104+
"""
105+
User without required permissions should be denied.
106+
This case validates that a non-staff user doesn't get access.
107+
"""
108+
non_staff_user = UserFactory()
109+
non_staff_client = APIClient()
110+
self.add_user_to_role(non_staff_user, COURSE_DATA_RESEARCHER.external_key)
111+
non_staff_client.force_authenticate(user=non_staff_user)
112+
113+
resp = non_staff_client.post(
114+
self.get_url(self.course_key),
115+
data=self.post_data,
116+
content_type="application/json"
117+
)
118+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)

‎cms/djangoapps/contentstore/rest_api/v0/views/authoring_grading.py‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@
22

33
import edx_api_doc_tools as apidocs
44
from opaque_keys.edx.keys import CourseKey
5+
from openedx_authz.constants.permissions import COURSES_EDIT_GRADING_SETTINGS
56
from rest_framework.request import Request
67
from rest_framework.response import Response
78
from rest_framework.views import APIView
89

910
from cms.djangoapps.models.settings.course_grading import CourseGradingModel
10-
from common.djangoapps.student.auth import has_studio_read_access
11+
from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
12+
from openedx.core.djangoapps.authz.decorators import authz_permission_required
1113
from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements
1214
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes
1315
from ..serializers import CourseGradingModelSerializer
@@ -31,7 +33,10 @@ class AuthoringGradingView(DeveloperErrorViewMixin, APIView):
3133
},
3234
)
3335
@verify_course_exists()
34-
def post(self, request: Request, course_id: str):
36+
# Please note: previous legacy permisison was checking for has_studio_read_access
37+
# So we are using LegacyAuthoringPermission.READ to keep compatibility
38+
@authz_permission_required(COURSES_EDIT_GRADING_SETTINGS.identifier, LegacyAuthoringPermission.READ)
39+
def post(self, request: Request, course_key: CourseKey):
3540
"""
3641
Update a course's grading.
3742
@@ -75,11 +80,6 @@ def post(self, request: Request, course_id: str):
7580
7681
If the request is successful, an HTTP 200 "OK" response is returned,
7782
"""
78-
course_key = CourseKey.from_string(course_id)
79-
80-
if not has_studio_read_access(request.user, course_key):
81-
self.permission_denied(request)
82-
8383
if 'minimum_grade_credit' in request.data:
8484
update_credit_course_requirements.delay(str(course_key))
8585

‎cms/djangoapps/contentstore/rest_api/v1/views/grading.py‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
import edx_api_doc_tools as apidocs
44
from django.conf import settings
55
from opaque_keys.edx.keys import CourseKey
6+
from openedx_authz.constants.permissions import COURSES_VIEW_GRADING_SETTINGS, COURSES_EDIT_GRADING_SETTINGS
67
from rest_framework.request import Request
78
from rest_framework.response import Response
89
from rest_framework.views import APIView
910

1011
from cms.djangoapps.models.settings.course_grading import CourseGradingModel
11-
from common.djangoapps.student.auth import has_studio_read_access
12+
from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
13+
from openedx.core.djangoapps.authz.decorators import authz_permission_required
1214
from openedx.core.djangoapps.credit.api import is_credit_course
1315
from openedx.core.djangoapps.credit.tasks import update_credit_course_requirements
1416
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes
@@ -36,7 +38,8 @@ class CourseGradingView(DeveloperErrorViewMixin, APIView):
3638
},
3739
)
3840
@verify_course_exists()
39-
def get(self, request: Request, course_id: str):
41+
@authz_permission_required(COURSES_VIEW_GRADING_SETTINGS.identifier, LegacyAuthoringPermission.READ)
42+
def get(self, request: Request, course_key: CourseKey):
4043
"""
4144
Get an object containing course grading settings with model.
4245
@@ -90,11 +93,6 @@ def get(self, request: Request, course_id: str):
9093
}
9194
```
9295
"""
93-
course_key = CourseKey.from_string(course_id)
94-
95-
if not has_studio_read_access(request.user, course_key):
96-
self.permission_denied(request)
97-
9896
with modulestore().bulk_operations(course_key):
9997
credit_eligibility_enabled = settings.FEATURES.get("ENABLE_CREDIT_ELIGIBILITY", False)
10098
show_credit_eligibility = is_credit_course(course_key) and credit_eligibility_enabled
@@ -118,13 +116,16 @@ def get(self, request: Request, course_id: str):
118116
},
119117
)
120118
@verify_course_exists()
121-
def post(self, request: Request, course_id: str):
119+
# Please note: previous legacy permisison was checking for has_studio_read_access
120+
# So we are using LegacyAuthoringPermission.READ to keep compatibility
121+
@authz_permission_required(COURSES_EDIT_GRADING_SETTINGS.identifier, LegacyAuthoringPermission.READ)
122+
def post(self, request: Request, course_key: CourseKey):
122123
"""
123124
Update a course's grading.
124125
125126
**Example Request**
126127
127-
PUT /api/contentstore/v1/course_grading/{course_id}
128+
POST /api/contentstore/v1/course_grading/{course_id}
128129
129130
**POST Parameters**
130131
@@ -162,11 +163,6 @@ def post(self, request: Request, course_id: str):
162163
163164
If the request is successful, an HTTP 200 "OK" response is returned,
164165
"""
165-
course_key = CourseKey.from_string(course_id)
166-
167-
if not has_studio_read_access(request.user, course_key):
168-
self.permission_denied(request)
169-
170166
if 'minimum_grade_credit' in request.data:
171167
update_credit_course_requirements.delay(str(course_key))
172168

‎cms/djangoapps/contentstore/rest_api/v1/views/tests/test_grading.py‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,16 @@
66

77
import ddt
88
from django.urls import reverse
9+
from openedx_authz.constants.roles import COURSE_DATA_RESEARCHER, COURSE_STAFF
910
from rest_framework import status
11+
from rest_framework.test import APIClient
1012

13+
from cms.djangoapps.contentstore.api.tests.base import BaseCourseViewTest
1114
from cms.djangoapps.contentstore.tests.utils import CourseTestCase
1215
from cms.djangoapps.contentstore.utils import get_proctored_exam_settings_url
1316
from cms.djangoapps.models.settings.course_grading import CourseGradingModel
17+
from common.djangoapps.student.tests.factories import UserFactory
18+
from openedx.core.djangoapps.authz.tests.mixins import CourseAuthzTestMixin
1419
from openedx.core.djangoapps.credit.tests.factories import CreditCourseFactory
1520

1621
from ...mixins import PermissionAccessMixin
@@ -117,3 +122,149 @@ def test_post_course_grading(self, mock_update_credit_course_requirements):
117122
)
118123
self.assertEqual(response.status_code, status.HTTP_200_OK)
119124
mock_update_credit_course_requirements.assert_called_once()
125+
126+
127+
class CourseGradingViewAuthzTest(CourseAuthzTestMixin, BaseCourseViewTest):
128+
"""
129+
Tests Course Grading Configuration API authorization using openedx-authz.
130+
The endpoint uses COURSES_VIEW_GRADING_SETTINGS and COURSES_EDIT_GRADING_SETTINGS permissions.
131+
"""
132+
133+
view_name = "cms.djangoapps.contentstore:v1:course_grading"
134+
authz_roles_to_assign = [COURSE_STAFF.external_key]
135+
post_data = json.dumps({
136+
"graders": [{
137+
"type": "Homework",
138+
"min_count": 1,
139+
"drop_count": 0,
140+
"short_label": "",
141+
"weight": 100,
142+
"id": 0
143+
}],
144+
"grade_cutoffs": {"A": 0.75, "B": 0.63, "C": 0.57, "D": 0.5},
145+
"grace_period": {"hours": 12, "minutes": 0},
146+
"minimum_grade_credit": 0.7,
147+
"is_credit_course": False,
148+
})
149+
150+
def test_authorized_user_can_access_get(self):
151+
"""User with COURSE_STAFF role can access."""
152+
resp = self.authorized_client.get(self.get_url(self.course_key))
153+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
154+
155+
def test_unauthorized_user_cannot_access_get(self):
156+
"""User without role cannot access."""
157+
resp = self.unauthorized_client.get(self.get_url(self.course_key))
158+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
159+
160+
def test_role_scoped_to_course_get(self):
161+
"""Authorization should only apply to the assigned course."""
162+
other_course = self.store.create_course("OtherOrg", "OtherCourse", "Run", self.staff.id)
163+
164+
resp = self.authorized_client.get(self.get_url(other_course.id))
165+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
166+
167+
def test_staff_user_allowed_via_legacy_get(self):
168+
"""
169+
Staff users should still pass through legacy fallback.
170+
"""
171+
self.client.login(username=self.staff.username, password=self.password)
172+
173+
resp = self.client.get(self.get_url(self.course_key))
174+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
175+
176+
def test_superuser_allowed_get(self):
177+
"""Superusers should always be allowed."""
178+
superuser = UserFactory(is_superuser=True)
179+
180+
client = APIClient()
181+
client.force_authenticate(user=superuser)
182+
183+
resp = client.get(self.get_url(self.course_key))
184+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
185+
186+
def test_non_staff_user_cannot_access_get(self):
187+
"""
188+
User without required permissions should be denied.
189+
This case validates that a non-staff user doesn't get access.
190+
"""
191+
non_staff_user = UserFactory()
192+
non_staff_client = APIClient()
193+
self.add_user_to_role(non_staff_user, COURSE_DATA_RESEARCHER.external_key)
194+
non_staff_client.force_authenticate(user=non_staff_user)
195+
196+
resp = non_staff_client.get(self.get_url(self.course_key))
197+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
198+
199+
def test_authorized_user_can_access_post(self):
200+
"""User with COURSE_STAFF role can access."""
201+
resp = self.authorized_client.post(
202+
self.get_url(self.course_key),
203+
data=self.post_data,
204+
content_type="application/json"
205+
)
206+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
207+
208+
def test_unauthorized_user_cannot_access_post(self):
209+
"""User without role cannot access."""
210+
resp = self.unauthorized_client.post(
211+
self.get_url(self.course_key),
212+
data=self.post_data,
213+
content_type="application/json"
214+
)
215+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
216+
217+
def test_role_scoped_to_course_post(self):
218+
"""Authorization should only apply to the assigned course."""
219+
other_course = self.store.create_course("OtherOrg", "OtherCourse", "Run", self.staff.id)
220+
221+
resp = self.authorized_client.post(
222+
self.get_url(other_course.id),
223+
data=self.post_data,
224+
content_type="application/json"
225+
)
226+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
227+
228+
def test_staff_user_allowed_via_legacy_post(self):
229+
"""
230+
Staff users should still pass through legacy fallback.
231+
"""
232+
self.client.login(username=self.staff.username, password=self.password)
233+
234+
resp = self.client.post(
235+
self.get_url(self.course_key),
236+
data=self.post_data,
237+
content_type="application/json"
238+
)
239+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
240+
241+
def test_superuser_allowed_post(self):
242+
"""Superusers should always be allowed."""
243+
superuser = UserFactory(is_superuser=True)
244+
245+
client = APIClient()
246+
client.force_authenticate(user=superuser)
247+
248+
resp = client.post(
249+
self.get_url(self.course_key),
250+
data=self.post_data,
251+
content_type="application/json"
252+
)
253+
self.assertEqual(resp.status_code, status.HTTP_200_OK)
254+
255+
def test_non_staff_user_cannot_access_post(self):
256+
"""
257+
User without required permissions should be denied.
258+
This case validates that a non-staff user doesn't get access.
259+
"""
260+
non_staff_user = UserFactory()
261+
non_staff_client = APIClient()
262+
self.add_user_to_role(non_staff_user, COURSE_DATA_RESEARCHER.external_key)
263+
non_staff_client.force_authenticate(user=non_staff_user)
264+
265+
resp = non_staff_client.post(
266+
self.get_url(self.course_key),
267+
data=self.post_data,
268+
content_type="application/json"
269+
)
270+
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)

0 commit comments

Comments
 (0)