Update the REST API course_team_handler to support library roles, fix bugs
This commit is contained in:
committed by
E. Kolpakov
parent
ea03f6ed76
commit
c477bab6f4
@@ -188,5 +188,6 @@ def library_blocks_view(library, user, response_format):
|
||||
'context_library': library,
|
||||
'component_templates': json.dumps(component_templates),
|
||||
'xblock_info': xblock_info,
|
||||
'templates': CONTAINER_TEMPATES
|
||||
'templates': CONTAINER_TEMPATES,
|
||||
'lib_users_url': reverse_library_url('manage_library_users', unicode(library.location.library_key)),
|
||||
})
|
||||
|
||||
@@ -70,7 +70,7 @@ class UsersTestCase(CourseTestCase):
|
||||
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 @@ class UsersTestCase(CourseTestCase):
|
||||
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 @@ class UsersTestCase(CourseTestCase):
|
||||
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 @@ class UsersTestCase(CourseTestCase):
|
||||
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
|
||||
|
||||
@@ -9,11 +9,12 @@ from edxmako.shortcuts import render_to_response
|
||||
|
||||
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 student.auth import has_course_author_access
|
||||
from student.auth import STUDIO_EDIT_ROLES, STUDIO_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_author_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_author_access(request.user, course_key):
|
||||
user_perms = get_user_permissions(request.user, course_key)
|
||||
if not (user_perms & STUDIO_VIEW_USERS):
|
||||
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_author_access(request.user, course_key, role=CourseInstructorRole),
|
||||
'allow_actions': bool(user_perms & STUDIO_EDIT_ROLES),
|
||||
})
|
||||
|
||||
|
||||
@@ -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_author_access(request.user, course_key, role=CourseInstructorRole):
|
||||
# instructors have full permissions
|
||||
pass
|
||||
elif has_course_author_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 (requester_perms & STUDIO_VIEW_USERS) 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 STUDIO_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 STUDIO_EDIT_ROLES permission or is editing themselves:
|
||||
if not ((requester_perms & STUDIO_EDIT_ROLES) 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)
|
||||
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)
|
||||
|
||||
auth.remove_users(request.user, CourseStaffRole(course_key), user)
|
||||
return JsonResponse()
|
||||
old_roles = set()
|
||||
role_added = False
|
||||
for role_type in role_hierarchy:
|
||||
role = role_type(course_key)
|
||||
if role_type.ROLE == new_role:
|
||||
if (requester_perms & STUDIO_EDIT_ROLES) or (user.id == request.user.id and old_roles):
|
||||
# User has STUDIO_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)
|
||||
|
||||
# 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 new_role and not role_added:
|
||||
return JsonResponse({"error": _("Invalid `role` specified.")}, 400)
|
||||
|
||||
if role == "instructor":
|
||||
if not has_course_author_access(request.user, course_key, role=CourseInstructorRole):
|
||||
msg = {
|
||||
"error": _("Only instructors may create other instructors")
|
||||
}
|
||||
for role in old_roles:
|
||||
if isinstance(role, CourseInstructorRole) and role.users_with_role().count() == 1:
|
||||
msg = {"error": _("You may not remove the last Admin. Add another Admin first.")}
|
||||
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)
|
||||
auth.remove_users(request.user, role, user)
|
||||
|
||||
# auto-enroll the course creator in the course so that "View Live" will work.
|
||||
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)
|
||||
|
||||
@@ -5,6 +5,11 @@ from django.conf.urls import patterns, include, url
|
||||
from ratelimitbackend import admin
|
||||
admin.autodiscover()
|
||||
|
||||
# Pattern to match a course key or a library key
|
||||
COURSELIKE_KEY_PATTERN = r'(?P<course_key_string>({}|{}))'.format(r'[^/]+/[^/]+/[^/]+', r'[^/:]+:[^/+]+\+[^/+]+(\+[^/]+)?')
|
||||
# Pattern to match a library key only
|
||||
LIBRARY_KEY_PATTERN = r'(?P<library_key_string>library-v1:[^/+]+\+[^/+]+)'
|
||||
|
||||
urlpatterns = patterns('', # nopep8
|
||||
|
||||
url(r'^transcripts/upload$', 'contentstore.views.upload_transcripts', name='upload_transcripts'),
|
||||
@@ -66,7 +71,7 @@ urlpatterns += patterns(
|
||||
url(r'^signin$', 'login_page', name='login'),
|
||||
url(r'^request_course_creator$', 'request_course_creator'),
|
||||
|
||||
url(r'^course_team/{}/(?P<email>.+)?$'.format(settings.COURSE_KEY_PATTERN), 'course_team_handler'),
|
||||
url(r'^course_team/{}/(?P<email>.+)?$'.format(COURSELIKE_KEY_PATTERN), 'course_team_handler'),
|
||||
url(r'^course_info/{}$'.format(settings.COURSE_KEY_PATTERN), 'course_info_handler'),
|
||||
url(
|
||||
r'^course_info_update/{}/(?P<provided_id>\d+)?$'.format(settings.COURSE_KEY_PATTERN),
|
||||
@@ -113,7 +118,6 @@ urlpatterns += patterns(
|
||||
)
|
||||
|
||||
if settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES'):
|
||||
LIBRARY_KEY_PATTERN = r'(?P<library_key_string>library-v1:[^/+]+\+[^/+]+)'
|
||||
urlpatterns += (
|
||||
url(r'^library/{}?$'.format(LIBRARY_KEY_PATTERN),
|
||||
'contentstore.views.library_handler', name='library_handler'),
|
||||
|
||||
@@ -12,6 +12,14 @@ from student.roles import GlobalStaff, CourseCreatorRole, CourseStaffRole, Cours
|
||||
CourseBetaTesterRole, OrgInstructorRole, OrgStaffRole, LibraryUserRole, OrgLibraryUserRole
|
||||
|
||||
|
||||
# Studio permissions:
|
||||
STUDIO_EDIT_ROLES = 8
|
||||
STUDIO_VIEW_USERS = 4
|
||||
STUDIO_EDIT_CONTENT = 2
|
||||
STUDIO_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 has_access(user, role):
|
||||
"""
|
||||
Check whether this user has access to this role (either direct or implied)
|
||||
@@ -41,7 +49,34 @@ def has_access(user, role):
|
||||
return False
|
||||
|
||||
|
||||
def has_studio_write_access(user, course_key, role=CourseStaffRole):
|
||||
def get_user_permissions(user, course_key, org=None):
|
||||
"""
|
||||
Get the bitmask 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.
|
||||
"""
|
||||
if org is None:
|
||||
org = course_key.org
|
||||
course_key = course_key.for_branch(None)
|
||||
else:
|
||||
assert course_key is None
|
||||
all_perms = STUDIO_EDIT_ROLES | STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_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 has_access(user, CourseInstructorRole(course_key)):
|
||||
return all_perms
|
||||
# Staff have all permissions except EDIT_ROLES:
|
||||
if OrgStaffRole(org=org).has_user(user) or (course_key and has_access(user, CourseStaffRole(course_key))):
|
||||
return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_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 has_access(user, LibraryUserRole(course_key)):
|
||||
return STUDIO_VIEW_USERS | STUDIO_VIEW_CONTENT
|
||||
return 0
|
||||
|
||||
|
||||
def has_studio_write_access(user, course_key):
|
||||
"""
|
||||
Return True if user has studio write access to the given course.
|
||||
Note that the CMS permissions model is with respect to courses.
|
||||
@@ -53,23 +88,15 @@ def has_studio_write_access(user, course_key, role=CourseStaffRole):
|
||||
|
||||
:param user:
|
||||
:param course_key: a CourseKey
|
||||
:param role: an AccessRole
|
||||
"""
|
||||
if GlobalStaff().has_user(user):
|
||||
return True
|
||||
if OrgInstructorRole(org=course_key.org).has_user(user):
|
||||
return True
|
||||
if OrgStaffRole(org=course_key.org).has_user(user):
|
||||
return True
|
||||
# temporary to ensure we give universal access given a course until we impl branch specific perms
|
||||
return has_access(user, role(course_key.for_branch(None)))
|
||||
return bool(STUDIO_EDIT_CONTENT & get_user_permissions(user, course_key))
|
||||
|
||||
|
||||
def has_course_author_access(*args, **kwargs):
|
||||
def has_course_author_access(user, course_key):
|
||||
"""
|
||||
Old name for has_studio_author_access
|
||||
Old name for has_studio_write_access
|
||||
"""
|
||||
return has_studio_read_access(*args, **kwargs)
|
||||
return has_studio_write_access(user, course_key)
|
||||
|
||||
|
||||
def has_studio_read_access(user, course_key):
|
||||
@@ -80,13 +107,7 @@ def has_studio_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_studio_write_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 bool(STUDIO_VIEW_CONTENT & get_user_permissions(user, course_key))
|
||||
|
||||
|
||||
def add_users(caller, role, *users):
|
||||
|
||||
Reference in New Issue
Block a user