From 2173a98ef8f5986ce6af9536b0ec2b2b413c818e Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Mon, 6 Dec 2021 12:03:55 +1030 Subject: [PATCH] refactor: deprecates ModuleSystem properties for code sandboxing and cache * Deprecates ModuleSystem can_execute_unsafe_code, get_python_lib_zip and cache properties * Adds a new CacheService and SandboxService to provide the deprecated property * Adds tests for the added CacheService and SandboxService * Updates the ModuleSystemShim tests in Lms and Studio --- cms/djangoapps/contentstore/views/preview.py | 26 ++-- .../contentstore/views/tests/test_preview.py | 62 ++++++--- .../djangoapps/util/tests/test_sandboxing.py | 77 +++++++++++- .../xmodule/modulestore/tests/django_utils.py | 25 +++- common/lib/xmodule/xmodule/util/sandboxing.py | 26 ++++ common/lib/xmodule/xmodule/x_module.py | 74 ++++++++--- lms/djangoapps/courseware/module_render.py | 18 +-- .../courseware/tests/test_module_render.py | 118 ++++++++++-------- .../core/djangoapps/xblock/runtime/runtime.py | 14 ++- openedx/core/lib/cache_utils.py | 24 ++++ openedx/core/lib/tests/test_cache_utils.py | 29 ++++- 11 files changed, 374 insertions(+), 119 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 6cb518ffe8..53b6d17832 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -4,6 +4,7 @@ import logging from functools import partial from django.conf import settings +from django.core.cache import cache from django.contrib.auth.decorators import login_required from django.http import Http404, HttpResponseBadRequest from django.urls import reverse @@ -15,6 +16,16 @@ from xblock.django.request import django_to_webob_request, webob_to_django_respo from xblock.exceptions import NoSuchHandlerError from xblock.runtime import KvsFieldData +from xmodule.contentstore.django import contentstore +from xmodule.error_module import ErrorBlock +from xmodule.exceptions import NotFoundError, ProcessingError +from xmodule.modulestore.django import ModuleI18nService, modulestore +from xmodule.partitions.partitions_service import PartitionService +from xmodule.services import SettingsService, TeamsConfigurationService +from xmodule.studio_editable import has_author_view +from xmodule.util.sandboxing import SandboxService +from xmodule.util.xmodule_django import add_webpack_to_fragment +from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, ModuleSystem, XModule, XModuleDescriptor from cms.djangoapps.xblock_config.models import StudioConfig from cms.lib.xblock.field_data import CmsFieldData from common.djangoapps import static_replace @@ -23,6 +34,7 @@ from common.djangoapps.edxmako.services import MakoService from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from lms.djangoapps.lms_xblock.field_data import LmsFieldData from openedx.core.lib.license import wrap_with_license +from openedx.core.lib.cache_utils import CacheService from openedx.core.lib.xblock_utils import ( replace_static_urls, request_token, @@ -31,16 +43,6 @@ from openedx.core.lib.xblock_utils import ( wrap_xblock_aside, xblock_local_resource_url ) -from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.error_module import ErrorBlock # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.exceptions import NotFoundError, ProcessingError # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.django import ModuleI18nService, modulestore # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.partitions.partitions_service import PartitionService # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.services import SettingsService, TeamsConfigurationService # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.studio_editable import has_author_view # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.util.sandboxing import can_execute_unsafe_code, get_python_lib_zip # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.util.xmodule_django import add_webpack_to_fragment # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.x_module import AUTHOR_VIEW, PREVIEW_VIEWS, STUDENT_VIEW, ModuleSystem, XModule, XModuleDescriptor # lint-amnesty, pylint: disable=wrong-import-order from ..utils import get_visibility_partition_info from .access import get_user_role @@ -198,8 +200,6 @@ def _preview_module_system(request, descriptor, field_data): get_module=partial(_load_preview_module, request), debug=True, replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id), - can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)), - get_python_lib_zip=(lambda: get_python_lib_zip(contentstore, course_id)), mixins=settings.XBLOCK_MIXINS, course_id=course_id, @@ -221,6 +221,8 @@ def _preview_module_system(request, descriptor, field_data): ), "partitions": StudioPartitionService(course_id=course_id), "teams_configuration": TeamsConfigurationService(), + "sandbox": SandboxService(contentstore=contentstore, course_id=course_id), + "cache": CacheService(cache), }, ) diff --git a/cms/djangoapps/contentstore/views/tests/test_preview.py b/cms/djangoapps/contentstore/views/tests/test_preview.py index eee4b7b158..9dcc20d03e 100644 --- a/cms/djangoapps/contentstore/views/tests/test_preview.py +++ b/cms/djangoapps/contentstore/views/tests/test_preview.py @@ -8,17 +8,19 @@ from unittest import mock import ddt from django.test.client import Client, RequestFactory +from django.test.utils import override_settings from web_fragments.fragment import Fragment from xblock.core import XBlock, XBlockAside +from xmodule.contentstore.django import contentstore +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, upload_file_to_course +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.tests.test_asides import AsideTestType from cms.djangoapps.contentstore.utils import reverse_usage_url from cms.djangoapps.xblock_config.models import StudioConfig from common.djangoapps.student.tests.factories import UserFactory -from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.test_asides import AsideTestType # lint-amnesty, pylint: disable=wrong-import-order from ..preview import _preview_module_system, get_preview_fragment @@ -220,22 +222,28 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): """ Tests that the deprecated attributes in the Module System (XBlock Runtime) return the expected values. """ + COURSE_ID = 'edX/CmsModuleShimTest/2021_Fall' + PYTHON_LIB_FILENAME = 'test_python_lib.zip' + PYTHON_LIB_SOURCE_FILE = './common/test/data/uploads/python_lib.zip' + def setUp(self): """ - Set up the user and other fields that will be used to instantiate the runtime. + Set up the user, course and other fields that will be used to instantiate the runtime. """ super().setUp() - self.course = CourseFactory.create() + org, number, run = self.COURSE_ID.split('/') + self.course = CourseFactory.create(org=org, number=number, run=run) self.user = UserFactory() self.request = RequestFactory().get('/dummy-url') self.request.user = self.user self.request.session = {} self.descriptor = ItemFactory(category="video", parent=self.course) self.field_data = mock.Mock() + self.contentstore = contentstore() self.runtime = _preview_module_system( self.request, - self.descriptor, - self.field_data, + descriptor=ItemFactory(category="problem", parent=self.course), + field_data=mock.Mock(), ) def test_get_user_role(self): @@ -247,12 +255,32 @@ class CmsModuleSystemShimTest(ModuleStoreTestCase): html = get_preview_fragment(self.request, descriptor, {'element_id': 142}).content assert '
Testing the MakoService
' in html - def test_xqueue_is_not_available_in_studio(self): - descriptor = ItemFactory(category="problem", parent=self.course) - runtime = _preview_module_system( - self.request, - descriptor=descriptor, - field_data=mock.Mock(), + @override_settings(COURSES_WITH_UNSAFE_CODE=[COURSE_ID]) + def test_can_execute_unsafe_code(self): + assert self.runtime.can_execute_unsafe_code() + + def test_cannot_execute_unsafe_code(self): + assert not self.runtime.can_execute_unsafe_code() + + @override_settings(PYTHON_LIB_FILENAME=PYTHON_LIB_FILENAME) + def test_get_python_lib_zip(self): + zipfile = upload_file_to_course( + course_key=self.course.id, + contentstore=self.contentstore, + source_file=self.PYTHON_LIB_SOURCE_FILE, + target_filename=self.PYTHON_LIB_FILENAME, ) - assert runtime.xqueue is None - assert runtime.service(descriptor, 'xqueue') is None + assert self.runtime.get_python_lib_zip() == zipfile + + def test_no_get_python_lib_zip(self): + zipfile = upload_file_to_course( + course_key=self.course.id, + contentstore=self.contentstore, + source_file=self.PYTHON_LIB_SOURCE_FILE, + target_filename=self.PYTHON_LIB_FILENAME, + ) + assert self.runtime.get_python_lib_zip() is None + + def test_cache(self): + assert hasattr(self.runtime.cache, 'get') + assert hasattr(self.runtime.cache, 'set') diff --git a/common/djangoapps/util/tests/test_sandboxing.py b/common/djangoapps/util/tests/test_sandboxing.py index c96ee3c5b2..a1c06172ed 100644 --- a/common/djangoapps/util/tests/test_sandboxing.py +++ b/common/djangoapps/util/tests/test_sandboxing.py @@ -3,12 +3,15 @@ Tests for sandboxing.py in util app """ +import ddt from django.test import TestCase from django.test.utils import override_settings from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import CourseLocator, LibraryLocator -from xmodule.util.sandboxing import can_execute_unsafe_code +from xmodule.contentstore.django import contentstore +from xmodule.modulestore.tests.django_utils import upload_file_to_course +from xmodule.util.sandboxing import can_execute_unsafe_code, SandboxService class SandboxingTest(TestCase): @@ -39,3 +42,75 @@ class SandboxingTest(TestCase): assert not can_execute_unsafe_code(CourseLocator('edX', 'full', '2012_Fall')) assert not can_execute_unsafe_code(CourseLocator('edX', 'full', '2013_Spring')) assert not can_execute_unsafe_code(LibraryLocator('edX', 'test_bank')) + + +@ddt.ddt +class SandboxServiceTest(TestCase): + """ + Test SandboxService methods. + """ + PYTHON_LIB_FILENAME = 'test_python_lib.zip' + PYTHON_LIB_SOURCE_FILE = './common/test/data/uploads/python_lib.zip' + + @classmethod + def setUpClass(cls): + """ + Upload the python lib file to the test course. + """ + super().setUpClass() + + course_key = CourseLocator('test', 'sandbox_test', '2021_01') + cls.sandbox_service = SandboxService(course_id=course_key, contentstore=contentstore) + cls.zipfile = upload_file_to_course( + course_key=course_key, + contentstore=cls.sandbox_service.contentstore(), + source_file=cls.PYTHON_LIB_SOURCE_FILE, + target_filename=cls.PYTHON_LIB_FILENAME, + ) + + @staticmethod + def validate_can_execute_unsafe_code(context_key, expected_result): + sandbox_service = SandboxService(course_id=context_key, contentstore=None) + assert expected_result == sandbox_service.can_execute_unsafe_code() + + @ddt.data( + CourseLocator('edX', 'notful', 'empty'), + LibraryLocator('edY', 'test_bank'), + ) + @override_settings(COURSES_WITH_UNSAFE_CODE=['edX/full/.*', 'library:v1-edX+.*']) + def test_sandbox_exclusion(self, context_key): + """ + Test to make sure that a non-match returns false + """ + self.validate_can_execute_unsafe_code(context_key, False) + + @ddt.data( + CourseKey.from_string('edX/full/2012_Fall'), + CourseKey.from_string('edX/full/2013_Spring'), + ) + @override_settings(COURSES_WITH_UNSAFE_CODE=['edX/full/.*']) + def test_sandbox_inclusion(self, context_key): + """ + Test to make sure that a match works across course runs + """ + self.validate_can_execute_unsafe_code(context_key, True) + self.validate_can_execute_unsafe_code(LibraryLocator('edX', 'test_bank'), False) + + @ddt.data( + CourseLocator('edX', 'full', '2012_Fall'), + CourseLocator('edX', 'full', '2013_Spring'), + LibraryLocator('edX', 'test_bank'), + ) + def test_courselikes_with_unsafe_code_default(self, context_key): + """ + Test that the default setting for COURSES_WITH_UNSAFE_CODE is an empty setting, + i.e., we don't use @override_settings in these tests + """ + self.validate_can_execute_unsafe_code(context_key, False) + + @override_settings(PYTHON_LIB_FILENAME=PYTHON_LIB_FILENAME) + def test_get_python_lib_zip(self): + assert self.sandbox_service.get_python_lib_zip() == self.zipfile + + def test_no_python_lib_zip(self): + assert self.sandbox_service.get_python_lib_zip() is None diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index e230673d10..ba724baa78 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -8,6 +8,7 @@ import functools import os from contextlib import contextmanager from enum import Enum +from mimetypes import guess_type from unittest.mock import patch from django.conf import settings @@ -16,6 +17,12 @@ from django.db import connections, transaction from django.test import TestCase from django.test.utils import override_settings +from xmodule.contentstore.content import StaticContent +from xmodule.contentstore.django import _CONTENTSTORE +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import SignalHandler, clear_existing_modulestores, modulestore +from xmodule.modulestore.tests.factories import XMODULE_FACTORY_LOCK +from xmodule.modulestore.tests.mongo_connection import MONGO_HOST, MONGO_PORT_NUM from lms.djangoapps.courseware.field_overrides import OverrideFieldData from openedx.core.djangolib.testing.utils import CacheIsolationMixin, CacheIsolationTestCase, FilteredQueryCountMixin from openedx.core.lib.tempdir import mkdtemp_clean @@ -23,11 +30,6 @@ from common.djangoapps.split_modulestore_django.models import SplitModulestoreCo from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.tests.factories import AdminFactory, UserFactory, InstructorFactory from common.djangoapps.student.tests.factories import StaffFactory -from xmodule.contentstore.django import _CONTENTSTORE -from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.django import SignalHandler, clear_existing_modulestores, modulestore -from xmodule.modulestore.tests.factories import XMODULE_FACTORY_LOCK -from xmodule.modulestore.tests.mongo_connection import MONGO_HOST, MONGO_PORT_NUM class CourseUserType(Enum): @@ -604,3 +606,16 @@ class ModuleStoreTestCase( self.store.update_item(course, user_id) updated_course = self.store.get_course(course.id) return updated_course + + +def upload_file_to_course(course_key, contentstore, source_file, target_filename): + ''' + Uploads the given source file to the given course, and returns the content of the file. + ''' + asset_key = course_key.make_asset_key('asset', target_filename) + with open(source_file, "rb") as f: + file_contents = f.read() + mimetype = guess_type(source_file)[0] + content = StaticContent(asset_key, target_filename, mimetype, file_contents, locked=False) + contentstore.save(content) + return file_contents diff --git a/common/lib/xmodule/xmodule/util/sandboxing.py b/common/lib/xmodule/xmodule/util/sandboxing.py index 7dc6a9320b..12c4243acf 100644 --- a/common/lib/xmodule/xmodule/util/sandboxing.py +++ b/common/lib/xmodule/xmodule/util/sandboxing.py @@ -41,3 +41,29 @@ def get_python_lib_zip(contentstore, course_id): return zip_lib.data else: return None + + +class SandboxService: + """ + A service which provides utilities for executing sandboxed Python code, for example, inside custom Python questions. + + Args: + contentstore(function): function which creates an instance of xmodule.content.ContentStore + course_id(string or CourseLocator): identifier for the course + """ + def __init__(self, contentstore, course_id, **kwargs): + super().__init__(**kwargs) + self.contentstore = contentstore + self.course_id = course_id + + def can_execute_unsafe_code(self): + """ + Returns a boolean, true if the course can run outside the sandbox. + """ + return can_execute_unsafe_code(self.course_id) + + def get_python_lib_zip(self): + """ + Return the bytes of the course code library file, if it exists. + """ + return get_python_lib_zip(self.contentstore, self.course_id) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 00c3bf66ab..46ae9ce156 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -34,13 +34,13 @@ from xblock.fields import ( ) from xblock.runtime import IdGenerator, IdReader, Runtime -from openedx.core.djangolib.markup import HTML from xmodule import block_metadata_utils from xmodule.errortracker import exc_info_to_str from xmodule.exceptions import UndefinedContext from xmodule.fields import RelativeTime from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.util.xmodule_django import add_webpack_to_fragment +from openedx.core.djangolib.markup import HTML from common.djangoapps.xblock_django.constants import ( ATTR_KEY_ANONYMOUS_USER_ID, @@ -1906,6 +1906,59 @@ class ModuleSystemShim: } return None + @property + def can_execute_unsafe_code(self): + """ + Returns a function which returns a boolean, indicating whether or not to allow the execution of unsafe, + unsandboxed code. + + Deprecated in favor of the sandbox service. + """ + warnings.warn( + 'runtime.can_execute_unsafe_code is deprecated. Please use the sandbox service instead.', + DeprecationWarning, stacklevel=3, + ) + sandbox_service = self._services.get('sandbox') + if sandbox_service: + return sandbox_service.can_execute_unsafe_code + # Default to saying "no unsafe code". + return lambda: False + + @property + def get_python_lib_zip(self): + """ + Returns a function returning a bytestring or None. + + The bytestring is the contents of a zip file that should be importable by other Python code running in the + module. + + Deprecated in favor of the sandbox service. + """ + warnings.warn( + 'runtime.get_python_lib_zip is deprecated. Please use the sandbox service instead.', + DeprecationWarning, stacklevel=3, + ) + sandbox_service = self._services.get('sandbox') + if sandbox_service: + return sandbox_service.get_python_lib_zip + # Default to saying "no lib data" + return lambda: None + + @property + def cache(self): + """ + Returns a cache object with two methods: + * .get(key) returns an object from the cache or None. + * .set(key, value, timeout_secs=None) stores a value in the cache with a timeout. + + Deprecated in favor of the cache service. + """ + warnings.warn( + 'runtime.cache is deprecated. Please use the cache service instead.', + DeprecationWarning, stacklevel=3, + ) + return self._services.get('cache') or DoNothingCache() + class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime): """ @@ -1925,10 +1978,10 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, replace_urls, descriptor_runtime, filestore=None, debug=False, hostname="", publish=None, node_path="", course_id=None, - cache=None, can_execute_unsafe_code=None, replace_course_urls=None, + replace_course_urls=None, replace_jump_to_id_urls=None, error_descriptor_class=None, field_data=None, rebind_noauth_module_to_user=None, - get_python_lib_zip=None, **kwargs): + **kwargs): """ Create a closure around the system environment. @@ -1956,17 +2009,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, publish(event) - A function that allows XModules to publish events (such as grade changes) - cache - A cache object with two methods: - .get(key) returns an object from the cache or None. - .set(key, value, timeout_secs=None) stores a value in the cache with a timeout. - - can_execute_unsafe_code - A function returning a boolean, whether or - not to allow the execution of unsafe, unsandboxed code. - - get_python_lib_zip - A function returning a bytestring or None. The - bytestring is the contents of a zip file that should be importable - by other Python code running in the module. - error_descriptor_class - The class to use to render XModules with errors field_data - the `FieldData` to use for backing XBlock storage. @@ -1993,10 +2035,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, if publish: self.publish = publish - - self.cache = cache or DoNothingCache() - self.can_execute_unsafe_code = can_execute_unsafe_code or (lambda: False) - self.get_python_lib_zip = get_python_lib_zip or (lambda: None) self.replace_course_urls = replace_course_urls self.replace_jump_to_id_urls = replace_jump_to_id_urls self.error_descriptor_class = error_descriptor_class diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index a8fb7cf8f6..4bf1a46dcd 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -39,6 +39,12 @@ from xblock.exceptions import NoSuchHandlerError, NoSuchViewError from xblock.reference.plugins import FSService from xblock.runtime import KvsFieldData +from xmodule.contentstore.django import contentstore +from xmodule.error_module import ErrorBlock, NonStaffErrorBlock +from xmodule.exceptions import NotFoundError, ProcessingError +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.util.sandboxing import SandboxService from common.djangoapps import static_replace from common.djangoapps.xblock_django.constants import ATTR_KEY_USER_ID from capa.xqueue_interface import XQueueService # lint-amnesty, pylint: disable=wrong-import-order @@ -90,12 +96,7 @@ from common.djangoapps.util import milestones_helpers from common.djangoapps.util.json_request import JsonResponse from common.djangoapps.edxmako.services import MakoService from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService -from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.error_module import ErrorBlock, NonStaffErrorBlock # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.exceptions import NotFoundError, ProcessingError # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.util.sandboxing import can_execute_unsafe_code, get_python_lib_zip # lint-amnesty, pylint: disable=wrong-import-order +from openedx.core.lib.cache_utils import CacheService log = logging.getLogger(__name__) @@ -772,9 +773,6 @@ def get_module_system_for_user( node_path=settings.NODE_PATH, publish=publish, course_id=course_id, - cache=cache, - can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)), - get_python_lib_zip=(lambda: get_python_lib_zip(contentstore, course_id)), # TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington) mixins=descriptor.runtime.mixologist._mixins, # pylint: disable=protected-access wrappers=block_wrappers, @@ -792,6 +790,8 @@ def get_module_system_for_user( 'grade_utils': GradesUtilService(course_id=course_id), 'user_state': UserStateService(), 'content_type_gating': ContentTypeGatingService(), + 'cache': CacheService(cache), + 'sandbox': SandboxService(contentstore=contentstore, course_id=course_id), 'xqueue': xqueue_service, }, descriptor_runtime=descriptor._runtime, # pylint: disable=protected-access diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 0ebcb2c979..b007a3358a 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -42,6 +42,21 @@ from xblock.test.tools import TestRuntime # lint-amnesty, pylint: disable=wrong from capa.tests.response_xml_factory import OptionResponseXMLFactory # lint-amnesty, pylint: disable=reimported from capa.xqueue_interface import XQueueInterface +from xmodule.capa_module import ProblemBlock +from xmodule.contentstore.django import contentstore +from xmodule.html_module import AboutBlock, CourseInfoBlock, HtmlBlock, StaticTabBlock +from xmodule.lti_module import LTIBlock +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ( + ModuleStoreTestCase, + SharedModuleStoreTestCase, + upload_file_to_course, +) +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, ToyCourseFactory, check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore.tests.test_asides import AsideTestType # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.video_module import VideoBlock # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.x_module import STUDENT_VIEW, CombinedSystem, XModule, XModuleDescriptor # lint-amnesty, pylint: disable=wrong-import-order from common.djangoapps.course_modes.models import CourseMode # lint-amnesty, pylint: disable=reimported from common.djangoapps.student.tests.factories import GlobalStaffFactory from common.djangoapps.student.tests.factories import RequestFactoryNoCsrf @@ -69,19 +84,6 @@ from openedx.core.lib.url_utils import quote_slashes from common.djangoapps.student.models import CourseEnrollment, anonymous_id_for_user from lms.djangoapps.verify_student.tests.factories import SoftwareSecurePhotoVerificationFactory from common.djangoapps.xblock_django.models import XBlockConfiguration -from xmodule.capa_module import ProblemBlock # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.html_module import AboutBlock, CourseInfoBlock, HtmlBlock, StaticTabBlock # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.lti_module import LTIBlock # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.django_utils import ( # lint-amnesty, pylint: disable=wrong-import-order - ModuleStoreTestCase, - SharedModuleStoreTestCase -) -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, ToyCourseFactory, check_mongo_calls # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.test_asides import AsideTestType # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.video_module import VideoBlock # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.x_module import STUDENT_VIEW, CombinedSystem, XModule, XModuleDescriptor # lint-amnesty, pylint: disable=wrong-import-order TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT @@ -2565,6 +2567,8 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): Tests that the deprecated attributes in the LMS Module System (XBlock Runtime) return the expected values. """ COURSE_ID = 'edX/LmsModuleShimTest/2021_Fall' + PYTHON_LIB_FILENAME = 'test_python_lib.zip' + PYTHON_LIB_SOURCE_FILE = './common/test/data/uploads/python_lib.zip' @classmethod def setUpClass(cls): @@ -2586,6 +2590,16 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): self.student_data = Mock() self.track_function = Mock() self.request_token = Mock() + self.contentstore = contentstore() + self.runtime, _ = render.get_module_system_for_user( + self.user, + self.student_data, + self.descriptor, + self.course.id, + self.track_function, + self.request_token, + course=self.course, + ) @ddt.data( ('seed', 232), @@ -2597,16 +2611,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): """ Tests that the deprecated attributes provided by the user service match expected values. """ - runtime, _ = render.get_module_system_for_user( - self.user, - self.student_data, - self.descriptor, - self.course.id, - self.track_function, - self.request_token, - course=self.course, - ) - assert getattr(runtime, attribute) == expected_value + assert getattr(self.runtime, attribute) == expected_value @patch('lms.djangoapps.courseware.module_render.has_access', Mock(return_value=True, autospec=True)) def test_user_is_staff(self): @@ -2636,16 +2641,7 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): assert runtime.get_user_role() == 'instructor' def test_anonymous_student_id(self): - runtime, _ = render.get_module_system_for_user( - self.user, - self.student_data, - self.descriptor, - self.course.id, - self.track_function, - self.request_token, - course=self.course, - ) - assert runtime.anonymous_student_id == anonymous_id_for_user(self.user, self.course.id) + assert self.runtime.anonymous_student_id == anonymous_id_for_user(self.user, self.course.id) def test_anonymous_student_id_bug(self): """ @@ -2715,29 +2711,11 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): assert runtime.get_real_user() == self.user # pylint: disable=not-callable def test_render_template(self): - runtime, _ = render.get_module_system_for_user( - self.user, - self.student_data, - self.descriptor, - self.course.id, - self.track_function, - self.request_token, - course=self.course, - ) - rendered = runtime.render_template('templates/edxmako.html', {'element_id': 'hi'}) # pylint: disable=not-callable + rendered = self.runtime.render_template('templates/edxmako.html', {'element_id': 'hi'}) # pylint: disable=not-callable assert rendered == '
Testing the MakoService
\n' def test_xqueue(self): - runtime, _ = render.get_module_system_for_user( - self.user, - self.student_data, - self.descriptor, - self.course.id, - self.track_function, - self.request_token, - course=self.course, - ) - xqueue = runtime.xqueue + xqueue = self.runtime.xqueue assert isinstance(xqueue['interface'], XQueueInterface) assert xqueue['interface'].url == 'http://sandbox-xqueue.edx.org' assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest' @@ -2777,3 +2755,37 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase): callback_url = f'http://alt.url/courses/edX/LmsModuleShimTest/2021_Fall/xqueue/232/{self.descriptor.location}' assert xqueue['construct_callback']() == f'{callback_url}/score_update' assert xqueue['construct_callback']('mock_dispatch') == f'{callback_url}/mock_dispatch' + + @override_settings(COURSES_WITH_UNSAFE_CODE=[COURSE_ID]) + def test_can_execute_unsafe_code_when_allowed(self): + assert self.runtime.can_execute_unsafe_code() + + @override_settings(COURSES_WITH_UNSAFE_CODE=['edX/full/2012_Fall']) + def test_cannot_execute_unsafe_code_when_disallowed(self): + assert not self.runtime.can_execute_unsafe_code() + + def test_cannot_execute_unsafe_code(self): + assert not self.runtime.can_execute_unsafe_code() + + @override_settings(PYTHON_LIB_FILENAME=PYTHON_LIB_FILENAME) + def test_get_python_lib_zip(self): + zipfile = upload_file_to_course( + course_key=self.course.id, + contentstore=self.contentstore, + source_file=self.PYTHON_LIB_SOURCE_FILE, + target_filename=self.PYTHON_LIB_FILENAME, + ) + assert self.runtime.get_python_lib_zip() == zipfile + + def test_no_get_python_lib_zip(self): + zipfile = upload_file_to_course( + course_key=self.course.id, + contentstore=self.contentstore, + source_file=self.PYTHON_LIB_SOURCE_FILE, + target_filename=self.PYTHON_LIB_FILENAME, + ) + assert self.runtime.get_python_lib_zip() is None + + def test_cache(self): + assert hasattr(self.runtime.cache, 'get') + assert hasattr(self.runtime.cache, 'set') diff --git a/openedx/core/djangoapps/xblock/runtime/runtime.py b/openedx/core/djangoapps/xblock/runtime/runtime.py index 0b290ab2ab..119c49c1bd 100644 --- a/openedx/core/djangoapps/xblock/runtime/runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/runtime.py @@ -10,6 +10,7 @@ from completion.waffle import ENABLE_COMPLETION_TRACKING_SWITCH from completion.models import BlockCompletion from completion.services import CompletionService from django.contrib.auth import get_user_model +from django.core.cache import cache from django.core.exceptions import PermissionDenied from functools import lru_cache # lint-amnesty, pylint: disable=wrong-import-order from eventtracking import tracker @@ -19,6 +20,10 @@ from xblock.field_data import SplitFieldData from xblock.fields import Scope from xblock.runtime import KvsFieldData, MemoryIdManager, Runtime +from xmodule.errortracker import make_error_tracker +from xmodule.contentstore.django import contentstore +from xmodule.modulestore.django import ModuleI18nService +from xmodule.util.sandboxing import SandboxService from common.djangoapps.edxmako.services import MakoService from common.djangoapps.track import contexts as track_contexts from common.djangoapps.track import views as track_views @@ -30,10 +35,9 @@ from openedx.core.djangoapps.xblock.runtime.blockstore_field_data import Blockst from openedx.core.djangoapps.xblock.runtime.ephemeral_field_data import EphemeralKeyValueStore from openedx.core.djangoapps.xblock.runtime.mixin import LmsBlockMixin from openedx.core.djangoapps.xblock.utils import get_xblock_id_for_anonymous_user +from openedx.core.lib.cache_utils import CacheService from openedx.core.lib.xblock_utils import wrap_fragment, xblock_local_resource_url from common.djangoapps.static_replace import process_static_urls -from xmodule.errortracker import make_error_tracker # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.django import ModuleI18nService # lint-amnesty, pylint: disable=wrong-import-order from .id_managers import OpaqueKeyReader from .shims import RuntimeShim, XBlockShim @@ -240,6 +244,12 @@ class XBlockRuntime(RuntimeShim, Runtime): return MakoService() elif service_name == "i18n": return ModuleI18nService(block=block) + elif service_name == 'sandbox': + context_key = block.scope_ids.usage_id.context_key + return SandboxService(contentstore=contentstore, course_id=context_key) + elif service_name == 'cache': + return CacheService(cache) + # Check if the XBlockRuntimeSystem wants to handle this: service = self.system.get_service(block, service_name) # Otherwise, fall back to the base implementation which loads services diff --git a/openedx/core/lib/cache_utils.py b/openedx/core/lib/cache_utils.py index 387aaeaecc..1b3b65f378 100644 --- a/openedx/core/lib/cache_utils.py +++ b/openedx/core/lib/cache_utils.py @@ -223,3 +223,27 @@ def get_cache(name): """ assert name is not None return RequestCache(name).data + + +class CacheService: + """ + An XBlock service which provides a cache. + + Args: + cache(object): provides get/set functions for retrieving/storing key/value pairs. + """ + def __init__(self, cache, **kwargs): + super().__init__(**kwargs) + self._cache = cache + + def get(self, key, *args, **kwargs): + """ + Returns the value cached against the given key, or None. + """ + return self._cache.get(key, *args, **kwargs) + + def set(self, key, value, *args, **kwargs): + """ + Caches the value against the given key. + """ + return self._cache.set(key, value, *args, **kwargs) diff --git a/openedx/core/lib/tests/test_cache_utils.py b/openedx/core/lib/tests/test_cache_utils.py index 2d051db517..e417a72dac 100644 --- a/openedx/core/lib/tests/test_cache_utils.py +++ b/openedx/core/lib/tests/test_cache_utils.py @@ -1,14 +1,16 @@ """ Tests for cache_utils.py """ - +from time import sleep from unittest import TestCase from unittest.mock import Mock import ddt from edx_django_utils.cache import RequestCache +from django.core.cache import cache +from django.test.utils import override_settings -from openedx.core.lib.cache_utils import request_cached +from openedx.core.lib.cache_utils import CacheService, request_cached @ddt.ddt @@ -295,3 +297,26 @@ class TestRequestCachedDecorator(TestCase): result = wrapped(3) assert result == 2 assert to_be_wrapped.call_count == 2 + + +class CacheServiceTest(TestCase): + """ + Test CacheService methods. + """ + @override_settings(CACHES={ + 'default': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + } + }) + def test_cache(self): + ''' + Ensure the default cache works as expected. + ''' + cache_service = CacheService(cache) + key = 'my_key' + value = 'some random value' + timeout = 1 + cache_service.set(key, value, timeout=timeout) + assert cache_service.get(key) == value + sleep(timeout) + assert cache_service.get(key) is None