Content libraries permissions UI (WIP)

cms/djangoapps/contentstore/views/access.py

@@ -8,6 +8,44 @@
 from student import auth
 
 
+# Studio permissions:
+EDIT_ROLES = 8
+VIEW_USERS = 4
+EDIT_CONTENT = 2
+VIEW_CONTENT = 1
+# In addition to the above, one is always allowed to "demote" oneself to a lower role within a course, or remove oneself.
+
+
+def get_user_permissions(user, course_key, org=None):
+    """
+    Get the set() of permissions that this user has in the given course context.
+    Can also set course_key=None and pass in an org to get the user's
+    permissions for that organization as a whole.
+
+    These permissions are specific to studio, but the roles that define them are
+    shared with the LMS.
+    """
+    if org is None:
+        org = course_key.org
+        course_key = course_key.for_branch(None)
+    else:
+        assert course_key is None
+    all_perms = {EDIT_ROLES, VIEW_USERS, EDIT_CONTENT, VIEW_CONTENT}
+    # global staff, org instructors, and course instructors have all permissions:
+    if GlobalStaff().has_user(user) or OrgInstructorRole(org=org).has_user(user):
+        return all_perms
+    if course_key and auth.has_access(user, CourseInstructorRole(course_key)):
+        return all_perms
+    # Staff have a all permissions except EDIT_ROLES:
+    if OrgStaffRole(org=org).has_user(user) or (course_key and auth.has_access(user, CourseStaffRole(course_key))):
+        return {VIEW_USERS, EDIT_CONTENT, VIEW_CONTENT}
+    # Otherwise, for libraries, users can view only:
+    if (course_key and isinstance(course_key, LibraryLocator)):
+        if OrgLibraryUserRole(org=org).has_user(user) or auth.has_access(user, LibraryUserRole(course_key)):
+            return {VIEW_USERS, VIEW_CONTENT}
+    return set()
+
+
 def has_course_access(user, course_key, role=CourseStaffRole):
     """
     Return True if user allowed to access this course_id
@@ -33,7 +71,7 @@ def has_write_access(user, course_key):
     Return True iff user is allowed to modify the given course/library.
     Currently equivalent to has_course_access but less amibguously named.
     """
-    return has_course_access(user, course_key)
+    return EDIT_CONTENT in get_user_permissions(user, course_key)
 
 
 def has_read_access(user, course_key):
