diff --git a/cms/envs/common.py b/cms/envs/common.py index 9b19779601..e382f6c1e8 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2093,7 +2093,10 @@ PROCTORING_SETTINGS = {} ################## BLOCKSTORE RELATED SETTINGS ######################### BLOCKSTORE_PUBLIC_URL_ROOT = 'http://localhost:18250' -BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1' +BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1/' +# Which of django's caches to use for storing anonymous user state for XBlocks +# in the blockstore-based XBlock runtime +XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE = 'default' ###################### LEARNER PORTAL ################################ LEARNER_PORTAL_URL_ROOT = 'https://learner-portal-localhost:18000' diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 73dab43d00..d0e719ed14 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -119,6 +119,20 @@ class ProblemBlock( shim_xmodule_js(fragment, 'Problem') return fragment + def public_view(self, context): + """ + Return the view seen by users who aren't logged in or who aren't + enrolled in the course. + """ + if getattr(self.runtime, 'suppports_state_for_anonymous_users', False): + # The new XBlock runtime can generally support capa problems for users who aren't logged in, so show the + # normal student_view. To prevent anonymous users from viewing specific problems, adjust course policies + # and/or content groups. + return self.student_view(context) + else: + # Show a message that this content requires users to login/enroll. + return super(ProblemBlock, self).public_view(context) + def author_view(self, context): """ Renders the Studio preview view. diff --git a/common/lib/xmodule/xmodule/unit_block.py b/common/lib/xmodule/xmodule/unit_block.py index 2635d91604..2516c7fe38 100644 --- a/common/lib/xmodule/xmodule/unit_block.py +++ b/common/lib/xmodule/xmodule/unit_block.py @@ -52,6 +52,8 @@ class UnitBlock(XBlock): result.add_content('') return result + public_view = student_view + def index_dictionary(self): """ Return dictionary prepared with module content and type for indexing, so diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index f2d70dc56a..91572c52c0 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -252,6 +252,9 @@ class VideoBlock( """ Returns a fragment that contains the html for the public view """ + if getattr(self.runtime, 'suppports_state_for_anonymous_users', False): + # The new runtime can support anonymous users as fully as regular users: + return self.student_view(context) return Fragment(self.get_html(view=PUBLIC_VIEW)) def get_html(self, view=STUDENT_VIEW): @@ -610,12 +613,8 @@ class VideoBlock( field_data = cls.parse_video_xml(node) for key, val in field_data.items(): setattr(video_block, key, cls.fields[key].from_json(val)) - # Update VAL with info extracted from `xml_object` - video_block.edx_video_id = video_block.import_video_info_into_val( - node, - runtime.resources_fs, - keys.usage_id.context_key, - ) + # Don't use VAL in the new runtime: + video_block.edx_video_id = None return video_block @classmethod diff --git a/lms/envs/common.py b/lms/envs/common.py index bffb3f977a..a399284486 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3834,7 +3834,10 @@ MAILCHIMP_NEW_USER_LIST_ID = "" ########################## BLOCKSTORE ##################################### BLOCKSTORE_PUBLIC_URL_ROOT = 'http://localhost:18250' -BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1' +BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1/' +# Which of django's caches to use for storing anonymous user state for XBlocks +# in the blockstore-based XBlock runtime +XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE = 'default' ########################## LEARNER PORTAL ############################## LEARNER_PORTAL_URL_ROOT = 'https://learner-portal-localhost:18000' diff --git a/lms/envs/test.py b/lms/envs/test.py index 69a0d9cb7f..322310a393 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -239,6 +239,7 @@ CACHES = { RUN_BLOCKSTORE_TESTS = os.environ.get('EDXAPP_RUN_BLOCKSTORE_TESTS', 'no').lower() in ('true', 'yes', '1') BLOCKSTORE_API_URL = os.environ.get('EDXAPP_BLOCKSTORE_API_URL', "http://edx.devstack.blockstore-test:18251/api/v1/") BLOCKSTORE_API_AUTH_TOKEN = os.environ.get('EDXAPP_BLOCKSTORE_API_AUTH_TOKEN', 'edxapp-test-key') +XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE = 'blockstore' # This must be set to a working cache for the tests to pass # Dummy secret key for dev SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' diff --git a/openedx/core/djangoapps/content_libraries/tests/test_runtime.py b/openedx/core/djangoapps/content_libraries/tests/test_runtime.py index ed71d49ca6..acde9be45f 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_runtime.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_runtime.py @@ -4,14 +4,12 @@ Test the Blockstore-based XBlock runtime and content libraries together. """ from __future__ import absolute_import, division, print_function, unicode_literals import json -import unittest from completion.test_utils import CompletionWaffleTestMixin -from django.test import TestCase +from django.test import TestCase, override_settings from organizations.models import Organization from rest_framework.test import APIClient -from xblock.core import XBlock, Scope -from xblock import fields +from xblock.core import XBlock from lms.djangoapps.courseware.model_data import get_score from openedx.core.djangoapps.content_libraries import api as library_api @@ -20,6 +18,7 @@ from openedx.core.djangoapps.content_libraries.tests.base import ( URL_BLOCK_RENDER_VIEW, URL_BLOCK_GET_HANDLER_URL, ) +from openedx.core.djangoapps.content_libraries.tests.user_state_block import UserStateTestBlock from openedx.core.djangoapps.xblock import api as xblock_api from openedx.core.djangolib.testing.utils import skip_unless_lms from openedx.core.lib import blockstore_api @@ -27,20 +26,6 @@ from student.tests.factories import UserFactory from xmodule.unit_block import UnitBlock -class UserStateTestBlock(XBlock): - """ - Block for testing variously scoped XBlock fields. - """ - BLOCK_TYPE = "user-state-test" - - display_name = fields.String(scope=Scope.content, name='User State Test Block') - # User-specific fields: - user_str = fields.String(scope=Scope.user_state, default='default value') # This usage, one user - uss_str = fields.String(scope=Scope.user_state_summary, default='default value') # This usage, all users - pref_str = fields.String(scope=Scope.preferences, default='default value') # Block type, one user - user_info_str = fields.String(scope=Scope.user_info, default='default value') # All blocks, one user - - class ContentLibraryContentTestMixin(object): """ Mixin for content library tests that creates two students and a library. @@ -69,6 +54,8 @@ class ContentLibraryContentTestMixin(object): @requires_blockstore +# EphemeralKeyValueStore requires a working cache, and the default test cache doesn't work: +@override_settings(XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE='blockstore') class ContentLibraryRuntimeTest(ContentLibraryContentTestMixin, TestCase): """ Basic tests of the Blockstore-based XBlock runtime using XBlocks in a @@ -168,13 +155,101 @@ class ContentLibraryXBlockUserStateTest(ContentLibraryContentTestMixin, TestCase self.assertEqual(block1_bob.pref_str, 'default value') self.assertEqual(block1_bob.user_info_str, 'default value') + @XBlock.register_temp_plugin(UserStateTestBlock, UserStateTestBlock.BLOCK_TYPE) + def test_state_for_anonymous_users(self): + """ + Test that anonymous users can interact with XBlocks and get/set their + state via handlers. + """ + # Create two XBlocks, block1 and block2 + block1_metadata = library_api.create_library_block(self.library.key, UserStateTestBlock.BLOCK_TYPE, "b3-1") + block1_usage_key = block1_metadata.usage_key + block2_metadata = library_api.create_library_block(self.library.key, UserStateTestBlock.BLOCK_TYPE, "b3-2") + block2_usage_key = block2_metadata.usage_key + library_api.publish_changes(self.library.key) + # Create two clients (anonymous user's browsers) + client1 = APIClient() + client2 = APIClient() + + def call_handler(client, block_key, handler_name, method, data=None): + """ Call an XBlock handler """ + url_result = client.get(URL_BLOCK_GET_HANDLER_URL.format(block_key=block_key, handler_name=handler_name)) + url = url_result.data["handler_url"] + data_json = json.dumps(data) if data else None + response = getattr(client, method)(url, data_json, content_type="application/json") + self.assertEqual(response.status_code, 200) + return response.json() + + # Now client1 sets all the fields via a handler: + call_handler(client1, block1_usage_key, "set_user_state", "post", { + "user_str": "1 was here", + "uss_str": "1 was here (USS)", + "pref_str": "1 was here (prefs)", + "user_info_str": "1 was here (user info)", + }) + + # Now load it back and expect the same data: + data = call_handler(client1, block1_usage_key, "get_user_state", "get") + self.assertEqual(data["user_str"], "1 was here") + self.assertEqual(data["uss_str"], "1 was here (USS)") + self.assertEqual(data["pref_str"], "1 was here (prefs)") + self.assertEqual(data["user_info_str"], "1 was here (user info)") + + # Now load a different XBlock and expect only pref_str and user_info_str to be set: + data = call_handler(client1, block2_usage_key, "get_user_state", "get") + self.assertEqual(data["user_str"], "default value") + self.assertEqual(data["uss_str"], "default value") + self.assertEqual(data["pref_str"], "1 was here (prefs)") + self.assertEqual(data["user_info_str"], "1 was here (user info)") + + # Now a different anonymous user loading the first block should see only the uss_str set: + data = call_handler(client2, block1_usage_key, "get_user_state", "get") + self.assertEqual(data["user_str"], "default value") + self.assertEqual(data["uss_str"], "1 was here (USS)") + self.assertEqual(data["pref_str"], "default value") + self.assertEqual(data["user_info_str"], "default value") + + # The "user state summary" should not be shared between registered and anonymous users: + client_registered = APIClient() + client_registered.login(username=self.student_a.username, password='edx') + data = call_handler(client_registered, block1_usage_key, "get_user_state", "get") + self.assertEqual(data["user_str"], "default value") + self.assertEqual(data["uss_str"], "default value") + self.assertEqual(data["pref_str"], "default value") + self.assertEqual(data["user_info_str"], "default value") + + def test_views_for_anonymous_users(self): + """ + Test that anonymous users can view XBlock's 'public_view' but not other + views + """ + # Create an XBlock + block_metadata = library_api.create_library_block(self.library.key, "html", "html1") + block_usage_key = block_metadata.usage_key + library_api.set_library_block_olx(block_usage_key, "Hello world") + library_api.publish_changes(self.library.key) + + anon_client = APIClient() + # View the public_view: + public_view_result = anon_client.get( + URL_BLOCK_RENDER_VIEW.format(block_key=block_usage_key, view_name='public_view'), + ) + self.assertEqual(public_view_result.status_code, 200) + self.assertIn("Hello world", public_view_result.data["content"]) + + # Try to view the student_view: + public_view_result = anon_client.get( + URL_BLOCK_RENDER_VIEW.format(block_key=block_usage_key, view_name='student_view'), + ) + self.assertEqual(public_view_result.status_code, 403) + @XBlock.register_temp_plugin(UserStateTestBlock, UserStateTestBlock.BLOCK_TYPE) def test_independent_instances(self): """ Test that independent instances of the same block don't share field data until .save() and re-loading, even when they're using the same runtime. """ - block_metadata = library_api.create_library_block(self.library.key, UserStateTestBlock.BLOCK_TYPE, "b3") + block_metadata = library_api.create_library_block(self.library.key, UserStateTestBlock.BLOCK_TYPE, "b4") block_usage_key = block_metadata.usage_key library_api.publish_changes(self.library.key) diff --git a/openedx/core/djangoapps/content_libraries/tests/user_state_block.py b/openedx/core/djangoapps/content_libraries/tests/user_state_block.py new file mode 100644 index 0000000000..2738f5854f --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/tests/user_state_block.py @@ -0,0 +1,52 @@ +# -*- coding: utf-8 -*- +""" +Block for testing variously scoped XBlock fields. +""" +from __future__ import absolute_import, division, print_function, unicode_literals +import json + +from webob import Response +from xblock.core import XBlock, Scope +from xblock import fields + + +class UserStateTestBlock(XBlock): + """ + Block for testing variously scoped XBlock fields. + """ + BLOCK_TYPE = "user-state-test" + has_score = False + + display_name = fields.String(scope=Scope.content, name='User State Test Block') + # User-specific fields: + user_str = fields.String(scope=Scope.user_state, default='default value') # This usage, one user + uss_str = fields.String(scope=Scope.user_state_summary, default='default value') # This usage, all users + pref_str = fields.String(scope=Scope.preferences, default='default value') # Block type, one user + user_info_str = fields.String(scope=Scope.user_info, default='default value') # All blocks, one user + + @XBlock.json_handler + def set_user_state(self, data, suffix): # pylint: disable=unused-argument + """ + Set the user-scoped fields + """ + self.user_str = data["user_str"] + self.uss_str = data["uss_str"] + self.pref_str = data["pref_str"] + self.user_info_str = data["user_info_str"] + return {} + + @XBlock.handler + def get_user_state(self, request, suffix=None): # pylint: disable=unused-argument + """ + Get the various user-scoped fields of this XBlock. + """ + return Response( + json.dumps({ + "user_str": self.user_str, + "uss_str": self.uss_str, + "pref_str": self.pref_str, + "user_info_str": self.user_info_str, + }), + content_type='application/json', + charset='UTF-8', + ) diff --git a/openedx/core/djangoapps/xblock/api.py b/openedx/core/djangoapps/xblock/api.py index 3f08999fc4..22b11f5184 100644 --- a/openedx/core/djangoapps/xblock/api.py +++ b/openedx/core/djangoapps/xblock/api.py @@ -24,7 +24,7 @@ from openedx.core.djangoapps.xblock.learning_context.manager import get_learning from openedx.core.djangoapps.xblock.runtime.blockstore_runtime import BlockstoreXBlockRuntime, xml_for_definition from openedx.core.djangoapps.xblock.runtime.runtime import XBlockRuntimeSystem from openedx.core.djangolib.blockstore_cache import BundleCache -from .utils import get_secure_token_for_xblock_handler +from .utils import get_secure_token_for_xblock_handler, get_xblock_id_for_anonymous_user log = logging.getLogger(__name__) @@ -160,11 +160,9 @@ def render_block_view(block, view_name, user): # pylint: disable=unused-argumen """ Get the HTML, JS, and CSS needed to render the given XBlock view. - The difference between this method and calling + The only difference between this method and calling load_block().render(view_name) - is that this method will automatically save any changes to field data that - resulted from rendering the view. If you don't want that, call .render() - directly. + is that this method can fall back from 'author_view' to 'student_view' Returns a Fragment. """ @@ -179,14 +177,10 @@ def render_block_view(block, view_name, user): # pylint: disable=unused-argumen else: raise - # TODO: save any changed user state fields - # TODO: if the view is anything other than student_view and we're not in the LMS, save any changed - # content/settings/children fields. - return fragment -def get_handler_url(usage_key, handler_name, user_id): +def get_handler_url(usage_key, handler_name, user): """ A method for getting the URL to any XBlock handler. The URL must be usable without any authentication (no cookie, no OAuth/JWT), and may expire. (So @@ -202,26 +196,30 @@ def get_handler_url(usage_key, handler_name, user_id): Params: usage_key - Usage Key (Opaque Key object or string) handler_name - Name of the handler or a dummy name like 'any_handler' - user_id - User ID or XBlockRuntimeSystem.ANONYMOUS_USER + user - Django User (registered or anonymous) This view does not check/care if the XBlock actually exists. """ usage_key_str = six.text_type(usage_key) site_root_url = get_xblock_app_config().get_site_root_url() - if user_id is None: + if not user: raise TypeError("Cannot get handler URLs without specifying a specific user ID.") - elif user_id == XBlockRuntimeSystem.ANONYMOUS_USER: - raise NotImplementedError("handler links for anonymous users are not yet implemented") # TODO: implement + elif user.is_authenticated: + user_id = user.id + elif user.is_anonymous: + user_id = get_xblock_id_for_anonymous_user(user) else: - # Normal case: generate a token-secured URL for this handler, specific - # to this user and this XBlock. - secure_token = get_secure_token_for_xblock_handler(user_id, usage_key_str) - path = reverse('xblock_api:xblock_handler', kwargs={ - 'usage_key_str': usage_key_str, - 'user_id': user_id, - 'secure_token': secure_token, - 'handler_name': handler_name, - }) + raise ValueError("Invalid user value") + # Now generate a token-secured URL for this handler, specific to this user + # and this XBlock: + secure_token = get_secure_token_for_xblock_handler(user_id, usage_key_str) + # Now generate the URL to that handler: + path = reverse('xblock_api:xblock_handler', kwargs={ + 'usage_key_str': usage_key_str, + 'user_id': user_id, + 'secure_token': secure_token, + 'handler_name': handler_name, + }) # We must return an absolute URL. We can't just use # rest_framework.reverse.reverse to get the absolute URL because this method # can be called by the XBlock from python as well and in that case we don't diff --git a/openedx/core/djangoapps/xblock/rest_api/urls.py b/openedx/core/djangoapps/xblock/rest_api/urls.py index efa95c0de4..92daffcfb6 100644 --- a/openedx/core/djangoapps/xblock/rest_api/urls.py +++ b/openedx/core/djangoapps/xblock/rest_api/urls.py @@ -20,7 +20,7 @@ urlpatterns = [ url(r'^handler_url/(?P[\w\-]+)/$', views.get_handler_url), # call one of this block's handlers url( - r'^handler/(?P\d+)-(?P\w+)/(?P[\w\-]+)/(?P.+)?$', + r'^handler/(?P\w+)-(?P\w+)/(?P[\w\-]+)/(?P.+)?$', views.xblock_handler, name='xblock_handler', ), diff --git a/openedx/core/djangoapps/xblock/rest_api/views.py b/openedx/core/djangoapps/xblock/rest_api/views.py index 055d070fe1..070656f264 100644 --- a/openedx/core/djangoapps/xblock/rest_api/views.py +++ b/openedx/core/djangoapps/xblock/rest_api/views.py @@ -42,7 +42,7 @@ def block_metadata(request, usage_key_str): @api_view(['GET']) -@view_auth_classes(is_authenticated=True) +@view_auth_classes(is_authenticated=False) @permission_classes((permissions.AllowAny, )) # Permissions are handled at a lower level, by the learning context def render_block_view(request, usage_key_str, view_name): """ @@ -57,7 +57,7 @@ def render_block_view(request, usage_key_str, view_name): @api_view(['GET']) -@view_auth_classes(is_authenticated=True) +@view_auth_classes(is_authenticated=False) def get_handler_url(request, usage_key_str, handler_name): """ Get an absolute URL which can be used (without any authentication) to call @@ -66,7 +66,7 @@ def get_handler_url(request, usage_key_str, handler_name): The URL will expire but is guaranteed to be valid for a minimum of 2 days. """ usage_key = UsageKey.from_string(usage_key_str) - handler_url = _get_handler_url(usage_key, handler_name, request.user.id) + handler_url = _get_handler_url(usage_key, handler_name, request.user) return Response({"handler_url": handler_url}) @@ -84,7 +84,6 @@ def xblock_handler(request, user_id, secure_token, usage_key_str, handler_name, auth token included in the URL (see below). As a result it can be exempt from CSRF, session auth, and JWT/OAuth. """ - user_id = int(user_id) # User ID comes from the URL, not session auth usage_key = UsageKey.from_string(usage_key_str) # To support sandboxed XBlocks, custom frontends, and other use cases, we @@ -94,13 +93,31 @@ def xblock_handler(request, user_id, secure_token, usage_key_str, handler_name, if not validate_secure_token_for_xblock_handler(user_id, usage_key_str, secure_token): raise PermissionDenied("Invalid/expired auth token.") if request.user.is_authenticated: - # The user authenticated twice, e.g. with session auth and the token - # So just make sure the session auth matches the token - if request.user.id != user_id: + # The user authenticated twice, e.g. with session auth and the token. + # This can happen if not running the XBlock in a sandboxed iframe. + # Just make sure the session auth matches the token: + if request.user.id != int(user_id): raise AuthenticationFailed("Authentication conflict.") user = request.user + elif user_id.isdigit(): + # This is a normal (integer) user ID for a registered user. + # This is the "normal" way this view gets used, with a sandboxed iframe. + user = User.objects.get(pk=int(user_id)) + elif user_id.startswith("anon"): + # This is a non-registered (anonymous) user: + assert request.user.is_anonymous + assert not hasattr(request.user, 'xblock_id_for_anonymous_user') + user = request.user # An AnonymousUser + # Since this particular view usually gets called from a sandboxed iframe + # we won't have access to the LMS session data for this user (the iframe + # has a new, empty session). So we need to save the identifier for this + # anonymous user (from the URL) on the user object, so that the runtime + # can get it (instead of generating a new one and saving it into this + # new empty session) + # See djangoapps.xblock.utils.get_xblock_id_for_anonymous_user() + user.xblock_id_for_anonymous_user = user_id else: - user = User.objects.get(pk=user_id) + raise AuthenticationFailed("Invalid user ID format.") request_webob = DjangoWebobRequest(request) # Convert from django request to the webob format that XBlocks expect block = load_block(usage_key, user) diff --git a/openedx/core/djangoapps/xblock/runtime/ephemeral_field_data.py b/openedx/core/djangoapps/xblock/runtime/ephemeral_field_data.py new file mode 100644 index 0000000000..1863fa357a --- /dev/null +++ b/openedx/core/djangoapps/xblock/runtime/ephemeral_field_data.py @@ -0,0 +1,62 @@ +""" +An :class:`~xblock.runtime.KeyValueStore` that stores data in the django cache + +This is used for low-priority ephemeral student state data: +* Anonymous users browsing and previewing content +* Studio authors testing out XBlocks + +We could also store this data in django sessions, but its a bit tricky to access +session data during any requests which don't have any cookies or other normal +authentication mechanisms (like XBlock handler calls from within XBlock