diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index 8b4f48b1fc5a..c6fdc00301cf 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -2,10 +2,16 @@ Content library unit tests that require the CMS runtime. """ from contentstore.tests.utils import AjaxEnabledTestClient, parse_json -from contentstore.utils import reverse_usage_url +from contentstore.utils import reverse_url, reverse_usage_url, reverse_library_url from contentstore.views.preview import _load_preview_module from contentstore.views.tests.test_library import LIBRARY_REST_URL import ddt +from mock import patch +from student.auth import has_studio_read_access, has_studio_write_access +from student.roles import ( + CourseInstructorRole, CourseStaffRole, CourseCreatorRole, LibraryUserRole, + OrgStaffRole, OrgInstructorRole, OrgLibraryUserRole, +) from xmodule.library_content_module import LibraryVersionReference from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore @@ -15,13 +21,12 @@ from opaque_keys.edx.locator import CourseKey, LibraryLocator -@ddt.ddt -class TestLibraries(ModuleStoreTestCase): +class LibraryTestCase(ModuleStoreTestCase): """ - High-level tests for libraries + Common functionality for content libraries tests """ def setUp(self): - user_password = super(TestLibraries, self).setUp() + user_password = super(LibraryTestCase, self).setUp() self.client = AjaxEnabledTestClient() self.client.login(username=self.user.username, password=user_password) @@ -62,7 +67,7 @@ def _add_library_content_block(self, course, library_key, other_settings=None): **(other_settings or {}) ) - def _refresh_children(self, lib_content_block): + def _refresh_children(self, lib_content_block, status_code_expected=200): """ Helper method: Uses the REST API to call the 'refresh_children' handler of a LibraryContent block @@ -71,7 +76,7 @@ def _refresh_children(self, lib_content_block): lib_content_block.runtime._services['user'] = Mock(user_id=self.user.id) # pylint: disable=protected-access handler_url = reverse_usage_url('component_handler', lib_content_block.location, kwargs={'handler': 'refresh_children'}) response = self.client.ajax_post(handler_url) - self.assertEqual(response.status_code, 200) + self.assertEqual(response.status_code, status_code_expected) return modulestore().get_item(lib_content_block.location) def _bind_module(self, descriptor, user=None): @@ -98,6 +103,20 @@ def _update_item(self, usage_key, metadata): } ) + def _list_libraries(self): + """ + Use the REST API to get a list of libraries visible to the current user. + """ + response = self.client.get_json(LIBRARY_REST_URL) + self.assertEqual(response.status_code, 200) + return parse_json(response) + + +@ddt.ddt +class TestLibraries(LibraryTestCase): + """ + High-level tests for libraries + """ @ddt.data( (2, 1, 1), (2, 2, 2), @@ -306,3 +325,268 @@ def test_change_after_first_sync(self): self.assertEqual(len(lc_block.children), 1) # Children should not be deleted due to a bad setting. html_block = modulestore().get_item(lc_block.children[0]) self.assertEqual(html_block.data, data_value) + + +@ddt.ddt +class TestLibraryAccess(LibraryTestCase): + """ + Test Roles and Permissions related to Content Libraries + """ + def setUp(self): + """ Create a library, staff user, and non-staff user """ + super(TestLibraryAccess, self).setUp() + self.non_staff_user, self.non_staff_user_password = self.create_non_staff_user() + + def _login_as_non_staff_user(self, logout_first=True): + """ Login as a user that starts out with no roles/permissions granted. """ + if logout_first: + self.client.logout() # We start logged in as a staff user + self.client.login(username=self.non_staff_user.username, password=self.non_staff_user_password) + + def _assert_cannot_create_library(self, org="org", library="libfail", expected_code=403): + """ Ensure the current user is not able to create a library. """ + self.assertTrue(expected_code >= 300) + response = self.client.ajax_post(LIBRARY_REST_URL, {'org': org, 'library': library, 'display_name': "Irrelevant"}) + self.assertEqual(response.status_code, expected_code) + key = LibraryLocator(org=org, library=library) + self.assertEqual(modulestore().get_library(key), None) + + def _can_access_library(self, library): + """ + Use the normal studio library URL to check if we have access + + `library` can be a LibraryLocator or the library's root XBlock + """ + if isinstance(library, (basestring, LibraryLocator)): + lib_key = library + else: + lib_key = library.location.library_key + response = self.client.get(reverse_library_url('library_handler', unicode(lib_key))) + self.assertIn(response.status_code, (200, 302, 403)) + return response.status_code == 200 + + def tearDown(self): + """ + Log out when done each test + """ + self.client.logout() + super(TestLibraryAccess, self).tearDown() + + def test_creation(self): + """ + The user that creates a library should have instructor (admin) and staff permissions + """ + # self.library has been auto-created by the staff user. + self.assertTrue(has_studio_write_access(self.user, self.lib_key)) + self.assertTrue(has_studio_read_access(self.user, self.lib_key)) + # Make sure the user was actually assigned the instructor role and not just using is_staff superpowers: + self.assertTrue(CourseInstructorRole(self.lib_key).has_user(self.user)) + + # Now log out and ensure we are forbidden from creating a library: + self.client.logout() + self._assert_cannot_create_library(expected_code=302) # 302 redirect to login expected + + # Now create a non-staff user with no permissions: + self._login_as_non_staff_user(logout_first=False) + self.assertFalse(CourseCreatorRole().has_user(self.non_staff_user)) + + # Now check that logged-in users without any permissions cannot create libraries + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}): + self._assert_cannot_create_library() + + @ddt.data( + CourseInstructorRole, + CourseStaffRole, + LibraryUserRole, + ) + def test_acccess(self, access_role): + """ + Test the various roles that allow viewing libraries are working correctly. + """ + # At this point, one library exists, created by the currently-logged-in staff user. + # Create another library as staff: + library2_key = self._create_library(library="lib2") + # Login as non_staff_user: + self._login_as_non_staff_user() + + # non_staff_user shouldn't be able to access any libraries: + lib_list = self._list_libraries() + self.assertEqual(len(lib_list), 0) + self.assertFalse(self._can_access_library(self.library)) + self.assertFalse(self._can_access_library(library2_key)) + + # Now manually intervene to give non_staff_user access to library2_key: + access_role(library2_key).add_users(self.non_staff_user) + + # Now non_staff_user should be able to access library2_key only: + lib_list = self._list_libraries() + self.assertEqual(len(lib_list), 1) + self.assertEqual(lib_list[0]["library_key"], unicode(library2_key)) + self.assertTrue(self._can_access_library(library2_key)) + self.assertFalse(self._can_access_library(self.library)) + + @ddt.data( + OrgStaffRole, + OrgInstructorRole, + OrgLibraryUserRole, + ) + def test_org_based_access(self, org_access_role): + """ + Test the various roles that allow viewing all of an organization's + libraries are working correctly. + """ + # Create some libraries as the staff user: + lib_key_pacific = self._create_library(org="PacificX", library="libP") + lib_key_atlantic = self._create_library(org="AtlanticX", library="libA") + + # Login as a non-staff: + self._login_as_non_staff_user() + + # Now manually intervene to give non_staff_user access to all "PacificX" libraries: + org_access_role(lib_key_pacific.org).add_users(self.non_staff_user) + + # Now non_staff_user should be able to access lib_key_pacific only: + lib_list = self._list_libraries() + self.assertEqual(len(lib_list), 1) + self.assertEqual(lib_list[0]["library_key"], unicode(lib_key_pacific)) + self.assertTrue(self._can_access_library(lib_key_pacific)) + self.assertFalse(self._can_access_library(lib_key_atlantic)) + self.assertFalse(self._can_access_library(self.lib_key)) + + @ddt.data(True, False) + def test_read_only_role(self, use_org_level_role): + """ + Test the read-only role (LibraryUserRole and its org-level equivalent) + """ + # As staff user, add a block to self.library: + block = ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + + # Login as a non_staff_user: + self._login_as_non_staff_user() + self.assertFalse(self._can_access_library(self.library)) + + block_url = reverse_usage_url('xblock_handler', block.location) + + def can_read_block(): + """ Check if studio lets us view the XBlock in the library """ + response = self.client.get_json(block_url) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + return response.status_code == 200 + + def can_edit_block(): + """ Check if studio lets us edit the XBlock in the library """ + response = self.client.ajax_post(block_url) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + return response.status_code == 200 + + def can_delete_block(): + """ Check if studio lets us delete the XBlock in the library """ + response = self.client.delete(block_url) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + return response.status_code == 200 + + def can_copy_block(): + """ Check if studio lets us duplicate the XBlock in the library """ + response = self.client.ajax_post(reverse_url('xblock_handler'), { + 'parent_locator': unicode(self.library.location), + 'duplicate_source_locator': unicode(block.location), + }) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + return response.status_code == 200 + + def can_create_block(): + """ Check if studio lets us make a new XBlock in the library """ + response = self.client.ajax_post(reverse_url('xblock_handler'), { + 'parent_locator': unicode(self.library.location), 'category': 'html', + }) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + return response.status_code == 200 + + # Check that we do not have read or write access to block: + self.assertFalse(can_read_block()) + self.assertFalse(can_edit_block()) + self.assertFalse(can_delete_block()) + self.assertFalse(can_copy_block()) + self.assertFalse(can_create_block()) + + # Give non_staff_user read-only permission: + if use_org_level_role: + OrgLibraryUserRole(self.lib_key.org).add_users(self.non_staff_user) + else: + LibraryUserRole(self.lib_key).add_users(self.non_staff_user) + + self.assertTrue(self._can_access_library(self.library)) + self.assertTrue(can_read_block()) + self.assertFalse(can_edit_block()) + self.assertFalse(can_delete_block()) + self.assertFalse(can_copy_block()) + self.assertFalse(can_create_block()) + + @ddt.data( + (LibraryUserRole, CourseStaffRole, True), + (CourseStaffRole, CourseStaffRole, True), + (None, CourseStaffRole, False), + (LibraryUserRole, None, False), + ) + @ddt.unpack + def test_duplicate_across_courses(self, library_role, course_role, expected_result): + """ + Test that the REST API will correctly allow/refuse when copying + from a library with (write, read, or no) access to a course with (write or no) access. + """ + # As staff user, add a block to self.library: + block = ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + # And create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + self._login_as_non_staff_user() + + # Assign roles: + if library_role: + library_role(self.lib_key).add_users(self.non_staff_user) + if course_role: + course_role(course.location.course_key).add_users(self.non_staff_user) + + # Copy block to the course: + response = self.client.ajax_post(reverse_url('xblock_handler'), { + 'parent_locator': unicode(course.location), + 'duplicate_source_locator': unicode(block.location), + }) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + duplicate_action_allowed = (response.status_code == 200) + self.assertEqual(duplicate_action_allowed, expected_result) + + @ddt.data( + (LibraryUserRole, CourseStaffRole, True), + (CourseStaffRole, CourseStaffRole, True), + (None, CourseStaffRole, False), + (LibraryUserRole, None, False), + ) + @ddt.unpack + def test_refresh_library_content_permissions(self, library_role, course_role, expected_result): + """ + Test that the LibraryContent block's 'refresh_children' handler will correctly + handle permissions and allow/refuse when updating its content with the latest + version of a library. We try updating from a library with (write, read, or no) + access to a course with (write or no) access. + """ + # As staff user, add a block to self.library: + ItemFactory.create(category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False) + # And create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + self._login_as_non_staff_user() + + # Assign roles: + if library_role: + library_role(self.lib_key).add_users(self.non_staff_user) + if course_role: + course_role(course.location.course_key).add_users(self.non_staff_user) + + # Try updating our library content block: + lc_block = self._add_library_content_block(course, self.lib_key) + self._bind_module(lc_block, user=self.non_staff_user) # We must use the CMS's module system in order to get permissions checks. + lc_block = self._refresh_children(lc_block, status_code_expected=200 if expected_result else 403) + self.assertEqual(len(lc_block.children), 1 if expected_result else 0) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 57ffa6b9e1ce..5bad2efa2207 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -48,7 +48,7 @@ from models.settings.course_metadata import CourseMetadata from util.json_request import expect_json from util.string_utils import _has_non_ascii_characters -from student.auth import has_course_author_access +from student.auth import has_studio_write_access, has_studio_read_access from .component import ( OPEN_ENDED_COMPONENT_TYPES, NOTE_COMPONENT_TYPES, @@ -96,7 +96,7 @@ def get_course_and_check_access(course_key, user, depth=0): Internal method used to calculate and return the locator and course module for the view functions in this file. """ - if not has_course_author_access(user, course_key): + if not has_studio_read_access(user, course_key): raise PermissionDenied() course_module = modulestore().get_course(course_key, depth=depth) return course_module @@ -130,7 +130,7 @@ def course_notifications_handler(request, course_key_string=None, action_state_i course_key = CourseKey.from_string(course_key_string) if response_format == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): - if not has_course_author_access(request.user, course_key): + if not has_studio_write_access(request.user, course_key): raise PermissionDenied() if request.method == 'GET': return _course_notifications_json_get(action_state_id) @@ -220,7 +220,7 @@ def course_handler(request, course_key_string=None): return JsonResponse(_course_outline_json(request, course_module)) elif request.method == 'POST': # not sure if this is only post. If one will have ids, it goes after access return _create_or_rerun_course(request) - elif not has_course_author_access(request.user, CourseKey.from_string(course_key_string)): + elif not has_studio_write_access(request.user, CourseKey.from_string(course_key_string)): raise PermissionDenied() elif request.method == 'PUT': raise NotImplementedError() @@ -292,7 +292,7 @@ def course_filter(course): if course.location.course == 'templates': return False - return has_course_author_access(request.user, course.id) + return has_studio_read_access(request.user, course.id) courses = filter(course_filter, modulestore().get_courses()) in_process_course_actions = [ @@ -300,7 +300,7 @@ def course_filter(course): CourseRerunState.objects.find_all( exclude_args={'state': CourseRerunUIStateManager.State.SUCCEEDED}, should_display=True ) - if has_course_author_access(request.user, course.course_key) + if has_studio_read_access(request.user, course.course_key) ] return courses, in_process_course_actions @@ -348,7 +348,7 @@ def _accessible_libraries_list(user): List all libraries available to the logged in user by iterating through all libraries """ # No need to worry about ErrorDescriptors - split's get_libraries() never returns them. - return [lib for lib in modulestore().get_libraries() if has_course_author_access(user, lib.location)] + return [lib for lib in modulestore().get_libraries() if has_studio_read_access(user, lib.location.library_key)] @login_required @@ -418,6 +418,7 @@ def format_library_for_view(library): 'url': reverse_library_url('library_handler', unicode(library.location.library_key)), 'org': library.display_org_with_default, 'number': library.display_number_with_default, + 'can_edit': has_studio_write_access(request.user, library.location.library_key), } # remove any courses in courses that are also in the in_process_course_actions list @@ -647,7 +648,7 @@ def _rerun_course(request, org, number, run, fields): source_course_key = CourseKey.from_string(request.json.get('source_course_key')) # verify user has access to the original course - if not has_course_author_access(request.user, source_course_key): + if not has_studio_write_access(request.user, source_course_key): raise PermissionDenied() # create destination course key @@ -728,7 +729,7 @@ def course_info_update_handler(request, course_key_string, provided_id=None): provided_id = None # check that logged in user has permissions to this item (GET shouldn't require this level?) - if not has_course_author_access(request.user, usage_key.course_key): + if not has_studio_write_access(request.user, usage_key.course_key): raise PermissionDenied() if request.method == 'GET': diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index fa1135e15f9c..32d3fc49b269 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -37,7 +37,7 @@ from util.json_request import expect_json, JsonResponse -from student.auth import has_course_author_access +from student.auth import has_studio_write_access, has_studio_read_access from contentstore.utils import find_release_date_source, find_staff_lock_source, is_currently_visible_to_students, \ ancestor_has_staff_lock from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primary_child_category, \ @@ -130,7 +130,8 @@ def xblock_handler(request, usage_key_string): if usage_key_string: usage_key = usage_key_with_run(usage_key_string) - if not has_course_author_access(request.user, usage_key.course_key): + access_check = has_studio_read_access if request.method == 'GET' else has_studio_write_access + if not access_check(request.user, usage_key.course_key): raise PermissionDenied() if request.method == 'GET': @@ -166,6 +167,11 @@ def xblock_handler(request, usage_key_string): parent_usage_key = usage_key_with_run(request.json['parent_locator']) duplicate_source_usage_key = usage_key_with_run(request.json['duplicate_source_locator']) + source_course = duplicate_source_usage_key.course_key + dest_course = parent_usage_key.course_key + if not has_studio_write_access(request.user, dest_course) or not has_studio_read_access(request.user, source_course): + raise PermissionDenied() + dest_usage_key = _duplicate_item( parent_usage_key, duplicate_source_usage_key, @@ -197,7 +203,7 @@ def xblock_view_handler(request, usage_key_string, view_name): the second is the resource description """ usage_key = usage_key_with_run(usage_key_string) - if not has_course_author_access(request.user, usage_key.course_key): + if not has_studio_read_access(request.user, usage_key.course_key): raise PermissionDenied() accept_header = request.META.get('HTTP_ACCEPT', 'application/json') @@ -228,6 +234,7 @@ def xblock_view_handler(request, usage_key_string, view_name): elif view_name in (PREVIEW_VIEWS + container_views): is_pages_view = view_name == STUDENT_VIEW # Only the "Pages" view uses student view in Studio + can_edit = has_studio_write_access(request.user, usage_key.course_key) # Determine the items to be shown as reorderable. Note that the view # 'reorderable_container_child_preview' is only rendered for xblocks that @@ -260,6 +267,7 @@ def xblock_view_handler(request, usage_key_string, view_name): context = { 'is_pages_view': is_pages_view, # This setting disables the recursive wrapping of xblocks 'is_unit_page': is_unit(xblock), + 'can_edit': can_edit, 'root_xblock': xblock if (view_name == 'container_preview') else None, 'reorderable_items': reorderable_items, 'paging': paging, @@ -304,7 +312,7 @@ def xblock_outline_handler(request, usage_key_string): a course. """ usage_key = usage_key_with_run(usage_key_string) - if not has_course_author_access(request.user, usage_key.course_key): + if not has_studio_read_access(request.user, usage_key.course_key): raise PermissionDenied() response_format = request.REQUEST.get('format', 'html') @@ -474,13 +482,12 @@ def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, def _create_item(request): """View for create items.""" usage_key = usage_key_with_run(request.json['parent_locator']) - category = request.json['category'] + if not has_studio_write_access(request.user, usage_key.course_key): + raise PermissionDenied() + category = request.json['category'] display_name = request.json.get('display_name') - if not has_course_author_access(request.user, usage_key.course_key): - raise PermissionDenied() - if isinstance(usage_key, LibraryUsageLocator): # Only these categories are supported at this time. if category not in ['html', 'problem', 'video']: @@ -627,7 +634,7 @@ def orphan_handler(request, course_key_string): """ course_usage_key = CourseKey.from_string(course_key_string) if request.method == 'GET': - if has_course_author_access(request.user, course_usage_key): + if has_studio_read_access(request.user, course_usage_key): return JsonResponse([unicode(item) for item in modulestore().get_orphans(course_usage_key)]) else: raise PermissionDenied() diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 1fdc8381a8f4..c599cc1559f9 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -9,7 +9,7 @@ import logging from contentstore.views.item import create_xblock_info -from contentstore.utils import reverse_library_url +from contentstore.utils import reverse_library_url, add_instructor from django.http import HttpResponseNotAllowed, Http404 from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied @@ -26,12 +26,12 @@ from xmodule.modulestore.django import modulestore from .component import get_component_templates, CONTAINER_TEMPATES -from student.auth import has_course_author_access -from student.roles import CourseCreatorRole +from student.auth import STUDIO_VIEW_USERS, STUDIO_EDIT_ROLES, get_user_permissions, has_studio_read_access, has_studio_write_access +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__) @@ -70,7 +70,7 @@ def _display_library(library_key_string, request): if not isinstance(library_key, LibraryLocator): log.exception("Non-library key passed to content libraries API.") # Should never happen due to url regex raise Http404 # This is not a library - if not has_course_author_access(request.user, library_key): + if not has_studio_read_access(request.user, library_key): log.exception(u"User %s tried to access library %s without permission", request.user.username, unicode(library_key)) raise PermissionDenied() @@ -83,7 +83,7 @@ def _display_library(library_key_string, request): if request.REQUEST.get('format', 'html') == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'text/html'): response_format = 'json' - return library_blocks_view(library, response_format) + return library_blocks_view(library, request.user, response_format) def _list_libraries(request): @@ -96,7 +96,7 @@ def _list_libraries(request): "library_key": unicode(lib.location.library_key), } for lib in modulestore().get_libraries() - if has_course_author_access(request.user, lib.location.library_key) + if has_studio_read_access(request.user, lib.location.library_key) ] return JsonResponse(lib_info) @@ -124,6 +124,8 @@ def _create_library(request): user_id=request.user.id, fields={"display_name": display_name}, ) + # Give the user admin ("Instructor") role for this library: + add_instructor(new_lib.location.library_key, request.user, request.user) except KeyError as error: log.exception("Unable to create library - missing required JSON key.") return JsonResponseBadRequest({ @@ -151,13 +153,15 @@ def _create_library(request): }) -def library_blocks_view(library, response_format): +def library_blocks_view(library, user, response_format): """ The main view of a course's content library. Shows all the XBlocks in the library, and allows adding/editing/deleting them. Can be called with response_format="json" to get a JSON-formatted list of the XBlocks in the library along with library metadata. + + Assumes that read permissions have been checked before calling this. """ assert isinstance(library.location.library_key, LibraryLocator) assert isinstance(library.location, LibraryUsageLocator) @@ -168,18 +172,56 @@ def library_blocks_view(library, response_format): prev_version = library.runtime.course_entry.structure['previous_version'] return JsonResponse({ "display_name": library.display_name, - "library_id": unicode(library.course_id), + "library_id": unicode(library.location.library_key), "version": unicode(library.runtime.course_entry.course_key.version), "previous_version": unicode(prev_version) if prev_version else None, "blocks": [unicode(x) for x in children], }) + can_edit = has_studio_write_access(user, library.location.library_key) + xblock_info = create_xblock_info(library, include_ancestor_info=False, graders=[]) - component_templates = get_component_templates(library, library=True) + component_templates = get_component_templates(library, library=True) if can_edit else [] return render_to_response('library.html', { + 'can_edit': can_edit, '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)), + }) + + +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 not (user_perms & STUDIO_VIEW_USERS): + 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': bool(user_perms & STUDIO_EDIT_ROLES), + 'library_key': unicode(library_key), + 'lib_users_url': reverse_library_url('manage_library_users', library_key_string), }) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 523d8d184636..63e1a6b79201 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -22,6 +22,7 @@ from xblock.django.request import webob_to_django_response, django_to_webob_request from xblock.exceptions import NoSuchHandlerError from xblock.fragment import Fragment +from student.auth import has_studio_read_access, has_studio_write_access from lms.djangoapps.lms_xblock.field_data import LmsFieldData from cms.lib.xblock.field_data import CmsFieldData @@ -124,6 +125,28 @@ def user_id(self): return self._request.user.id +class StudioPermissionsService(object): + """ + Service that can provide information about a user's permissions. + + Deprecated. To be replaced by a more general authorization service. + + Only used by LibraryContentDescriptor (and library_tools.py). + """ + + def __init__(self, request): + super(StudioPermissionsService, self).__init__() + self._request = request + + def can_read(self, course_key): + """ Does the user have read access to the given course/library? """ + return has_studio_read_access(self._request.user, course_key) + + def can_write(self, course_key): + """ Does the user have read access to the given course/library? """ + return has_studio_write_access(self._request.user, course_key) + + def _preview_module_system(request, descriptor, field_data): """ Returns a ModuleSystem for the specified descriptor that is specialized for @@ -153,6 +176,7 @@ def _preview_module_system(request, descriptor, field_data): ] descriptor.runtime._services['user'] = StudioUserService(request) # pylint: disable=protected-access + descriptor.runtime._services['studio_user_permissions'] = StudioPermissionsService(request) # pylint: disable=protected-access return PreviewModuleSystem( static_url=settings.STATIC_URL, @@ -226,6 +250,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'content': frag.content, 'is_root': is_root, 'is_reorderable': is_reorderable, + 'can_edit': context.get('can_edit', True), } html = render_to_string('studio_xblock_wrapper.html', template_context) frag = wrap_fragment(frag, html) diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index 9ab5bc06ccf0..0b0abe2e3b29 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -4,12 +4,14 @@ More important high-level tests are in contentstore/tests/test_libraries.py """ from contentstore.tests.utils import AjaxEnabledTestClient, parse_json +from contentstore.utils import reverse_course_url, reverse_library_url from contentstore.views.component import get_component_templates from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import LibraryFactory from mock import patch from opaque_keys.edx.locator import CourseKey, LibraryLocator import ddt +from student.roles import LibraryUserRole LIBRARY_REST_URL = '/library/' # URL for GET/POST requests involving libraries @@ -197,3 +199,27 @@ def test_get_component_templates(self): self.assertIn('problem', templates) self.assertNotIn('discussion', templates) self.assertNotIn('advanced', templates) + + def test_manage_library_users(self): + """ + Simple test that the Library "User Access" view works. + Also tests that we can use the REST API to assign a user to a library. + """ + library = LibraryFactory.create() + extra_user, _ = self.create_non_staff_user() + manage_users_url = reverse_library_url('manage_library_users', unicode(library.location.library_key)) + + response = self.client.get(manage_users_url) + self.assertEqual(response.status_code, 200) + # extra_user has not been assigned to the library so should not show up in the list: + self.assertNotIn(extra_user.username, response.content) + + # Now add extra_user to the library: + user_details_url = reverse_course_url('course_team_handler', library.location.library_key, kwargs={'email': extra_user.email}) + edit_response = self.client.ajax_post(user_details_url, {"role": LibraryUserRole.ROLE}) + self.assertIn(edit_response.status_code, (200, 204)) + + # Now extra_user should apear in the list: + response = self.client.get(manage_users_url) + self.assertEqual(response.status_code, 200) + self.assertIn(extra_user.username, response.content) diff --git a/cms/djangoapps/contentstore/views/tests/test_user.py b/cms/djangoapps/contentstore/views/tests/test_user.py index f4fcc609d346..ce99a266ef8d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_user.py +++ b/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 diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index c73ecc651ff9..812890a451dc 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/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 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) - - 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_author_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 (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) + + 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 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) - - # 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) diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index b83442a9a630..33a816203155 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -256,6 +256,7 @@ define([ "js/spec/views/pages/course_outline_spec", "js/spec/views/pages/course_rerun_spec", "js/spec/views/pages/index_spec", + "js/spec/views/pages/library_users_spec", "js/spec/views/modals/base_modal_spec", "js/spec/views/modals/edit_xblock_spec", diff --git a/cms/static/js/factories/library.js b/cms/static/js/factories/library.js index 76ac47413ddc..59f447dead35 100644 --- a/cms/static/js/factories/library.js +++ b/cms/static/js/factories/library.js @@ -11,7 +11,8 @@ function($, _, XBlockInfo, PagedContainerPage, LibraryContainerView, ComponentTe model: new XBlockInfo(XBlockInfoJson, {parse: true}), templates: new ComponentTemplates(componentTemplates, {parse: true}), action: 'view', - viewClass: LibraryContainerView + viewClass: LibraryContainerView, + canEdit: true }; xmoduleLoader.done(function () { diff --git a/cms/static/js/factories/manage_users.js b/cms/static/js/factories/manage_users.js index 42272a859df0..04b444ae207c 100644 --- a/cms/static/js/factories/manage_users.js +++ b/cms/static/js/factories/manage_users.js @@ -32,7 +32,7 @@ define(['jquery', 'underscore', 'gettext', 'js/views/feedback_prompt'], function msg = new PromptView.Warning({ title: gettext('Already a course team member'), message: _.template( - gettext("{email} is already on the “{course}” team. If you're trying to add a new member, please double-check the email address you provided."), { + gettext("{email} is already on the {course} team. Recheck the email address if you want to add a new member."), { email: email, course: course.escape('name') }, {interpolate: /\{(.+?)\}/g} diff --git a/cms/static/js/factories/manage_users_lib.js b/cms/static/js/factories/manage_users_lib.js new file mode 100644 index 000000000000..388ec56c733b --- /dev/null +++ b/cms/static/js/factories/manage_users_lib.js @@ -0,0 +1,159 @@ +/* + Code for editing users and assigning roles within a library context. +*/ +define(['jquery', 'underscore', 'gettext', 'js/views/feedback_prompt', 'js/views/utils/view_utils'], +function($, _, gettext, PromptView, ViewUtils) { + 'use strict'; + return function (libraryName, allUserEmails, tplUserURL) { + var unknownErrorMessage = gettext('Unknown'), + $createUserForm = $('#create-user-form'), + $createUserFormWrapper = $createUserForm.closest('.wrapper-create-user'), + $cancelButton; + + // Our helper method that calls the RESTful API to add/remove/change user roles: + var changeRole = function(email, newRole, opts) { + var url = tplUserURL.replace('@@EMAIL@@', email); + var errMessage = opts.errMessage || gettext("There was an error changing the user's role"); + var onSuccess = opts.onSuccess || function(data){ ViewUtils.reload(); }; + var onError = opts.onError || function(){}; + $.ajax({ + url: url, + type: newRole ? 'POST' : 'DELETE', + dataType: 'json', + contentType: 'application/json', + notifyOnError: false, + data: JSON.stringify({role: newRole}), + success: onSuccess, + error: function(jqXHR, textStatus, errorThrown) { + var message, prompt; + try { + message = JSON.parse(jqXHR.responseText).error || unknownErrorMessage; + } catch (e) { + message = unknownErrorMessage; + } + prompt = new PromptView.Error({ + title: errMessage, + message: message, + actions: { + primary: { text: gettext('OK'), click: function(view) { view.hide(); onError(); } } + } + }); + prompt.show(); + } + }); + }; + + $createUserForm.bind('submit', function(event) { + event.preventDefault(); + var email = $('#user-email-input').val().trim(); + var msg; + + if(!email) { + msg = new PromptView.Error({ + title: gettext('A valid email address is required'), + message: gettext('You must enter a valid email address in order to add an instructor'), + actions: { + primary: { + text: gettext('Return and add email address'), + click: function(view) { view.hide(); $('#user-email-input').focus(); } + } + } + }); + msg.show(); + return; + } + + if(_.contains(allUserEmails, email)) { + msg = new PromptView.Warning({ + title: gettext('Already a library team member'), + message: _.template( + gettext("{email} is already on the {course} team. Recheck the email address if you want to add a new member."), { + email: email, + course: libraryName + }, {interpolate: /\{(.+?)\}/g} + ), + actions: { + primary: { + text: gettext('Return to team listing'), + click: function(view) { view.hide(); $('#user-email-input').focus(); } + } + } + }); + msg.show(); + return; + } + + // Use the REST API to create the user, giving them a role of "library_user" for now: + changeRole( + $('#user-email-input').val().trim(), + 'library_user', + { + errMessage: gettext('Error adding user'), + onError: function() { $('#user-email-input').focus(); } + } + ); + }); + + $cancelButton = $createUserForm.find('.action-cancel'); + $cancelButton.on('click', function(event) { + event.preventDefault(); + $('.create-user-button').toggleClass('is-disabled'); + $createUserFormWrapper.toggleClass('is-shown'); + $('#user-email-input').val(''); + }); + + $('.create-user-button').on('click', function(event) { + event.preventDefault(); + $('.create-user-button').toggleClass('is-disabled'); + $createUserFormWrapper.toggleClass('is-shown'); + $createUserForm.find('#user-email-input').focus(); + }); + + $('body').on('keyup', function(event) { + if(event.which == jQuery.ui.keyCode.ESCAPE && $createUserFormWrapper.is('.is-shown')) { + $cancelButton.click(); + } + }); + + $('.remove-user').click(function() { + var email = $(this).closest('li[data-email]').data('email'), + msg = new PromptView.Warning({ + title: gettext('Are you sure?'), + message: _.template(gettext('Are you sure you want to delete {email} from the library “{library}”?'), {email: email, library: libraryName}, {interpolate: /\{(.+?)\}/g}), + actions: { + primary: { + text: gettext('Delete'), + click: function(view) { + // User the REST API to delete the user: + changeRole(email, null, { errMessage: gettext('Error removing user') }); + } + }, + secondary: { + text: gettext('Cancel'), + click: function(view) { view.hide(); } + } + } + }); + msg.show(); + }); + + $('.user-actions .make-instructor').click(function(event) { + event.preventDefault(); + var email = $(this).closest('li[data-email]').data('email'); + changeRole(email, 'instructor', {}); + }); + + $('.user-actions .make-staff').click(function(event) { + event.preventDefault(); + var email = $(this).closest('li[data-email]').data('email'); + changeRole(email, 'staff', {}); + }); + + $('.user-actions .make-user').click(function(event) { + event.preventDefault(); + var email = $(this).closest('li[data-email]').data('email'); + changeRole(email, 'library_user', {}); + }); + + }; +}); diff --git a/cms/static/js/spec/views/modals/edit_xblock_spec.js b/cms/static/js/spec/views/modals/edit_xblock_spec.js index 99de1404c87e..b159998e7faf 100644 --- a/cms/static/js/spec/views/modals/edit_xblock_spec.js +++ b/cms/static/js/spec/views/modals/edit_xblock_spec.js @@ -49,7 +49,7 @@ define(["jquery", "underscore", "js/common_helpers/ajax_helpers", "js/spec_helpe var requests = AjaxHelpers.requests(this); modal = showModal(requests, mockXBlockEditorHtml); expect(modal.$('.action-save')).not.toBeVisible(); - expect(modal.$('.action-cancel').text()).toBe('OK'); + expect(modal.$('.action-cancel').text()).toBe('Close'); }); it('shows the correct title', function() { diff --git a/cms/static/js/spec/views/pages/library_users_spec.js b/cms/static/js/spec/views/pages/library_users_spec.js new file mode 100644 index 000000000000..376d95fb23b4 --- /dev/null +++ b/cms/static/js/spec/views/pages/library_users_spec.js @@ -0,0 +1,78 @@ +define([ + "jquery", "js/common_helpers/ajax_helpers", "js/spec_helpers/view_helpers", + "js/factories/manage_users_lib", "js/views/utils/view_utils" +], +function ($, AjaxHelpers, ViewHelpers, ManageUsersFactory, ViewUtils) { + "use strict"; + describe("Library Instructor Access Page", function () { + var mockHTML = readFixtures('mock/mock-manage-users-lib.underscore'); + + beforeEach(function () { + ViewHelpers.installMockAnalytics(); + appendSetFixtures(mockHTML); + ManageUsersFactory( + "Mock Library", + ["honor@example.com", "audit@example.com", "staff@example.com"], + "dummy_change_role_url" + ); + }); + + afterEach(function () { + ViewHelpers.removeMockAnalytics(); + }); + + it("can give a user permission to use the library", function () { + var requests = AjaxHelpers.requests(this); + var reloadSpy = spyOn(ViewUtils, 'reload'); + $('.create-user-button').click(); + expect($('.wrapper-create-user')).toHaveClass('is-shown'); + $('.user-email-input').val('other@example.com'); + $('.form-create.create-user .action-primary').click(); + AjaxHelpers.expectJsonRequest(requests, 'POST', 'dummy_change_role_url', {role: 'library_user'}); + AjaxHelpers.respondWithJson(requests, {'result': 'ok'}); + expect(reloadSpy).toHaveBeenCalled(); + }); + + it("can cancel adding a user to the library", function () { + $('.create-user-button').click(); + $('.form-create.create-user .action-secondary').click(); + expect($('.wrapper-create-user')).not.toHaveClass('is-shown'); + }); + + it("displays an error when the required field is blank", function () { + var requests = AjaxHelpers.requests(this); + $('.create-user-button').click(); + $('.user-email-input').val(''); + var errorPromptSelector = '.wrapper-prompt.is-shown .prompt.error'; + expect($(errorPromptSelector).length).toEqual(0); + $('.form-create.create-user .action-primary').click(); + expect($(errorPromptSelector).length).toEqual(1); + expect($(errorPromptSelector)).toContainText('You must enter a valid email address'); + expect(requests.length).toEqual(0); + }); + + it("displays an error when the user has already been added", function () { + var requests = AjaxHelpers.requests(this); + $('.create-user-button').click(); + $('.user-email-input').val('honor@example.com'); + var warningPromptSelector = '.wrapper-prompt.is-shown .prompt.warning'; + expect($(warningPromptSelector).length).toEqual(0); + $('.form-create.create-user .action-primary').click(); + expect($(warningPromptSelector).length).toEqual(1); + expect($(warningPromptSelector)).toContainText('Already a library team member'); + expect(requests.length).toEqual(0); + }); + + + it("can remove a user's permission to access the library", function () { + var requests = AjaxHelpers.requests(this); + var reloadSpy = spyOn(ViewUtils, 'reload'); + $('.user-item[data-email="honor@example.com"] .action-delete .delete').click(); + expect($('.wrapper-prompt.is-shown .prompt.warning').length).toEqual(1); + $('.wrapper-prompt.is-shown .action-primary').click(); + AjaxHelpers.expectJsonRequest(requests, 'DELETE', 'dummy_change_role_url', {role: null}); + AjaxHelpers.respondWithJson(requests, {'result': 'ok'}); + expect(reloadSpy).toHaveBeenCalled(); + }); + }); +}); diff --git a/cms/static/js/views/modals/edit_xblock.js b/cms/static/js/views/modals/edit_xblock.js index 67e9de6f88e1..ef2de588a008 100644 --- a/cms/static/js/views/modals/edit_xblock.js +++ b/cms/static/js/views/modals/edit_xblock.js @@ -65,7 +65,8 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie onDisplayXBlock: function() { var editorView = this.editorView, - title = this.getTitle(); + title = this.getTitle(), + readOnlyView = (this.editOptions && this.editOptions.readOnlyView) || !editorView.xblock.save; // Notify the runtime that the modal has been shown editorView.notifyRuntime('modal-shown', this); @@ -88,7 +89,7 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie // If the xblock is not using custom buttons then choose which buttons to show if (!editorView.hasCustomButtons()) { // If the xblock does not support save then disable the save button - if (!editorView.xblock.save) { + if (readOnlyView) { this.disableSave(); } this.getActionBar().show(); @@ -101,8 +102,8 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie disableSave: function() { var saveButton = this.getActionButton('save'), cancelButton = this.getActionButton('cancel'); - saveButton.hide(); - cancelButton.text(gettext('OK')); + saveButton.parent().hide(); + cancelButton.text(gettext('Close')); cancelButton.addClass('action-primary'); }, diff --git a/cms/static/js/views/paged_container.js b/cms/static/js/views/paged_container.js index a8cd7aec3242..c2b97f3b5430 100644 --- a/cms/static/js/views/paged_container.js +++ b/cms/static/js/views/paged_container.js @@ -52,7 +52,7 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex success: function(fragment) { self.handleXBlockFragment(fragment, options); self.processPaging({ requested_page: options.page_number }); - self.page.renderAddXBlockComponents() + self.page.renderAddXBlockComponents(); } }); }, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 406e6e9b0354..cea6c4312090 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -20,7 +20,8 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views }, options: { - collapsedClass: 'is-collapsed' + collapsedClass: 'is-collapsed', + canEdit: true // If not specified, assume user has permission to make changes }, view: 'container_preview', @@ -113,9 +114,8 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views // Notify the runtime that the page has been successfully shown xblockView.notifyRuntime('page-shown', self); - // Render the add buttons. Paged containers should do this on their own. if (self.components_on_init) { - // Render the add buttons + // Render the add buttons. Paged containers should do this on their own. self.renderAddXBlockComponents(); } @@ -146,14 +146,18 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views renderAddXBlockComponents: function() { var self = this; - this.$('.add-xblock-component').each(function(index, element) { - var component = new AddXBlockComponent({ - el: element, - createComponent: _.bind(self.createComponent, self), - collection: self.options.templates + if (self.options.canEdit) { + this.$('.add-xblock-component').each(function(index, element) { + var component = new AddXBlockComponent({ + el: element, + createComponent: _.bind(self.createComponent, self), + collection: self.options.templates + }); + component.render(); }); - component.render(); - }); + } else { + this.$('.add-xblock-component').remove(); + } }, editXBlock: function(event) { @@ -163,6 +167,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views event.preventDefault(); modal.edit(xblockElement, this.model, { + readOnlyView: !this.options.canEdit, refresh: function() { self.refreshXBlock(xblockElement, false); } diff --git a/cms/static/sass/views/_dashboard.scss b/cms/static/sass/views/_dashboard.scss index 1a1430f0127e..63f55a9c097e 100644 --- a/cms/static/sass/views/_dashboard.scss +++ b/cms/static/sass/views/_dashboard.scss @@ -495,26 +495,21 @@ .metadata-item { display: inline-block; - &:after { + & + .metadata-item:before { content: "/"; margin-left: ($baseline/10); margin-right: ($baseline/10); color: $gray-l4; } - &:last-child { - - &:after { - content: ""; - margin-left: 0; - margin-right: 0; - } - } - .label { @extend %cont-text-sr; } } + + .extra-metadata { + margin-left: ($baseline/10); + } } .course-actions { diff --git a/cms/static/sass/views/_users.scss b/cms/static/sass/views/_users.scss index 11c89dd19635..a729d9416720 100644 --- a/cms/static/sass/views/_users.scss +++ b/cms/static/sass/views/_users.scss @@ -116,11 +116,16 @@ &.flag-role-admin { background: $pink; } + + &.flag-role-user { + background: $yellow-d1; + .msg-you { color: $yellow-l1; } + } } // ELEM: item - metadata .item-metadata { - width: flex-grid(5, 9); + width: flex-grid(4, 9); @include margin-right(flex-gutter()); .user-username, .user-email { @@ -143,7 +148,7 @@ // ELEM: item - actions .item-actions { - width: flex-grid(4, 9); + width: flex-grid(5, 9); position: static; // nasty reset needed due to base.scss text-align: right; @@ -153,12 +158,34 @@ } .action-role { - width: flex-grid(3, 4); + width: flex-grid(7, 8); margin-right: flex-gutter(); + + .add-admin-role { + @include blue-button; + @include transition(all .15s); + @extend %t-action2; + @extend %t-strong; + display: inline-block; + padding: ($baseline/5) $baseline; + } + + .remove-admin-role { + @include grey-button; + @include transition(all .15s); + @extend %t-action2; + @extend %t-strong; + display: inline-block; + padding: ($baseline/5) $baseline; + } + .notoggleforyou { + @extend %t-copy-sub1; + color: $gray-l2; + } } .action-delete { - width: flex-grid(1, 4); + width: flex-grid(1, 8); // STATE: disabled &.is-disabled { @@ -178,33 +205,6 @@ float: none; color: inherit; } - - // ELEM: admin role controls - .toggle-admin-role { - - &.add-admin-role { - @include blue-button; - @include transition(all .15s); - @extend %t-action2; - @extend %t-strong; - display: inline-block; - padding: ($baseline/5) $baseline; - } - - &.remove-admin-role { - @include grey-button; - @include transition(all .15s); - @extend %t-action2; - @extend %t-strong; - display: inline-block; - padding: ($baseline/5) $baseline; - } - } - - .notoggleforyou { - @extend %t-copy-sub1; - color: $gray-l2; - } } // STATE: hover diff --git a/cms/templates/container.html b/cms/templates/container.html index e36914506b1b..9a6ce236a1c8 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -33,7 +33,8 @@ ${component_templates | n}, ${json.dumps(xblock_info) | n}, "${action}", { - isUnitPage: ${json.dumps(is_unit_page)} + isUnitPage: ${json.dumps(is_unit_page)}, + canEdit: true } ); }); diff --git a/cms/templates/index.html b/cms/templates/index.html index 56f2d5b7ee00..cd2f8978aecb 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -456,6 +456,9 @@

