Skip to content
Snippets Groups Projects
Commit d95de6e8 authored by michaelroytman's avatar michaelroytman
Browse files

fix: bug in rending buttons and message in PII sharing consent dialog

This commit fixes a bug in the PII sharing consent dialog.

The bug resulted in bizarre behavior when there were more than one LTI component in a unit. For example, if there were two LTI inline launches in a unit, two "OK" button would appear in a single component, instead of in their respective components. Another example is that clicking the "View resource in a [modal|new] window" buttons under two LTI components resulted in the "OK" and "Cancel" buttons as well as the PII sharing prompt appearing in a single component, instead of in their respective components.

This is because the dialog-container div that is dynamically created in the Javascript was not scoped to the LTI component, so there was a div with a id of "dialog-container" for each component configured to share PII. When dynamically inserting and removing buttons and the PII sharing prompt, the Javascript would simply find the first div with the dialog-container ID and operate on it, instead of the div appropriate to the component the user is interacting with.
parent 53823ea8
No related branches found
No related tags found
No related merge requests found
......@@ -16,6 +16,12 @@ Please See the [releases tab](https://github.com/openedx/xblock-lti-consumer/rel
Unreleased
~~~~~~~~~~
7.2.3 - 2023-01-24
------------------
* This release fixes a bug in the way that the PII sharing consent dialog renders. The bug resulted in the "OK" and
"Cancel" buttons as well as the text of the PII sharing consent prompt appearing inside an inappropriate component
when there was more than one LTI component in a unit.
7.2.2 - 2023-01-12
------------------
* Fixes LTI 1.3 grade injection vulnerability that allowed LTI integrations to modify scores for any block.
......
......@@ -4,4 +4,4 @@ Runtime will load the XBlock class from here.
from .apps import LTIConsumerApp
from .lti_xblock import LtiConsumerXBlock
__version__ = '7.2.2'
__version__ = '7.2.3'
......@@ -107,30 +107,39 @@ function LtiConsumerXBlock(runtime, element) {
function confirmDialog(message, triggerElement, showCancelButton) {
var def = $.Deferred();
// In order to scope the dialog container to the lti-consumer-container, grab the ID of the
// lti-consumer-container ancestor and append it to the ID of the dialog container.
var container_id = triggerElement.closest(".lti-consumer-container").attr("id");
var dialog_container_id = "dialog-container-" + container_id;
// 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.
$('<div id="' + dialog_container_id + '"></div>').insertAfter(triggerElement) // TODO: this will need some cute styling. It looks like trash but it works.
.append('<p>' + message + '</p>')
var $dialog_container = $("#" + dialog_container_id);
if (showCancelButton) {
$('#dialog-container')
$dialog_container
.append('<button style="margin-right:1rem" id="cancel-button">Cancel</button>');
}
$('#dialog-container').append('<button id="confirm-button">OK</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 () {
$dialog_container.find('#confirm-button').click(function () {
// Show the button that triggered the event, i.e. the launch button.
triggerElement.show();
$("#dialog-container").remove()
$dialog_container.remove()
$('body').append('<h1>Confirm Dialog Result: <i>Yes</i></h1>');
def.resolve("OK");
})
$('#cancel-button').click(function () {
$dialog_container.find('#cancel-button').click(function () {
// Hide the button that triggered the event, i.e. the launch button.
triggerElement.show()
$("#dialog-container").remove()
$dialog_container.remove()
$('body').append('<h1>Confirm Dialog Result: <i>No</i></h1>');
def.resolve("Cancel");
})
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment