diff --git a/README.rst b/README.rst
index bd2fbf8181b9d65dbb59da724825891b1628cd6d..ee17c403c5e23bd2e149795649fdcfab9fbbae27 100644
--- a/README.rst
+++ b/README.rst
@@ -139,9 +139,11 @@ Instructions:
.. admonition:: Testing using ``ngrok``
- When launching LTI 1.3 requests through ``ngrok``, make sure you set ``DCS_SESSION_COOKIE_SAMESITE = 'None'`` in your
- ``devstack.py`` (located in /edx/app/edxapp/edx-platform/(lms|cms)/envs``) when doing LTI 1.3 launches in the
- devstack through ngrok. Do not forget to restart your services after updating the ``.py`` files.
+ When launching LTI 1.3 requests through ``ngrok``, make sure your LMS is serving session cookies marked as
+ ``Secure`` and with the ``SameSite`` attribute set to ``None``. You can do this by changing ``SESSION_COOKIE_SECURE: true``
+ and ``DCS_SESSION_COOKIE_SAMESITE: None`` in your ``lms.yml`` configuration files. Note that this will break logins
+ for locally accessed URLs in the devstack.
+
Custom LTI Parameters
=====================
@@ -367,6 +369,17 @@ Changelog
Please See the [releases tab](https://github.com/edx/xblock-lti-consumer/releases) for the complete changelog.
+3.4.6 - 2022-03-31
+------------------
+
+* Fix rendering of `lti_1p3_launch_error.html` and `lti_1p3_permission_error.html` templates
+
+3.4.5 - 2022-03-16
+------------------
+
+* Fix LTI Deep Linking return endpoint permission checking method by replacing the old one with the proper
+ Studio API call.
+
3.4.4 - 2022-03-03
------------------
diff --git a/lti_consumer/__init__.py b/lti_consumer/__init__.py
index ed92eb5d89f723f051797376ec443aaf8e65e40b..e78f3aeb05935b044c3e9b2e2b14df80c3b545aa 100644
--- a/lti_consumer/__init__.py
+++ b/lti_consumer/__init__.py
@@ -4,4 +4,4 @@ Runtime will load the XBlock class from here.
from .apps import LTIConsumerApp
from .lti_xblock import LtiConsumerXBlock
-__version__ = '3.4.4'
+__version__ = '3.4.6'
diff --git a/lti_consumer/lti_xblock.py b/lti_consumer/lti_xblock.py
index 21b3e939ef5bed1e3a2d465a9461d2fa5973f04a..336df6e7abaddbdc3775cce85aecec5afe460f6b 100644
--- a/lti_consumer/lti_xblock.py
+++ b/lti_consumer/lti_xblock.py
@@ -685,7 +685,8 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
"""
Get system user role and convert it to LTI role.
"""
- return ROLE_MAP.get(self.runtime.get_user_role(), 'Student')
+ role = self.runtime.service(self, 'user').get_current_user().opt_attrs.get('edx-platform.user_role', 'student')
+ return ROLE_MAP.get(role, 'Student')
@property
def course(self):
@@ -723,11 +724,15 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
def user_id(self):
"""
Returns the opaque anonymous_student_id for the current user.
+ This defaults to 'student' when testing in studio.
+ It will return the proper anonymous ID in the LMS.
"""
- user_id = self.runtime.anonymous_student_id
+ user_id = self.runtime.service(self, 'user').get_current_user().opt_attrs.get(
+ 'edx-platform.anonymous_user_id', None)
+
if user_id is None:
raise LtiError(self.ugettext("Could not get user id for current request"))
- return str(urllib.parse.quote(user_id))
+ return str(user_id)
def get_icon_class(self):
""" Returns the icon class """
@@ -912,22 +917,20 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
"""
Extract and return real user data from the runtime
"""
+ user = self.runtime.service(self, 'user').get_current_user()
user_data = {
'user_email': None,
'user_username': None,
'user_language': None,
}
- if callable(self.runtime.get_real_user):
- real_user_object = self.runtime.get_real_user(self.runtime.anonymous_student_id)
- user_data['user_email'] = getattr(real_user_object, "email", "")
- user_data['user_username'] = getattr(real_user_object, "username", "")
- user_preferences = getattr(real_user_object, "preferences", None)
+ try:
+ user_data['user_email'] = user.emails[0]
+ except IndexError:
+ user_data['user_email'] = None
- if user_preferences is not None:
- language_preference = user_preferences.filter(key='pref-lang')
- if len(language_preference) == 1:
- user_data['user_language'] = language_preference[0].value
+ user_data['user_username'] = user.opt_attrs.get("edx-platform.username", None)
+ user_data['user_language'] = user.opt_attrs.get("edx-platform.user_preferences", {}).get("pref-lang", None)
return user_data
@@ -1163,7 +1166,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
str(self.location), # pylint: disable=no-member
exc,
)
- template = loader.render_mako_template('/templates/html/lti_1p3_launch_error.html', context)
+ template = loader.render_django_template('/templates/html/lti_1p3_launch_error.html', context)
return Response(template, status=400, content_type='text/html')
except AssertionError as exc:
log.warning(
@@ -1172,7 +1175,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
self.external_user_id,
exc,
)
- template = loader.render_mako_template('/templates/html/lti_1p3_permission_error.html', context)
+ template = loader.render_django_template('/templates/html/lti_1p3_permission_error.html', context)
return Response(template, status=403, content_type='text/html')
@XBlock.handler
diff --git a/lti_consumer/plugin/compat.py b/lti_consumer/plugin/compat.py
index d64a5aeab8cfd479cc357c57ff7391a90dd323a9..e2acf520a25bc4c6550ee3c547d472d504f93cde 100644
--- a/lti_consumer/plugin/compat.py
+++ b/lti_consumer/plugin/compat.py
@@ -113,6 +113,18 @@ def user_has_access(*args, **kwargs):
return has_access(*args, **kwargs)
+def user_has_studio_write_access(*args, **kwargs):
+ """
+ Import and run `has_studio_write_access` from common modules.
+
+ Used to check if someone saving deep linking content has the
+ correct write permissions for a given.
+ """
+ # pylint: disable=import-error,import-outside-toplevel
+ from common.djangoapps.student.auth import has_studio_write_access
+ return has_studio_write_access(*args, **kwargs)
+
+
def get_course_by_id(course_key):
"""
Import and run `get_course_by_id` from LMS
diff --git a/lti_consumer/plugin/views.py b/lti_consumer/plugin/views.py
index 4cc4bd8777c23c3e204699a69c6ca7223219de2d..abe52d27ecf61b5783db58c1cec9767a22159c4c 100644
--- a/lti_consumer/plugin/views.py
+++ b/lti_consumer/plugin/views.py
@@ -63,13 +63,6 @@ from lti_consumer.utils import _
log = logging.getLogger(__name__)
-def user_has_staff_access(user, course_key):
- """
- Check if an user has write permissions to a given course.
- """
- return compat.user_has_access(user, "staff", course_key)
-
-
def has_block_access(user, block, course_key):
"""
Checks if a user has access to given xblock.
@@ -183,26 +176,31 @@ def access_token_endpoint(request, usage_id=None):
@require_http_methods(["POST"])
def deep_linking_response_endpoint(request, lti_config_id=None):
"""
- Deep Linking response endpoint where tool can send back
+ Deep Linking response endpoint where tool can send back Deep Linking
+ content selected by instructions in the tool's UI.
+
+ For this feature to work, the LMS session cookies need to be Secure
+ and have the `SameSite` attribute set to `None`, otherwise we won't
+ be able to check user permissions.
"""
try:
# Retrieve LTI configuration
lti_config = LtiConfiguration.objects.get(id=lti_config_id)
- # First, check if the user has sufficient permissions to
- # save LTI Deep Linking content through the student.auth API.
- course_key = lti_config.location.course_key
- if not user_has_staff_access(request.user, course_key):
- raise PermissionDenied()
-
# Get LTI consumer
lti_consumer = lti_config.get_lti_consumer()
- # Retrieve Deep Linking return message and validate parameters
+ # Validate Deep Linking return message and return decoded message
content_items = lti_consumer.check_and_decode_deep_linking_token(
request.POST.get("JWT")
)
+ # Check if the user has sufficient permissions to
+ # save LTI Deep Linking content through the student.auth API.
+ course_key = lti_config.location.course_key
+ if not compat.user_has_studio_write_access(request.user, course_key):
+ raise PermissionDenied()
+
# On a transaction, clear older DeepLinking selections, then
# verify and save each content item passed from the tool.
with transaction.atomic():
diff --git a/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py b/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py
index 58f77660bd02c159a1fb988b8138ed127217089d..77505f9a28c2fe20d171c97c75fc1839dd26c433 100644
--- a/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py
+++ b/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py
@@ -89,10 +89,17 @@ class LtiDeepLinkingResponseEndpointTestCase(LtiDeepLinkingTestCase):
super().setUp()
# Patch method that calls platform core to ask for user permissions
- studio_access_patcher = patch('lti_consumer.plugin.views.user_has_staff_access')
- self.addCleanup(studio_access_patcher.stop)
- self._mock_has_studio_write_acess = studio_access_patcher.start()
- self._mock_has_studio_write_acess.return_value = True
+ compat_mock = patch("lti_consumer.signals.compat")
+ self.addCleanup(compat_mock.stop)
+ self._compat_mock = compat_mock.start()
+ self._compat_mock.user_has_studio_write_access.return_value = True
+
+ has_studio_write_acess_patcher = patch(
+ 'lti_consumer.plugin.views.compat.user_has_studio_write_access',
+ return_value=True
+ )
+ self.addCleanup(has_studio_write_acess_patcher.stop)
+ self._mock_has_studio_write_acess = has_studio_write_acess_patcher.start()
# Deep Linking response endpoint
self.url = '/lti_consumer/v1/lti/{}/lti-dl/response'.format(self.lti_config.id)
diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py
index 9a3e6a0201cdaecedb12a4afdbe75c3ce6ef4f53..fbada0498baae8b66b83e515d4a65f84169aea41 100644
--- a/lti_consumer/tests/unit/test_lti_xblock.py
+++ b/lti_consumer/tests/unit/test_lti_xblock.py
@@ -5,7 +5,7 @@ Unit tests for LtiConsumerXBlock
import json
import logging
from datetime import timedelta
-from unittest.mock import Mock, NonCallableMock, PropertyMock, patch
+from unittest.mock import Mock, PropertyMock, patch
import ddt
from Cryptodome.PublicKey import RSA
@@ -132,16 +132,29 @@ class TestProperties(TestLtiConsumerXBlock):
"""
Test `role` returns the correct LTI role string
"""
- self.xblock.runtime.get_user_role.return_value = 'student'
+ fake_user = Mock()
+ fake_user.opt_attrs = {
+ 'edx-platform.user_role': 'student'
+ }
+ self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
self.assertEqual(self.xblock.role, 'Student')
- self.xblock.runtime.get_user_role.return_value = 'guest'
+ fake_user.opt_attrs = {
+ 'edx-platform.user_role': 'guest'
+ }
+ self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
self.assertEqual(self.xblock.role, 'Student')
- self.xblock.runtime.get_user_role.return_value = 'staff'
+ fake_user.opt_attrs = {
+ 'edx-platform.user_role': 'staff'
+ }
+ self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
self.assertEqual(self.xblock.role, 'Administrator')
- self.xblock.runtime.get_user_role.return_value = 'instructor'
+ fake_user.opt_attrs = {
+ 'edx-platform.user_role': 'instructor'
+ }
+ self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
self.assertEqual(self.xblock.role, 'Instructor')
def test_course(self):
@@ -218,21 +231,25 @@ class TestProperties(TestLtiConsumerXBlock):
"""
Test `user_id` returns the user_id string
"""
- self.xblock.runtime.anonymous_student_id = FAKE_USER_ID
- self.assertEqual(self.xblock.user_id, FAKE_USER_ID)
+ fake_user = Mock()
+ fake_user.opt_attrs = {
+ 'edx-platform.anonymous_user_id': FAKE_USER_ID
+ }
- def test_user_id_url_encoded(self):
- """
- Test `user_id` url encodes the user id
- """
- self.xblock.runtime.anonymous_student_id = 'user_id?&. '
- self.assertEqual(self.xblock.user_id, 'user_id%3F%26.%20')
+ self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
+ self.assertEqual(self.xblock.user_id, FAKE_USER_ID)
def test_user_id_none(self):
"""
Test `user_id` raises LtiError when the user id cannot be returned
"""
- self.xblock.runtime.anonymous_student_id = None
+ fake_user = Mock()
+ fake_user.opt_attrs = {
+ 'edx-platform.anonymous_user_id': None
+ }
+
+ self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
+
with self.assertRaises(LtiError):
__ = self.xblock.user_id
@@ -490,48 +507,47 @@ class TestExtractRealUserData(TestLtiConsumerXBlock):
Unit tests for LtiConsumerXBlock.extract_real_user_data()
"""
- def test_get_real_user_not_callable(self):
- """
- Test user_email, user_username, and user_language not available
- """
- self.xblock.runtime.get_real_user = NonCallableMock()
-
- real_user_data = self.xblock.extract_real_user_data()
- self.assertIsNone(real_user_data['user_email'])
- self.assertIsNone(real_user_data['user_username'])
- self.assertIsNone(real_user_data['user_language'])
-
def test_get_real_user_callable(self):
"""
Test user_email, and user_username available, but not user_language
+ See also documentation of new user service:
+ https://github.com/openedx/XBlock/blob/master/xblock/reference/user_service.py
"""
fake_user = Mock()
- fake_user.email = 'abc@example.com'
- fake_user.username = 'fake'
- fake_user.preferences = None
+ fake_user_email = 'abc@example.com'
+ fake_user.emails = [fake_user_email]
+ fake_username = 'fake'
+ fake_user.opt_attrs = {
+ "edx-platform.username": fake_username
+ }
- self.xblock.runtime.get_real_user = Mock(return_value=fake_user)
+ self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
real_user_data = self.xblock.extract_real_user_data()
- self.assertEqual(real_user_data['user_email'], fake_user.email)
- self.assertEqual(real_user_data['user_username'], fake_user.username)
+ self.assertEqual(real_user_data['user_email'], fake_user_email)
+ self.assertEqual(real_user_data['user_username'], fake_username)
self.assertIsNone(real_user_data['user_language'])
def test_get_real_user_callable_with_language_preference(self):
"""
Test user_language available
+ See also documentation of new user service:
+ https://github.com/openedx/XBlock/blob/master/xblock/reference/user_service.py
"""
fake_user = Mock()
- fake_user.email = 'abc@example.com'
- fake_user.username = 'fake'
- mock_language_pref = Mock()
- mock_language_pref.value = PropertyMock(return_value='en')
- fake_user.preferences.filter = Mock(return_value=[mock_language_pref])
+ fake_user.emails = ['abc@example.com']
+ fake_user.full_name = 'fake'
+ pref_language = "en"
+ fake_user.opt_attrs = {
+ "edx-platform.user_preferences": {
+ "pref-lang": "en"
+ }
+ }
- self.xblock.runtime.get_real_user = Mock(return_value=fake_user)
+ self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
real_user_data = self.xblock.extract_real_user_data()
- self.assertEqual(real_user_data['user_language'], mock_language_pref.value)
+ self.assertEqual(real_user_data['user_language'], pref_language)
class TestStudentView(TestLtiConsumerXBlock):
@@ -630,7 +646,16 @@ class TestLtiLaunchHandler(TestLtiConsumerXBlock):
self.xblock._get_lti_consumer = Mock(return_value=self.mock_lti_consumer) # pylint: disable=protected-access
self.xblock.due = timezone.now()
self.xblock.graceperiod = timedelta(days=1)
- self.xblock.runtime.get_real_user = Mock(return_value=None)
+
+ fake_user = Mock()
+ fake_user_email = 'abc@example.com'
+ fake_user.emails = [fake_user_email]
+ fake_username = 'fake'
+ fake_user.opt_attrs = {
+ "edx-platform.username": fake_username
+ }
+
+ self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
@patch('lti_consumer.lti_xblock.LtiConsumerXBlock.course')
@patch('lti_consumer.lti_xblock.LtiConsumerXBlock.user_id', PropertyMock(return_value=FAKE_USER_ID))
@@ -1268,6 +1293,7 @@ class TestLtiConsumer1p3XBlock(TestCase):
response_body = response.body.decode('utf-8')
self.assertIn("There was an error while launching the LTI 1.3 tool.", response_body)
+ self.assertNotIn("% trans", response_body)
def test_launch_callback_endpoint_when_using_lti_1p1(self):
"""
@@ -1372,6 +1398,9 @@ class TestLtiConsumer1p3XBlock(TestCase):
# Check response
self.assertEqual(response.status_code, 403)
+ response_body = response.body.decode('utf-8')
+ self.assertIn("Students don't have permissions to perform", response_body)
+ self.assertNotIn("% trans", response_body)
@patch('lti_consumer.api.get_deep_linking_data')
def test_callback_endpoint_dl_content_launch(self, mock_dl_data):