diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index 6f02429b08..5558c57625 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -23,6 +23,8 @@ from mock import Mock from opaque_keys.edx.locator import CourseKey, LibraryLocator from openedx.core.djangoapps.content.course_structures.tests import SignalDisconnectTestMixin from xblock_django.user_service import DjangoXBlockUserService +from xmodule.x_module import STUDIO_VIEW +from student import auth class LibraryTestCase(ModuleStoreTestCase): @@ -30,16 +32,22 @@ class LibraryTestCase(ModuleStoreTestCase): Common functionality for content libraries tests """ def setUp(self): - user_password = super(LibraryTestCase, self).setUp() + self.user_password = super(LibraryTestCase, self).setUp() self.client = AjaxEnabledTestClient() - self.client.login(username=self.user.username, password=user_password) + self._login_as_staff_user(logout_first=False) self.lib_key = self._create_library() self.library = modulestore().get_library(self.lib_key) self.session_data = {} # Used by _bind_module + def _login_as_staff_user(self, logout_first=True): + """ Login as a staff user """ + if logout_first: + self.client.logout() + self.client.login(username=self.user.username, password=self.user_password) + def _create_library(self, org="org", library="lib", display_name="Test Library"): """ Helper method used to create a library. Uses the REST API. @@ -729,6 +737,64 @@ class TestLibraryAccess(SignalDisconnectTestMixin, LibraryTestCase): 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) + def test_studio_user_permissions(self): + """ + Test that user could attach to the problem only libraries that he has access (or which were created by him). + This test was created on the basis of bug described in the pull requests on github: + https://github.com/edx/edx-platform/pull/11331 + https://github.com/edx/edx-platform/pull/11611 + """ + self._create_library(org='admin_org_1', library='lib_adm_1', display_name='admin_lib_1') + self._create_library(org='admin_org_2', library='lib_adm_2', display_name='admin_lib_2') + + self._login_as_non_staff_user() + + self._create_library(org='staff_org_1', library='lib_staff_1', display_name='staff_lib_1') + self._create_library(org='staff_org_2', library='lib_staff_2', display_name='staff_lib_2') + + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + instructor_role = CourseInstructorRole(course.id) + auth.add_users(self.user, instructor_role, self.non_staff_user) + + lib_block = ItemFactory.create( + category='library_content', + parent_location=course.location, + user_id=self.non_staff_user.id, + publish_item=False + ) + + def _get_settings_html(): + """ + Helper function to get block settings HTML + Used to check the available libraries. + """ + edit_view_url = reverse_usage_url("xblock_view_handler", lib_block.location, {"view_name": STUDIO_VIEW}) + + resp = self.client.get_json(edit_view_url) + self.assertEquals(resp.status_code, 200) + + return parse_json(resp)['html'] + + self._login_as_staff_user() + staff_settings_html = _get_settings_html() + self.assertIn('staff_lib_1', staff_settings_html) + self.assertIn('staff_lib_2', staff_settings_html) + self.assertIn('admin_lib_1', staff_settings_html) + self.assertIn('admin_lib_2', staff_settings_html) + + self._login_as_non_staff_user() + response = self.client.get_json(LIBRARY_REST_URL) + staff_libs = parse_json(response) + self.assertEquals(2, len(staff_libs)) + + non_staff_settings_html = _get_settings_html() + self.assertIn('staff_lib_1', non_staff_settings_html) + self.assertIn('staff_lib_2', non_staff_settings_html) + self.assertNotIn('admin_lib_1', non_staff_settings_html) + self.assertNotIn('admin_lib_2', non_staff_settings_html) + @ddt.ddt @override_settings(SEARCH_ENGINE=None) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index d7ac594700..68c7d78e18 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -21,7 +21,7 @@ from xblock.runtime import Mixologist from contentstore.utils import get_lms_link_for_item from contentstore.views.helpers import get_parent_xblock, is_unit, xblock_type_display_name -from contentstore.views.item import create_xblock_info, add_container_page_publishing_info +from contentstore.views.item import create_xblock_info, add_container_page_publishing_info, StudioEditModuleRuntime from opaque_keys.edx.keys import UsageKey @@ -330,6 +330,7 @@ def component_handler(request, usage_key_string, handler, suffix=''): usage_key = UsageKey.from_string(usage_key_string) descriptor = modulestore().get_item(usage_key) + descriptor.xmodule_runtime = StudioEditModuleRuntime(request.user) # Let the module handle the AJAX req = django_to_webob_request(request) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 1109600af0..81325cc9fa 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -21,6 +21,7 @@ from opaque_keys.edx.locator import LibraryUsageLocator from pytz import UTC from xblock.fields import Scope from xblock.fragment import Fragment +from xblock_django.user_service import DjangoXBlockUserService from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW from contentstore.utils import ( @@ -51,6 +52,7 @@ from xmodule.modulestore.inheritance import own_metadata from xmodule.tabs import CourseTabList from xmodule.x_module import PREVIEW_VIEWS, STUDIO_VIEW, STUDENT_VIEW, DEPRECATION_VSCOMPAT_EVENT + __all__ = [ 'orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler', 'xblock_container_handler' ] @@ -198,6 +200,49 @@ def xblock_handler(request, usage_key_string): ) +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, user): + self._user = user + + def can_read(self, course_key): + """ Does the user have read access to the given course/library? """ + return has_studio_read_access(self._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._user, course_key) + + +class StudioEditModuleRuntime(object): + """ + An extremely minimal ModuleSystem shim used for XBlock edits and studio_view. + (i.e. whenever we're not using PreviewModuleSystem.) This is required to make information + about the current user (especially permissions) available via services as needed. + """ + def __init__(self, user): + self._user = user + + def service(self, block, service_name): + """ + This block is not bound to a user but some blocks (LibraryContentModule) may need + user-specific services to check for permissions, etc. + If we return None here, CombinedSystem will load services from the descriptor runtime. + """ + if block.service_declaration(service_name) is not None: + if service_name == "user": + return DjangoXBlockUserService(self._user) + if service_name == "studio_user_permissions": + return StudioPermissionsService(self._user) + return None + + @require_http_methods(("GET")) @login_required @expect_json @@ -231,6 +276,9 @@ def xblock_view_handler(request, usage_key_string, view_name): )) if view_name in (STUDIO_VIEW, VISIBILITY_VIEW): + if view_name == STUDIO_VIEW and xblock.xmodule_runtime is None: + xblock.xmodule_runtime = StudioEditModuleRuntime(request.user) + try: fragment = xblock.render(view_name) # catch exceptions indiscriminately, since after this point they escape the @@ -375,6 +423,7 @@ def _update_with_callback(xblock, user, old_metadata=None, old_content=None): old_metadata = own_metadata(xblock) if old_content is None: old_content = xblock.get_explicitly_set_fields_by_scope(Scope.content) + xblock.xmodule_runtime = StudioEditModuleRuntime(user) xblock.editor_saved(user, old_metadata, old_content) # Update after the callback so any changes made in the callback will get persisted. @@ -624,6 +673,7 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ # Allow an XBlock to do anything fancy it may need to when duplicated from another block. # These blocks may handle their own children or parenting if needed. Let them return booleans to # let us know if we need to handle these or not. + dest_module.xmodule_runtime = StudioEditModuleRuntime(user) children_handled = dest_module.studio_post_duplicate(store, source_item) # Children are not automatically copied over (and not all xblocks have a 'children' attribute). diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 7856254fa0..ce87bcc39f 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -16,7 +16,6 @@ from xmodule.x_module import PREVIEW_VIEWS, STUDENT_VIEW, AUTHOR_VIEW from xmodule.contentstore.django import contentstore from xmodule.error_module import ErrorDescriptor from xmodule.exceptions import NotFoundError, ProcessingError -from xmodule.library_tools import LibraryToolsService from xmodule.services import SettingsService from xmodule.modulestore.django import modulestore, ModuleI18nService from xmodule.mixin import wrap_with_license @@ -150,28 +149,6 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method return result -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 @@ -213,8 +190,6 @@ def _preview_module_system(request, descriptor, field_data): # stick the license wrapper in front wrappers.insert(0, wrap_with_license) - descriptor.runtime._services['studio_user_permissions'] = StudioPermissionsService(request) # pylint: disable=protected-access - return PreviewModuleSystem( static_url=settings.STATIC_URL, # TODO (cpennington): Do we want to track how instructors are using the preview problems? @@ -241,7 +216,6 @@ def _preview_module_system(request, descriptor, field_data): services={ "field-data": field_data, "i18n": ModuleI18nService, - "library_tools": LibraryToolsService(modulestore()), "settings": SettingsService(), "user": DjangoXBlockUserService(request.user), }, diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index 438aec726d..7d7a174038 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -576,12 +576,10 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe """ lib_tools = self.runtime.service(self, 'library_tools') user_perms = self.runtime.service(self, 'studio_user_permissions') - all_libraries = lib_tools.list_available_libraries() - if user_perms: - all_libraries = [ - (key, name) for key, name in all_libraries - if user_perms.can_read(key) or self.source_library_id == unicode(key) - ] + all_libraries = [ + (key, name) for key, name in lib_tools.list_available_libraries() + if user_perms.can_read(key) or self.source_library_id == unicode(key) + ] all_libraries.sort(key=lambda entry: entry[1]) # Sort by name if self.source_library_id and self.source_library_key not in [entry[0] for entry in all_libraries]: all_libraries.append((self.source_library_id, _(u"Invalid Library"))) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index b349a82747..6dbfadc815 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1482,8 +1482,9 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime): """ potential_set = set(super(DescriptorSystem, self).applicable_aside_types(block)) if getattr(block, 'xmodule_runtime', None) is not None: - application_set = set(block.xmodule_runtime.applicable_aside_types(block)) - return list(potential_set.intersection(application_set)) + if hasattr(block.xmodule_runtime, 'applicable_aside_types'): + application_set = set(block.xmodule_runtime.applicable_aside_types(block)) + return list(potential_set.intersection(application_set)) return list(potential_set) def resource_url(self, resource): diff --git a/common/static/js/vendor/jasmine-jquery.js b/common/static/js/vendor/jasmine-jquery.js deleted file mode 100644 index 6a9a3aba98..0000000000 --- a/common/static/js/vendor/jasmine-jquery.js +++ /dev/null @@ -1,366 +0,0 @@ -var readFixtures = function() { - return jasmine.getFixtures().proxyCallTo_('read', arguments) -} - -var preloadFixtures = function() { - jasmine.getFixtures().proxyCallTo_('preload', arguments) -} - -var loadFixtures = function() { - jasmine.getFixtures().proxyCallTo_('load', arguments) -} - -var appendLoadFixtures = function() { - jasmine.getFixtures().proxyCallTo_('appendLoad', arguments) -} - -var setFixtures = function(html) { - jasmine.getFixtures().proxyCallTo_('set', arguments) -} - -var appendSetFixtures = function() { - jasmine.getFixtures().proxyCallTo_('appendSet', arguments) -} - -var sandbox = function(attributes) { - return jasmine.getFixtures().sandbox(attributes) -} - -var spyOnEvent = function(selector, eventName) { - jasmine.JQuery.events.spyOn(selector, eventName) -} - -jasmine.getFixtures = function() { - return jasmine.currentFixtures_ = jasmine.currentFixtures_ || new jasmine.Fixtures() -} - -jasmine.Fixtures = function() { - this.containerId = 'jasmine-fixtures' - this.fixturesCache_ = {} - this.fixturesPath = 'spec/javascripts/fixtures' -} - -jasmine.Fixtures.prototype.set = function(html) { - this.cleanUp() - this.createContainer_(html) -} - -jasmine.Fixtures.prototype.appendSet= function(html) { - this.addToContainer_(html) -} - -jasmine.Fixtures.prototype.preload = function() { - this.read.apply(this, arguments) -} - -jasmine.Fixtures.prototype.load = function() { - this.cleanUp() - this.createContainer_(this.read.apply(this, arguments)) -} - -jasmine.Fixtures.prototype.appendLoad = function() { - this.addToContainer_(this.read.apply(this, arguments)) -} - -jasmine.Fixtures.prototype.read = function() { - var htmlChunks = [] - - var fixtureUrls = arguments - for(var urlCount = fixtureUrls.length, urlIndex = 0; urlIndex < urlCount; urlIndex++) { - htmlChunks.push(this.getFixtureHtml_(fixtureUrls[urlIndex])) - } - - return htmlChunks.join('') -} - -jasmine.Fixtures.prototype.clearCache = function() { - this.fixturesCache_ = {} -} - -jasmine.Fixtures.prototype.cleanUp = function() { - jQuery('#' + this.containerId).remove() -} - -jasmine.Fixtures.prototype.sandbox = function(attributes) { - var attributesToSet = attributes || {} - return jQuery('
').attr(attributesToSet) -} - -jasmine.Fixtures.prototype.createContainer_ = function(html) { - var container - if(html instanceof jQuery) { - container = jQuery('
') - container.html(html) - } else { - container = '
' + html + '
' - } - jQuery('body').append(container) -} - -jasmine.Fixtures.prototype.addToContainer_ = function(html){ - var container = jQuery('body').find('#'+this.containerId).append(html) - if(!container.length){ - this.createContainer_(html) - } -} - -jasmine.Fixtures.prototype.getFixtureHtml_ = function(url) { - if (typeof this.fixturesCache_[url] === 'undefined') { - this.loadFixtureIntoCache_(url) - } - return this.fixturesCache_[url] -} - -jasmine.Fixtures.prototype.loadFixtureIntoCache_ = function(relativeUrl) { - var url = this.makeFixtureUrl_(relativeUrl) - var request = new XMLHttpRequest() - request.open("GET", url + "?" + new Date().getTime(), false) - request.send(null) - this.fixturesCache_[relativeUrl] = request.responseText -} - -jasmine.Fixtures.prototype.makeFixtureUrl_ = function(relativeUrl){ - return this.fixturesPath.match('/$') ? this.fixturesPath + relativeUrl : this.fixturesPath + '/' + relativeUrl -} - -jasmine.Fixtures.prototype.proxyCallTo_ = function(methodName, passedArguments) { - return this[methodName].apply(this, passedArguments) -} - - -jasmine.JQuery = function() {} - -jasmine.JQuery.browserTagCaseIndependentHtml = function(html) { - return jQuery('
').append(html).html() -} - -jasmine.JQuery.elementToString = function(element) { - var domEl = $(element).get(0) - if (domEl == undefined || domEl.cloneNode) - return jQuery('
').append($(element).clone()).html() - else - return element.toString() -} - -jasmine.JQuery.matchersClass = {}; - -!function(namespace) { - var data = { - spiedEvents: {}, - handlers: [] - } - - namespace.events = { - spyOn: function(selector, eventName) { - var handler = function(e) { - data.spiedEvents[[selector, eventName]] = e - } - jQuery(selector).bind(eventName, handler) - data.handlers.push(handler) - }, - - wasTriggered: function(selector, eventName) { - return !!(data.spiedEvents[[selector, eventName]]) - }, - - wasPrevented: function(selector, eventName) { - return data.spiedEvents[[selector, eventName]].isDefaultPrevented() - }, - - cleanUp: function() { - data.spiedEvents = {} - data.handlers = [] - } - } -}(jasmine.JQuery) - -!function(){ - var jQueryMatchers = { - toHaveClass: function(className) { - return this.actual.hasClass(className) - }, - - toHaveCss: function(css){ - for (var prop in css){ - if (this.actual.css(prop) !== css[prop]) return false - } - return true - }, - - toBeVisible: function() { - return this.actual.is(':visible') - }, - - toBeHidden: function() { - return this.actual.is(':hidden') - }, - - toBeSelected: function() { - return this.actual.is(':selected') - }, - - toBeChecked: function() { - return this.actual.is(':checked') - }, - - toBeEmpty: function() { - return this.actual.is(':empty') - }, - - toExist: function() { - return $(document).find(this.actual).length - }, - - toHaveAttr: function(attributeName, expectedAttributeValue) { - return hasProperty(this.actual.attr(attributeName), expectedAttributeValue) - }, - - toHaveProp: function(propertyName, expectedPropertyValue) { - return hasProperty(this.actual.prop(propertyName), expectedPropertyValue) - }, - - toHaveId: function(id) { - return this.actual.attr('id') == id - }, - - toHaveHtml: function(html) { - return this.actual.html() == jasmine.JQuery.browserTagCaseIndependentHtml(html) - }, - - toContainHtml: function(html){ - var actualHtml = this.actual.html() - var expectedHtml = jasmine.JQuery.browserTagCaseIndependentHtml(html) - return (actualHtml.indexOf(expectedHtml) >= 0) - }, - - toHaveText: function(text) { - var trimmedText = $.trim(this.actual.text()) - if (text && jQuery.isFunction(text.test)) { - return text.test(trimmedText) - } else { - return trimmedText == text - } - }, - - toHaveValue: function(value) { - return this.actual.val() == value - }, - - toHaveData: function(key, expectedValue) { - return hasProperty(this.actual.data(key), expectedValue) - }, - - toBe: function(selector) { - return this.actual.is(selector) - }, - - toContain: function(selector) { - return this.actual.find(selector).length - }, - - toBeDisabled: function(selector){ - return this.actual.is(':disabled') - }, - - toBeFocused: function(selector) { - return this.actual.is(':focus') - }, - - toHandle: function(event) { - - var events = this.actual.data('events') - - if(!events || !event || typeof event !== "string") { - return false - } - - var namespaces = event.split(".") - var eventType = namespaces.shift() - var sortedNamespaces = namespaces.slice(0).sort() - var namespaceRegExp = new RegExp("(^|\\.)" + sortedNamespaces.join("\\.(?:.*\\.)?") + "(\\.|$)") - - if(events[eventType] && namespaces.length) { - for(var i = 0; i < events[eventType].length; i++) { - var namespace = events[eventType][i].namespace - if(namespaceRegExp.test(namespace)) { - return true - } - } - } else { - return events[eventType] && events[eventType].length > 0 - } - }, - - // tests the existence of a specific event binding + handler - toHandleWith: function(eventName, eventHandler) { - var stack = this.actual.data("events")[eventName] - for (var i = 0; i < stack.length; i++) { - if (stack[i].handler == eventHandler) return true - } - return false - } - } - - var hasProperty = function(actualValue, expectedValue) { - if (expectedValue === undefined) return actualValue !== undefined - return actualValue == expectedValue - } - - var bindMatcher = function(methodName) { - var builtInMatcher = jasmine.Matchers.prototype[methodName] - - jasmine.JQuery.matchersClass[methodName] = function() { - if (this.actual - && (this.actual instanceof jQuery - || jasmine.isDomNode(this.actual))) { - this.actual = $(this.actual) - var result = jQueryMatchers[methodName].apply(this, arguments) - var element; - if (this.actual.get && (element = this.actual.get()[0]) && !$.isWindow(element) && element.tagName !== "HTML") - this.actual = jasmine.JQuery.elementToString(this.actual) - return result - } - - if (builtInMatcher) { - return builtInMatcher.apply(this, arguments) - } - - return false - } - } - - for(var methodName in jQueryMatchers) { - bindMatcher(methodName) - } -}() - -beforeEach(function() { - this.addMatchers(jasmine.JQuery.matchersClass) - this.addMatchers({ - toHaveBeenTriggeredOn: function(selector) { - this.message = function() { - return [ - "Expected event " + this.actual + " to have been triggered on " + selector, - "Expected event " + this.actual + " not to have been triggered on " + selector - ] - } - return jasmine.JQuery.events.wasTriggered($(selector), this.actual) - } - }) - this.addMatchers({ - toHaveBeenPreventedOn: function(selector) { - this.message = function() { - return [ - "Expected event " + this.actual + " to have been prevented on " + selector, - "Expected event " + this.actual + " not to have been prevented on " + selector - ] - } - return jasmine.JQuery.events.wasPrevented(selector, this.actual) - } - }) -}) - -afterEach(function() { - jasmine.getFixtures().cleanUp() - jasmine.JQuery.events.cleanUp() -}) diff --git a/lms/djangoapps/bulk_email/models.py b/lms/djangoapps/bulk_email/models.py index bf79d023b0..488ba9700b 100644 --- a/lms/djangoapps/bulk_email/models.py +++ b/lms/djangoapps/bulk_email/models.py @@ -12,6 +12,7 @@ file and check it in at the same time as your model changes. To do that, """ import logging +import markupsafe from django.conf import settings from django.contrib.auth.models import User from django.db import models @@ -176,7 +177,7 @@ class CourseEmailTemplate(models.Model): which is rendered using format() with the provided `context` dict. Any keywords encoded in the form %%KEYWORD%% found in the message - body are subtituted with user data before the body is inserted into + body are substituted with user data before the body is inserted into the template. Output is returned as a unicode string. It is not encoded as utf-8. @@ -215,6 +216,10 @@ class CourseEmailTemplate(models.Model): Convert HTML text body (`htmltext`) into HTML email message using the stored HTML template and the provided `context` dict. """ + # HTML-escape string values in the context (used for keyword substitution). + for key, value in context.iteritems(): + if isinstance(value, basestring): + context[key] = markupsafe.escape(value) return CourseEmailTemplate._render(self.html_template, htmltext, context) diff --git a/lms/djangoapps/bulk_email/tests/test_models.py b/lms/djangoapps/bulk_email/tests/test_models.py index ef274558e2..f382071f68 100644 --- a/lms/djangoapps/bulk_email/tests/test_models.py +++ b/lms/djangoapps/bulk_email/tests/test_models.py @@ -97,6 +97,15 @@ class CourseEmailTemplateTest(TestCase): context['course_image_url'] = "/location/of/course/image/url" return context + def _add_xss_fields(self, context): + """ Add fields to the context for XSS testing. """ + context['course_title'] = "