From bade391bf220d541cb9fa02e2d4bc33b54c71f82 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 6 Nov 2014 20:01:03 -0800 Subject: [PATCH 1/2] 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/access.py | 30 +++++++++++++++++++- cms/djangoapps/contentstore/views/course.py | 5 ++-- cms/djangoapps/contentstore/views/item.py | 21 ++++++++------ cms/djangoapps/contentstore/views/library.py | 24 ++++++++++------ cms/static/js/factories/container.js | 5 ++-- cms/static/js/views/pages/container.js | 12 ++++++-- cms/static/sass/views/_dashboard.scss | 15 ++++------ cms/templates/container.html | 2 +- cms/templates/index.html | 3 ++ cms/templates/library.html | 8 ++++-- common/djangoapps/student/roles.py | 22 ++++++++++++++ common/djangoapps/xmodule_django/models.py | 7 ++--- 12 files changed, 113 insertions(+), 41 deletions(-) diff --git a/cms/djangoapps/contentstore/views/access.py b/cms/djangoapps/contentstore/views/access.py index 6e9eab48b6c7..9d984d983d4e 100644 --- a/cms/djangoapps/contentstore/views/access.py +++ b/cms/djangoapps/contentstore/views/access.py @@ -1,6 +1,10 @@ """ Helper methods for determining user access permissions in Studio """ -from student.roles import CourseStaffRole, GlobalStaff, CourseInstructorRole, OrgStaffRole, OrgInstructorRole +from opaque_keys.edx.locator import LibraryLocator +from student.roles import ( + GlobalStaff, CourseStaffRole, CourseInstructorRole, LibraryUserRole, + OrgStaffRole, OrgInstructorRole, OrgLibraryUserRole +) from student import auth @@ -24,6 +28,30 @@ def has_course_access(user, course_key, role=CourseStaffRole): return auth.has_access(user, role(course_key.for_branch(None))) +def has_write_access(user, course_key): + """ + Return True iff user is allowed to modify the given course/library. + Currently equivalent to has_course_access but less amibguously named. + """ + return has_course_access(user, course_key) + + +def has_read_access(user, course_key): + """ + Return True iff user is allowed to view this course/library. + + There is currently no such thing as read-only course access in studio, but + there is read-only access to content libraries. + """ + if has_course_access(user, course_key): + return True # Global, Org, or Course "Instructors" and "Staff" can read and write + if isinstance(course_key, LibraryLocator): + if OrgLibraryUserRole(org=course_key.org).has_user(user): + return True # User has read-only access to all libraries in this organization + return LibraryUserRole(course_key.for_branch(None)).has_user(user) # User has read-only access this library + return False + + def get_user_role(user, course_id): """ What type of access: staff or instructor does this user have in Studio? diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 860e2e55d640..70d32eb8ae9e 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 .access import has_course_access +from .access import has_course_access, has_read_access, has_write_access from .component import ( OPEN_ENDED_COMPONENT_TYPES, NOTE_COMPONENT_TYPES, @@ -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_access(user, lib.location)] + return [lib for lib in modulestore().get_libraries() if has_read_access(user, lib.location.library_key)] @login_required @@ -415,6 +415,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_write_access(request.user, library.location.library_key), } # remove any courses in courses that are also in the in_process_course_actions list diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index c42fb711fb48..1d9a06ea2022 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 .access import has_course_access +from .access import has_write_access, has_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_access(request.user, usage_key.course_key): + access_check = has_read_access if request.method == 'GET' else has_write_access + if not access_check(request.user, usage_key.course_key): raise PermissionDenied() if request.method == 'GET': @@ -166,6 +167,9 @@ 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']) + if not has_write_access(request.user, parent_usage_key.course_key) or not has_read_access(request.user, duplicate_source_usage_key): + raise PermissionDenied() + dest_usage_key = _duplicate_item( parent_usage_key, duplicate_source_usage_key, @@ -197,7 +201,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_access(request.user, usage_key.course_key): + if not has_read_access(request.user, usage_key.course_key): raise PermissionDenied() accept_header = request.META.get('HTTP_ACCEPT', 'application/json') @@ -284,7 +288,7 @@ def xblock_outline_handler(request, usage_key_string): a course. """ usage_key = usage_key_with_run(usage_key_string) - if not has_course_access(request.user, usage_key.course_key): + if not has_read_access(request.user, usage_key.course_key): raise PermissionDenied() response_format = request.REQUEST.get('format', 'html') @@ -453,13 +457,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_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_access(request.user, usage_key.course_key): - raise PermissionDenied() - store = modulestore() with store.bulk_operations(usage_key.course_key): parent = store.get_item(usage_key) @@ -599,7 +602,7 @@ def orphan_handler(request, course_key_string): """ course_usage_key = CourseKey.from_string(course_key_string) if request.method == 'GET': - if has_course_access(request.user, course_usage_key): + if has_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 7ea3ebf21932..c5da7aa8963b 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 @@ -24,7 +24,7 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from .access import has_course_access +from .access import has_read_access, has_write_access from .component import get_component_templates from student.roles import CourseCreatorRole from student import auth @@ -54,7 +54,7 @@ def library_handler(request, library_key_string=None): library_key = CourseKey.from_string(library_key_string) if not isinstance(library_key, LibraryLocator): raise Http404 # This is not a library - if not has_course_access(request.user, library_key): + if not has_read_access(request.user, library_key): raise PermissionDenied() library = modulestore().get_library(library_key) @@ -62,7 +62,7 @@ def library_handler(request, library_key_string=None): raise Http404 if request.method == 'GET': - return library_blocks_view(library, response_format) + return library_blocks_view(library, request.user, response_format) return HttpResponseNotAllowed(['GET']) elif request.method == 'POST': @@ -74,9 +74,10 @@ def library_handler(request, library_key_string=None): { "display_name": lib.display_name, "library_key": unicode(lib.location.library_key), + "can_edit": has_write_access(request.user, lib.location.library_key) } for lib in modulestore().get_libraries() - if has_course_access(request.user, lib.location.library_key) + if has_read_access(request.user, lib.location.library_key) ] return JsonResponse(lib_info) else: @@ -104,6 +105,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: return JsonResponseBadRequest({ "ErrMsg": _("Unable to create library - missing expected JSON key '{err}'").format(err=error.message)} @@ -124,13 +127,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. """ children = library.children if response_format == "json": @@ -138,20 +143,23 @@ 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.location.course_key), # library.course_id raises UndefinedContext - fix? + "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_write_access(user, library.location.library_key) + xblock_info = create_xblock_info(library, include_ancestor_info=False, graders=[]) - component_templates = get_component_templates(library) + component_templates = get_component_templates(library) if can_edit else [] assert isinstance(library.location.library_key, LibraryLocator) assert isinstance(library.location, LibraryUsageLocator) return render_to_response('library.html', { + 'can_edit': can_edit, 'context_library': library, 'action': 'view', 'xblock': library, diff --git a/cms/static/js/factories/container.js b/cms/static/js/factories/container.js index 93cdeb8fd991..0ff2bc5495a0 100644 --- a/cms/static/js/factories/container.js +++ b/cms/static/js/factories/container.js @@ -5,7 +5,7 @@ define([ ], function($, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { 'use strict'; - return function (componentTemplates, XBlockInfoJson, action, isUnitPage) { + return function (componentTemplates, XBlockInfoJson, action, isUnitPage, canEdit) { var templates = new ComponentTemplates(componentTemplates, {parse: true}), mainXBlockInfo = new XBlockInfo(XBlockInfoJson, {parse: true}); @@ -15,7 +15,8 @@ function($, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { model: mainXBlockInfo, action: action, templates: templates, - isUnitPage: isUnitPage + isUnitPage: isUnitPage, + canEdit: canEdit }); view.render(); }); diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 7a62e535919d..fec3b54db1e3 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', @@ -98,7 +99,11 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views xblockView.notifyRuntime('page-shown', self); // Render the add buttons - self.renderAddXBlockComponents(); + if (self.options.canEdit) { + self.renderAddXBlockComponents(); + } else { + self.$el.find('.add-xblock-component').remove(); + } // Refresh the views now that the xblock is visible self.onXBlockRefresh(xblockView); @@ -120,6 +125,9 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views onXBlockRefresh: function(xblockView) { this.xblockView.refresh(); + if (!this.options.canEdit) { + xblockView.$el.find('.action-duplicate, .action-delete, .action-drag').remove(); + } // Update publish and last modified information from the server. this.model.fetch(); }, diff --git a/cms/static/sass/views/_dashboard.scss b/cms/static/sass/views/_dashboard.scss index c20ecb8c5d72..0f4602b6b1ef 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 b24cffe2ea10..2c764ca04624 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -38,7 +38,7 @@ require(["js/factories/container"], function(ContainerFactory) { ContainerFactory( ${component_templates | n}, ${json.dumps(xblock_info) | n}, - "${action}", ${json.dumps(is_unit_page)} + "${action}", ${json.dumps(is_unit_page)}, true ); }); diff --git a/cms/templates/index.html b/cms/templates/index.html index 15ba6494c41a..6d0807e00b64 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -441,6 +441,9 @@

${library_info['display_name']}

${_("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 8276a6d524c7..921d634196af 100644 --- a/cms/templates/library.html +++ b/cms/templates/library.html @@ -29,7 +29,7 @@ <%block name="requirejs"> require(["js/factories/container"], function(ContainerFactory) { ContainerFactory( - ${component_templates | n}, ${json.dumps(xblock_info) | n}, "${action}", false + ${component_templates | n}, ${json.dumps(xblock_info) | n}, "${action}", false, ${"true" if can_edit else "false"} ); }); @@ -40,7 +40,7 @@
-

${xblock.display_name_with_default | h}

@@ -49,12 +49,14 @@

${_("Page Actions")}

@@ -73,10 +75,12 @@

${_("Page Actions")}