${library_info['display_name']}

${_("Course Number:")} ${library_info['number']} + % if not library_info["can_edit"]: + ${_("(Read-only)")} + % endif diff --git a/cms/templates/js/mock/mock-manage-users-lib.underscore b/cms/templates/js/mock/mock-manage-users-lib.underscore new file mode 100644 index 000000000000..d5b9ebf97ba2 --- /dev/null +++ b/cms/templates/js/mock/mock-manage-users-lib.underscore @@ -0,0 +1,146 @@ +
+
+

+ Settings + > Instructor Access +

+ + +
+
+ +
+
+
+
+
+
+

Grant Instructor Access to This Library

+ +
+ New Instructor Information + +
    +
  1. + + + Please provide the email address of the instructor you'd like to add +
  2. +
+
+
+ +
+ + +
+
+
+ +
    + +
  1. + + + + Current Role: + + Staff + + + + + + + + +
  2. + +
  3. + + + + Current Role: + + Admin + + + + + + + + +
  4. + +
  5. + + + + Current Role: + + User + + + + + + + + +
  6. +
+ +
+
+
diff --git a/cms/templates/js/system-feedback.underscore b/cms/templates/js/system-feedback.underscore index aa63fa558d84..049194a595a8 100644 --- a/cms/templates/js/system-feedback.underscore +++ b/cms/templates/js/system-feedback.underscore @@ -4,6 +4,7 @@ id="<%= type %>-<%= intent %>" aria-hidden="<% if(obj.shown) { %>false<% } else { %>true<% } %>" aria-labelledby="<%= type %>-<%= intent %>-title" + tabindex="-1" <% if (obj.message) { %>aria-describedby="<%= type %>-<%= intent %>-description" <% } %> <% if (obj.actions) { %>role="dialog"<% } %> > diff --git a/cms/templates/library.html b/cms/templates/library.html index 5d06317f04cb..119900f87304 100644 --- a/cms/templates/library.html +++ b/cms/templates/library.html @@ -22,10 +22,12 @@ <%block name="requirejs"> require(["js/factories/library"], function(LibraryFactory) { LibraryFactory( - ${component_templates | n}, ${json.dumps(xblock_info) | n}, + ${component_templates | n}, + ${json.dumps(xblock_info) | n}, { isUnitPage: false, - page_size: 10 + page_size: 10, + canEdit: ${"true" if can_edit else "false"} } ); }); @@ -65,10 +67,12 @@
${_("Library ID")}

