diff --git a/docs/reusable_configuration.rst b/docs/reusable_configuration.rst new file mode 100644 index 00000000..8177be49 --- /dev/null +++ b/docs/reusable_configuration.rst @@ -0,0 +1,80 @@ +########################## +LTI Reusable configuration +########################## + +Currently, this library supports a mechanism that allows LTI configuration +pluggability and re-usability, this allows instructors to be able to re-use +LTI configuration values across multiple XBlocks, reducing the work a +instructor needs to do to set up an LTI consumer XBlock. This feature works +for both LTI 1.1 and LTI 1.3. This feature, in the case of LTI 1.3 greatly +reduces the work an instructor needs to dedicate to setting up multiple +XBlocks that use the same configuration, since all values, including the access +token and keyset URL, are shared across all XBlocks using the same +configuration, eliminating the need to have a tool deployment for each XBlock. + +*********************** +How to use this feature +*********************** + +Setup Openedx LTI Store +======================= + +1. Install the openedx-ltistore plugin on the LMS and studio + (https://github.com/open-craft/openedx-ltistore): + +.. code-block:: bash + + make lms-shell + pip install -e /edx/src/openedx-ltistore + +2. Setup any existing openedx-filters configurations for both LMS and studio: + +.. code-block:: python + + OPEN_EDX_FILTERS_CONFIG = { + "org.openedx.xblock.lti_consumer.configuration.listed.v1": { + "fail_silently": False, + "pipeline": [ + "lti_store.pipelines.GetLtiConfigurations" + ] + } + } + +3. Restart the LMS & Studio for the latest config to take effect. + +Setup course waffle flag +======================== + +1. Go to LMS admin > WAFFLE_UTILS > Waffle flag course override + (http://localhost:18010/admin/waffle_utils/waffleflagcourseoverridemodel/). +2. Create a waffle flag course override with these values: + - Waffle flag: lti_consumer.enable_external_config_filter + - Course id: + - Override choice: Force On + - Enabled: True + +Create reusable LTI configuration +================================= + +1. Go to LMS admin > LTI_STORE > External lti configurations + (http://localhost:18010/admin/lti_store/externallticonfiguration/). +2. Create a new external LTI configuration. +3. On the list of external LTI configurations, note down the "Filter Key" value + of the newly created configuration (Example: lti_store:1). + +Setup LTI consumer XBlock on studio +=================================== + +1. Add "lti_consumer" to the "Advanced Module List" in + the "Advanced Settings" of your course. +2. Add a new unit to the course and add "LTI Consumer" + from the "Advanced" blocks. +3. Click the "Edit" button of the LTI Consumer block + and set "Configuration Type" to "Reusable Configuration". +4. Set the "External configuration ID" to the filter key value we copied + when we created the LTI configuration (Example: lti_store:1). +5. Click "Save". +6. (Optional) On an LTI 1.3 consumer XBlock, note down all the values + you need to set up on the LTI tool. +6. Go to your live course and execute a launch. +7. That launch should work as expected. diff --git a/lti_consumer/lti_xblock.py b/lti_consumer/lti_xblock.py index cc4f86cf..50ec65f7 100644 --- a/lti_consumer/lti_xblock.py +++ b/lti_consumer/lti_xblock.py @@ -82,6 +82,7 @@ external_config_filter_enabled, external_user_id_1p1_launches_enabled, database_config_enabled, + EXTERNAL_ID_REGEX, ) log = logging.getLogger(__name__) @@ -107,8 +108,6 @@ ) # Catch a value enclosed by ${}, the value enclosed can contain any charater except "=". CUSTOM_PARAMETER_TEMPLATE_REGEX = re.compile(r'^(\${[^%s]+})$' % CUSTOM_PARAMETER_SEPARATOR) -SLUG_CHARACTER_CLASS = '[-a-zA-Z0-9_]' -EXTERNAL_ID_REGEX = re.compile(rf'^({SLUG_CHARACTER_CLASS}+:{SLUG_CHARACTER_CLASS}+)$') def parse_handler_suffix(suffix): @@ -693,18 +692,14 @@ def validate_field_data(self, validation, data): _('Custom Parameters should be strings in "x=y" format.'), ))) - # Validate external config ID is not missing. - if data.config_type == 'external' and not data.external_config: + # Validate the external config ID. + if ( + data.config_type == 'external' and not + (data.external_config and EXTERNAL_ID_REGEX.match(str(data.external_config))) + ): _ = self.runtime.service(self, 'i18n').ugettext validation.add(ValidationMessage(ValidationMessage.ERROR, str( - _('Reusable configuration ID must be set when using external config.'), - ))) - - # Validate external config ID format. - if data.config_type == 'external' and not EXTERNAL_ID_REGEX.match(str(data.external_config)): - _ = self.runtime.service(self, 'i18n').ugettext - validation.add(ValidationMessage(ValidationMessage.ERROR, str( - _('Reusable configuration ID should be a string in "x:y" format.'), + _('Reusable configuration ID must be set when using external config (Example: "x:y").'), ))) # keyset URL and public key are mutually exclusive diff --git a/lti_consumer/models.py b/lti_consumer/models.py index 4d1b422f..3c7d87ea 100644 --- a/lti_consumer/models.py +++ b/lti_consumer/models.py @@ -15,6 +15,7 @@ from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField from opaque_keys.edx.keys import CourseKey from config_models.models import ConfigurationModel +from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ from lti_consumer.filters import get_external_config_from_filter @@ -31,6 +32,7 @@ get_lti_nrps_context_membership_url, choose_lti_1p3_redirect_uris, model_to_dict, + EXTERNAL_ID_REGEX, ) log = logging.getLogger(__name__) @@ -251,9 +253,11 @@ def clean(self): raise ValidationError({ "config_store": _("LTI Configuration stores on XBlock needs a block location set."), }) - if self.config_store == self.CONFIG_EXTERNAL and self.external_id is None: + if self.config_store == self.CONFIG_EXTERNAL and not EXTERNAL_ID_REGEX.match(str(self.external_id)): raise ValidationError({ - "config_store": _("LTI Configuration using reusable configuration needs a external ID set."), + "config_store": _( + 'LTI Configuration using reusable configuration needs a external ID in "x:y" format.', + ), }) if self.version == self.LTI_1P3 and self.config_store == self.CONFIG_ON_DB: if self.lti_1p3_tool_public_key == "" and self.lti_1p3_tool_keyset_url == "": @@ -368,6 +372,13 @@ def lti_1p3_public_jwk(self): self._generate_lti_1p3_keys_if_missing() return json.loads(self.lti_1p3_internal_public_jwk) + @cached_property + def external_config(self): + """ + Return the external resuable configuration. + """ + return get_external_config_from_filter({}, self.external_id) + def _get_lti_1p1_consumer(self): """ Return a class of LTI 1.1 consumer. @@ -378,10 +389,9 @@ def _get_lti_1p1_consumer(self): key, secret = block.lti_provider_key_secret launch_url = block.launch_url elif self.config_store == self.CONFIG_EXTERNAL: - config = get_external_config_from_filter({}, self.external_id) - key = config.get("lti_1p1_client_key") - secret = config.get("lti_1p1_client_secret") - launch_url = config.get("lti_1p1_launch_url") + key = self.external_config.get("lti_1p1_client_key") + secret = self.external_config.get("lti_1p1_client_secret") + launch_url = self.external_config.get("lti_1p1_launch_url") else: key = self.lti_1p1_client_key secret = self.lti_1p1_client_secret @@ -396,8 +406,7 @@ def get_lti_advantage_ags_mode(self): if self.config_store == self.CONFIG_ON_DB: return self.lti_advantage_ags_mode elif self.config_store == self.CONFIG_EXTERNAL: - config = get_external_config_from_filter({}, self.external_id) - return config.get('lti_advantage_ags_mode') + return self.external_config.get('lti_advantage_ags_mode') else: block = compat.load_enough_xblock(self.location) return block.lti_advantage_ags_mode @@ -409,8 +418,7 @@ def get_lti_advantage_deep_linking_enabled(self): if self.config_store == self.CONFIG_ON_DB: return self.lti_advantage_deep_linking_enabled elif self.config_store == self.CONFIG_EXTERNAL: - config = get_external_config_from_filter({}, self.external_id) - return config.get('lti_advantage_deep_linking_enabled') + return self.external_config.get('lti_advantage_deep_linking_enabled') else: block = compat.load_enough_xblock(self.location) return block.lti_advantage_deep_linking_enabled @@ -422,8 +430,7 @@ def get_lti_advantage_deep_linking_launch_url(self): if self.config_store == self.CONFIG_ON_DB: return self.lti_advantage_deep_linking_launch_url elif self.config_store == self.CONFIG_EXTERNAL: - config = get_external_config_from_filter({}, self.external_id) - return config.get('lti_advantage_deep_linking_launch_url') + return self.external_config.get('lti_advantage_deep_linking_launch_url') else: block = compat.load_enough_xblock(self.location) return block.lti_advantage_deep_linking_launch_url @@ -435,8 +442,7 @@ def get_lti_advantage_nrps_enabled(self): if self.config_store == self.CONFIG_ON_DB: return self.lti_advantage_enable_nrps elif self.config_store == self.CONFIG_EXTERNAL: - config = get_external_config_from_filter({}, self.external_id) - return config.get('lti_advantage_enable_nrps') + return self.external_config.get('lti_advantage_enable_nrps') else: block = compat.load_enough_xblock(self.location) return block.lti_1p3_enable_nrps @@ -578,22 +584,20 @@ def _get_lti_1p3_consumer(self): tool_keyset_url=self.lti_1p3_tool_keyset_url, ) elif self.config_store == self.CONFIG_EXTERNAL: - config = get_external_config_from_filter({}, self.external_id) - consumer = consumer_class( iss=get_lti_api_base(), - lti_oidc_url=config.get('lti_1p3_oidc_url'), - lti_launch_url=config.get('lti_1p3_launch_url'), - client_id=config.get('lti_1p3_client_id'), + lti_oidc_url=self.external_config.get('lti_1p3_oidc_url'), + lti_launch_url=self.external_config.get('lti_1p3_launch_url'), + client_id=self.external_config.get('lti_1p3_client_id'), # Deployment ID hardcoded to 1 since # we're not using multi-tenancy. deployment_id='1', - rsa_key=config.get('lti_1p3_private_key'), - rsa_key_id=config.get('lti_1p3_private_key_id'), + rsa_key=self.external_config.get('lti_1p3_private_key'), + rsa_key_id=self.external_config.get('lti_1p3_private_key_id'), # Registered redirect uris redirect_uris=self.get_lti_1p3_redirect_uris(), - tool_key=config.get('lti_1p3_tool_public_key'), - tool_keyset_url=config.get('lti_1p3_tool_keyset_url'), + tool_key=self.external_config.get('lti_1p3_tool_public_key'), + tool_keyset_url=self.external_config.get('lti_1p3_tool_keyset_url'), ) else: # This should not occur, but raise an error if self.config_store is not @@ -621,10 +625,9 @@ def get_lti_1p3_redirect_uris(self): Return pre-registered redirect uris or sensible defaults """ if self.config_store == self.CONFIG_EXTERNAL: - config = get_external_config_from_filter({}, self.external_id) - redirect_uris = config.get('lti_1p3_redirect_uris') - launch_url = config.get('lti_1p3_launch_url') - deep_link_launch_url = config.get('lti_advantage_deep_linking_launch_url') + redirect_uris = self.external_config.get('lti_1p3_redirect_uris') + launch_url = self.external_config.get('lti_1p3_launch_url') + deep_link_launch_url = self.external_config.get('lti_advantage_deep_linking_launch_url') elif self.config_store == self.CONFIG_ON_DB: redirect_uris = self.lti_1p3_redirect_uris launch_url = self.lti_1p3_launch_url diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py index 24cacbc0..c088b012 100644 --- a/lti_consumer/tests/unit/test_lti_xblock.py +++ b/lti_consumer/tests/unit/test_lti_xblock.py @@ -6,7 +6,7 @@ import string from datetime import timedelta from itertools import product -from unittest.mock import Mock, PropertyMock, patch, call +from unittest.mock import Mock, PropertyMock, patch import ddt from Cryptodome.PublicKey import RSA @@ -220,16 +220,12 @@ def test_validate_empty_external_config_with_external_config_type( self.xblock.validate() - add_mock.assert_has_calls([ - call(mock_validation_message( - 'error', - 'Reusable configuration ID must be set when using external config.', - )), - call(mock_validation_message( + add_mock.assert_called_once_with( + mock_validation_message( 'error', - 'Reusable configuration ID should be a string in "x:y" format.', - )), - ]) + 'Reusable configuration ID must be set when using external config (Example: "x:y").', + ), + ) @ddt.data('apptest', 'app:', ':test', 'app::test', f'{string.punctuation}:{string.punctuation}') @patch('lti_consumer.lti_xblock.ValidationMessage') @@ -250,7 +246,7 @@ def test_validate_invalid_external_config_with_external_config_type( add_mock.assert_called_once_with( mock_validation_message( 'error', - 'Reusable configuration ID should be a string in "x:y" format.', + 'Reusable configuration ID must be set when using external config (Example: "x:y").', ), ) diff --git a/lti_consumer/tests/unit/test_models.py b/lti_consumer/tests/unit/test_models.py index 4c004ac9..f95daf21 100644 --- a/lti_consumer/tests/unit/test_models.py +++ b/lti_consumer/tests/unit/test_models.py @@ -374,6 +374,11 @@ def test_clean(self): self.lti_1p3_config.config_store = self.lti_1p3_config.CONFIG_EXTERNAL self.lti_1p3_config.external_id = None + with self.assertRaises(ValidationError): + self.lti_1p3_config.clean() + + self.lti_1p3_config.external_id = 'invalid-external-id' + with self.assertRaises(ValidationError): self.lti_1p3_config.clean() diff --git a/lti_consumer/utils.py b/lti_consumer/utils.py index 192c2bc1..a6acbd4d 100644 --- a/lti_consumer/utils.py +++ b/lti_consumer/utils.py @@ -3,6 +3,7 @@ """ import copy import logging +import re from importlib import import_module from urllib.parse import urlencode @@ -19,6 +20,9 @@ log = logging.getLogger(__name__) +SLUG_CHARACTER_CLASS = '[-a-zA-Z0-9_]' +EXTERNAL_ID_REGEX = re.compile(rf'^({SLUG_CHARACTER_CLASS}+:{SLUG_CHARACTER_CLASS}+)$') + def _(text): """