diff --git a/README.rst b/README.rst
index 95d0ee491b26c5fee8ed01ab8239609bd45794ac..781b9859afcd12f4fdac6cf3d9a5992f5c0d7c88 100644
--- a/README.rst
+++ b/README.rst
@@ -76,6 +76,75 @@ http://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/lat
tests for this repo running inside an LMS container). From here, you can see the contents of the
messages that we are sending as an LTI Consumer in the "Message Parameters" part of the "Message" tab.
+Custom LTI Parameters
+---------------------
+This XBlock sends a number of parameters to the provider including some optional parameters. To keep the XBlock
+somewhat minimal, some parameters were omitted like ``lis_person_name_full`` among others.
+At the same time the XBlock allows passing extra parameters to the LTI provider via parameter processor functions.
+
+Defining an LTI Parameter Processors
+====================================
+The parameter processor is a function that expects an XBlock instance, and returns a ``dict`` of
+additional parameters for the LTI.
+If a processor throws an exception, the exception is logged and suppressed.
+If a processor returns ``None`` or any falsy value, no parameters will be added.
+
+.. code:: python
+
+ def team_info(xblock):
+ course = get_team(xblock.user, lti_params.course.id)
+ if not course:
+ return
+
+ return {
+ 'custom_course_id': unicode(course.id),
+ 'custom_course_name': course.name,
+ }
+
+A processor can define a list of default parameters ``lti_xblock_default_params``,
+which is useful in case the processor had an exception.
+
+It is recommended to define default parameters anyway, because it can simplify the implementation of the processor
+function. Below is an example:
+
+.. code:: python
+
+ def dummy_processor(xblock):
+ course = get_team(xblock.user, lti_params.course.id) # If something went wrong default params will be used
+ if not course:
+ return # Will use the default params
+
+ return {
+ 'custom_course_id': unicode(course.id),
+ 'custom_course_name': course.name,
+ }
+
+ dummy_processor.lti_xblock_default_params = {
+ 'custom_course_id': '',
+ 'custom_course_name': '',
+ }
+
+If you're looking for a more realistic example, you can check the
+`Tahoe LTI <https://github.com/appsembler/tahoe-lti>`_ repository at the
+`Appsembler GitHub organization <https://github.com/appsembler/>`_.
+
+Configuring the Parameter Processors Settings
+=============================================
+
+Using the standard XBlock settings interface the developer can provide a list of processor functions:
+Those parameters are not sent by default. The course author can enable that on per XBlock instance
+(aka module) by setting the **Send extra parameters** to ``true`` in Studio.
+
+To configure parameter processors add the following snippet to your Ansible variable files:
+
+.. code:: yaml
+
+ EDXAPP_XBLOCK_SETTINGS:
+ lti_consumer:
+ parameter_processors:
+ - 'customer_package.lti_processors:team_and_cohort'
+ - 'example_package.lti_processors:extra_lti_params'
+
Workbench installation and settings
-----------------------------------
diff --git a/lti_consumer/lti.py b/lti_consumer/lti.py
index 367bb01282244fa73b97dcffc73246485d262080..ea0b591a109b211070db5a43e906bc4831941b04 100644
--- a/lti_consumer/lti.py
+++ b/lti_consumer/lti.py
@@ -174,6 +174,16 @@ class LtiConsumer(object):
# Appending custom parameter for signing.
lti_parameters.update(self.xblock.prefixed_custom_parameters)
+ for processor in self.xblock.get_parameter_processors():
+ try:
+ default_params = getattr(processor, 'lti_xblock_default_params', {})
+ lti_parameters.update(default_params)
+ lti_parameters.update(processor(self.xblock) or {})
+ except Exception: # pylint: disable=broad-except
+ # Log the error without causing a 500-error.
+ # Useful for catching casual runtime errors in the processors.
+ log.exception('Error in XBlock LTI parameter processor "%s"', processor)
+
headers = {
# This is needed for body encoding:
'Content-Type': 'application/x-www-form-urlencoded',
diff --git a/lti_consumer/lti_consumer.py b/lti_consumer/lti_consumer.py
index 7d2f9553e523f876c09224f7dd48d08494aad485..c7cec87198583045b130aae16fe0776182e37feb 100644
--- a/lti_consumer/lti_consumer.py
+++ b/lti_consumer/lti_consumer.py
@@ -53,6 +53,7 @@ What is supported:
import logging
import bleach
import re
+from importlib import import_module
import json
import urllib
@@ -164,6 +165,7 @@ class LaunchTarget(object):
@XBlock.needs('i18n')
+@XBlock.wants('settings')
@XBlock.wants('lti-configuration')
class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
"""
@@ -247,6 +249,8 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
Otherwise error message from LTI provider is generated.
"""
+ block_settings_key = 'lti_consumer'
+
display_name = String(
display_name=_("Display Name"),
help=_(
@@ -421,20 +425,85 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
default=False,
scope=Scope.settings
)
+ enable_processors = Boolean(
+ display_name=_("Send extra parameters"),
+ help=_("Select True to send the extra parameters, which might contain Personally Identifiable Information. "
+ "The processors are site-wide, please consult the site administrator if you have any questions."),
+ default=False,
+ scope=Scope.settings
+ )
# Possible editable fields
editable_field_names = (
'display_name', 'description', 'lti_id', 'launch_url', 'custom_parameters',
'launch_target', 'button_text', 'inline_height', 'modal_height', 'modal_width',
'has_score', 'weight', 'hide_launch', 'accept_grades_past_due', 'ask_to_send_username',
- 'ask_to_send_email'
+ 'ask_to_send_email', 'enable_processors',
)
+ @staticmethod
+ def workbench_scenarios():
+ """
+ Gather scenarios to be displayed in the workbench
+ """
+ scenarios = [
+ ('LTI Consumer XBlock',
+ '''<sequence_demo>
+ <lti_consumer
+ display_name="LTI Consumer - New Window"
+ lti_id="test"
+ description=""
+ ask_to_send_username="False"
+ ask_to_send_email="False"
+ enable_processors="True"
+ launch_target="new_window"
+ launch_url="https://lti.tools/saltire/tp" />
+
+ <lti_consumer
+ display_name="LTI Consumer - IFrame"
+ lti_id="test"
+ ask_to_send_username="False"
+ ask_to_send_email="False"
+ enable_processors="True"
+ description=""
+ launch_target="iframe"
+ launch_url="https://lti.tools/saltire/tp" />
+ </sequence_demo>
+ '''),
+ ]
+ return scenarios
+
def validate_field_data(self, validation, data):
if not isinstance(data.custom_parameters, list):
_ = self.runtime.service(self, "i18n").ugettext
validation.add(ValidationMessage(ValidationMessage.ERROR, unicode(_("Custom Parameters must be a list"))))
+ def get_settings(self):
+ """
+ Get the XBlock settings bucket via the SettingsService.
+ """
+ settings_service = self.runtime.service(self, 'settings')
+ if settings_service:
+ return settings_service.get_settings_bucket(self)
+
+ return {}
+
+ def get_parameter_processors(self):
+ """
+ Read the parameter processor functions from the settings and return their functions.
+ """
+ if not self.enable_processors:
+ return
+
+ try:
+ for path in self.get_settings().get('parameter_processors', []):
+ module_path, func_name = path.split(':', 1)
+ module = import_module(module_path)
+ yield getattr(module, func_name)
+ except Exception:
+ log.exception('Something went wrong in reading the LTI XBlock configuration.')
+ raise
+
@property
def editable_fields(self):
"""
diff --git a/lti_consumer/tests/unit/test_lti.py b/lti_consumer/tests/unit/test_lti.py
index 16eb4385e035d27f87d3dcd5d19d9c8bacdb2561..60d721af7e8a4c09290fe7bd7ba675cfba3e4d54 100644
--- a/lti_consumer/tests/unit/test_lti.py
+++ b/lti_consumer/tests/unit/test_lti.py
@@ -11,7 +11,7 @@ from six import text_type
from django.utils import timezone
-from lti_consumer.tests.unit.test_utils import FAKE_USER_ID, make_request
+from lti_consumer.tests.unit.test_utils import make_request, patch_signed_parameters
from lti_consumer.tests.unit.test_lti_consumer import TestLtiConsumerXBlock
from lti_consumer.lti import parse_result_json, LtiConsumer
@@ -129,30 +129,27 @@ class TestLtiConsumer(TestLtiConsumerXBlock):
super(TestLtiConsumer, self).setUp()
self.lti_consumer = LtiConsumer(self.xblock)
- @patch(
- 'lti_consumer.lti.get_oauth_request_signature',
- Mock(return_value=(
- 'OAuth oauth_nonce="fake_nonce", '
- 'oauth_timestamp="fake_timestamp", oauth_version="fake_version", oauth_signature_method="fake_method", '
- 'oauth_consumer_key="fake_consumer_key", oauth_signature="fake_signature"'
- ))
- )
- @patch(
- 'lti_consumer.lti_consumer.LtiConsumerXBlock.prefixed_custom_parameters',
- PropertyMock(return_value={u'custom_param_1': 'custom1', u'custom_param_2': 'custom2'})
- )
- @patch(
- 'lti_consumer.lti_consumer.LtiConsumerXBlock.lti_provider_key_secret',
- PropertyMock(return_value=('t', 's'))
- )
- @patch('lti_consumer.lti_consumer.LtiConsumerXBlock.user_id', PropertyMock(return_value=FAKE_USER_ID))
- def test_get_signed_lti_parameters(self):
+ def _update_xblock_for_signed_parameters(self):
"""
- Test `get_signed_lti_parameters` returns the correct dict
+ Prepare the LTI XBlock for signing the parameters.
"""
self.lti_consumer.xblock.due = timezone.now()
self.lti_consumer.xblock.graceperiod = timedelta(days=1)
+ self.lti_consumer.xblock.has_score = True
+ self.lti_consumer.xblock.ask_to_send_username = True
+ self.lti_consumer.xblock.ask_to_send_email = True
+ self.lti_consumer.xblock.runtime.get_real_user.return_value = Mock(
+ email='edx@example.com',
+ username='edx',
+ preferences=Mock(filter=Mock(return_value=[Mock(value='en')]))
+ )
+ @patch_signed_parameters
+ def test_get_signed_lti_parameters(self):
+ """
+ Test `get_signed_lti_parameters` returns the correct dict
+ """
+ self._update_xblock_for_signed_parameters()
expected_lti_parameters = {
text_type('user_id'): self.lti_consumer.xblock.user_id,
text_type('oauth_callback'): 'about:blank',
@@ -181,14 +178,6 @@ class TestLtiConsumer(TestLtiConsumerXBlock):
text_type('context_label'): self.lti_consumer.xblock.course.display_org_with_default,
text_type('context_title'): self.lti_consumer.xblock.course.display_name_with_default,
}
- self.lti_consumer.xblock.has_score = True
- self.lti_consumer.xblock.ask_to_send_username = True
- self.lti_consumer.xblock.ask_to_send_email = True
- self.lti_consumer.xblock.runtime.get_real_user.return_value = Mock(
- email='edx@example.com',
- username='edx',
- preferences=Mock(filter=Mock(return_value=[Mock(value='en')]))
- )
self.assertEqual(self.lti_consumer.get_signed_lti_parameters(), expected_lti_parameters)
# Test that `lis_person_sourcedid`, `lis_person_contact_email_primary`, and `launch_presentation_locale`
@@ -199,6 +188,53 @@ class TestLtiConsumer(TestLtiConsumerXBlock):
del expected_lti_parameters['launch_presentation_locale']
self.assertEqual(self.lti_consumer.get_signed_lti_parameters(), expected_lti_parameters)
+ @patch_signed_parameters
+ @patch('lti_consumer.lti.log')
+ def test_parameter_processors(self, mock_log):
+ self._update_xblock_for_signed_parameters()
+ self.xblock.enable_processors = True
+
+ with patch('lti_consumer.lti_consumer.LtiConsumerXBlock.get_settings', return_value={
+ 'parameter_processors': [
+ 'lti_consumer.tests.unit.test_utils:dummy_processor',
+ ],
+ }):
+ params = self.lti_consumer.get_signed_lti_parameters()
+ assert '' == params['custom_author_country']
+ assert 'author@example.com' == params['custom_author_email']
+ assert not mock_log.exception.called
+
+ @patch_signed_parameters
+ @patch('lti_consumer.lti.log')
+ def test_default_params(self, mock_log):
+ self._update_xblock_for_signed_parameters()
+ self.xblock.enable_processors = True
+
+ with patch('lti_consumer.lti_consumer.LtiConsumerXBlock.get_settings', return_value={
+ 'parameter_processors': [
+ 'lti_consumer.tests.unit.test_utils:defaulting_processor',
+ ],
+ }):
+ params = self.lti_consumer.get_signed_lti_parameters()
+ assert '' == params['custom_country']
+ assert 'Lex' == params['custom_name']
+ assert not mock_log.exception.called
+
+ @patch_signed_parameters
+ @patch('lti_consumer.lti.log')
+ def test_default_params_with_error(self, mock_log):
+ self._update_xblock_for_signed_parameters()
+ self.xblock.enable_processors = True
+
+ with patch('lti_consumer.lti_consumer.LtiConsumerXBlock.get_settings', return_value={
+ 'parameter_processors': [
+ 'lti_consumer.tests.unit.test_utils:faulty_processor',
+ ],
+ }):
+ params = self.lti_consumer.get_signed_lti_parameters()
+ assert 'Lex' == params['custom_name']
+ assert mock_log.exception.called
+
def test_get_result(self):
"""
Test `get_result` returns valid json response
@@ -259,7 +295,8 @@ class TestLtiConsumer(TestLtiConsumerXBlock):
with self.assertRaises(LtiError):
self.lti_consumer.verify_result_headers(request)
- self.assertTrue(mock_log.called)
+
+ assert mock_log.error.called
@patch('lti_consumer.lti.verify_oauth_body_signature', Mock(return_value=True))
@patch('lti_consumer.lti_consumer.LtiConsumerXBlock.lti_provider_key_secret', PropertyMock(return_value=('t', 's')))
@@ -297,7 +334,8 @@ class TestLtiConsumer(TestLtiConsumerXBlock):
with self.assertRaises(LtiError):
self.lti_consumer.verify_result_headers(request)
- self.assertTrue(mock_log.called)
+
+ assert mock_log.error.called
@patch('lti_consumer.lti.verify_oauth_body_signature', Mock(side_effect=ValueError))
@patch('lti_consumer.lti_consumer.LtiConsumerXBlock.lti_provider_key_secret', PropertyMock(return_value=('t', 's')))
@@ -311,4 +349,5 @@ class TestLtiConsumer(TestLtiConsumerXBlock):
with self.assertRaises(LtiError):
self.lti_consumer.verify_result_headers(request)
- self.assertTrue(mock_log.called)
+
+ assert mock_log.error.called
diff --git a/lti_consumer/tests/unit/test_lti_consumer.py b/lti_consumer/tests/unit/test_lti_consumer.py
index 141cfb0003d200692e89f91cb6d466a4507aedcb..0560e8c82d88eb071e70587d3b8e5809d35b93b0 100644
--- a/lti_consumer/tests/unit/test_lti_consumer.py
+++ b/lti_consumer/tests/unit/test_lti_consumer.py
@@ -2,17 +2,18 @@
Unit tests for LtiConsumerXBlock
"""
-import unittest
-
from datetime import timedelta
+import ddt
from mock import Mock, PropertyMock, patch
+from django.test.testcases import TestCase
from django.utils import timezone
from lti_consumer.tests.unit.test_utils import FAKE_USER_ID, make_xblock, make_request
from lti_consumer.lti_consumer import LtiConsumerXBlock, parse_handler_suffix
from lti_consumer.exceptions import LtiError
+from lti_consumer.tests.unit import test_utils
HTML_PROBLEM_PROGRESS = '<div class="problem-progress">'
@@ -22,7 +23,7 @@ HTML_LAUNCH_NEW_WINDOW_BUTTON = 'btn-lti-new-window'
HTML_IFRAME = '<iframe'
-class TestLtiConsumerXBlock(unittest.TestCase):
+class TestLtiConsumerXBlock(TestCase):
"""
Unit tests for LtiConsumerXBlock.max_score()
"""
@@ -46,6 +47,41 @@ class TestProperties(TestLtiConsumerXBlock):
"""
self.assertEqual(self.xblock.descriptor, self.xblock)
+ def test_workbench_scenarios(self):
+ """
+ Basic tests that `workbench_scenarios()` returns a well formed scenario.
+ """
+ scenarios = self.xblock.workbench_scenarios()
+ assert isinstance(scenarios, list)
+ assert len(scenarios) == 1, 'Keep it to a single scenario with multiple squences.'
+
+ scenario = scenarios[0]
+ assert scenario[0] == 'LTI Consumer XBlock'
+ assert '<lti_consumer' in scenario[1]
+
+ def test_settings(self):
+ """
+ Test that the XBlock is using the SettingsService correctly.
+ """
+ sample_settings_bucket = {
+ 'parameter_processors': [],
+ }
+
+ self.xblock.runtime.service = Mock(
+ return_value=Mock(
+ get_settings_bucket=Mock(return_value=sample_settings_bucket)
+ )
+ )
+
+ assert self.xblock.get_settings() == sample_settings_bucket
+
+ def test_settings_without_service(self):
+ """
+ Test that the XBlock can work without the SettingsService.
+ """
+ self.xblock.runtime.service = Mock(return_value=None)
+ assert self.xblock.get_settings() == {}
+
def test_context_id(self):
"""
Test `context_id` returns unicode course id
@@ -790,6 +826,59 @@ class TestGetContext(TestLtiConsumerXBlock):
self.assertIn(key, context)
+@ddt.ddt
+class TestProcessorSettings(TestLtiConsumerXBlock):
+ """
+ Unit tests for the adding custom LTI parameters.
+ """
+ def test_no_processors_by_default(self):
+ processors = list(self.xblock.get_parameter_processors())
+ assert not processors, 'The processor list should empty by default.'
+
+ def test_enable_processor(self):
+ self.xblock.enable_processors = True
+ with patch('lti_consumer.lti_consumer.LtiConsumerXBlock.get_settings', return_value={
+ 'parameter_processors': [
+ 'lti_consumer.tests.unit.test_utils:dummy_processor',
+ ],
+ }):
+ processors = list(self.xblock.get_parameter_processors())
+ assert len(processors) == 1, 'One processor should be enabled'
+ assert processors[0] == test_utils.dummy_processor, 'Should load the correct function'
+
+ def test_disabled_processors(self):
+ self.xblock.enable_processors = False
+ with patch('lti_consumer.lti_consumer.LtiConsumerXBlock.get_settings', return_value={
+ 'parameter_processors': [
+ 'lti_consumer.tests.unit.test_utils:dummy_processor',
+ ],
+ }):
+ processors = list(self.xblock.get_parameter_processors())
+ assert not processors, 'No processor should be enabled'
+
+ @ddt.data({
+ # Bad processor list
+ 'parameter_processors': False,
+ }, {
+ # Bad object path, no separator
+ 'parameter_processors': [
+ 'zzzzz',
+ ],
+ }, {
+ # Non-existent processor
+ 'parameter_processors': [
+ 'lti_consumer.tests.unit.test_utils:non_existent',
+ ],
+ })
+ @patch('lti_consumer.lti_consumer.log')
+ def test_faulty_configs(self, settings, mock_log):
+ self.xblock.enable_processors = True
+ with patch('lti_consumer.lti_consumer.LtiConsumerXBlock.get_settings', return_value=settings):
+ with self.assertRaises(Exception):
+ list(self.xblock.get_parameter_processors())
+ assert mock_log.exception.called
+
+
class TestGetModalPositionOffset(TestLtiConsumerXBlock):
"""
Unit tests for LtiConsumerXBlock._get_modal_position_offset()
diff --git a/lti_consumer/tests/unit/test_utils.py b/lti_consumer/tests/unit/test_utils.py
index b309edc6a83dde8341fcf1bf6d85c4f23c89f475..9c32789adb9beb3c5f5d6a0d10f7f0c47a90257e 100644
--- a/lti_consumer/tests/unit/test_utils.py
+++ b/lti_consumer/tests/unit/test_utils.py
@@ -3,7 +3,7 @@ Utility functions used within unit tests
"""
from webob import Request
-from mock import Mock
+from mock import patch, Mock, PropertyMock
from xblock.fields import ScopeIds
from xblock.runtime import KvsFieldData, DictKeyValueStore
@@ -54,3 +54,68 @@ def make_request(body, method='POST'):
request.body = body.encode('utf-8')
request.method = method
return request
+
+
+def patch_signed_parameters(func):
+ """
+ Prepare the patches for the get_signed_lti_parameters function for tests.
+ """
+ func = patch(
+ 'lti_consumer.lti.get_oauth_request_signature',
+ Mock(return_value=(
+ 'OAuth oauth_nonce="fake_nonce", '
+ 'oauth_timestamp="fake_timestamp", oauth_version="fake_version", oauth_signature_method="fake_method", '
+ 'oauth_consumer_key="fake_consumer_key", oauth_signature="fake_signature"'
+ ))
+ )(func)
+
+ func = patch(
+ 'lti_consumer.lti_consumer.LtiConsumerXBlock.prefixed_custom_parameters',
+ PropertyMock(return_value={u'custom_param_1': 'custom1', u'custom_param_2': 'custom2'})
+ )(func)
+
+ func = patch(
+ 'lti_consumer.lti_consumer.LtiConsumerXBlock.lti_provider_key_secret',
+ PropertyMock(return_value=('t', 's'))
+ )(func)
+
+ func = patch(
+ 'lti_consumer.lti_consumer.LtiConsumerXBlock.user_id', PropertyMock(return_value=FAKE_USER_ID)
+ )(func)
+
+ return func
+
+
+def dummy_processor(_xblock):
+ """
+ A dummy LTI parameter processor.
+ """
+ return {
+ 'custom_author_email': 'author@example.com',
+ 'custom_author_country': '',
+ }
+
+
+def defaulting_processor(_xblock):
+ """
+ A dummy LTI parameter processor with default params.
+ """
+ pass
+
+
+defaulting_processor.lti_xblock_default_params = {
+ 'custom_name': 'Lex',
+ 'custom_country': '',
+}
+
+
+def faulty_processor(_xblock):
+ """
+ A dummy LTI parameter processor with default params that throws an error.
+ """
+ raise Exception()
+
+
+faulty_processor.lti_xblock_default_params = {
+ 'custom_name': 'Lex',
+}
diff --git a/setup.py b/setup.py
index 4a8c5e57a7290b46d11c797be6540b16c0183f1b..8e4be4194af6c9b8f697dea547b4e86ab158d0af 100644
--- a/setup.py
+++ b/setup.py
@@ -22,7 +22,7 @@ def package_data(pkg, roots):
setup(
name='lti_consumer-xblock',
- version='1.1.8',
+ version='1.2.0',
description='This XBlock implements the consumer side of the LTI specification.',
packages=[
'lti_consumer',
diff --git a/test_requirements.txt b/test_requirements.txt
index a985a8b0eb3feedbc16abf31de9e77da49ba9189..c7bb14178c9b6c64c46d15d03d7fd1184377bea6 100644
--- a/test_requirements.txt
+++ b/test_requirements.txt
@@ -2,6 +2,7 @@
django-nose==1.4.4
astroid==1.3.8 # Pinning to avoid backwards incompatibility issue with pylint/pylint-django
+ddt
coveralls
mock
pep8