+ % if can_edit:

${_("Adding content components")}

${_("You can add components to the library. Help text here.")}

+ % endif
${_("Learn more about content libraries")}
diff --git a/cms/templates/manage_users_lib.html b/cms/templates/manage_users_lib.html new file mode 100644 index 000000000000..0f3959ed93aa --- /dev/null +++ b/cms/templates/manage_users_lib.html @@ -0,0 +1,170 @@ +<%! import json %> +<%! from django.utils.translation import ugettext as _ %> +<%! from django.core.urlresolvers import reverse %> +<%inherit file="base.html" /> +<%def name="online_help_token()"><% return "team" %> +<%block name="title">${_("Library User Access")} +<%block name="bodyclass">is-signedin course users view-team + +<%block name="content"> + +
+
+

+ ${_("Settings")} + > ${_("User Access")} +

+ + +
+
+ +
+
+
+ %if allow_actions: +
+
+
+

${_("Grant Access to This Library")}

+ +
+ ${_("New Team Member Information")} + +
    +
  1. + + + ${_("Provide the email address of the user you want to add")} +
  2. +
+
+
+ +
+ + +
+
+
+ %endif + +
    + % for user in all_users: + <% + is_instructor = user in instructors + is_staff = user in staff + role_id = 'admin' if is_instructor else ('staff' if is_staff else 'user') + role_desc = _("Admin") if is_instructor else (_("Staff") if is_staff else _("User")) + %> + +
  1. + + + + ${_("Current Role:")} + + ${role_desc} + % if request.user.id == user.id: + ${_("You!")} + % endif + + + + + + + % if allow_actions: + + % elif request.user.id == user.id: + + % endif + +
  2. + % endfor +
+ + % if allow_actions and len(all_users) == 1: +
+
+

${_('Add More Users to This Library')}

+
+

${_('Grant other members of your course team access to this library. New library users must have an active {studio_name} account.').format(studio_name=settings.STUDIO_SHORT_NAME)}

+
+
+ + +
+ %endif +
+ + +
+
+ + +<%block name="requirejs"> + require(["js/factories/manage_users_lib"], function(ManageUsersFactory) { + ManageUsersFactory( + "${context_library.display_name_with_default | h}", + ${json.dumps([user.email for user in all_users])}, + "${reverse('contentstore.views.course_team_handler', kwargs={'course_key_string': library_key, 'email': '@@EMAIL@@'})}" + ); + }); + diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 4ad50666d57c..fb75c7818f0d 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -55,31 +55,39 @@
diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index cd509abf6a2a..6233c1aaf3db 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -39,7 +39,7 @@