Only instructors may make other instructors on a course

cms/djangoapps/contentstore/tests/test_users.py

@@ -243,3 +243,77 @@ class UsersTestCase(CourseTestCase):
         ext_user = User.objects.get(email=self.ext_user.email)
         groups = [g.name for g in ext_user.groups.all()]
         self.assertIn(self.inst_groupname, groups)
+
+    def test_permission_denied_self(self):
+        group, _ = Group.objects.get_or_create(name=self.staff_groupname)
+        self.user.groups.add(group)
+        self.user.is_staff = False
+        self.user.save()
+
+        self_url = reverse("course_team_user", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+            "email": self.user.email,
+        })
+
+        resp = self.client.post(
+            self_url,
+            data={"role": "instructor"},
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+
+    def test_permission_denied_other(self):
+        group, _ = Group.objects.get_or_create(name=self.staff_groupname)
+        self.user.groups.add(group)
+        self.user.is_staff = False
+        self.user.save()
+
+        resp = self.client.post(
+            self.detail_url,
+            data={"role": "instructor"},
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+
+    def test_staff_can_delete_self(self):
+        group, _ = Group.objects.get_or_create(name=self.staff_groupname)
+        self.user.groups.add(group)
+        self.user.is_staff = False
+        self.user.save()
+
+        self_url = reverse("course_team_user", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+            "email": self.user.email,
+        })
+
+        resp = self.client.delete(self_url)
+        self.assert2XX(resp.status_code)
+        # reload user from DB
+        user = User.objects.get(email=self.user.email)
+        groups = [g.name for g in user.groups.all()]
+        self.assertNotIn(self.staff_groupname, groups)
+
+    def test_staff_cannot_delete_other(self):
+        group, _ = Group.objects.get_or_create(name=self.staff_groupname)
+        self.user.groups.add(group)
+        self.user.is_staff = False
+        self.user.save()
+        self.ext_user.groups.add(group)
+        self.ext_user.save()
+
+        resp = self.client.delete(self.detail_url)
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertIn(self.staff_groupname, groups)

cms/djangoapps/contentstore/views/user.py

@@ -106,8 +106,17 @@ def manage_users(request, org, course, name):
 def course_team_user(request, org, course, name, email):
     location = Location('i4x', org, course, 'course', name)
     # check that logged in user has permissions to this item
-    if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME) and not has_access(request.user, location, role=STAFF_ROLE_NAME):
-        raise PermissionDenied()
+    if has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME):
+        # instructors have full permissions
+        pass
+    elif has_access(request.user, location, role=STAFF_ROLE_NAME) and email == request.user.email:
+        # staff can only affect themselves
+        pass
+    else:
+        msg = {
+            "error": _("Insufficient permissions")
+        }
+        return JsonResponse(msg, 400)
 
     try:
         user = User.objects.get(email=email)
@@ -153,14 +162,18 @@ def course_team_user(request, org, course, name, email):
         # remove all roles in this course from this user: but fail if the user
         # is the last instructor in the course team
         instructors = set(inst_group.user_set.all())
+        staff = set(staff_group.user_set.all())
         if user in instructors and len(instructors) == 1:
             msg = {
                 "error": _("You may not remove the last instructor from a course")
             }
             return JsonResponse(msg, 400)
 
-        for role in roles:
-            remove_user_from_course_group(request.user, user, location, role)
+        if user in instructors:
+            user.groups.remove(inst_group)
+        if user in staff:
+            user.groups.remove(staff_group)
+        user.save()
         return JsonResponse()
 
     # all other operations require the requesting user to specify a role
@@ -179,6 +192,11 @@ def course_team_user(request, org, course, name, email):
         role = request.POST["role"]
 
     if role == "instructor":
+        if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME):
+            msg = {
+                "error": _("Only instructors may create other instructors")
+            }
+            return JsonResponse(msg, 400)
         add_user_to_course_group(request.user, user, location, role)
     elif role == "staff":
         # if we're trying to downgrade a user from "instructor" to "staff",