@@ -43,13 +81,7 @@ def has_read_access(user, course_key):
     There is currently no such thing as read-only course access in studio, but
     there is read-only access to content libraries.
     """
-    if has_course_access(user, course_key):
-        return True  # Global, Org, or Course "Instructors" and "Staff" can read and write
-    if isinstance(course_key, LibraryLocator):
-        if OrgLibraryUserRole(org=course_key.org).has_user(user):
-            return True  # User has read-only access to all libraries in this organization
-        return LibraryUserRole(course_key.for_branch(None)).has_user(user)  # User has read-only access this library
-    return False
+    return VIEW_CONTENT in get_user_permissions(user, course_key)
 
 
 def get_user_role(user, course_id):

cms/djangoapps/contentstore/views/library.py

@@ -24,13 +24,13 @@
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 
-from .access import has_read_access, has_write_access
+from .access import VIEW_USERS, EDIT_ROLES, has_read_access, has_write_access, get_user_permissions
 from .component import get_component_templates
-from student.roles import CourseCreatorRole
+from student.roles import CourseCreatorRole, CourseInstructorRole, CourseStaffRole, LibraryUserRole
 from student import auth
 from util.json_request import expect_json, JsonResponse, JsonResponseBadRequest
 
-__all__ = ['library_handler']
+__all__ = ['library_handler', 'manage_library_users']
 
 log = logging.getLogger(__name__)
 
@@ -167,4 +167,39 @@ def library_blocks_view(library, user, response_format):
         'unit': None,
         'component_templates': json.dumps(component_templates),
         'xblock_info': xblock_info,
+        'lib_users_url': reverse_library_url('manage_library_users', unicode(library.location.library_key)),
+    })
+
+
+def manage_library_users(request, library_key_string):
+    """
+    Studio UI for editing the users within a library.
+
+    Uses the /course_team/:library_key/:user_email/ REST API to make changes.
+    """
+    library_key = CourseKey.from_string(library_key_string)
+    if not isinstance(library_key, LibraryLocator):
+        raise Http404  # This is not a library
+    user_perms = get_user_permissions(request.user, library_key)
+    if VIEW_USERS not in user_perms:
+        raise PermissionDenied()
+    library = modulestore().get_library(library_key)
+    if library is None:
+        raise Http404
+
+    # Segment all the users explicitly associated with this library, ensuring each user only has one role listed:
+    instructors = set(CourseInstructorRole(library_key).users_with_role())
+    staff = set(CourseStaffRole(library_key).users_with_role()) - instructors
+    users = set(LibraryUserRole(library_key).users_with_role()) - instructors - staff
+    all_users = instructors | staff | users
+
+    return render_to_response('manage_users_lib.html', {
+        'context_library': library,
+        'staff': staff,
+        'instructors': instructors,
+        'users': users,
+        'all_users': all_users,
+        'allow_actions': EDIT_ROLES in user_perms,
+        'library_key': unicode(library_key),
+        'lib_users_url': reverse_library_url('manage_library_users', library_key_string),
     })

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

@@ -70,7 +70,7 @@ def test_detail_invalid(self):
     def test_detail_post(self):
         resp = self.client.post(
             self.detail_url,
-            data={"role": None},
+            data={"role": ""},
         )
         self.assertEqual(resp.status_code, 204)
         # reload user from DB
@@ -218,7 +218,7 @@ def test_permission_denied_self(self):
             data={"role": "instructor"},
             HTTP_ACCEPT="application/json",
         )
-        self.assertEqual(resp.status_code, 400)
+        self.assertEqual(resp.status_code, 403)
         result = json.loads(resp.content)
         self.assertIn("error", result)
 
@@ -232,7 +232,7 @@ def test_permission_denied_other(self):
             data={"role": "instructor"},
             HTTP_ACCEPT="application/json",
         )
-        self.assertEqual(resp.status_code, 400)
+        self.assertEqual(resp.status_code, 403)
         result = json.loads(resp.content)
         self.assertIn("error", result)
 
@@ -255,7 +255,7 @@ def test_staff_cannot_delete_other(self):
         self.user.save()
 
         resp = self.client.delete(self.detail_url)
-        self.assertEqual(resp.status_code, 400)
+        self.assertEqual(resp.status_code, 403)
         result = json.loads(resp.content)
         self.assertIn("error", result)
         # reload user from DB

cms/djangoapps/contentstore/views/user.py

@@ -9,11 +9,12 @@
 
 from xmodule.modulestore.django import modulestore
 from opaque_keys.edx.keys import CourseKey
+from opaque_keys.edx.locator import LibraryLocator
 from util.json_request import JsonResponse, expect_json
-from student.roles import CourseInstructorRole, CourseStaffRole
+from student.roles import CourseInstructorRole, CourseStaffRole, LibraryUserRole
 from course_creators.views import user_requested_access
 
-from .access import has_course_access
+from .access import EDIT_ROLES, VIEW_USERS, get_user_permissions
 
 from student.models import CourseEnrollment
 from django.http import HttpResponseNotFound
@@ -50,8 +51,7 @@ def course_team_handler(request, course_key_string=None, email=None):
         json: remove a particular course team member from the course team (email is required).
     """
     course_key = CourseKey.from_string(course_key_string) if course_key_string else None
-    if not has_course_access(request.user, course_key):
-        raise PermissionDenied()
+    # No permissions check here - each helper method does its own check.
 
     if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):
         return _course_team_user(request, course_key, email)
@@ -66,7 +66,8 @@ def _manage_users(request, course_key):
     This view will return all CMS users who are editors for the specified course
     """
     # check that logged in user has permissions to this item
-    if not has_course_access(request.user, course_key):
+    user_perms = get_user_permissions(request.user, course_key)
+    if VIEW_USERS not in user_perms:
         raise PermissionDenied()
 
     course_module = modulestore().get_course(course_key)
@@ -78,7 +79,7 @@ def _manage_users(request, course_key):
         'context_course': course_module,
         'staff': staff,
         'instructors': instructors,
-        'allow_actions': has_course_access(request.user, course_key, role=CourseInstructorRole),
+        'allow_actions': EDIT_ROLES in user_perms,
     })
 
 
@@ -88,17 +89,14 @@ def _course_team_user(request, course_key, email):
     Handle the add, remove, promote, demote requests ensuring the requester has authority
     """
     # check that logged in user has permissions to this item
-    if has_course_access(request.user, course_key, role=CourseInstructorRole):
-        # instructors have full permissions
-        pass
-    elif has_course_access(request.user, course_key, role=CourseStaffRole) and email == request.user.email:
-        # staff can only affect themselves
+    requester_perms = get_user_permissions(request.user, course_key)
+    permissions_error_response = JsonResponse({"error": _("Insufficient permissions")}, 403)
+    if (VIEW_USERS in requester_perms) or (email == request.user.email):
+        # This user has permissions to at least view the list of users or is editing themself
         pass
     else:
-        msg = {
-            "error": _("Insufficient permissions")
-        }
-        return JsonResponse(msg, 400)
+        # This user is not even allowed to know who the authorized users are.
+        return permissions_error_response
 
     try:
         user = User.objects.get(email=email)
@@ -108,7 +106,13 @@ def _course_team_user(request, course_key, email):
         }
         return JsonResponse(msg, 404)
 
