From a7fcf621bfacb80bae787999c3182b5feb5f2614 Mon Sep 17 00:00:00 2001
From: michaelroytman <mroytman@edx.org>
Date: Thu, 15 Dec 2022 07:40:18 -0500
Subject: [PATCH] feat: enable sharing username, email in LTI 1.3 launches and
support blocking PII transmission
This commit enables sharing username and email in LTI 1.3 basic launches.
This commit adds preferred_username and email as attributes of the Lti1p3LaunchData. The application or context that instantiates Lti1p3LaunchData is responsible for ensuring that username and email can be sent via an LTI 1.3 launch and supplying these data, if appropriate.
This commit sends username and email as part of an LTI 1.3 basic launch when the XBlock fields ask_to_send_username and ask_to_send_email are set to True, respectively.
Code was also added to block the transmission of username and email in both LTI 1.1 and LTI 1.3 launches if the value of the lti_access_to_learners_editable method of the LTI configuration service (i.e. the value of the CourseAllowPIISharingInLTIFlag ConfigurationModel) returns False, as originally intended and documented in the "Unified Flag for Enabling Sharing of PII in LTI
" decision record. However, the LTI configuration service is not currently available or defined in all runtime contexts, so this behavior only works when editing the XBlock in Studio (i.e. the studio_view). It does not work from the XBlock preview in Studio (i.e. the author_view) or from the LMS (i.e. the student_view).
The impact of this is that the ask_to_send_username and ask_to_send_email fields will be hidden in LTI XBlocks in courses for which an instance of the CourseAllowPIISharingInLTIFlag ConfigurationModel does not exist or for which an existing instance of the CourseAllowPIISharingInLTIFlag ConfigurationModel is disabled. If there already exists an instance of the CourseAllowPIISharingInLTIFlag ConfigurationModel for a course, then disabling the flag will only hide the ask_to_send_username and ask_to_send_email in the LTI XBlock edit menu. It will not prevent the transmission of username or email via the launch in Studio preview or via the launch in the LMS. If a course has already set ask_to_send_username or ask_to_send_email to True in the XBlock edit menu, that information will continue to be sent via the LTI 1.1 or LTI 1.3 launch.
We plan to fix this bug in the future.
---
lti_consumer/data.py | 4 +
lti_consumer/lti_1p3/consumer.py | 8 +-
lti_consumer/lti_1p3/tests/test_consumer.py | 12 +-
lti_consumer/lti_xblock.py | 63 ++++++--
lti_consumer/plugin/views.py | 2 +
lti_consumer/tests/test_utils.py | 16 ++
lti_consumer/tests/unit/test_lti_xblock.py | 154 +++++++++++++++++---
7 files changed, 222 insertions(+), 37 deletions(-)
diff --git a/lti_consumer/data.py b/lti_consumer/data.py
index b450115..5b10a3f 100644
--- a/lti_consumer/data.py
+++ b/lti_consumer/data.py
@@ -45,6 +45,8 @@ class Lti1p3LaunchData:
* config_id (required): The config_id field of an LtiConfiguration to use for the launch.
* resource_link_id (required): A unique identifier that is guaranteed to be unique for each placement of the LTI
link.
+ * preferred_username (optional): The user's username.
+ * email (optional): The user's email.
* external_user_id (optional): A unique identifier for the user that is requesting the LTI 1.3 launch that can be
shared externally. The identifier must be stable to the issuer. This value will be sent to the the Tool in the
form of both the login_hint in the login initiation request and the sub claim in the ID token of the LTI 1.3
@@ -73,6 +75,8 @@ class Lti1p3LaunchData:
user_role = field()
config_id = field()
resource_link_id = field()
+ preferred_username = field(default=None)
+ email = field(default=None)
external_user_id = field(default=None)
launch_presentation_document_target = field(default=None)
launch_presentation_return_url = field(default=None)
diff --git a/lti_consumer/lti_1p3/consumer.py b/lti_consumer/lti_1p3/consumer.py
index 6360227..00a31bf 100644
--- a/lti_consumer/lti_1p3/consumer.py
+++ b/lti_consumer/lti_1p3/consumer.py
@@ -132,7 +132,8 @@ class LtiConsumer1p3:
user_id,
role,
full_name=None,
- email_address=None
+ email_address=None,
+ preferred_username=None,
):
"""
Set user data/roles and convert to IMS Specification
@@ -162,6 +163,11 @@ class LtiConsumer1p3:
"email": email_address,
})
+ if preferred_username:
+ self.lti_claim_user_data.update({
+ "preferred_username": preferred_username,
+ })
+
def set_resource_link_claim(
self,
resource_link_id,
diff --git a/lti_consumer/lti_1p3/tests/test_consumer.py b/lti_consumer/lti_1p3/tests/test_consumer.py
index 9528e34..7ba2e68 100644
--- a/lti_consumer/lti_1p3/tests/test_consumer.py
+++ b/lti_consumer/lti_1p3/tests/test_consumer.py
@@ -210,12 +210,20 @@ class TestLti1p3Consumer(TestCase):
),
# User with extra data
(
- {"user_id": "1", "role": '', "full_name": "Jonh", "email_address": "jonh@example.com"},
+ {
+ "user_id": "1",
+ "role": '',
+ "full_name":
+ "Jonh",
+ "email_address":
+ "jonh@example.com",
+ "preferred_username": "johnuser"},
{
"sub": "1",
"https://purl.imsglobal.org/spec/lti/claim/roles": [],
"name": "Jonh",
- "email": "jonh@example.com"
+ "email": "jonh@example.com",
+ "preferred_username": "johnuser",
}
),
)
diff --git a/lti_consumer/lti_xblock.py b/lti_consumer/lti_xblock.py
index fcb53d3..f5b912c 100644
--- a/lti_consumer/lti_xblock.py
+++ b/lti_consumer/lti_xblock.py
@@ -673,6 +673,26 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
log.exception('Something went wrong in reading the LTI XBlock configuration.')
raise
+ def get_pii_sharing_enabled(self):
+ """
+ Returns whether PII can be transmitted via this XBlock. This controls both whether the PII sharing XBlock
+ fields ask_to_send_username and ask_to_send_email are displayed in Studio and whether these data are shared
+ in LTI launches, regardless of the values of the settings on the XBlock.
+ """
+ config_service = self.runtime.service(self, 'lti-configuration')
+ if config_service:
+ is_already_sharing_learner_info = self.ask_to_send_email or self.ask_to_send_username
+
+ return config_service.configuration.lti_access_to_learners_editable(
+ self.scope_ids.usage_id.context_key,
+ is_already_sharing_learner_info,
+ )
+
+ # TODO: The LTI configuration service is currently only available from the studio_view. This means that
+ # the CourseAllowPIISharingInLTIFlag does not control PII sharing in the author_view or student_view,
+ # because the service is not defined in those contexts.
+ return True
+
@property
def editable_fields(self):
"""
@@ -710,14 +730,9 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
# update the editable fields if this XBlock is configured to not to allow the
# editing of 'ask_to_send_username' and 'ask_to_send_email'.
- config_service = self.runtime.service(self, 'lti-configuration')
- if config_service:
- is_already_sharing_learner_info = self.ask_to_send_email or self.ask_to_send_username
- if not config_service.configuration.lti_access_to_learners_editable(
- self.scope_ids.usage_id.context_key,
- is_already_sharing_learner_info,
- ):
- noneditable_fields.extend(['ask_to_send_username', 'ask_to_send_email'])
+ pii_sharing_enabled = self.get_pii_sharing_enabled()
+ if not pii_sharing_enabled:
+ noneditable_fields.extend(['ask_to_send_username', 'ask_to_send_email'])
editable_fields = tuple(
field
@@ -1164,10 +1179,13 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
username = None
email = None
- if self.ask_to_send_username and real_user_data['user_username']:
- username = real_user_data['user_username']
- if self.ask_to_send_email and real_user_data['user_email']:
- email = real_user_data['user_email']
+ # Send PII fields only if this XBlock is configured to allow the sending PII.
+ pii_sharing_enabled = self.get_pii_sharing_enabled()
+ if pii_sharing_enabled:
+ if self.ask_to_send_username and real_user_data['user_username']:
+ username = real_user_data['user_username']
+ if self.ask_to_send_email and real_user_data['user_email']:
+ email = real_user_data['user_email']
lti_consumer.set_user_data(
user_id,
@@ -1478,12 +1496,26 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
location = self.scope_ids.usage_id
course_key = str(location.context_key)
+ username = None
+ email = None
+
+ pii_sharing_enabled = self.get_pii_sharing_enabled()
+ if pii_sharing_enabled:
+ user_data = self.extract_real_user_data()
+
+ if self.ask_to_send_username and user_data['user_username']:
+ username = user_data['user_username']
+ if self.ask_to_send_email and user_data['user_email']:
+ email = user_data['user_email']
+
launch_data = Lti1p3LaunchData(
user_id=self.lms_user_id,
user_role=self.role,
config_id=config_id,
resource_link_id=str(location),
external_user_id=self.external_user_id,
+ preferred_username=username,
+ email=email,
launch_presentation_document_target="iframe",
context_id=course_key,
context_type=["course_offering"],
@@ -1568,6 +1600,9 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
lti_block_launch_handler = self._get_lti_block_launch_handler()
lti_1p3_launch_url = self._get_lti_1p3_launch_url(lti_consumer)
+ # The values of ask_to_send_username and ask_to_send_email should only apply if PII sharing is enabled.
+ pii_sharing_enabled = self.get_pii_sharing_enabled()
+
return {
'launch_url': launch_url.strip(),
'lti_1p3_launch_url': lti_1p3_launch_url,
@@ -1582,8 +1617,8 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
'module_score': self.module_score,
'comment': sanitized_comment,
'description': self.description,
- 'ask_to_send_username': self.ask_to_send_username,
- 'ask_to_send_email': self.ask_to_send_email,
+ 'ask_to_send_username': self.ask_to_send_username if pii_sharing_enabled else False,
+ 'ask_to_send_email': self.ask_to_send_email if pii_sharing_enabled else False,
'button_text': self.button_text,
'inline_height': self.inline_height,
'modal_vertical_offset': self._get_modal_position_offset(self.modal_height),
diff --git a/lti_consumer/plugin/views.py b/lti_consumer/plugin/views.py
index 4e7428d..c6503b2 100644
--- a/lti_consumer/plugin/views.py
+++ b/lti_consumer/plugin/views.py
@@ -187,6 +187,8 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume
lti_consumer.set_user_data(
user_id=user_id,
role=user_role,
+ email_address=launch_data.email,
+ preferred_username=launch_data.preferred_username,
)
# Set resource_link claim.
diff --git a/lti_consumer/tests/test_utils.py b/lti_consumer/tests/test_utils.py
index f31c078..727e012 100644
--- a/lti_consumer/tests/test_utils.py
+++ b/lti_consumer/tests/test_utils.py
@@ -92,3 +92,19 @@ defaulting_processor.lti_xblock_default_params = {
'custom_name': 'Lex',
'custom_country': '',
}
+
+
+def get_mock_lti_configuration(editable):
+ """
+ Returns a mock object of lti-configuration service
+
+ Arguments:
+ editable (bool): This indicates whether the LTI fields (i.e. 'ask_to_send_username' and
+ 'ask_to_send_email') are editable.
+ """
+ lti_configuration = Mock()
+ lti_configuration.configuration = Mock()
+ lti_configuration.configuration.lti_access_to_learners_editable = Mock(
+ return_value=editable
+ )
+ return lti_configuration
diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py
index 2f8987d..eb4a8c7 100644
--- a/lti_consumer/tests/unit/test_lti_xblock.py
+++ b/lti_consumer/tests/unit/test_lti_xblock.py
@@ -23,7 +23,13 @@ from lti_consumer.data import Lti1p3LaunchData
from lti_consumer.lti_xblock import LtiConsumerXBlock, parse_handler_suffix, valid_config_type_values
from lti_consumer.lti_1p3.tests.utils import create_jwt
from lti_consumer.tests import test_utils
-from lti_consumer.tests.test_utils import FAKE_USER_ID, make_jwt_request, make_request, make_xblock
+from lti_consumer.tests.test_utils import (
+ FAKE_USER_ID,
+ get_mock_lti_configuration,
+ make_jwt_request,
+ make_request,
+ make_xblock,
+)
from lti_consumer.utils import resolve_custom_parameter_template
HTML_PROBLEM_PROGRESS = '<div class="problem-progress">'
@@ -460,21 +466,6 @@ class TestEditableFields(TestLtiConsumerXBlock):
self.mock_database_config_enabled_patcher.stop()
super().tearDown()
- def get_mock_lti_configuration(self, editable):
- """
- Returns a mock object of lti-configuration service
-
- Arguments:
- editable (bool): This indicates whether the LTI fields (i.e. 'ask_to_send_username' and
- 'ask_to_send_email') are editable.
- """
- lti_configuration = Mock()
- lti_configuration.configuration = Mock()
- lti_configuration.configuration.lti_access_to_learners_editable = Mock(
- return_value=editable
- )
- return lti_configuration
-
def are_fields_editable(self, fields):
"""
Returns whether the fields passed in as an argument, are editable.
@@ -499,7 +490,7 @@ class TestEditableFields(TestLtiConsumerXBlock):
are editable when this XBlock is configured to allow it.
"""
# this XBlock is configured to allow editing of LTI fields
- self.xblock.runtime.service.return_value = self.get_mock_lti_configuration(editable=True)
+ self.xblock.runtime.service.return_value = get_mock_lti_configuration(editable=True)
# Assert that 'ask_to_send_username' and 'ask_to_send_email' are editable.
self.assertTrue(self.are_fields_editable(fields=['ask_to_send_username', 'ask_to_send_email']))
@@ -509,7 +500,7 @@ class TestEditableFields(TestLtiConsumerXBlock):
are not editable when this XBlock is configured to not to allow it.
"""
# this XBlock is configured to not to allow editing of LTI fields
- self.xblock.runtime.service.return_value = self.get_mock_lti_configuration(editable=False)
+ self.xblock.runtime.service.return_value = get_mock_lti_configuration(editable=False)
# Assert that 'ask_to_send_username' and 'ask_to_send_email' are not editable.
self.assertFalse(self.are_fields_editable(fields=['ask_to_send_username', 'ask_to_send_email']))
@@ -818,11 +809,11 @@ class TestStudentView(TestLtiConsumerXBlock):
)
+@ddt.ddt
class TestLtiLaunchHandler(TestLtiConsumerXBlock):
"""
Unit tests for LtiConsumerXBlock.lti_launch_handler()
"""
-
def setUp(self):
super().setUp()
self.mock_lti_consumer = Mock(
@@ -923,6 +914,34 @@ class TestLtiLaunchHandler(TestLtiConsumerXBlock):
}
)
+ @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.anonymous_user_id', PropertyMock(return_value=FAKE_USER_ID))
+ @ddt.idata(product([True, False], [True, False], [True, False]))
+ @ddt.unpack
+ def test_lti_launch_pii_sharing(self, pii_sharing_enabled, ask_to_send_username, ask_to_send_email):
+ """
+ Test that the values of the LTI 1.1 PII fields person_sourcedid and person_contact_email_primary that are set
+ on the LTI consumer are actual values for those fields only when PII sharing is enabled. If PII sharing is not
+ enabled, then the values should be None.
+ """
+ self.xblock.get_pii_sharing_enabled = Mock(return_value=pii_sharing_enabled)
+
+ self.xblock.ask_to_send_username = ask_to_send_username
+ self.xblock.ask_to_send_email = ask_to_send_email
+
+ request = make_request('', 'GET')
+ self.xblock.lti_launch_handler(request)
+
+ set_user_data_kwargs = {
+ 'result_sourcedid': self.xblock.lis_result_sourcedid,
+ }
+
+ set_user_data_kwargs['person_sourcedid'] = 'fake' if pii_sharing_enabled and ask_to_send_username else None
+ set_user_data_kwargs['person_contact_email_primary'] = (
+ 'abc@example.com' if pii_sharing_enabled and ask_to_send_email else None
+ )
+
+ self.mock_lti_consumer.set_user_data.assert_called_with(FAKE_USER_ID, 'Student,Learner', **set_user_data_kwargs)
+
class TestOutcomeServiceHandler(TestLtiConsumerXBlock):
"""
@@ -1467,6 +1486,26 @@ class TestGetContext(TestLtiConsumerXBlock):
context = self.xblock._get_context_for_template() # pylint: disable=protected-access
self.assertEqual(context['lti_1p3_launch_url'], lti_1p3_launch_url)
+ @ddt.idata(product([True, False], [True, False], [True, False]))
+ @ddt.unpack
+ def test_context_pii_sharing(self, pii_sharing_enabled, ask_to_send_username, ask_to_send_email):
+ """
+ Test that the values for context keys ask_to_send_username and ask_to_send_email are the values of the
+ corresponding XBlock fields only when PII sharing is enabled. Otherwise, they should always be False.
+ """
+ self.xblock.get_pii_sharing_enabled = Mock(return_value=pii_sharing_enabled)
+ self.xblock.ask_to_send_username = ask_to_send_username
+ self.xblock.ask_to_send_email = ask_to_send_email
+
+ context = self.xblock._get_context_for_template() # pylint: disable=protected-access
+
+ if pii_sharing_enabled:
+ self.assertEqual(context['ask_to_send_username'], self.xblock.ask_to_send_username)
+ self.assertEqual(context['ask_to_send_email'], self.xblock.ask_to_send_email)
+ else:
+ self.assertEqual(context['ask_to_send_username'], False)
+ self.assertEqual(context['ask_to_send_email'], False)
+
@ddt.ddt
class TestProcessorSettings(TestLtiConsumerXBlock):
@@ -1533,6 +1572,7 @@ class TestGetModalPositionOffset(TestLtiConsumerXBlock):
self.assertEqual(offset, 10)
+@ddt.ddt
class TestLtiConsumer1p3XBlock(TestCase):
"""
Unit tests for LtiConsumerXBlock when using an LTI 1.3 tool.
@@ -1554,16 +1594,22 @@ class TestLtiConsumer1p3XBlock(TestCase):
self.addCleanup(self.mock_filter_enabled_patcher.stop)
self.addCleanup(self.mock_database_config_enabled_patcher.stop)
- def test_get_lti_1p3_launch_data(self):
+ @ddt.idata(product([True, False], [True, False], [True, False]))
+ @ddt.unpack
+ def test_get_lti_1p3_launch_data(self, pii_sharing_enabled, send_username, send_email):
"""
Test that get_lti_1p3_launch_data returns an instance of Lti1p3LaunchData with the correct data.
"""
# Mock out the user role and external_user_id properties.
fake_user = Mock()
+ fake_user_email = 'fake_email@example.com'
+ fake_username = 'fake_username'
+ fake_user.emails = [fake_user_email]
fake_user.opt_attrs = {
'edx-platform.user_id': 1,
'edx-platform.user_role': 'instructor',
'edx-platform.is_authenticated': True,
+ 'edx-platform.username': fake_username,
}
self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user)
self.xblock.runtime.service(self, 'user').get_external_user_id = Mock(return_value="external_user_id")
@@ -1571,9 +1617,34 @@ class TestLtiConsumer1p3XBlock(TestCase):
# Mock out get_context_title to avoid calling into the compatability layer.
self.xblock.get_context_title = Mock(return_value="context_title")
+ # Mock out get_pii_sharing_enabled to reduce the amount of mocking we have to do.
+ self.xblock.get_pii_sharing_enabled = Mock(return_value=pii_sharing_enabled)
+
launch_data = self.xblock.get_lti_1p3_launch_data()
course_key = str(self.xblock.scope_ids.usage_id.course_key)
+
+ expected_launch_data_kwargs = {
+ "user_id": 1,
+ "user_role": "instructor",
+ "config_id": config_id_for_block(self.xblock),
+ "resource_link_id": str(self.xblock.scope_ids.usage_id),
+ "external_user_id": "external_user_id",
+ "launch_presentation_document_target": "iframe",
+ "message_type": "LtiResourceLinkRequest",
+ "context_id": course_key,
+ "context_type": ["course_offering"],
+ "context_title": "context_title",
+ "context_label": course_key,
+ }
+
+ if pii_sharing_enabled:
+ if send_username:
+ expected_launch_data_kwargs["preferred_username"] = fake_username
+
+ if send_email:
+ expected_launch_data_kwargs["email"] = fake_user_email
+
expected_launch_data = Lti1p3LaunchData(
user_id=1,
user_role="instructor",
@@ -1928,3 +1999,46 @@ class TestSubmitStudioEditsHandler(TestLtiConsumerXBlock):
)
external_config_flag_patcher.start()
self.addCleanup(external_config_flag_patcher.stop)
+
+
+@ddt.ddt
+class TestGetPiiSharingEnabled(TestLtiConsumerXBlock):
+ """
+ Unit tests for LtiConsumerXBlock.get_pii_sharing_enabled.
+ """
+ def test_no_service(self):
+ self.assertTrue(self.xblock.get_pii_sharing_enabled())
+
+ @ddt.data(True, False)
+ def test_lti_access_to_learners_editable(self, lti_access_to_learners_editable):
+ """
+ Test that the get_pii_sharing_enabled method returns the value of calling the lti_access_to_learners_editable
+ method of the LTI configuration service, so long as as the configuration service is available and defined.
+ """
+ self.xblock.runtime.service.return_value = get_mock_lti_configuration(
+ editable=lti_access_to_learners_editable
+ )
+ self.assertEqual(self.xblock.get_pii_sharing_enabled(), lti_access_to_learners_editable)
+
+ @ddt.idata(product([True, False], [True, False]))
+ @ddt.unpack
+ def test_lti_access_to_learners_editable_args(self, ask_to_send_username, ask_to_send_email):
+ """
+ Test that the lti_access_to_learners_editable_mock method of the LTI configuration service is called with the
+ the correct arguments.
+ """
+ lti_configuration = Mock()
+ lti_configuration.configuration = Mock()
+ lti_access_to_learners_editable_mock = Mock()
+ lti_configuration.configuration.lti_access_to_learners_editable = lti_access_to_learners_editable_mock
+ self.xblock.runtime.service.return_value = lti_configuration
+
+ self.xblock.ask_to_send_username = ask_to_send_username
+ self.xblock.ask_to_send_email = ask_to_send_email
+
+ self.xblock.get_pii_sharing_enabled()
+
+ lti_access_to_learners_editable_mock.assert_called_once_with(
+ self.xblock.scope_ids.usage_id.context_key,
+ ask_to_send_username or ask_to_send_email,
+ )
--
GitLab