diff --git a/lms/djangoapps/ccx/api/v0/tests/test_views.py b/lms/djangoapps/ccx/api/v0/tests/test_views.py index c2096d9a29..a48193ad35 100644 --- a/lms/djangoapps/ccx/api/v0/tests/test_views.py +++ b/lms/djangoapps/ccx/api/v0/tests/test_views.py @@ -39,7 +39,11 @@ from lms.djangoapps.ccx.models import CcxFieldOverride, CustomCourseForEdX from lms.djangoapps.ccx.overrides import override_field_for_ccx from lms.djangoapps.ccx.tests.utils import CcxTestCase from opaque_keys.edx.keys import CourseKey -from student.roles import CourseCcxCoachRole +from student.roles import ( + CourseInstructorRole, + CourseCcxCoachRole, + CourseStaffRole, +) from student.tests.factories import AdminFactory @@ -62,6 +66,8 @@ class CcxRestApiTest(CcxTestCase, APITestCase): # OAUTH2 setup # create a specific user for the application app_user = User.objects.create_user('test_app_user', 'test_app_user@openedx.org', 'test') + # add staff role to the app user + CourseStaffRole(self.master_course_key).add_users(app_user) # create an oauth client app entry self.app_client = Client.objects.create( user=app_user, @@ -135,15 +141,15 @@ class CcxListTest(CcxRestApiTest): """ super(CcxListTest, self).setUp() self.list_url = reverse('ccx_api:v0:ccx:list') + self.list_url_master_course = urlparse.urljoin( + self.list_url, + '?master_course_id={0}'.format(urllib.quote_plus(self.master_course_key_str)) + ) def test_authorization(self): """ Test that only the right token is authorized """ - url = urlparse.urljoin( - self.list_url, - '?master_course_id={0}'.format(urllib.quote_plus(self.master_course_key_str)) - ) auth_list = [ "Wrong token-type-obviously", "Bearer wrong token format", @@ -153,42 +159,108 @@ class CcxListTest(CcxRestApiTest): ] # all the auths in the list fail to authorize for auth in auth_list: - resp = self.client.get(url, {}, HTTP_AUTHORIZATION=auth) + resp = self.client.get(self.list_url_master_course, {}, HTTP_AUTHORIZATION=auth) self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED) - resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) + resp = self.client.get(self.list_url_master_course, {}, HTTP_AUTHORIZATION=self.auth) self.assertEqual(resp.status_code, status.HTTP_200_OK) + def test_authorization_no_oauth_staff(self): + """ + Check authorization for staff users logged in without oauth + """ + # create a staff user + staff_user = User.objects.create_user('test_staff_user', 'test_staff_user@openedx.org', 'test') + # add staff role to the staff user + CourseStaffRole(self.master_course_key).add_users(staff_user) + + data = { + 'master_course_id': self.master_course_key_str, + 'max_students_allowed': 111, + 'display_name': 'CCX Test Title', + 'coach_email': self.coach.email + } + # the staff user can perform the request + self.client.login(username=staff_user.username, password='test') + resp = self.client.get(self.list_url_master_course) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + resp = self.client.post(self.list_url, data, format='json') + self.assertEqual(resp.status_code, status.HTTP_201_CREATED) + + def test_authorization_no_oauth_instructor(self): + """ + Check authorization for instructor users logged in without oauth + """ + # create an instructor user + instructor_user = User.objects.create_user('test_instructor_user', 'test_instructor_user@openedx.org', 'test') + # add instructor role to the instructor user + CourseInstructorRole(self.master_course_key).add_users(instructor_user) + + data = { + 'master_course_id': self.master_course_key_str, + 'max_students_allowed': 111, + 'display_name': 'CCX Test Title', + 'coach_email': self.coach.email + } + + # the instructor user can perform the request + self.client.login(username=instructor_user.username, password='test') + resp = self.client.get(self.list_url_master_course) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + resp = self.client.post(self.list_url, data, format='json') + self.assertEqual(resp.status_code, status.HTTP_201_CREATED) + + def test_authorization_no_oauth(self): + """ + Check authorization for coach users logged in without oauth + """ + # create an coach user + coach_user = User.objects.create_user('test_coach_user', 'test_coach_user@openedx.org', 'test') + # add coach role to the coach user + CourseCcxCoachRole(self.master_course_key).add_users(coach_user) + + data = { + 'master_course_id': self.master_course_key_str, + 'max_students_allowed': 111, + 'display_name': 'CCX Test Title', + 'coach_email': self.coach.email + } + # the coach user cannot perform the request: this type of user can only get her own CCX + self.client.login(username=coach_user.username, password='test') + resp = self.client.get(self.list_url_master_course) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + resp = self.client.post(self.list_url, data, format='json') + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + def test_get_list_wrong_master_course(self): """ Test for various get requests with wrong master course string """ - # case with no master_course_id provided - resp = self.client.get(self.list_url, {}, HTTP_AUTHORIZATION=self.auth) - self.expect_error(status.HTTP_400_BAD_REQUEST, 'master_course_id_not_provided', resp) - base_url = urlparse.urljoin(self.list_url, '?master_course_id=') - # case with empty master_course_id - resp = self.client.get(base_url, {}, HTTP_AUTHORIZATION=self.auth) - self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_id_not_valid', resp) - # case with invalid master_course_id - url = '{0}invalid_master_course_str'.format(base_url) - resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) - self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_id_not_valid', resp) - # case with inexistent master_course_id - url = '{0}course-v1%3Aorg_foo.0%2Bcourse_bar_0%2BRun_0'.format(base_url) - resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) - self.expect_error(status.HTTP_404_NOT_FOUND, 'course_id_does_not_exist', resp) + # mock the permission class these cases can be tested + mock_class_str = 'openedx.core.lib.api.permissions.IsMasterCourseStaffInstructor.has_permission' + with mock.patch(mock_class_str, autospec=True) as mocked_perm_class: + mocked_perm_class.return_value = True + # case with no master_course_id provided + resp = self.client.get(self.list_url, {}, HTTP_AUTHORIZATION=self.auth) + self.expect_error(status.HTTP_400_BAD_REQUEST, 'master_course_id_not_provided', resp) + base_url = urlparse.urljoin(self.list_url, '?master_course_id=') + # case with empty master_course_id + resp = self.client.get(base_url, {}, HTTP_AUTHORIZATION=self.auth) + self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_id_not_valid', resp) + # case with invalid master_course_id + url = '{0}invalid_master_course_str'.format(base_url) + resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) + self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_id_not_valid', resp) + # case with inexistent master_course_id + url = '{0}course-v1%3Aorg_foo.0%2Bcourse_bar_0%2BRun_0'.format(base_url) + resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) + self.expect_error(status.HTTP_404_NOT_FOUND, 'course_id_does_not_exist', resp) def test_get_list(self): """ Tests the API to get a list of CCX Courses """ - # get the list of ccx - url = urlparse.urljoin( - self.list_url, - '?master_course_id={0}'.format(urllib.quote_plus(self.master_course_key_str)) - ) # there are no CCX courses - resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) + resp = self.client.get(self.list_url_master_course, {}, HTTP_AUTHORIZATION=self.auth) self.assertIn('count', resp.data) # pylint: disable=no-member self.assertEqual(resp.data['count'], 0) # pylint: disable=no-member @@ -196,7 +268,7 @@ class CcxListTest(CcxRestApiTest): num_ccx = 10 for _ in xrange(num_ccx): self.make_ccx() - resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) + resp = self.client.get(self.list_url_master_course, {}, HTTP_AUTHORIZATION=self.auth) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertIn('count', resp.data) # pylint: disable=no-member self.assertEqual(resp.data['count'], num_ccx) # pylint: disable=no-member @@ -220,13 +292,8 @@ class CcxListTest(CcxRestApiTest): ccx.display_name = title_str.format(string.ascii_lowercase[-(num + 1)]) ccx.save() - # get the list of ccx - base_url = urlparse.urljoin( - self.list_url, - '?master_course_id={0}'.format(urllib.quote_plus(self.master_course_key_str)) - ) # sort by display name - url = '{0}&order_by=display_name'.format(base_url) + url = '{0}&order_by=display_name'.format(self.list_url_master_course) resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(len(resp.data['results']), num_ccx) # pylint: disable=no-member @@ -234,7 +301,7 @@ class CcxListTest(CcxRestApiTest): for num, ccx in enumerate(resp.data['results']): # pylint: disable=no-member self.assertEqual(title_str.format(string.ascii_lowercase[-(num_ccx - num)]), ccx['display_name']) # add sort order desc - url = '{0}&order_by=display_name&sort_order=desc'.format(base_url) + url = '{0}&order_by=display_name&sort_order=desc'.format(self.list_url_master_course) resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) # the only thing I can check is that the display name is in alphabetically reversed order # in the same way when the field has been updated above, so with the id asc @@ -249,15 +316,10 @@ class CcxListTest(CcxRestApiTest): num_ccx = 357 for _ in xrange(num_ccx): self.make_ccx() - # get the list of ccx - base_url = urlparse.urljoin( - self.list_url, - '?master_course_id={0}'.format(urllib.quote_plus(self.master_course_key_str)) - ) page_size = settings.REST_FRAMEWORK.get('PAGE_SIZE', 10) num_pages = int(math.ceil(num_ccx / float(page_size))) # get first page - resp = self.client.get(base_url, {}, HTTP_AUTHORIZATION=self.auth) + resp = self.client.get(self.list_url_master_course, {}, HTTP_AUTHORIZATION=self.auth) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(resp.data['count'], num_ccx) # pylint: disable=no-member self.assertEqual(resp.data['num_pages'], num_pages) # pylint: disable=no-member @@ -266,7 +328,7 @@ class CcxListTest(CcxRestApiTest): self.assertIsNotNone(resp.data['next']) # pylint: disable=no-member self.assertIsNone(resp.data['previous']) # pylint: disable=no-member # get a page in the middle - url = '{0}&page=24'.format(base_url) + url = '{0}&page=24'.format(self.list_url_master_course) resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(resp.data['count'], num_ccx) # pylint: disable=no-member @@ -276,7 +338,7 @@ class CcxListTest(CcxRestApiTest): self.assertIsNotNone(resp.data['next']) # pylint: disable=no-member self.assertIsNotNone(resp.data['previous']) # pylint: disable=no-member # get last page - url = '{0}&page={1}'.format(base_url, num_pages) + url = '{0}&page={1}'.format(self.list_url_master_course, num_pages) resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(resp.data['count'], num_ccx) # pylint: disable=no-member @@ -286,7 +348,7 @@ class CcxListTest(CcxRestApiTest): self.assertIsNone(resp.data['next']) # pylint: disable=no-member self.assertIsNotNone(resp.data['previous']) # pylint: disable=no-member # last page + 1 - url = '{0}&page={1}'.format(base_url, num_pages + 1) + url = '{0}&page={1}'.format(self.list_url_master_course, num_pages + 1) resp = self.client.get(url, {}, HTTP_AUTHORIZATION=self.auth) self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) @@ -322,9 +384,13 @@ class CcxListTest(CcxRestApiTest): """ Test for various post requests with wrong master course string """ - # case with no master_course_id provided - resp = self.client.post(self.list_url, data, format='json', HTTP_AUTHORIZATION=self.auth) - self.expect_error(expected_http_error, expected_error_string, resp) + # mock the permission class these cases can be tested + mock_class_str = 'openedx.core.lib.api.permissions.IsMasterCourseStaffInstructor.has_permission' + with mock.patch(mock_class_str, autospec=True) as mocked_perm_class: + mocked_perm_class.return_value = True + # case with no master_course_id provided + resp = self.client.post(self.list_url, data, format='json', HTTP_AUTHORIZATION=self.auth) + self.expect_error(expected_http_error, expected_error_string, resp) def test_post_list_wrong_master_course_special_cases(self): """ @@ -536,6 +602,69 @@ class CcxDetailTest(CcxRestApiTest): resp = self.client.get(self.detail_url, {}, HTTP_AUTHORIZATION=self.auth) self.assertEqual(resp.status_code, status.HTTP_200_OK) + def test_authorization_no_oauth_staff(self): + """ + Check authorization for staff users logged in without oauth + """ + # create a staff user + staff_user = User.objects.create_user('test_staff_user', 'test_staff_user@openedx.org', 'test') + # add staff role to the staff user + CourseStaffRole(self.master_course_key).add_users(staff_user) + + data = {'display_name': 'CCX Title'} + # the staff user can perform the request + self.client.login(username=staff_user.username, password='test') + resp = self.client.get(self.detail_url) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + resp = self.client.patch(self.detail_url, data, format='json') + self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) + + def test_authorization_no_oauth_instructor(self): + """ + Check authorization for users logged in without oauth + """ + # create an instructor user + instructor_user = User.objects.create_user('test_instructor_user', 'test_instructor_user@openedx.org', 'test') + # add instructor role to the instructor user + CourseInstructorRole(self.master_course_key).add_users(instructor_user) + + data = {'display_name': 'CCX Title'} + # the instructor user can perform the request + self.client.login(username=instructor_user.username, password='test') + resp = self.client.get(self.detail_url) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + resp = self.client.patch(self.detail_url, data, format='json') + self.assertEqual(resp.status_code, status.HTTP_204_NO_CONTENT) + + def test_authorization_no_oauth_other_coach(self): + """ + Check authorization for other coach users logged in without oauth + """ + # create an coach user + coach_user = User.objects.create_user('test_coach_user', 'test_coach_user@openedx.org', 'test') + # add coach role to the coach user + CourseCcxCoachRole(self.master_course_key).add_users(coach_user) + + data = {'display_name': 'CCX Title'} + # the coach user cannot perform the request: this type of user can only get her own CCX + self.client.login(username=coach_user.username, password='test') + resp = self.client.get(self.detail_url) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + resp = self.client.patch(self.detail_url, data, format='json') + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + + def test_authorization_no_oauth_ccx_coach(self): + """ + Check authorization for ccx coach users logged in without oauth + """ + data = {'display_name': 'CCX Title'} + # the coach owner of the CCX can perform the request only if it is a get + self.client.login(username=self.coach.username, password='test') + resp = self.client.get(self.detail_url) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + resp = self.client.patch(self.detail_url, data, format='json') + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + def test_resolve_get_detail(self): """ Test for the ccx detail view resolver. This is needed because it is assumed @@ -569,18 +698,38 @@ class CcxDetailTest(CcxRestApiTest): """ client_request = getattr(self.client, http_method) # get a detail url with a master_course id string + mock_class_str = 'openedx.core.lib.api.permissions.IsCourseStaffInstructor.has_object_permission' url = reverse('ccx_api:v0:ccx:detail', kwargs={'ccx_course_id': self.master_course_key_str}) + + # the permission class will give a 403 error because will not find the CCX resp = client_request(url, {}, HTTP_AUTHORIZATION=self.auth) - self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_id_not_valid_ccx_id', resp) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + # bypassing the permission class we get another kind of error + with mock.patch(mock_class_str, autospec=True) as mocked_perm_class: + mocked_perm_class.return_value = True + resp = client_request(url, {}, HTTP_AUTHORIZATION=self.auth) + self.expect_error(status.HTTP_400_BAD_REQUEST, 'course_id_not_valid_ccx_id', resp) # use an non existing ccx id url = reverse('ccx_api:v0:ccx:detail', kwargs={'ccx_course_id': 'ccx-v1:foo.0+course_bar_0+Run_0+ccx@1'}) + # the permission class will give a 403 error because will not find the CCX resp = client_request(url, {}, HTTP_AUTHORIZATION=self.auth) - self.expect_error(status.HTTP_404_NOT_FOUND, 'ccx_course_id_does_not_exist', resp) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + # bypassing the permission class we get another kind of error + with mock.patch(mock_class_str, autospec=True) as mocked_perm_class: + mocked_perm_class.return_value = True + resp = client_request(url, {}, HTTP_AUTHORIZATION=self.auth) + self.expect_error(status.HTTP_404_NOT_FOUND, 'ccx_course_id_does_not_exist', resp) # get a valid ccx key and add few 0s to get a non existing ccx for a valid course ccx_key_str = '{0}000000'.format(self.ccx_key_str) url = reverse('ccx_api:v0:ccx:detail', kwargs={'ccx_course_id': ccx_key_str}) + # the permission class will give a 403 error because will not find the CCX resp = client_request(url, {}, HTTP_AUTHORIZATION=self.auth) - self.expect_error(status.HTTP_404_NOT_FOUND, 'ccx_course_id_does_not_exist', resp) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + # bypassing the permission class we get another kind of error + with mock.patch(mock_class_str, autospec=True) as mocked_perm_class: + mocked_perm_class.return_value = True + resp = client_request(url, {}, HTTP_AUTHORIZATION=self.auth) + self.expect_error(status.HTTP_404_NOT_FOUND, 'ccx_course_id_does_not_exist', resp) def test_get_detail(self): """ diff --git a/lms/djangoapps/ccx/api/v0/views.py b/lms/djangoapps/ccx/api/v0/views.py index fac2525a6f..9a4207fbfb 100644 --- a/lms/djangoapps/ccx/api/v0/views.py +++ b/lms/djangoapps/ccx/api/v0/views.py @@ -23,7 +23,7 @@ from instructor.enrollment import ( from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.lib.api.permissions import IsCourseInstructor +from openedx.core.lib.api import permissions from student.models import CourseEnrollment from student.roles import CourseCcxCoachRole @@ -301,7 +301,7 @@ class CCXListView(GenericAPIView): } """ authentication_classes = (OAuth2Authentication, SessionAuthentication,) - permission_classes = (IsAuthenticated, IsCourseInstructor) + permission_classes = (IsAuthenticated, permissions.IsMasterCourseStaffInstructor) serializer_class = CCXCourseSerializer pagination_class = CCXAPIPagination @@ -510,9 +510,17 @@ class CCXDetailView(GenericAPIView): """ authentication_classes = (OAuth2Authentication, SessionAuthentication,) - permission_classes = (IsAuthenticated, IsCourseInstructor) + permission_classes = (IsAuthenticated, permissions.IsCourseStaffInstructor) serializer_class = CCXCourseSerializer + def get_object(self, course_id, is_ccx=False): # pylint: disable=arguments-differ + """ + Override the default get_object to allow a custom getter for the CCX + """ + course_object, course_key, error_code, http_status = get_valid_course(course_id, is_ccx) + self.check_object_permissions(self.request, course_object) + return course_object, course_key, error_code, http_status + def get(self, request, ccx_course_id=None): """ Gets a CCX Course information. @@ -524,7 +532,7 @@ class CCXDetailView(GenericAPIView): Return: A JSON serialized representation of the CCX course. """ - ccx_course_object, _, error_code, http_status = get_valid_course(ccx_course_id, is_ccx=True) + ccx_course_object, _, error_code, http_status = self.get_object(ccx_course_id, is_ccx=True) if ccx_course_object is None: return Response( status=http_status, @@ -543,7 +551,7 @@ class CCXDetailView(GenericAPIView): request (Request): Django request object. ccx_course_id (string): URI element specifying the CCX course location. """ - ccx_course_object, ccx_course_key, error_code, http_status = get_valid_course(ccx_course_id, is_ccx=True) + ccx_course_object, ccx_course_key, error_code, http_status = self.get_object(ccx_course_id, is_ccx=True) if ccx_course_object is None: return Response( status=http_status, @@ -571,7 +579,7 @@ class CCXDetailView(GenericAPIView): request (Request): Django request object. ccx_course_id (string): URI element specifying the CCX course location. """ - ccx_course_object, ccx_course_key, error_code, http_status = get_valid_course(ccx_course_id, is_ccx=True) + ccx_course_object, ccx_course_key, error_code, http_status = self.get_object(ccx_course_id, is_ccx=True) if ccx_course_object is None: return Response( status=http_status, diff --git a/openedx/core/lib/api/permissions.py b/openedx/core/lib/api/permissions.py index fb8b0f2abb..2d7f050d8b 100644 --- a/openedx/core/lib/api/permissions.py +++ b/openedx/core/lib/api/permissions.py @@ -6,6 +6,8 @@ from django.conf import settings from django.http import Http404 from rest_framework import permissions +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey from student.roles import CourseStaffRole, CourseInstructorRole @@ -64,13 +66,49 @@ class IsUserInUrl(permissions.BasePermission): return True -class IsCourseInstructor(permissions.BasePermission): +class IsCourseStaffInstructor(permissions.BasePermission): """ - Permission to check that user is a course instructor. + Permission to check that user is a course instructor or staff of + a master course given a course object or the user is a coach of + the course itself. """ def has_object_permission(self, request, view, obj): - return hasattr(request, 'user') and CourseInstructorRole(obj.course_id).has_user(request.user) + return (hasattr(request, 'user') and + # either the user is a staff or instructor of the master course + (hasattr(obj, 'course_id') and + (CourseInstructorRole(obj.course_id).has_user(request.user) or + CourseStaffRole(obj.course_id).has_user(request.user))) or + # or it is a safe method and the user is a coach on the course object + (request.method in permissions.SAFE_METHODS + and hasattr(obj, 'coach') and obj.coach == request.user)) + + +class IsMasterCourseStaffInstructor(permissions.BasePermission): + """ + Permission to check that user is instructor or staff of the master course. + """ + def has_permission(self, request, view): + """ + This method is assuming that a `master_course_id` parameter + is available in the request as a GET parameter, a POST parameter + or it is in the JSON payload included in the request. + The reason is because this permission class is going + to check if the user making the request is an instructor + for the specified course. + """ + master_course_id = (request.GET.get('master_course_id') + or request.POST.get('master_course_id') + or request.data.get('master_course_id')) + if master_course_id is not None: + try: + course_key = CourseKey.from_string(master_course_id) + except InvalidKeyError: + raise Http404() + return (hasattr(request, 'user') and + (CourseInstructorRole(course_key).has_user(request.user) or + CourseStaffRole(course_key).has_user(request.user))) + return False class IsUserInUrlOrStaff(IsUserInUrl): diff --git a/openedx/core/lib/api/tests/test_permissions.py b/openedx/core/lib/api/tests/test_permissions.py index c045e2b60a..f44b60f1f9 100644 --- a/openedx/core/lib/api/tests/test_permissions.py +++ b/openedx/core/lib/api/tests/test_permissions.py @@ -1,10 +1,16 @@ """ Tests for API permissions classes. """ import ddt +from django.contrib.auth.models import AnonymousUser +from django.http import Http404 from django.test import TestCase, RequestFactory from student.roles import CourseStaffRole, CourseInstructorRole -from openedx.core.lib.api.permissions import IsStaffOrOwner, IsCourseInstructor +from openedx.core.lib.api.permissions import ( + IsStaffOrOwner, + IsCourseStaffInstructor, + IsMasterCourseStaffInstructor, +) from student.tests.factories import UserFactory from opaque_keys.edx.keys import CourseKey @@ -16,35 +22,88 @@ class TestObject(object): self.course_id = course_id -class IsCourseInstructorTests(TestCase): - """ Test for IsCourseInstructor permission class. """ +class TestCcxObject(TestObject): + """ Fake class for object permission for CCX Courses """ + def __init__(self, user=None, course_id=None): + super(TestCcxObject, self).__init__(user, course_id) + self.coach = user + + +class IsCourseStaffInstructorTests(TestCase): + """ Test for IsCourseStaffInstructor permission class. """ def setUp(self): - super(IsCourseInstructorTests, self).setUp() - self.permission = IsCourseInstructor() + super(IsCourseStaffInstructorTests, self).setUp() + self.permission = IsCourseStaffInstructor() + self.coach = UserFactory.create() + self.user = UserFactory.create() self.request = RequestFactory().get('/') + self.request.user = self.user self.course_key = CourseKey.from_string('edx/test123/run') - self.obj = TestObject(course_id=self.course_key) + self.obj = TestCcxObject(user=self.coach, course_id=self.course_key) - def test_course_staff_has_no_access(self): - user = UserFactory.create() - self.request.user = user - CourseStaffRole(course_key=self.course_key).add_users(user) - - self.assertFalse( - self.permission.has_object_permission(self.request, None, self.obj)) + def test_course_staff_has_access(self): + CourseStaffRole(course_key=self.course_key).add_users(self.user) + self.assertTrue(self.permission.has_object_permission(self.request, None, self.obj)) def test_course_instructor_has_access(self): - user = UserFactory.create() - self.request.user = user - CourseInstructorRole(course_key=self.course_key).add_users(user) + CourseInstructorRole(course_key=self.course_key).add_users(self.user) + self.assertTrue(self.permission.has_object_permission(self.request, None, self.obj)) - self.assertTrue( - self.permission.has_object_permission(self.request, None, self.obj)) + def test_course_coach_has_access(self): + self.request.user = self.coach + self.assertTrue(self.permission.has_object_permission(self.request, None, self.obj)) + + def test_any_user_has_no_access(self): + self.assertFalse(self.permission.has_object_permission(self.request, None, self.obj)) def test_anonymous_has_no_access(self): - self.assertFalse( - self.permission.has_object_permission(self.request, None, self.obj)) + self.request.user = AnonymousUser() + self.assertFalse(self.permission.has_object_permission(self.request, None, self.obj)) + + +class IsMasterCourseStaffInstructorTests(TestCase): + """ Test for IsMasterCourseStaffInstructorTests permission class. """ + + def setUp(self): + super(IsMasterCourseStaffInstructorTests, self).setUp() + self.permission = IsMasterCourseStaffInstructor() + master_course_id = 'edx/test123/run' + self.user = UserFactory.create() + self.get_request = RequestFactory().get('/?master_course_id={}'.format(master_course_id)) + self.get_request.user = self.user + self.post_request = RequestFactory().post('/', data={'master_course_id': master_course_id}) + self.post_request.user = self.user + self.course_key = CourseKey.from_string(master_course_id) + + def test_course_staff_has_access(self): + CourseStaffRole(course_key=self.course_key).add_users(self.user) + self.assertTrue(self.permission.has_permission(self.get_request, None)) + self.assertTrue(self.permission.has_permission(self.post_request, None)) + + def test_course_instructor_has_access(self): + CourseInstructorRole(course_key=self.course_key).add_users(self.user) + self.assertTrue(self.permission.has_permission(self.get_request, None)) + self.assertTrue(self.permission.has_permission(self.post_request, None)) + + def test_any_user_has_partial_access(self): + self.assertFalse(self.permission.has_permission(self.get_request, None)) + self.assertFalse(self.permission.has_permission(self.post_request, None)) + + def test_anonymous_has_no_access(self): + user = AnonymousUser() + self.get_request.user = user + self.post_request.user = user + self.assertFalse(self.permission.has_permission(self.get_request, None)) + self.assertFalse(self.permission.has_permission(self.post_request, None)) + + def test_wrong_course_id_raises(self): + get_request = RequestFactory().get('/?master_course_id=this_is_invalid') + with self.assertRaises(Http404): + self.permission.has_permission(get_request, None) + post_request = RequestFactory().post('/', data={'master_course_id': 'this_is_invalid'}) + with self.assertRaises(Http404): + self.permission.has_permission(post_request, None) @ddt.ddt