Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: LTI 1.3 reusable configuration #390

80 changes: 80 additions & 0 deletions docs/reusable_configuration.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
##########################
LTI Reusable configuration
##########################
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved

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.
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved

***********************
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.
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved

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: <your 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).
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved

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".
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved
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.
10 changes: 9 additions & 1 deletion lti_consumer/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,18 @@ def get_lti_1p3_launch_info(
deep_linking_content_items = [item.attributes for item in dl_content_items]

config_id = lti_config.config_id
client_id = lti_config.lti_1p3_client_id

# Display LTI launch information from external configuration.
# if an external configuration is being used.
if lti_config.config_store == lti_config.CONFIG_EXTERNAL:
external_config = get_external_config_from_filter({}, lti_config.external_id)
config_id = lti_config.external_id.replace(':', '/')
client_id = external_config.get('lti_1p3_client_id')

# Return LTI launch information for end user configuration
return {
'client_id': lti_config.lti_1p3_client_id,
'client_id': client_id,
'keyset_url': get_lms_lti_keyset_link(config_id),
'deployment_id': '1',
'oidc_callback': get_lms_lti_launch_link(),
Expand Down
7 changes: 7 additions & 0 deletions lti_consumer/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,10 @@ class LtiError(Exception):
"""
General error class for LTI Consumer usage.
"""


class ExternalConfigurationNotFound(Exception):
"""
This exception is used when a reusable external configuration
is not found for a given external ID.
"""
22 changes: 14 additions & 8 deletions lti_consumer/lti_xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
external_config_filter_enabled,
external_user_id_1p1_launches_enabled,
database_config_enabled,
EXTERNAL_ID_REGEX,
)

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -691,6 +692,16 @@ def validate_field_data(self, validation, data):
_('Custom Parameters should be strings in "x=y" format.'),
)))

# 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 (Example: "x:y").'),
)))
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved

# keyset URL and public key are mutually exclusive
if data.lti_1p3_tool_key_mode == 'keyset_url':
data.lti_1p3_tool_public_key = ''
Expand Down Expand Up @@ -1664,10 +1675,7 @@ def _get_lti_block_launch_handler(self):
"""
Return the LTI block launch handler.
"""
# The "external" config_type is not supported for LTI 1.3, only LTI 1.1. Therefore, ensure that we define
# the lti_block_launch_handler using LTI 1.1 logic for "external" config_types.
# TODO: This needs to change when the LTI 1.3 support is added to the external config_type in the future.
if self.lti_version == 'lti_1p1' or self.config_type == "external":
if self.lti_version == 'lti_1p1':
lti_block_launch_handler = self.runtime.handler_url(self, 'lti_launch_handler').rstrip('/?')
else:
launch_data = self.get_lti_1p3_launch_data()
Expand All @@ -1687,10 +1695,8 @@ def _get_lti_1p3_launch_url(self, consumer):
"""
lti_1p3_launch_url = self.lti_1p3_launch_url.strip()

# The "external" config_type is not supported for LTI 1.3, only LTI 1.1. Therefore, ensure that we define
# the lti_1p3_launch_url using the LTI 1.3 consumer only for config_types that support LTI 1.3.
# TODO: This needs to change when the LTI 1.3 support is added to the external config_type in the future.
if consumer and self.lti_version == "lti_1p3" and self.config_type == "database":
# Get LTI launch URL from consumer if using database or external configuration type.
if consumer and self.lti_version == 'lti_1p3' and self.config_type in ('database', 'external'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something odd happens when I try to use the external configuration directly from the XBlock. When I set the Configuration Type to Reusable Configuration and provide a valid LTI Reusable Configuration ID, the XBlock break with Error: (1048, "Column 'version' cannot be null"). The relevant part of the traceback is:

File "/edx/shared-src/xblock-lti-consumer/lti_consumer/lti_xblock.py", line 1177, in author_view
  return self.student_view(context)
File "/edx/shared-src/xblock-lti-consumer/lti_consumer/lti_xblock.py", line 1227, in student_view
  context.update(self._get_context_for_template())
File "/edx/shared-src/xblock-lti-consumer/lti_consumer/lti_xblock.py", line 1730, in _get_context_for_template
  lti_consumer = self._get_lti_consumer()
File "/edx/shared-src/xblock-lti-consumer/lti_consumer/lti_xblock.py", line 1128, in _get_lti_consumer
  return get_lti_consumer(config_id_for_block(self))
File "/edx/shared-src/xblock-lti-consumer/lti_consumer/api.py", line 96, in config_id_for_block
  config = _get_lti_config_for_block(block)
File "/edx/shared-src/xblock-lti-consumer/lti_consumer/api.py", line 76, in _get_lti_config_for_block
  lti_config = _get_or_create_local_lti_config(
File "/edx/shared-src/xblock-lti-consumer/lti_consumer/api.py", line 46, in _get_or_create_local_lti_config
  lti_config.save()
File "/edx/shared-src/xblock-lti-consumer/lti_consumer/models.py", line 313, in save
  super().save(*args, **kwargs)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because the external ID set on the XBlock is not found, this is either because the reusable configuration feature is not properly setup or the external configuration doesn't exist. If you check this specific line you can notice that if no config is found the "version" value will return None, once this value is sent to the _get_or_create_local_lti_config function, when it tries to save the new values to the LtiConfiguration it fails since "version" value cannot be null. This could be fixed by adding an extra validation to any required field from the external config on _get_lti_config_for_block but I think it's out of the scope of this PR, an issue about this should be created.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #420.

lti_1p3_launch_url = consumer.launch_url

return lti_1p3_launch_url
Expand Down
71 changes: 49 additions & 22 deletions lti_consumer/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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__)
Expand Down Expand Up @@ -251,6 +253,12 @@ 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 not EXTERNAL_ID_REGEX.match(str(self.external_id)):
raise ValidationError({
"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 == "":
raise ValidationError({
Expand Down Expand Up @@ -364,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):
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved
"""
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.
Expand All @@ -374,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
Expand All @@ -389,11 +403,10 @@ def get_lti_advantage_ags_mode(self):
"""
Return LTI 1.3 Advantage Assignment and Grade Services mode.
"""
if self.config_store == self.CONFIG_EXTERNAL:
# TODO: Add support for CONFIG_EXTERNAL for LTI 1.3.
raise NotImplementedError
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved
if self.config_store == self.CONFIG_ON_DB:
return self.lti_advantage_ags_mode
elif self.config_store == self.CONFIG_EXTERNAL:
return self.external_config.get('lti_advantage_ags_mode')
else:
block = compat.load_enough_xblock(self.location)
return block.lti_advantage_ags_mode
Expand All @@ -402,11 +415,10 @@ def get_lti_advantage_deep_linking_enabled(self):
"""
Return whether LTI 1.3 Advantage Deep Linking is enabled.
"""
if self.config_store == self.CONFIG_EXTERNAL:
# TODO: Add support for CONFIG_EXTERNAL for LTI 1.3.
raise NotImplementedError("CONFIG_EXTERNAL is not supported for LTI 1.3 Advantage services: %s")
if self.config_store == self.CONFIG_ON_DB:
return self.lti_advantage_deep_linking_enabled
elif self.config_store == self.CONFIG_EXTERNAL:
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
Expand All @@ -415,11 +427,10 @@ def get_lti_advantage_deep_linking_launch_url(self):
"""
Return the LTI 1.3 Advantage Deep Linking launch URL.
"""
if self.config_store == self.CONFIG_EXTERNAL:
# TODO: Add support for CONFIG_EXTERNAL for LTI 1.3.
raise NotImplementedError("CONFIG_EXTERNAL is not supported for LTI 1.3 Advantage services: %s")
if self.config_store == self.CONFIG_ON_DB:
return self.lti_advantage_deep_linking_launch_url
elif self.config_store == self.CONFIG_EXTERNAL:
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
Expand All @@ -428,11 +439,10 @@ def get_lti_advantage_nrps_enabled(self):
"""
Return whether LTI 1.3 Advantage Names and Role Provisioning Services is enabled.
"""
if self.config_store == self.CONFIG_EXTERNAL:
# TODO: Add support for CONFIG_EXTERNAL for LTI 1.3.
raise NotImplementedError("CONFIG_EXTERNAL is not supported for LTI 1.3 Advantage services: %s")
if self.config_store == self.CONFIG_ON_DB:
return self.lti_advantage_enable_nrps
elif self.config_store == self.CONFIG_EXTERNAL:
return self.external_config.get('lti_advantage_enable_nrps')
else:
block = compat.load_enough_xblock(self.location)
return block.lti_1p3_enable_nrps
Expand All @@ -453,6 +463,7 @@ def _setup_lti_1p3_ags(self, consumer):
return

