diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index 5828c223f3..c6fdc00301 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -67,7 +67,7 @@ class LibraryTestCase(ModuleStoreTestCase): **(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 @@ -76,7 +76,7 @@ class LibraryTestCase(ModuleStoreTestCase): 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): @@ -556,3 +556,37 @@ class TestLibraryAccess(LibraryTestCase): 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/preview.py b/cms/djangoapps/contentstore/views/preview.py index 9f9aca8da1..63e1a6b792 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -22,6 +22,7 @@ from xblock.runtime import KvsFieldData 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 @@ class StudioUserService(object): 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, diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 8571540f75..5004a5d51a 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -279,6 +279,7 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): @XBlock.wants('user') @XBlock.wants('library_tools') # Only needed in studio +@XBlock.wants('studio_user_permissions') # Only available in studio class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDescriptor, StudioEditableDescriptor): """ Descriptor class for LibraryContentModule XBlock. @@ -307,8 +308,9 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe """ lib_tools = self.runtime.service(self, 'library_tools') user_service = self.runtime.service(self, 'user') + user_perms = self.runtime.service(self, 'studio_user_permissions') user_id = user_service.user_id if user_service else None # May be None when creating bok choy test fixtures - lib_tools.update_children(self, user_id, update_db) + lib_tools.update_children(self, user_id, user_perms, update_db) return Response() def validate(self): diff --git a/common/lib/xmodule/xmodule/library_tools.py b/common/lib/xmodule/xmodule/library_tools.py index bd52f3e2ac..f8dcadf80e 100644 --- a/common/lib/xmodule/xmodule/library_tools.py +++ b/common/lib/xmodule/xmodule/library_tools.py @@ -2,6 +2,7 @@ XBlock runtime services for LibraryContentModule """ import hashlib +from django.core.exceptions import PermissionDenied from opaque_keys.edx.locator import LibraryLocator from xblock.fields import Scope from xmodule.library_content_module import LibraryVersionReference @@ -44,7 +45,7 @@ class LibraryToolsService(object): return library.location.library_key.version_guid return None - def update_children(self, dest_block, user_id, update_db=True): + def update_children(self, dest_block, user_id, user_perms=None, update_db=True): """ This method is to be used when any of the libraries that a LibraryContentModule references have been updated. It will re-fetch all matching blocks from @@ -62,6 +63,8 @@ class LibraryToolsService(object): anyways. Otherwise, orphaned blocks may be created. """ root_children = [] + if user_perms and not user_perms.can_write(dest_block.location.course_key): + raise PermissionDenied() with self.store.bulk_operations(dest_block.location.course_key): # Currently, ALL children are essentially deleted and then re-added @@ -76,6 +79,8 @@ class LibraryToolsService(object): library = self._get_library(library_key) if library is None: raise ValueError("Required library not found.") + if user_perms and not user_perms.can_read(library_key): + raise PermissionDenied() libraries.append((library_key, library)) # Next, delete all our existing children to avoid block_id conflicts when we add them: diff --git a/common/test/acceptance/tests/studio/test_studio_library_container.py b/common/test/acceptance/tests/studio/test_studio_library_container.py index d7a592c79f..42fdfc4dc5 100644 --- a/common/test/acceptance/tests/studio/test_studio_library_container.py +++ b/common/test/acceptance/tests/studio/test_studio_library_container.py @@ -2,8 +2,11 @@ Acceptance tests for Library Content in LMS """ import ddt -from .base_studio_test import StudioLibraryTest, ContainerBase +from .base_studio_test import StudioLibraryTest +from ...fixtures.course import CourseFixture +from ..helpers import UniqueCourseTest from ...pages.studio.library import StudioLibraryContentXBlockEditModal, StudioLibraryContainerXBlockWrapper +from ...pages.studio.overview import CourseOutlinePage from ...fixtures.course import XBlockFixtureDesc SECTION_NAME = 'Test Section' @@ -12,7 +15,7 @@ UNIT_NAME = 'Test Unit' @ddt.ddt -class StudioLibraryContainerTest(ContainerBase, StudioLibraryTest): +class StudioLibraryContainerTest(StudioLibraryTest, UniqueCourseTest): """ Test Library Content block in LMS """ @@ -21,6 +24,12 @@ class StudioLibraryContainerTest(ContainerBase, StudioLibraryTest): Install library with some content and a course using fixtures """ super(StudioLibraryContainerTest, self).setUp() + # Also create a course: + self.course_fixture = CourseFixture(self.course_info['org'], self.course_info['number'], self.course_info['run'], self.course_info['display_name']) + self.populate_course_fixture(self.course_fixture) + self.course_fixture.install() + self.outline = CourseOutlinePage(self.browser, self.course_info['org'], self.course_info['number'], self.course_info['run']) + self.outline.visit() subsection = self.outline.section(SECTION_NAME).subsection(SUBSECTION_NAME) self.unit_page = subsection.toggle_expand().unit(UNIT_NAME).go_to()