Merge pull request #8931 from edx/ziafazal/fix-SOL-1008

SOL-1008: only edx PM can activate/deactivate or delete certificate configuration

cms/djangoapps/contentstore/views/certificates.py

@@ -35,6 +35,7 @@ from edxmako.shortcuts import render_to_response
 from opaque_keys.edx.keys import CourseKey, AssetKey
 from eventtracking import tracker
 from student.auth import has_studio_write_access
+from student.roles import GlobalStaff
 from util.db import generate_int_id, MYSQL_MAX_INT
 from util.json_request import JsonResponse
 from xmodule.modulestore import EdxJSONEncoder
@@ -293,6 +294,9 @@ def certificate_activation_handler(request, course_key_string):
     POST
         json: is_active. update the activation state of certificate
     """
+    # Only global staff (PMs) are able to activate/deactivate certificate configuration
+    if not GlobalStaff().has_user(request.user):
+        raise PermissionDenied()
     course_key = CourseKey.from_string(course_key_string)
     store = modulestore()
     try:
@@ -452,6 +456,10 @@ def certificates_detail_handler(request, course_key_string, certificate_id):
         return JsonResponse(serialized_certificate, status=201)
 
     elif request.method == "DELETE":
+        # Only global staff (PMs) are able to activate/deactivate certificate configuration
+        if not GlobalStaff().has_user(request.user):
+            raise PermissionDenied()
+
         if not match_cert:
             return JsonResponse(status=404)
         CertificateManager.remove_certificate(

cms/djangoapps/contentstore/views/tests/test_certificates.py

@@ -20,6 +20,7 @@ from xmodule.contentstore.django import contentstore
 from xmodule.contentstore.content import StaticContent
 from xmodule.exceptions import NotFoundError
 from student.models import CourseEnrollment
+from student.roles import CourseInstructorRole, CourseStaffRole
 from student.tests.factories import UserFactory
 from contentstore.views.certificates import CertificateManager
 from django.test.utils import override_settings
@@ -491,6 +492,23 @@ class CertificatesDetailHandlerTestCase(EventTestMixin, CourseTestCase, Certific
         )
         self.assertEqual(response.status_code, 403)
 
+    def test_delete_certificate_without_global_staff_permissions(self):
+        """
+        Tests certificate deletion without global staff permission on course.
+        """
+        self._add_course_certificates(count=2, signatory_count=1)
+        user = UserFactory()
+        for role in [CourseInstructorRole, CourseStaffRole]:
+            role(self.course.id).add_users(user)
+        self.client.login(username=user.username, password='test')
+        response = self.client.delete(
+            self._url(cid=1),
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+            HTTP_X_REQUESTED_WITH="XMLHttpRequest",
+        )
+        self.assertEqual(response.status_code, 403)
+
     def test_delete_non_existing_certificate(self):
         """
         Try to delete a non existing certificate. It should return status code 404 Not found.
@@ -605,6 +623,27 @@ class CertificatesDetailHandlerTestCase(EventTestMixin, CourseTestCase, Certific
         )
         self.assertEquals(response.status_code, 403)
 
+    @ddt.data(True, False)
+    def test_certificate_activation_without_global_staff_permissions(self, activate):
+        """
+        Tests certificate Activate and Deactivate should not be allowed if user
+        does not have global staff permissions on course.
+        """
+        test_url = reverse_course_url('certificates.certificate_activation_handler', self.course.id)
+        self._add_course_certificates(count=1, signatory_count=2)
+        user = UserFactory()
+        for role in [CourseInstructorRole, CourseStaffRole]:
+            role(self.course.id).add_users(user)
+        self.client.login(username=user.username, password='test')
+        response = self.client.post(
+            test_url,
+            data=json.dumps({"is_active": activate}),
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+            HTTP_X_REQUESTED_WITH="XMLHttpRequest"
+        )
+        self.assertEquals(response.status_code, 403)
+
     def test_certificate_activation_failure(self):
         """
         Certificate activation should fail when user has not read access to course then permission denied exception

common/test/acceptance/tests/studio/test_studio_settings_certificates.py

@@ -10,8 +10,8 @@ class CertificatesTest(StudioCourseTest):
     """
     Tests for settings/certificates Page.
     """
-    def setUp(self, is_staff=False):
-        super(CertificatesTest, self).setUp(is_staff)
+    def setUp(self):  # pylint: disable=arguments-differ
+        super(CertificatesTest, self).setUp(is_staff=True)
         self.certificates_page = CertificatesPage(
             self.browser,
             self.course_info['org'],
@@ -131,7 +131,6 @@ class CertificatesTest(StudioCourseTest):
         # Delete the certificate we just created
         certificate.click_delete_certificate_button()
         self.certificates_page.click_confirmation_prompt_primary_button()
-        self.certificates_page.wait_for_first_certificate_button()
 
         # Reload the page and confirm there are no certificates
         self.certificates_page.visit()