diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/course_details.py b/cms/djangoapps/contentstore/rest_api/v1/views/course_details.py index d5ccf3c6165e..3b78fb2240bf 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/course_details.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/course_details.py @@ -4,23 +4,28 @@ from django.core.exceptions import ValidationError from common.djangoapps.util.json_request import JsonResponseBadRequest from opaque_keys.edx.keys import CourseKey +from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView -from common.djangoapps.student.auth import has_studio_read_access +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from openedx.core.djangoapps.models.course_details import CourseDetails -from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes +from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists from xmodule.modulestore.django import modulestore +from cms.djangoapps.contentstore.views.permissions import HasStudioReadAccess from ..serializers import CourseDetailsSerializer from ....utils import update_course_details -@view_auth_classes(is_authenticated=True) class CourseDetailsView(DeveloperErrorViewMixin, APIView): """ View for getting and setting the course details. """ + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated, HasStudioReadAccess) + serializer_class = CourseDetailsSerializer @apidocs.schema( parameters=[ apidocs.string_parameter("course_id", apidocs.ParameterLocation.PATH, description="Course ID"), @@ -98,11 +103,8 @@ def get(self, request: Request, course_id: str): ``` """ course_key = CourseKey.from_string(course_id) - if not has_studio_read_access(request.user, course_key): - self.permission_denied(request) - course_details = CourseDetails.fetch(course_key) - serializer = CourseDetailsSerializer(course_details) + serializer = self.serializer_class(course_details) return Response(serializer.data) @apidocs.schema( @@ -141,9 +143,6 @@ def put(self, request: Request, course_id: str): along with all the course's details similar to a ``GET`` request. """ course_key = CourseKey.from_string(course_id) - if not has_studio_read_access(request.user, course_key): - self.permission_denied(request) - course_block = modulestore().get_course(course_key) try: @@ -151,5 +150,5 @@ def put(self, request: Request, course_id: str): except ValidationError as err: return JsonResponseBadRequest({"error": err.message}) - serializer = CourseDetailsSerializer(updated_data) + serializer = self.serializer_class(updated_data) return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_details.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_details.py index cbc2fdc98c3f..248067a224d3 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_details.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_details.py @@ -5,10 +5,13 @@ from unittest.mock import patch import ddt +from django.test import TestCase from django.urls import reverse from rest_framework import status +from rest_framework.test import APIClient from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from common.djangoapps.student.tests.factories import UserFactory from ...mixins import PermissionAccessMixin @@ -111,3 +114,72 @@ def test_put_course_details(self): content_type="application/json", ) self.assertEqual(response.status_code, status.HTTP_200_OK) + + +class CourseDetailsViewPermissionsTest(CourseTestCase): + """ + ADR 0026 – permission regression tests for CourseDetailsView. + + Verifies that permission_classes = (IsAuthenticated, HasStudioReadAccess) enforces + the same access rules previously split between @view_auth_classes(is_authenticated=True) + and the inline has_studio_read_access() checks in get() and put(). + """ + + def setUp(self): + super().setUp() + self.url = reverse( + "cms.djangoapps.contentstore:v1:course_details", + kwargs={"course_id": self.course.id}, + ) + + # --- Unauthenticated --- + def test_unauthenticated_get_returns_401(self): + """ + Unauthenticated GET must return 401. + + Before ADR 0026: enforced by @view_auth_classes(is_authenticated=True). + After ADR 0026: enforced by IsAuthenticated in permission_classes. + """ + self.client.logout() + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_unauthenticated_put_returns_401(self): + """ + Unauthenticated PUT must return 401. + + Before ADR 0026: enforced by @view_auth_classes(is_authenticated=True). + After ADR 0026: enforced by IsAuthenticated in permission_classes. + """ + self.client.logout() + response = self.client.put(self.url, data={}, content_type="application/json") + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + # --- Authenticated but no course access --- + def test_non_staff_get_returns_403(self): + """ + Authenticated user without course staff role must receive 403 on GET. + + Before ADR 0026: enforced by inline has_studio_read_access() check in get(). + After ADR 0026: enforced by HasStudioReadAccess in permission_classes. + """ + client, _ = self.create_non_staff_authed_user_client() + response = client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_non_staff_put_returns_403(self): + """ + Authenticated user without course staff role must receive 403 on PUT. + + Before ADR 0026: enforced by inline has_studio_read_access() check in put(). + After ADR 0026: enforced by HasStudioReadAccess in permission_classes. + """ + client, _ = self.create_non_staff_authed_user_client() + response = client.put(self.url, data={}, content_type="application/json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # --- Authorized --- + def test_course_staff_get_returns_200(self): + """Course staff/instructor must receive 200 on GET.""" + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/cms/djangoapps/contentstore/views/permissions.py b/cms/djangoapps/contentstore/views/permissions.py index 6d63c7a124f4..9dec0bec3270 100644 --- a/cms/djangoapps/contentstore/views/permissions.py +++ b/cms/djangoapps/contentstore/views/permissions.py @@ -4,7 +4,7 @@ from rest_framework.permissions import BasePermission -from common.djangoapps.student.auth import has_studio_write_access +from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access from openedx.core.lib.api.view_utils import validate_course_key @@ -20,3 +20,20 @@ def has_permission(self, request, view): course_key_string = view.kwargs.get("course_key_string") course_key = validate_course_key(course_key_string) return has_studio_write_access(request.user, course_key) + + +class HasStudioReadAccess(BasePermission): + """ + Check if the user has read access to studio for the requested course. + + ADR 0026: replaces inline ``if not has_studio_read_access(request.user, course_key): + self.permission_denied(request)`` checks previously embedded in view methods + (e.g. CourseDetailsView.get() and CourseDetailsView.put()). + + Expects the view to receive the course identifier as a ``course_id`` URL kwarg. + """ + + def has_permission(self, request, view): + course_key_string = view.kwargs.get("course_id") + course_key = validate_course_key(course_key_string) + return has_studio_read_access(request.user, course_key)