From 480c1ff99f605f97fcc93c473fc5b2dd5fd73d67 Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Fri, 14 Aug 2026 14:54:29 -0600 Subject: [PATCH] feat: split read vs write authz checks for certificates Use COURSES_VIEW_CERTIFICATES for GET access and COURSES_MANAGE_CERTIFICATES for write access. Add can_manage flag to the response so the frontend knows whether to render edit controls. Course Editors and Auditors can now view certificates in read-only mode. ENG45-714 --- .../rest_api/v1/serializers/certificates.py | 1 + .../rest_api/v1/views/certificates.py | 14 ++++-- .../v1/views/tests/test_certificates.py | 45 +++++++++++++++---- 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/certificates.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/certificates.py index 9e536efa4550..32e8c16f1d1f 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/certificates.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/certificates.py @@ -50,3 +50,4 @@ class CourseCertificatesSerializer(serializers.Serializer): course_number = serializers.CharField(source="context_course.number") course_title = serializers.CharField(source="context_course.display_name_with_default") course_number_override = serializers.CharField(source="context_course.display_coursenumber") + can_manage = serializers.BooleanField(default=False, read_only=True) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/certificates.py b/cms/djangoapps/contentstore/rest_api/v1/views/certificates.py index 43069553a067..8943a61f74db 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/certificates.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/certificates.py @@ -2,7 +2,7 @@ import edx_api_doc_tools as apidocs from opaque_keys.edx.keys import CourseKey -from openedx_authz.constants.permissions import COURSES_MANAGE_CERTIFICATES +from openedx_authz.constants.permissions import COURSES_MANAGE_CERTIFICATES, COURSES_VIEW_CERTIFICATES from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView @@ -94,14 +94,22 @@ def get(self, request: Request, course_id: str): if not user_has_course_permission( request.user, - COURSES_MANAGE_CERTIFICATES.identifier, + COURSES_VIEW_CERTIFICATES.identifier, course_key, - LegacyAuthoringPermission.WRITE + LegacyAuthoringPermission.READ ): self.permission_denied(request) + can_manage = user_has_course_permission( + request.user, + COURSES_MANAGE_CERTIFICATES.identifier, + course_key, + LegacyAuthoringPermission.WRITE + ) + with store.bulk_operations(course_key): course = modulestore().get_course(course_key) certificates_context = get_certificates_context(course, request.user) + certificates_context['can_manage'] = can_manage serializer = CourseCertificatesSerializer(certificates_context) return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_certificates.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_certificates.py index 9ae3ba84d8bc..46fb221e61ca 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_certificates.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_certificates.py @@ -2,7 +2,7 @@ Unit tests for the course's certificate. """ from django.urls import reverse -from openedx_authz.constants.roles import COURSE_EDITOR, COURSE_STAFF +from openedx_authz.constants.roles import COURSE_AUDITOR, COURSE_EDITOR, COURSE_STAFF from rest_framework import status from cms.djangoapps.contentstore.tests.utils import CourseTestCase @@ -59,14 +59,41 @@ def test_authorized_user_can_access(self): self._add_course_certificates(count=2, signatory_count=2) self.add_user_to_role_in_course(self.authorized_user, COURSE_STAFF.external_key, self.course.id) resp = self.authorized_client.get(self.url) - self.assertEqual(resp.status_code, status.HTTP_200_OK) # noqa: PT009 + assert resp.status_code == status.HTTP_200_OK - def test_non_staff_user_cannot_access(self): - """ - User without permissions should be denied. - This case validates that a non-staff user cannot access. - """ - self._add_course_certificates(count=2, signatory_count=2) + def test_staff_role_has_can_manage_true(self): + """User with COURSE_STAFF role gets can_manage=True in response.""" + self._add_course_certificates(count=1, signatory_count=1) + self.add_user_to_role_in_course(self.authorized_user, COURSE_STAFF.external_key, self.course.id) + resp = self.authorized_client.get(self.url) + assert resp.status_code == status.HTTP_200_OK + assert resp.data["can_manage"] is True + + def test_editor_can_view_certificates(self): + """User with COURSE_EDITOR role can view certificates (has view_certificates).""" + self._add_course_certificates(count=1, signatory_count=1) self.add_user_to_role_in_course(self.authorized_user, COURSE_EDITOR.external_key, self.course.id) resp = self.authorized_client.get(self.url) - self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) # noqa: PT009 + assert resp.status_code == status.HTTP_200_OK + + def test_editor_has_can_manage_false(self): + """User with COURSE_EDITOR role gets can_manage=False (no manage_certificates).""" + self._add_course_certificates(count=1, signatory_count=1) + self.add_user_to_role_in_course(self.authorized_user, COURSE_EDITOR.external_key, self.course.id) + resp = self.authorized_client.get(self.url) + assert resp.status_code == status.HTTP_200_OK + assert resp.data["can_manage"] is False + + def test_unauthorized_user_cannot_access(self): + """User without any role cannot access.""" + self._add_course_certificates(count=1, signatory_count=1) + resp = self.unauthorized_client.get(self.url) + assert resp.status_code == status.HTTP_403_FORBIDDEN + + def test_auditor_can_view_certificates(self): + """User with COURSE_AUDITOR role can view certificates (has view_certificates).""" + self._add_course_certificates(count=1, signatory_count=1) + self.add_user_to_role_in_course(self.authorized_user, COURSE_AUDITOR.external_key, self.course.id) + resp = self.authorized_client.get(self.url) + assert resp.status_code == status.HTTP_200_OK + assert resp.data["can_manage"] is False