diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index bda54186ba6b8a0da1b13ea53c47475d5ab5bdd7..8f895f9c0768647aa0368870fe1faccccc338c57 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -16,6 +16,41 @@ Please See the [releases tab](https://github.com/openedx/xblock-lti-consumer/rel
Unreleased
~~~~~~~~~~
+7.2.0 - 2022-12-15
+------------------
+
+This release addresses a number of issues with and bugs in sharing personally identifiable information (PII) in LTI
+launches.
+
+* Replaces the PII sharing consent modal with an inline PII sharing consent dialog to better suit the three different
+ LTI launch types (i.e. ``inline``, ``modal``, and ``new_window``).
+* Adds a PII consent dialog for ``inline`` LTI launches.
+* Fixes a bug in the ``modal`` LTI launch in LTI 1.3 that was preventing the LTI launch.
+* Fixes a bug in evaluating and caching whether PII sharing is enabled via the ``CourseAllowPIISharingInLTIFlag``.
+
+ * This fixes a bug where the PII sharing fields in the LTI XBlock edit menu appeared regardless of the existence or
+ value of this flag. The PII sharing fields will now always be hidden if either no ``CourseAllowPIISharingInLTIFlag``
+ exists for a course or if a ``CourseAllowPIISharingInLTIFlag`` exists for the course but is not enabled.
+ * This fixes a bug in the backwards compatibility code in ``lti_access_to_learners_editable``. Now,
+ ``CourseAllowPIISharingInLTIFlag`` will always be created for courses that contain (an) LTI XBlock(s) that have (a)
+ PII sharing field(s) set to True when a user opens the LTI XBlock edit menu. Before, this would occur inconsistently
+ due to a bug in the caching code.
+
+* Enables sharing username and email in LTI 1.3 launches.
+
+ * Adds ``preferred_username`` and ``email`` attributes to the ``Lti1p3LaunchData`` class. 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.
+
+* Adds code to eventually support the value of ``CourseAllowPIISharingInLTIFlag`` controlling PII sharing for a given
+ course in LTI 1.1 and LTI 1.3 launches.
+
+ * This code does not currently work, because the LTI configuration service is not available or defined in all runtime
+ contexts. This code works in the LTI XBlock edit menu (i.e. the ``studio_view``), but it does not work in the Studio
+ preview context (i.e. the ``author_view``) or the LMS (i.e. the ``student_view``). The effect is that
+ the ``CourseAllowPIISharingInLTIFlag`` can only control the appearance of the username and email PII sharing fields
+ in the XBlock edit menu; it does not control PII sharing. We plan to fix this bug in the future.
+
7.1.0 - 2022-12-09
------------------
* Add support for platform setting `LTI_NRPS_DISALLOW_PII` to prevent sharing of pii over the names and roles
diff --git a/lti_consumer/__init__.py b/lti_consumer/__init__.py
index 564229846033e0656995329bc63a9570828be7e2..a2af6c5c15c6008438a52ac0fb5cb27ff166dc6e 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.1.0'
+__version__ = '7.2.0'
diff --git a/lti_consumer/data.py b/lti_consumer/data.py
index b450115db5d3becfaed4a1cb8bc41357ed5e8c43..5b10a3f02dbda9525154c48d2a69c1caf7806f3f 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 6360227de84f07c2f2340fcf784ef9cb9a606ef5..00a31bf1f81252740f728774ee6d643db89c301f 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 9528e344c2a927cf43d5924b98b205de25c526aa..7ba2e6865e0baa27d7cc32a7af6676516e71034e 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 39e9dcdf573b0fce4923074b0a5a8a278e514786..f5b912c13a50b2a013db701c081b92c3d2e130a1 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,14 +1617,15 @@ 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),
'modal_horizontal_offset': self._get_modal_position_offset(self.modal_width),
'modal_width': self.modal_width,
'accept_grades_past_due': self.accept_grades_past_due,
+ 'lti_version': self.lti_version,
}
def _get_modal_position_offset(self, viewport_percentage):
diff --git a/lti_consumer/models.py b/lti_consumer/models.py
index 95b6863215179ac53fc9178d73b6718df5c2ac21..4c6ebc42b4661f26a631a422608b3d3d539706fe 100644
--- a/lti_consumer/models.py
+++ b/lti_consumer/models.py
@@ -23,7 +23,6 @@ from lti_consumer.lti_1p1.consumer import LtiConsumer1p1
from lti_consumer.lti_1p3.consumer import LtiAdvantageConsumer, LtiProctoringConsumer
from lti_consumer.lti_1p3.key_handlers import PlatformKeyHandler
from lti_consumer.plugin import compat
-from lti_consumer.plugin.compat import request_cached
from lti_consumer.utils import (
get_lms_base,
get_lti_ags_lineitems_url,
@@ -778,7 +777,6 @@ class CourseAllowPIISharingInLTIFlag(ConfigurationModel):
course_id = CourseKeyField(max_length=255, db_index=True)
@classmethod
- @request_cached
def lti_access_to_learners_editable(cls, course_id: CourseKey, is_already_sharing_learner_info: bool) -> bool:
"""
Looks at the currently active configuration model to determine whether
diff --git a/lti_consumer/plugin/views.py b/lti_consumer/plugin/views.py
index 4e7428dadfaa9de72220253d2b41b86e5f408b6d..c6503b280fbdff6edd941821d3201e6ccf570e2c 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/static/js/xblock_lti_consumer.js b/lti_consumer/static/js/xblock_lti_consumer.js
index db835be4f4995022507c72b2ae97ce4e1be4b578..d32e00821cd66fc5172ec394f0354ae7b40c9a4a 100644
--- a/lti_consumer/static/js/xblock_lti_consumer.js
+++ b/lti_consumer/static/js/xblock_lti_consumer.js
@@ -8,51 +8,64 @@ function LtiConsumerXBlock(runtime, element) {
iframeModal: function (options) {
var $trigger = $(this);
var modal_id = $trigger.data("target");
- var defaults = {top: 100, overlay: 0.5, closeButton: null};
+ var defaults = { top: 100, overlay: 0.5, closeButton: null };
var overlay_id = (modal_id + '_lean-overlay').replace('#', '');
var overlay = $("<div id='" + overlay_id + "' class='lean-overlay'></div>");
$("body").append(overlay);
options = $.extend(defaults, options);
return this.each(function () {
var o = options;
- $(this).click(function (e) {
- var $modal = $(modal_id);
- // If we are already in an iframe, skip creation of the modal, since
- // it won't look good, anyway. Instead, we post a message to the parent
- // window, requesting creation of a modal there.
- // This is used by the courseware microfrontend.
- if (window !== window.parent) {
- window.parent.postMessage(
- {
- 'type': 'plugin.modal',
- 'payload': {
- 'url': window.location.origin + $modal.data('launch-url'),
- 'title': $modal.find('iframe').attr('title'),
- 'width': $modal.data('width')
- }
- },
- document.referrer
- );
- return;
+
+ var $modal = $(modal_id);
+ // If we are already in an iframe, skip creation of the modal, since
+ // it won't look good, anyway. Instead, we post a message to the parent
+ // window, requesting creation of a modal there.
+ // This is used by the courseware microfrontend.
+ if (window !== window.parent) {
+ // LTI 1.1 launch URLs are XBlock handler URLs that point to the lti_launch_handler method of
+ // the XBlock. When we get the handler URL from the runtime, it returns a relative URL without a
+ // protocol or hostname. However, in LTI 1.3, the launch URLs come from user input, including
+ // the values of fields on the XBlock or fields in the database. These URLs should be absolute
+ // URLs with a protocol and hostname, so we should not prepend a protocol and hostname to those
+ // URLs.
+ var launch_url = $modal.data('launch-url');
+
+ if (ltiVersion === 'lti_1p1') {
+ launch_url = window.location.origin + launch_url
}
- // Set iframe src attribute to launch LTI provider
- $modal.find('iframe').attr('src', $modal.data('launch-url'));
- $("#" + overlay_id).click(function () {
- close_modal(modal_id)
- });
- $(o.closeButton).click(function () {
- close_modal(modal_id)
- });
- var modal_height = $(modal_id).outerHeight();
- var modal_width = $(modal_id).outerWidth();
- $("#" + overlay_id).css({"display": "block", opacity: 0});
- $("#" + overlay_id).fadeTo(200, o.overlay);
- $(modal_id).css({
- "display": "block"
- });
- $(modal_id).fadeTo(200, 1);
- $(modal_id).attr('aria-hidden', false);
- $('body').css('overflow', 'hidden');
+
+ window.parent.postMessage(
+ {
+ 'type': 'plugin.modal',
+ 'payload': {
+ 'url': launch_url,
+ 'title': $modal.find('iframe').attr('title'),
+ 'width': $modal.data('width')
+ }
+ },
+ document.referrer
+ );
+ return;
+ }
+
+ // Set iframe src attribute to launch LTI provider.
+ $modal.find('iframe').attr('src', $modal.data('launch-url'));
+ $("#" + overlay_id).click(function () {
+ close_modal(modal_id)
+ });
+ $(o.closeButton).click(function () {
+ close_modal(modal_id)
+ });
+ var modal_height = $(modal_id).outerHeight();
+ var modal_width = $(modal_id).outerWidth();
+ $("#" + overlay_id).css({ "display": "block", opacity: 0 });
+ $("#" + overlay_id).fadeTo(200, o.overlay);
+ $(modal_id).css({
+ "display": "block"
+ });
+ $(modal_id).fadeTo(200, 1);
+ $(modal_id).attr('aria-hidden', false);
+ $('body').css('overflow', 'hidden');
e.preventDefault();
@@ -71,19 +84,19 @@ function LtiConsumerXBlock(runtime, element) {
}
});
- /* Redirect non-iframe tab to close button */
- var $inputs = $('select, input, textarea, button, a').filter(':visible').not(o.closeButton);
- $inputs.on('focus', function(e) {
- e.preventDefault();
- $(options.closeButton).focus();
- });
+ /* Redirect non-iframe tab to close button */
+ var $inputs = $('select, input, textarea, button, a').filter(':visible').not(o.closeButton);
+ $inputs.on('focus', function (e) {
+ e.preventDefault();
+ $(options.closeButton).focus();
});
+
});
function close_modal(modal_id) {
var $modal = $(modal_id);
$('select, input, textarea, button, a').off('focus');
$("#" + overlay_id).fadeOut(200);
- $modal.css({"display": "none"});
+ $modal.css({ "display": "none" });
$modal.attr('aria-hidden', true);
$modal.find('iframe').attr('src', '');
$('body').css('overflow', 'auto');
@@ -92,68 +105,99 @@ function LtiConsumerXBlock(runtime, element) {
}
});
+ function confirmDialog(message, triggerElement, showCancelButton) {
+ var def = $.Deferred();
+ // Hide the button that triggered the event, i.e. the launch button.
+ triggerElement.hide();
+
+ $('<div id="dialog-container"></div>').insertAfter(triggerElement) // TODO: this will need some cute styling. It looks like trash but it works.
+ .append('<p>' + message + '</p>')
+ if (showCancelButton) {
+ $('#dialog-container')
+ .append('<button style="margin-right:1rem" id="cancel-button">Cancel</button>');
+ }
+ $('#dialog-container').append('<button id="confirm-button">OK</button>');
+
+ // When a learner clicks "OK" or "Cancel" in the consent dialog, remove the consent dialog, show the launch
+ // button, and resolve the promise.
+ $('#confirm-button').click(function () {
+ // Show the button that triggered the event, i.e. the launch button.
+ triggerElement.show();
+ $("#dialog-container").remove()
+ $('body').append('<h1>Confirm Dialog Result: <i>Yes</i></h1>');
+ def.resolve("OK");
+ })
+ $('#cancel-button').click(function () {
+ // Hide the button that triggered the event, i.e. the launch button.
+ triggerElement.show()
+ $("#dialog-container").remove()
+ $('body').append('<h1>Confirm Dialog Result: <i>No</i></h1>');
+ def.resolve("Cancel");
+ })
+ return def.promise();
+ };
+
var $element = $(element);
var $ltiContainer = $element.find('.lti-consumer-container');
var askToSendUsername = $ltiContainer.data('ask-to-send-username') == 'True';
var askToSendEmail = $ltiContainer.data('ask-to-send-email') == 'True';
+ var ltiVersion = $ltiContainer.data('lti-version');
- // Apply click handler to modal launch button
- $element.find('.btn-lti-modal').iframeModal({top: 200, closeButton: '.close-modal'});
-
- // Apply click handler to new window launch button
- $element.find('.btn-lti-new-window').click(function(){
-
- // If this instance is configured to require username and/or email, ask user if it is okay to send them
- // Do not launch if it is not okay
- var destination = $(this).data('target')
-
- function confirmDialog(message) {
- var def = $.Deferred();
- $('<div></div>').appendTo('body') // TODO: this will need some cute styling. It looks like trash but it works.
- .html('<div><p>' + message + '</p></div>')
- .dialog({
- modal: true,
- title: 'Confirm',
- zIndex: 10000,
- autoOpen: true,
- width: 'auto',
- resizable: false,
- dialogClass: 'confirm-dialog',
- buttons: {
- OK: function() {
- $('body').append('<h1>Confirm Dialog Result: <i>Yes</i></h1>');
- def.resolve("OK");
- $(this).dialog("close");
- },
- Cancel: function() {
- $('body').append('<h1>Confirm Dialog Result: <i>No</i></h1>');
- def.resolve("Cancel");
- $(this).dialog("close");
- }
- },
- close: function(event, ui) {
- $(this).remove();
- }
- }).prev().css('background', 'white').css('color', '#000').css('border-color', 'transparent');
- return def.promise();
- };
-
- if(askToSendUsername && askToSendEmail) {
+ function renderPIIConsentPromptIfRequired(onSuccess, showCancelButton=true) {
+ if (askToSendUsername && askToSendEmail) {
msg = gettext("Click OK to have your username and e-mail address sent to a 3rd party application.\n\nClick Cancel to return to this page without sending your information.");
} else if (askToSendUsername) {
msg = gettext("Click OK to have your username sent to a 3rd party application.\n\nClick Cancel to return to this page without sending your information.");
} else if (askToSendEmail) {
msg = gettext("Click OK to have your e-mail address sent to a 3rd party application.\n\nClick Cancel to return to this page without sending your information.");
} else {
- window.open(destination);
+ onSuccess("OK");
+ return;
}
- $.when(confirmDialog(msg)).then(
- function(status) {
+ $.when(confirmDialog(msg, $(this), showCancelButton)).then(onSuccess);
+ }
+
+ // Render consent dialog for inline elements immediately.
+ var $ltiIframeContainerElement = $element.find('#lti-iframe-container');
+ $ltiIframeContainerElement.each(function () {
+ var ltiIframeTarget = $ltiIframeContainerElement.data('target')
+ renderPIIConsentPromptIfRequired.apply(this, [
+ function (status) {
+ if (status === 'OK') {
+ // After getting consent to share PII, set the src attribute of the iframe to start the launch.
+ $ltiIframeContainerElement.find('iframe').attr('src', ltiIframeTarget);
+ }
+ },
+ false
+ ]);
+ })
+
+ // Apply click handler to modal launch button.
+ var $ltiModalButton = $element.find('.btn-lti-modal');
+ $ltiModalButton.click(function () {
+ renderPIIConsentPromptIfRequired.apply(this, [
+ function (status) {
+ if (status === 'OK') {
+ $ltiModalButton.iframeModal({
+ top: 200, closeButton: '.close-modal'
+ })
+ }
+ }
+ ]);
+ });
+
+ // Apply click handler to new window launch button.
+ var $ltiNewWindowButton = $element.find('.btn-lti-new-window');
+ $ltiNewWindowButton.click(function () {
+ renderPIIConsentPromptIfRequired.apply(this, [
+ function (status) {
if (status == "OK") {
- window.open(destination);
+ window.open(
+ $ltiNewWindowButton.data('target')
+ );
}
}
- );
+ ]);
});
});
}
diff --git a/lti_consumer/templates/html/student.html b/lti_consumer/templates/html/student.html
index bd14c773c8c6e0d1441a2c51cf452d7a084a8187..d50a4e0e1f50f2cdd98cac5d4f9036d6d6e6ad29 100644
--- a/lti_consumer/templates/html/student.html
+++ b/lti_consumer/templates/html/student.html
@@ -18,6 +18,7 @@
class="${element_class} lti-consumer-container"
data-ask-to-send-username="${ask_to_send_username}"
data-ask-to-send-email="${ask_to_send_email}"
+ data-lti-version="${lti_version}"
>
% if (launch_url or lti_1p3_launch_url) and not hide_launch:
@@ -68,9 +69,11 @@
</section>
% endif
% if launch_target == 'iframe':
- <div style="height:${inline_height}px;">
+ <div id="lti-iframe-container" data-target="${form_url}" style="height:${inline_height}px;">
## The result of the LTI launch form submit will be rendered here.
- <%include file="templates/html/lti_iframe.html" args="initial_launch_url=form_url"/>
+ ## Don't pass in the initial_launch_url. Let the Javascript set the src, so we can get PII sharing consent
+ ## before the launch occurs, if needed.
+ <%include file="templates/html/lti_iframe.html" args="initial_launch_url=''"/>
</div>
% endif
% elif not hide_launch:
diff --git a/lti_consumer/tests/test_utils.py b/lti_consumer/tests/test_utils.py
index f31c0781799941ff87e14cf200c829239de9aa6b..727e012341ffa148bd3a352e9889e631337632e8 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 106828571c1ac82dd8056fbbb35a5c7afbfa4815..bfc9175abb0beeedb4938e63b5e253f836097c19 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):
"""
@@ -1391,7 +1410,7 @@ class TestGetContext(TestLtiConsumerXBlock):
'display_name', 'form_url', 'hide_launch', 'has_score', 'weight', 'module_score',
'comment', 'description', 'ask_to_send_username', 'ask_to_send_email', 'button_text',
'modal_vertical_offset', 'modal_horizontal_offset', 'modal_width',
- 'accept_grades_past_due'
+ 'accept_grades_past_due', 'lti_version',
)
# This test isn't testing the value of any of the above keys. Calling _get_lti_block_launch_handler raises an
@@ -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,38 +1594,62 @@ 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, ask_to_send_username, ask_to_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")
+ self.xblock.ask_to_send_username = ask_to_send_username
+ self.xblock.ask_to_send_email = ask_to_send_email
# 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 ask_to_send_username:
+ expected_launch_data_kwargs["preferred_username"] = fake_username
+
+ if ask_to_send_email:
+ expected_launch_data_kwargs["email"] = fake_user_email
+
expected_launch_data = Lti1p3LaunchData(
- 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,
+ **expected_launch_data_kwargs
)
self.assertEqual(
@@ -1928,3 +1992,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,
+ )