Modified permission classes for CCX REST APIs

Modified how the per object permissions are enforced in the CCX REST APIs

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):

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