From 480c1ff99f605f97fcc93c473fc5b2dd5fd73d67 Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Fri, 14 Aug 2026 14:54:29 -0600 Subject: [PATCH 1/2] 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 From 304d08aaf5511258b1715d6da1e92f458b695de3 Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Wed, 26 Aug 2026 15:17:11 -0600 Subject: [PATCH 2/2] refactor: address review feedback on certificates authz - add can_manage to the docstring example response - consolidate editor/auditor view tests using ddt --- .../rest_api/v1/views/certificates.py | 3 ++- .../v1/views/tests/test_certificates.py | 24 +++++-------------- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/certificates.py b/cms/djangoapps/contentstore/rest_api/v1/views/certificates.py index 8943a61f74db..adb835d30c2c 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/certificates.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/certificates.py @@ -85,7 +85,8 @@ def get(self, request: Request, course_id: str): "mfe_proctored_exam_settings_url": "", "course_number": "DemoX", "course_title": "Demonstration Course", - "course_number_override": "Course Number Display String" + "course_number_override": "Course Number Display String", + "can_manage": true } ``` """ 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 46fb221e61ca..8c0e0e79b4d2 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 @@ -1,6 +1,7 @@ """ Unit tests for the course's certificate. """ +import ddt from django.urls import reverse from openedx_authz.constants.roles import COURSE_AUDITOR, COURSE_EDITOR, COURSE_STAFF from rest_framework import status @@ -40,6 +41,7 @@ def test_success_response(self): self.assertEqual(response_data["course_number"], self.course.number) # noqa: PT009 +@ddt.ddt class CourseCertificatesAuthzViewTest( CourseAuthoringAuthzTestMixin, CourseTestCase, PermissionAccessMixin, HelperMethods ): @@ -69,17 +71,11 @@ def test_staff_role_has_can_manage_true(self): 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).""" + @ddt.data(COURSE_EDITOR, COURSE_AUDITOR) + def test_view_only_role_can_view_without_manage(self, role): + """Course editor/auditor can view certificates but gets can_manage=False.""" 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 - - 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) + self.add_user_to_role_in_course(self.authorized_user, role.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 @@ -89,11 +85,3 @@ def test_unauthorized_user_cannot_access(self): 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