diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 7b70f257a648a3707325e0c802da8b78f900e291..b92ea47807970962c4006650bf9928e306a4958d 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -16,7 +16,16 @@ Please See the [releases tab](https://github.com/openedx/xblock-lti-consumer/rel
Unreleased
~~~~~~~~~~
-=======
+7.0.2 - 2022-11-29
+------------------
+* Fix the LTI 1.1 Outcome Results Service to be able to tie an outcome pass back to a user when the user ID is an
+ `external_user_id`.
+* Fix the LTI 2.0 Result Service to be able to tie a result pass back to a user when the user ID is an
+ `external_user_id`.
+* Update the `RESULT_SERVICE_SUFFIX_PARSER` regex string to be able to parse UUIDs to accommodate `external_user_ids`.
+* Add a `get_lti_1p1_user_from_user_id` method to the `LtiConsumerXBlock` to get the user object associated with a user
+ ID.
+
7.0.1 - 2022-11-29
------------------
diff --git a/lti_consumer/__init__.py b/lti_consumer/__init__.py
index a920083c27989e8694e367c12fc09e9867109f9e..73e477f21d3a93b92318e291987d0659fae3146e 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__ = '7.0.1'
+__version__ = '7.0.2'
diff --git a/lti_consumer/lti_xblock.py b/lti_consumer/lti_xblock.py
index fb3e1d96cd1ffb52dcf3bc2099ea8e13997faf45..39e9dcdf573b0fce4923074b0a5a8a278e514786 100644
--- a/lti_consumer/lti_xblock.py
+++ b/lti_consumer/lti_xblock.py
@@ -93,7 +93,7 @@ DOCS_ANCHOR_TAG_OPEN = (
"/projects/open-edx-building-and-running-a-course/en/latest/exercises_tools/lti_component.html"
"'>"
)
-RESULT_SERVICE_SUFFIX_PARSER = re.compile(r"^user/(?P<anon_id>\w+)", re.UNICODE)
+RESULT_SERVICE_SUFFIX_PARSER = re.compile(r"^user/(?P<anon_id>[\w-]+)", re.UNICODE)
LTI_1P1_ROLE_MAP = {
'student': 'Student,Learner',
'staff': 'Administrator',
@@ -835,7 +835,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
def get_lti_1p1_user_id(self):
"""
- Returns the user ID to send to an LTI tool during an LTI 1.1 launch. If the
+ Returns the user ID to send to an LTI tool during an LTI 1.1/2.0 launch. If the
enable_external_user_id_1p1_launches CourseWaffleFlag is enabled for the course, returns the external_user_id
defined by the external_user_ids Djangoapp. Otherwise, returns the anonymous_user_id.
@@ -849,6 +849,23 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
return self.anonymous_user_id
+ def get_lti_1p1_user_from_user_id(self, user_id):
+ """
+ Returns the user object associated with a user_id. This is used in LTI 1.1/2.0 integrations for calls to the
+ LTI 1.1 Basic Outcomes service and the LTI 2.0 Results service. Tool Providers may make calls to this library's
+ endpoints with a user identifier. This function returns a user object associated with that user identifier.
+
+ The user identifier may be a course-anonymized user ID (i.e. the anonymous_user_id) or the global, consistent
+ user ID (i.e. the external_user_id). This functions returns the correct User object.
+ """
+ if external_user_id_1p1_launches_enabled(self.scope_ids.usage_id.context_key):
+ try:
+ return compat.get_user_from_external_user_id(user_id)
+ except LtiError:
+ return None
+ else:
+ return self.runtime.service(self, 'user').get_user_by_anonymous_id(user_id)
+
@property
def resource_link_id(self):
"""
@@ -1293,7 +1310,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
except LtiError:
return Response(status=401) # Unauthorized in this case. 401 is right
- user = self.runtime.service(self, 'user').get_user_by_anonymous_id(anon_id)
+ user = self.get_lti_1p1_user_from_user_id(anon_id)
if not user: # that means we can't save to database, as we do not have real user id.
msg = _("[LTI]: Real user not found against anon_id: {}").format(anon_id)
log.info(msg)
diff --git a/lti_consumer/outcomes.py b/lti_consumer/outcomes.py
index b05ae0db0bfbb19be8e44557828919ae7de154b7..94112ec54bd6047bb7e5fe144cd3d83c7f6b9c40 100644
--- a/lti_consumer/outcomes.py
+++ b/lti_consumer/outcomes.py
@@ -183,8 +183,9 @@ class OutcomeService:
log.debug("[LTI]: %s", error_message)
return response_xml_template.format(**failure_values)
- anon_id = unquote(sourced_id.split(':')[-1])
- real_user = self.xblock.runtime.service(self, 'user').get_user_by_anonymous_id(anon_id)
+ user_id = unquote(sourced_id.split(':')[-1])
+ real_user = self.xblock.get_lti_1p1_user_from_user_id(user_id)
+
if not real_user: # that means we can't save to database, as we do not have real user id.
failure_values['imsx_messageIdentifier'] = escape(imsx_message_identifier)
failure_values['imsx_description'] = "User not found."
diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py
index 0b8631cd1ab3ef0a92ff381395a86d24cd9417ee..106828571c1ac82dd8056fbbb35a5c7afbfa4815 100644
--- a/lti_consumer/tests/unit/test_lti_xblock.py
+++ b/lti_consumer/tests/unit/test_lti_xblock.py
@@ -671,8 +671,8 @@ class TestExtractRealUserData(TestLtiConsumerXBlock):
@ddt.ddt
-class TestGetLti1p1UserId(TestLtiConsumerXBlock):
- """ Unit tests for the get_lti_1p1_user_id method"""
+class TestLti1p1UserId(TestLtiConsumerXBlock):
+ """ Unit tests for the get_lti_1p1_user_id and get_lti_1p1_user_from_user_id methods"""
def setUp(self):
super().setUp()
@@ -698,6 +698,40 @@ class TestGetLti1p1UserId(TestLtiConsumerXBlock):
external_user_id_1p1_launches_enabled.return_value = external_user_id_1p1_launches_enabled_value
self.assertEqual(self.xblock.get_lti_1p1_user_id(), expected_value)
+ @patch('lti_consumer.lti_xblock.compat')
+ @ddt.data(True, False)
+ def test_get_lti_1p1_user_from_user_id(
+ self,
+ external_user_id_1p1_launches_enabled,
+ compat_mock):
+
+ # Set the mock user objects for user objects associated with an anonymous_user_id and an external_user_id.
+ mock_anonymous_user = Mock()
+ mock_external_user = Mock()
+
+ self.xblock.runtime.service(self, 'user').get_user_by_anonymous_id = Mock(return_value=mock_anonymous_user)
+ compat_mock.get_user_from_external_user_id.return_value = mock_external_user
+
+ with patch('lti_consumer.lti_xblock.external_user_id_1p1_launches_enabled') as \
+ mock_external_user_id_1p1_launches_enabled:
+ mock_external_user_id_1p1_launches_enabled.return_value = external_user_id_1p1_launches_enabled
+
+ user = self.xblock.get_lti_1p1_user_from_user_id('user_id')
+
+ if external_user_id_1p1_launches_enabled:
+ self.assertEqual(user, mock_external_user)
+ else:
+ self.assertEqual(user, mock_anonymous_user)
+
+ @patch('lti_consumer.lti_xblock.external_user_id_1p1_launches_enabled')
+ @patch('lti_consumer.lti_xblock.compat')
+ def test_get_lti_1p1_user_from_user_id_lti_error(self, compat_mock, mock_external_user_id_1p1_launches_enabled):
+ mock_external_user_id_1p1_launches_enabled.return_value = True
+ compat_mock.get_user_from_external_user_id.side_effect = LtiError()
+
+ user = self.xblock.get_lti_1p1_user_from_user_id('user_id')
+ self.assertEqual(user, None)
+
class TestStudentView(TestLtiConsumerXBlock):
"""
@@ -917,11 +951,15 @@ class TestResultServiceHandler(TestLtiConsumerXBlock):
super().setUp()
self.lti_provider_key = 'test'
self.lti_provider_secret = 'secret'
- self.xblock.runtime.get_real_user = Mock()
self.xblock.accept_grades_past_due = True
self.mock_lti_consumer = Mock()
self.xblock._get_lti_consumer = Mock(return_value=self.mock_lti_consumer) # pylint: disable=protected-access
+ mock_user = Mock()
+ mock_id = PropertyMock(return_value=1)
+ type(mock_user).id = mock_id
+ self.xblock.get_lti_1p1_user_from_user_id = Mock(return_value=mock_user)
+
@override_settings(DEBUG=True)
@patch('lti_consumer.lti_xblock.log_authorization_header')
@patch('lti_consumer.lti_xblock.LtiConsumerXBlock.lti_provider_key_secret')
@@ -998,11 +1036,8 @@ class TestResultServiceHandler(TestLtiConsumerXBlock):
Test 404 response returned when a user cannot be found
"""
mock_parse_suffix.return_value = FAKE_USER_ID
- self.xblock.runtime.service = Mock(
- return_value=Mock(
- get_user_by_anonymous_id=Mock(return_value=None)
- )
- )
+ self.xblock.get_lti_1p1_user_from_user_id.return_value = None
+
response = self.xblock.result_service_handler(make_request('', 'GET'))
self.assertEqual(response.status_code, 404)
@@ -1327,6 +1362,15 @@ class TestParseSuffix(TestLtiConsumerXBlock):
parsed = parse_handler_suffix(f"user/{FAKE_USER_ID}")
self.assertEqual(parsed, FAKE_USER_ID)
+ def test_suffix_match_uuid(self):
+ """
+ Test `parse_handler_suffix` when `suffix` is a UUID. Note that we may send UUIDs as user IDs when the
+ lti_consumer.enable_external_user_id_1p1_launches CourseWaffleFlag is enabled, so we must be able to parse
+ UUID user IDs.
+ """
+ parsed = parse_handler_suffix("user/2e9ec4fa-e1cc-4591-9f19-cf1e94454c21")
+ self.assertEqual(parsed, "2e9ec4fa-e1cc-4591-9f19-cf1e94454c21")
+
@ddt.ddt
class TestGetContext(TestLtiConsumerXBlock):
diff --git a/lti_consumer/tests/unit/test_outcomes.py b/lti_consumer/tests/unit/test_outcomes.py
index dcc1e5de3462402cd9d3baeb493c00517007fa19..8008f9f7d2fe90dc137691ad1ae966d6d826b76e 100644
--- a/lti_consumer/tests/unit/test_outcomes.py
+++ b/lti_consumer/tests/unit/test_outcomes.py
@@ -5,13 +5,14 @@ Unit tests for lti_consumer.outcomes module
import textwrap
import unittest
from copy import copy
-
from unittest.mock import Mock, PropertyMock, patch
+import ddt
+
from lti_consumer.exceptions import LtiError
from lti_consumer.outcomes import OutcomeService, parse_grade_xml_body
-from lti_consumer.tests.unit.test_lti_xblock import TestLtiConsumerXBlock
from lti_consumer.tests.test_utils import make_request
+from lti_consumer.tests.unit.test_lti_xblock import TestLtiConsumerXBlock
REQUEST_BODY_TEMPLATE_VALID = textwrap.dedent("""
<?xml version="1.0" encoding="UTF-8"?>
@@ -326,6 +327,7 @@ class TestParseGradeXmlBody(unittest.TestCase):
self.assertEqual(action, 'ţéšţ_action')
+@ddt.ddt
class TestOutcomeService(TestLtiConsumerXBlock):
"""
Unit tests for OutcomeService
@@ -333,7 +335,17 @@ class TestOutcomeService(TestLtiConsumerXBlock):
def setUp(self):
super().setUp()
- self.outcome_servce = OutcomeService(self.xblock)
+ self.outcome_service = OutcomeService(self.xblock)
+
+ # Set up user mock for LtiConsumerXBlock.get_lti_1p1_user_from_user_id method.
+ self.mock_get_user_id_patcher = patch('lti_consumer.lti_xblock.LtiConsumerXBlock.get_lti_1p1_user_from_user_id')
+ self.addCleanup(self.mock_get_user_id_patcher.stop)
+ self.mock_get_user_id_patcher_enabled = self.mock_get_user_id_patcher.start()
+
+ mock_user = Mock()
+ mock_id = PropertyMock(return_value=1)
+ type(mock_user).id = mock_id
+ self.mock_get_user_id_patcher_enabled.return_value = mock_user
@patch('lti_consumer.outcomes.verify_oauth_body_signature', Mock(return_value=True))
@patch('lti_consumer.lti_xblock.LtiConsumerXBlock.lti_provider_key_secret', PropertyMock(return_value=('t', 's')))
@@ -343,6 +355,7 @@ class TestOutcomeService(TestLtiConsumerXBlock):
Test replace result request returns with success indicator
"""
request = make_request('')
+
values = {
'code': 'success',
'description': 'Score for is now 0.5',
@@ -351,7 +364,7 @@ class TestOutcomeService(TestLtiConsumerXBlock):
}
self.assertEqual(
- self.outcome_servce.handle_request(request).strip(),
+ self.outcome_service.handle_request(request).strip(),
RESPONSE_BODY_TEMPLATE.format(**values).strip()
)
@@ -362,7 +375,7 @@ class TestOutcomeService(TestLtiConsumerXBlock):
"""
request = make_request('')
self.xblock.accept_grades_past_due = False
- response = self.outcome_servce.handle_request(request)
+ response = self.outcome_service.handle_request(request)
self.assertIn('failure', response)
self.assertIn('Grade is past due', response)
@@ -376,7 +389,7 @@ class TestOutcomeService(TestLtiConsumerXBlock):
request = make_request('test_string')
mock_parse.side_effect = LtiError
- response = self.outcome_servce.handle_request(request)
+ response = self.outcome_service.handle_request(request)
self.assertNotIn('TypeError', response)
self.assertNotIn('a bytes-like object is required', response)
self.assertIn('Request body XML parsing error', response)
@@ -389,7 +402,7 @@ class TestOutcomeService(TestLtiConsumerXBlock):
request = make_request('')
mock_parse.side_effect = LtiError
- response = self.outcome_servce.handle_request(request)
+ response = self.outcome_service.handle_request(request)
self.assertIn('failure', response)
self.assertIn('Request body XML parsing error', response)
@@ -403,10 +416,10 @@ class TestOutcomeService(TestLtiConsumerXBlock):
request = make_request('')
mock_verify.side_effect = ValueError
- self.assertIn('failure', self.outcome_servce.handle_request(request))
+ self.assertIn('failure', self.outcome_service.handle_request(request))
mock_verify.side_effect = LtiError
- self.assertIn('failure', self.outcome_servce.handle_request(request))
+ self.assertIn('failure', self.outcome_service.handle_request(request))
@patch('lti_consumer.outcomes.verify_oauth_body_signature', Mock(return_value=True))
@patch('lti_consumer.lti_xblock.LtiConsumerXBlock.lti_provider_key_secret', PropertyMock(return_value=('t', 's')))
@@ -416,12 +429,9 @@ class TestOutcomeService(TestLtiConsumerXBlock):
Test user not found returns failure response
"""
request = make_request('')
- self.xblock.runtime.service = Mock(
- return_value=Mock(
- get_user_by_anonymous_id=Mock(return_value=None)
- )
- )
- response = self.outcome_servce.handle_request(request)
+
+ self.mock_get_user_id_patcher_enabled.return_value = None
+ response = self.outcome_service.handle_request(request)
self.assertIn('failure', response)
self.assertIn('User not found', response)
@@ -434,7 +444,7 @@ class TestOutcomeService(TestLtiConsumerXBlock):
Test unsupported action returns unsupported response
"""
request = make_request('')
- response = self.outcome_servce.handle_request(request)
+ response = self.outcome_service.handle_request(request)
self.assertIn('unsupported', response)
self.assertIn('Target does not support the requested operation.', response)