From 9711cd5fc3680657e9b7aa9155137eaa7b19d869 Mon Sep 17 00:00:00 2001 From: Zia Fazal Date: Fri, 3 Jul 2015 15:59:23 +0500 Subject: [PATCH] certificate add/update/delete operations permission check address quality violation --- .../contentstore/views/certificates.py | 4 +- .../views/tests/test_certificates.py | 50 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/certificates.py b/cms/djangoapps/contentstore/views/certificates.py index 154605dfe5..96e2f522e6 100644 --- a/cms/djangoapps/contentstore/views/certificates.py +++ b/cms/djangoapps/contentstore/views/certificates.py @@ -33,7 +33,7 @@ from django.views.decorators.http import require_http_methods from contentstore.utils import reverse_course_url from edxmako.shortcuts import render_to_response from opaque_keys.edx.keys import CourseKey, AssetKey -from student.auth import has_studio_read_access +from student.auth import has_studio_write_access from util.db import generate_int_id, MYSQL_MAX_INT from util.json_request import JsonResponse from xmodule.modulestore import EdxJSONEncoder @@ -53,7 +53,7 @@ def _get_course_and_check_access(course_key, user, depth=0): Internal method used to calculate and return the locator and course module for the view functions in this file. """ - if not has_studio_read_access(user, course_key): + if not has_studio_write_access(user, course_key): raise PermissionDenied() course_module = modulestore().get_course(course_key, depth=depth) return course_module diff --git a/cms/djangoapps/contentstore/views/tests/test_certificates.py b/cms/djangoapps/contentstore/views/tests/test_certificates.py index 7997e9b575..ec3a8a8f87 100644 --- a/cms/djangoapps/contentstore/views/tests/test_certificates.py +++ b/cms/djangoapps/contentstore/views/tests/test_certificates.py @@ -5,6 +5,7 @@ Group Configuration Tests. """ import json import mock +import ddt from django.conf import settings from django.test.utils import override_settings @@ -19,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.tests.factories import UserFactory from contentstore.views.certificates import CertificateManager from django.test.utils import override_settings from contentstore.utils import get_lms_link_for_certificate_web_view @@ -230,6 +232,19 @@ class CertificatesListHandlerTestCase(CourseTestCase, CertificatesBaseTestCase, self._remove_ids(content) # pylint: disable=unused-variable self.assertEqual(content, expected) + def test_cannot_create_certificate_if_user_has_no_write_permissions(self): + """ + Tests user without write permissions on course should not able to create certificate + """ + user = UserFactory() + self.client.login(username=user.username, password='test') + response = self.client.ajax_post( + self._url(), + data=CERTIFICATE_JSON + ) + + self.assertEqual(response.status_code, 403) + @override_settings(LMS_BASE=None) def test_no_lms_base_for_certificate_web_view_link(self): test_link = get_lms_link_for_certificate_web_view( @@ -330,6 +345,7 @@ class CertificatesListHandlerTestCase(CourseTestCase, CertificatesBaseTestCase, self.assertNotEqual(new_certificate.get('id'), prev_certificate.get('id')) +@ddt.ddt @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) class CertificatesDetailHandlerTestCase(CourseTestCase, CertificatesBaseTestCase, HelperMethods): """ @@ -433,6 +449,21 @@ class CertificatesDetailHandlerTestCase(CourseTestCase, CertificatesBaseTestCase self.assertEqual(certificates[0].get('name'), 'Name 0') self.assertEqual(certificates[0].get('description'), 'Description 0') + def test_delete_certificate_without_write_permissions(self): + """ + Tests certificate deletion without write permission on course. + """ + self._add_course_certificates(count=2, signatory_count=1) + user = UserFactory() + 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. @@ -523,6 +554,25 @@ class CertificatesDetailHandlerTestCase(CourseTestCase, CertificatesBaseTestCase certificates = course.certificates['certificates'] self.assertEqual(certificates[0].get('is_active'), is_active) + @ddt.data(True, False) + def test_certificate_activation_without_write_permissions(self, activate): + """ + Tests certificate Activate and Deactivate should not be allowed if user + does not have write 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() + 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