lineitem = self.ltiagslineitem_set.first()

# If using the declarative approach, we should create a LineItem if it
# doesn't exist. This is because on this mode the tool is not able to create
# and manage lineitems using the AGS endpoints.
Expand Down Expand Up @@ -572,9 +583,25 @@ def _get_lti_1p3_consumer(self):
tool_key=self.lti_1p3_tool_public_key,
tool_keyset_url=self.lti_1p3_tool_keyset_url,
)
elif self.config_store == self.CONFIG_EXTERNAL:
consumer = consumer_class(
iss=get_lti_api_base(),
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=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=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 CONFIG_ON_XBLOCK
# or CONFIG_ON_DB.
# This should not occur, but raise an error if self.config_store is not
# CONFIG_ON_XBLOCK, CONFIG_ON_DB or CONFIG_EXTERNAL.
raise NotImplementedError

if isinstance(consumer, LtiAdvantageConsumer):
Expand All @@ -598,10 +625,10 @@ def get_lti_1p3_redirect_uris(self):
Return pre-registered redirect uris or sensible defaults
"""
if self.config_store == self.CONFIG_EXTERNAL:
# TODO: Add support for CONFIG_EXTERNAL for LTI 1.3.
raise NotImplementedError

if self.config_store == self.CONFIG_ON_DB:
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
deep_link_launch_url = self.lti_advantage_deep_linking_launch_url
Expand Down
14 changes: 14 additions & 0 deletions lti_consumer/plugin/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@
public_keyset_endpoint,
name='lti_consumer.public_keyset_endpoint_via_location'
),
# The external ID is split into slashes to make the URL more readable
# and avoid clashing with USAGE_ID_PATTERN.
path(
'lti_consumer/v1/public_keysets/<slug:external_app>/<slug:external_slug>',
public_keyset_endpoint,
name='lti_consumer.public_keyset_endpoint_via_external_id'
),
re_path(
'lti_consumer/v1/launch/(?:/(?P<suffix>.*))?$',
launch_gate_endpoint,
Expand All @@ -46,6 +53,13 @@
access_token_endpoint,
name='lti_consumer.access_token_via_location'
),
# The external ID is split into slashes to make the URL more readable
# and avoid clashing with USAGE_ID_PATTERN.
path(
'lti_consumer/v1/token/<slug:external_app>/<slug:external_slug>',
access_token_endpoint,
name='lti_consumer.access_token_via_external_id'
),
re_path(
r'lti_consumer/v1/lti/(?P<lti_config_id>[-\w]+)/lti-dl/response',
deep_linking_response_endpoint,
Expand Down
Loading
Loading