-    # role hierarchy: globalstaff > "instructor" > "staff" (in a course)
+    is_library = isinstance(course_key, LibraryLocator)
+    # Ordered list of roles: can always move self to the right, but need EDIT_ROLES to move any user left
+    if is_library:
+        role_hierarchy = (CourseInstructorRole, CourseStaffRole, LibraryUserRole)
+    else:
+        role_hierarchy = (CourseInstructorRole, CourseStaffRole)
+
     if request.method == "GET":
         # just return info about the user
         msg = {
@@ -117,12 +121,17 @@ def _course_team_user(request, course_key, email):
             "role": None,
         }
         # what's the highest role that this user has? (How should this report global staff?)
-        for role in [CourseInstructorRole(course_key), CourseStaffRole(course_key)]:
-            if role.has_user(user):
+        for role in role_hierarchy:
+            if role(course_key).has_user(user):
                 msg["role"] = role.ROLE
                 break
         return JsonResponse(msg)
 
+    # All of the following code is for editing/promoting/deleting users.
+    # Check that the user has EDIT_ROLES permission or is editing themselves:
+    if not (EDIT_ROLES in requester_perms or (user.id == request.user.id)):
+        return permissions_error_response
+
     # can't modify an inactive user
     if not user.is_active:
         msg = {
@@ -131,60 +140,43 @@ def _course_team_user(request, course_key, email):
         return JsonResponse(msg, 400)
 
     if request.method == "DELETE":
-        try:
-            try_remove_instructor(request, course_key, user)
-        except CannotOrphanCourse as oops:
-            return JsonResponse(oops.msg, 400)
-
-        auth.remove_users(request.user, CourseStaffRole(course_key), user)
-        return JsonResponse()
-
-    # all other operations require the requesting user to specify a role
-    role = request.json.get("role", request.POST.get("role"))
-    if role is None:
-        return JsonResponse({"error": _("`role` is required")}, 400)
-
-    if role == "instructor":
-        if not has_course_access(request.user, course_key, role=CourseInstructorRole):
-            msg = {
-                "error": _("Only instructors may create other instructors")
-            }
+        new_role = None
+    else:
+        # only other operation supported is to promote/demote a user by changing their role:
+        # role may be None or "" (equivalent to a DELETE request) but must be set.
+        # Check that the new role was specified:
+        if "role" in request.json or "role" in request.POST:
+            new_role = request.json.get("role", request.POST.get("role"))
+        else:
+            return JsonResponse({"error": _("No `role` specified.")}, 400)
+
+    old_roles = set()
+    role_added = False
+    for role_type in role_hierarchy:
+        role = role_type(course_key)
+        if role_type.ROLE == new_role:
+            if EDIT_ROLES in requester_perms or (user.id == request.user.id and old_roles):
+                # User has EDIT_ROLES permission or is currently a member of a higher role, and is thus demoting themself
+                auth.add_users(request.user, role, user)
+                role_added = True
+            else:
+                return permissions_error_response
+        elif role.has_user(user):
+            # Remove the user from this old role:
+            old_roles.add(role)
+
+    if new_role and not role_added:
+        return JsonResponse({"error": _("Invalid `role` specified.")}, 400)
+
+    for role in old_roles:
+        if isinstance(role, CourseInstructorRole) and role.users_with_role().count() == 1:
+            msg = {"error": _("You may not remove the last instructor from a course")}
             return JsonResponse(msg, 400)
-        auth.add_users(request.user, CourseInstructorRole(course_key), user)
-        # auto-enroll the course creator in the course so that "View Live" will work.
-        CourseEnrollment.enroll(user, course_key)
-    elif role == "staff":
-        # add to staff regardless (can't do after removing from instructors as will no longer
-        # be allowed)
-        auth.add_users(request.user, CourseStaffRole(course_key), user)
-        try:
-            try_remove_instructor(request, course_key, user)
-        except CannotOrphanCourse as oops:
-            return JsonResponse(oops.msg, 400)
-
-        # auto-enroll the course creator in the course so that "View Live" will work.
+        auth.remove_users(request.user, role, user)
+
+    if new_role and not is_library:
+        # The user may be newly added to this course.
+        # auto-enroll the user in the course so that "View Live" will work.
         CourseEnrollment.enroll(user, course_key)
 
     return JsonResponse()
-
-
-class CannotOrphanCourse(Exception):
-    """
-    Exception raised if an attempt is made to remove all responsible instructors from course.
-    """
-    def __init__(self, msg):
-        self.msg = msg
-        Exception.__init__(self)
-
-
-def try_remove_instructor(request, course_key, user):
-
-    # remove all roles in this course from this user: but fail if the user
-    # is the last instructor in the course team
-    instructors = CourseInstructorRole(course_key)
-    if instructors.has_user(user):
-        if instructors.users_with_role().count() == 1:
-            msg = {"error": _("You may not remove the last instructor from a course")}
-            raise CannotOrphanCourse(msg)
-        else:
-            auth.remove_users(request.user, instructors, user)