From fefc70c4055dc01d47172c2f39a0e0a76b48c7f8 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 6 Nov 2014 20:01:03 -0800 Subject: [PATCH] Three levels of user permissions for content libraries: Admin ("Instructor") - Can edit and assign permissions to other users Normal ("Staff") - Can edit User - Can view the library and use content from it but cannot edit it or its blocks. --- cms/djangoapps/contentstore/views/course.py | 19 ++++++------ cms/djangoapps/contentstore/views/item.py | 25 +++++++++------- cms/djangoapps/contentstore/views/library.py | 23 ++++++++++----- cms/static/js/factories/library.js | 3 +- cms/static/js/views/paged_container.js | 3 +- cms/static/js/views/pages/container.js | 31 +++++++++++++------- cms/static/sass/views/_dashboard.scss | 15 ++++------ cms/templates/container.html | 3 +- cms/templates/index.html | 3 ++ cms/templates/library.html | 8 +++-- common/djangoapps/student/auth.py | 31 ++++++++++++++++++-- common/djangoapps/student/roles.py | 22 ++++++++++++++ 12 files changed, 131 insertions(+), 55 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 57ffa6b9e1..5bad2efa22 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -48,7 +48,7 @@ from models.settings.course_grading import CourseGradingModel 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 _accessible_courses_list(request): 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 _accessible_courses_list(request): 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 course_listing(request): '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 3ed4305a2c..a98a5cf193 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -37,7 +37,7 @@ from util.date_utils import get_default_time_display 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') @@ -304,7 +310,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 +480,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'] - - display_name = request.json.get('display_name') - - 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() + category = request.json['category'] + display_name = request.json.get('display_name') + if isinstance(usage_key, LibraryUsageLocator): # Only these categories are supported at this time. if category not in ['html', 'problem', 'video']: @@ -627,7 +632,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 1fdc8381a8..afebda11f7 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -9,7 +9,7 @@ import json 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,7 +26,7 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from .component import get_component_templates, CONTAINER_TEMPATES -from student.auth import has_course_author_access +from student.auth import has_studio_write_access, has_studio_read_access from student.roles import CourseCreatorRole from student import auth from util.json_request import expect_json, JsonResponse, JsonResponseBadRequest @@ -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,16 +172,19 @@ 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, diff --git a/cms/static/js/factories/library.js b/cms/static/js/factories/library.js index 76ac47413d..59f447dead 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/views/paged_container.js b/cms/static/js/views/paged_container.js index a8cd7aec32..4b300843b5 100644 --- a/cms/static/js/views/paged_container.js +++ b/cms/static/js/views/paged_container.js @@ -52,7 +52,8 @@ 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(); + self.page.updateBlockActions(); } }); }, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 406e6e9b03..018b03fe8f 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(); } @@ -140,20 +140,31 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views onXBlockRefresh: function(xblockView, block_added) { this.xblockView.refresh(block_added); + this.updateBlockActions(); // Update publish and last modified information from the server. this.model.fetch(); }, 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(); + } + }, + + updateBlockActions: function() { + if (!this.options.canEdit) { + this.xblockView.$el.find('.action-duplicate, .action-delete, .action-drag').remove(); + } }, editXBlock: function(event) { diff --git a/cms/static/sass/views/_dashboard.scss b/cms/static/sass/views/_dashboard.scss index c41fc0c7fd..1afd5e4612 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/templates/container.html b/cms/templates/container.html index d2c7ce3167..574d0c7ae1 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -33,7 +33,8 @@ from django.utils.translation import ugettext as _ ${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 120bfa2c50..4bce500604 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -456,6 +456,9 @@ ${_("Course Number:")} ${library_info['number']} + % if not library_info["can_edit"]: + ${_("(Read-only)")} + % endif diff --git a/cms/templates/library.html b/cms/templates/library.html index 736e0aca29..7e4955fad7 100644 --- a/cms/templates/library.html +++ b/cms/templates/library.html @@ -22,10 +22,12 @@ from django.utils.translation import ugettext as _ <%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,6 +67,7 @@ from django.utils.translation import ugettext as _

+ % if can_edit:

${_("Adding content to your library")}

${_("Add components to your library for use in courses, using Add New Component at the bottom of this page.")}

@@ -72,6 +75,7 @@ from django.utils.translation import ugettext as _

${_("Using library content in courses")}

${_("Use library content in courses by adding the {em_start}library_content{em_end} policy key to Advanced Settings, then adding a Randomized Content Block to your courseware. In the settings for each Randomized Content Block, enter the Library ID for each library from which you want to draw content, and specify the number of problems to be randomly selected and displayed to each student.").format(em_start='', em_end="")}

+ % endif
${_("Learn more about content libraries")}
diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index 1a0caac948..d272249b3d 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -6,9 +6,10 @@ to decide whether to check course creator role, and other such functions. """ from django.core.exceptions import PermissionDenied from django.conf import settings +from opaque_keys.edx.locator import LibraryLocator from student.roles import GlobalStaff, CourseCreatorRole, CourseStaffRole, CourseInstructorRole, CourseRole, \ - CourseBetaTesterRole, OrgInstructorRole, OrgStaffRole + CourseBetaTesterRole, OrgInstructorRole, OrgStaffRole, LibraryUserRole, OrgLibraryUserRole def has_access(user, role): @@ -40,9 +41,9 @@ def has_access(user, role): return False -def has_course_author_access(user, course_key, role=CourseStaffRole): +def has_studio_write_access(user, course_key, role=CourseStaffRole): """ - Return True if user has studio (write) access to the given course. + Return True if user has studio write access to the given course. Note that the CMS permissions model is with respect to courses. There is a super-admin permissions if user.is_staff is set. Also, since we're unifying the user database between LMS and CAS, @@ -64,6 +65,30 @@ def has_course_author_access(user, course_key, role=CourseStaffRole): return has_access(user, role(course_key.for_branch(None))) +def has_course_author_access(*args, **kwargs): + """ + Old name for has_studio_author_access + """ + return has_studio_read_access(*args, **kwargs) + + +def has_studio_read_access(user, course_key): + """ + Return True iff user is allowed to view this course/library in studio. + Will also return True if user has write access in studio (has_course_author_access) + + 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 + + def add_users(caller, role, *users): """ The caller requests adding the given users to the role. Checks that the caller diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 5165966acb..a0eb5856c4 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -219,6 +219,17 @@ class CourseBetaTesterRole(CourseRole): super(CourseBetaTesterRole, self).__init__(self.ROLE, *args, **kwargs) +class LibraryUserRole(CourseRole): + """ + A user who can view a library and import content from it, but not edit it. + Used in Studio only. + """ + ROLE = 'library_user' + + def __init__(self, *args, **kwargs): + super(LibraryUserRole, self).__init__(self.ROLE, *args, **kwargs) + + class OrgStaffRole(OrgRole): """An organization staff member""" def __init__(self, *args, **kwargs): @@ -231,6 +242,17 @@ class OrgInstructorRole(OrgRole): super(OrgInstructorRole, self).__init__('instructor', *args, **kwargs) +class OrgLibraryUserRole(OrgRole): + """ + A user who can view any libraries in an org and import content from them, but not edit them. + Used in Studio only. + """ + ROLE = LibraryUserRole.ROLE + + def __init__(self, *args, **kwargs): + super(OrgLibraryUserRole, self).__init__(self.ROLE, *args, **kwargs) + + class CourseCreatorRole(RoleBase): """ This is the group of people who have permission to create new courses (we may want to eventually