Check permissions when updating LibraryContent blocks
This commit is contained in:
committed by
E. Kolpakov
parent
95c8125609
commit
7303966c13
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user