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.

Sign up for GitHub

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

Jump to bottom

feat: LTI 1.3 reusable configuration #390

Merged
merged 9 commits into from
Oct 24, 2023
Merged

feat: LTI 1.3 reusable configuration #390

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a28dcc1
feat: LTI 1.3 reusable configuration
kuipumu Jul 10, 2023
681abaf
fix: CONFIG_EXTERNAL XBlock or DB values fallback
kuipumu Aug 31, 2023
1a9ca9a
Merge branch 'master' into kuipumu/lti_1p3_reusable_configuration
kuipumu Aug 31, 2023
3c20fab
fix: CI linting errors
kuipumu Aug 31, 2023
f15a78c
feat: Get keyset and access token using external ID
kuipumu Sep 6, 2023
0af3753
fix: Apply suggestions
kuipumu Sep 27, 2023
c8031eb
fix: Apply suggestions
kuipumu Oct 11, 2023
8087346
fix: Apply suggestions, sync_configurations method
kuipumu Oct 19, 2023
c9a2521
feat: Update version, changelog and readme
kuipumu Oct 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions lti_consumer/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,25 @@ 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
token_url = get_lms_lti_access_token_link(config_id)
keyset_url = get_lms_lti_keyset_link(config_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)
client_id = external_config.get('lti_1p3_client_id')
token_url = get_lms_lti_access_token_link(lti_config.external_id.replace(':', '/'))
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved
keyset_url = get_lms_lti_keyset_link(lti_config.external_id.replace(':', '/'))

# Return LTI launch information for end user configuration
return {
'client_id': lti_config.lti_1p3_client_id,
'keyset_url': get_lms_lti_keyset_link(config_id),
'client_id': client_id,
'keyset_url': keyset_url,
'deployment_id': '1',
'oidc_callback': get_lms_lti_launch_link(),
'token_url': get_lms_lti_access_token_link(config_id),
'token_url': token_url,
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved
'deep_linking_launch_url': deep_linking_launch_url,
'deep_linking_content_items':
json.dumps(deep_linking_content_items, indent=4) if deep_linking_content_items else None,
Expand Down
18 changes: 10 additions & 8 deletions lti_consumer/lti_xblock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,13 @@ 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:
_ = self.runtime.service(self, 'i18n').ugettext
validation.add(ValidationMessage(ValidationMessage.ERROR, str(
_('Reusable configuration ID must be set when using external config.'),
)))
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 +1671,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 +1691,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
47 changes: 29 additions & 18 deletions lti_consumer/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,6 @@ 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
else:
Expand All @@ -402,9 +399,6 @@ 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
else:
Expand All @@ -415,11 +409,11 @@ 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:
config = get_external_config_from_filter({}, self.external_id)
return 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,9 +422,6 @@ 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
else:
Expand All @@ -453,6 +444,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 +564,27 @@ 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:
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'),
# 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'),
# 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'),
)
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 +608,11 @@ 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:
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')
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
10 changes: 10 additions & 0 deletions lti_consumer/plugin/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
public_keyset_endpoint,
name='lti_consumer.public_keyset_endpoint_via_location'
),
path(
'lti_consumer/v1/public_keysets/<str: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 +51,11 @@
access_token_endpoint,
name='lti_consumer.access_token_via_location'
),
path(
'lti_consumer/v1/token/<str:external_app>/<slug:external_slug>',
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved
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
104 changes: 85 additions & 19 deletions lti_consumer/plugin/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from django.conf import settings
from django.contrib.auth import get_user_model
from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError
from django.core.exceptions import PermissionDenied, ValidationError
from django.db import transaction
from django.http import Http404, JsonResponse
from django.shortcuts import render
Expand All @@ -26,7 +26,7 @@

from lti_consumer.api import get_lti_pii_sharing_state_for_course, validate_lti_1p3_launch_data
from lti_consumer.exceptions import LtiError
from lti_consumer.lti_1p3.consumer import LtiProctoringConsumer
from lti_consumer.lti_1p3.consumer import LtiConsumer1p3, LtiProctoringConsumer
from lti_consumer.lti_1p3.exceptions import (BadJwtSignature, InvalidClaimValue, Lti1p3Exception,
LtiDeepLinkingContentTypeNotSupported, MalformedJwtToken,
MissingRequiredClaim, NoSuitableKeys, TokenSignatureExpired,
Expand All @@ -48,7 +48,8 @@
from lti_consumer.plugin import compat
from lti_consumer.signals.signals import LTI_1P3_PROCTORING_ASSESSMENT_STARTED
from lti_consumer.track import track_event
from lti_consumer.utils import _, get_data_from_cache, get_lti_1p3_context_types_claim
from lti_consumer.utils import _, get_data_from_cache, get_lti_1p3_context_types_claim, get_lti_api_base
from lti_consumer.filters import get_external_config_from_filter

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -83,36 +84,64 @@ def has_block_access(user, block, course_key):


@require_http_methods(["GET"])
def public_keyset_endpoint(request, usage_id=None, lti_config_id=None):
def public_keyset_endpoint(
request,
usage_id=None,
lti_config_id=None,
external_app=None,
external_slug=None,
):
"""
Gate endpoint to fetch public keysets from a problem

This is basically a passthrough function that uses the
OIDC response parameter `login_hint` to locate the block
and run the proper handler.

Arguments:
lti_config_id (UUID): config_id of the LtiConfiguration
usage_id (UsageKey): location of the Block
external_app (str): App name of the external LTI configuration
external_slug (str): Slug of the external LTI configuration.

Returns:
JsonResponse or Http404
"""
external_id = f"{external_app}:{external_slug}"

try:
if usage_id:
lti_config = LtiConfiguration.objects.get(location=UsageKey.from_string(usage_id))
version = lti_config.version
public_jwk = lti_config.lti_1p3_public_jwk
elif lti_config_id:
lti_config = LtiConfiguration.objects.get(config_id=lti_config_id)
version = lti_config.version
public_jwk = lti_config.lti_1p3_public_jwk
elif external_app and external_slug:
lti_config = get_external_config_from_filter({}, external_id)

if not lti_config:
raise ValueError("External LTI configuration not found")
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved

if lti_config.version != lti_config.LTI_1P3:
version = lti_config.get("version")
public_jwk = lti_config.get("lti_1p3_public_jwk", {})

if version != LtiConfiguration.LTI_1P3:
raise LtiError(
"LTI Error: LTI 1.1 blocks do not have a public keyset endpoint."
)

# Retrieve block's Public JWK
# The underlying method will generate a new Private-Public Pair if one does
# not exist, and retrieve the values.
response = JsonResponse(lti_config.lti_1p3_public_jwk)
response = JsonResponse(public_jwk)
response['Content-Disposition'] = 'attachment; filename=keyset.json'
return response
except (LtiError, InvalidKeyError, ObjectDoesNotExist) as exc:
except (InvalidKeyError, LtiConfiguration.DoesNotExist, ValueError, LtiError) as exc:
log.info(
"Error while retrieving keyset for usage_id (%r) or lit_config_id (%s): %s",
usage_id,
lti_config_id,
"Error while retrieving keyset for ID %s: %s",
usage_id or lti_config_id or external_id,
exc,
exc_info=True,
)
Expand Down Expand Up @@ -362,7 +391,13 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume
@csrf_exempt
@xframe_options_sameorigin
@require_http_methods(["POST"])
def access_token_endpoint(request, lti_config_id=None, usage_id=None):
def access_token_endpoint(
request,
lti_config_id=None,
usage_id=None,
external_app=None,
external_slug=None,
):
"""
Gate endpoint to enable tools to retrieve access tokens for the LTI 1.3 tool.

Expand All @@ -371,6 +406,8 @@ def access_token_endpoint(request, lti_config_id=None, usage_id=None):
Arguments:
lti_config_id (UUID): config_id of the LtiConfiguration
usage_id (UsageKey): location of the Block
external_app (str): App name of the external LTI configuration
external_slug (str): Slug of the external LTI configuration.

Returns:
JsonResponse or Http404
Expand All @@ -379,21 +416,50 @@ def access_token_endpoint(request, lti_config_id=None, usage_id=None):
Sucess: https://tools.ietf.org/html/rfc6749#section-4.4.3
Failure: https://tools.ietf.org/html/rfc6749#section-5.2
"""
external_id = f"{external_app}:{external_slug}"

try:
if lti_config_id:
if usage_id:
lti_config = LtiConfiguration.objects.get(location=UsageKey.from_string(usage_id))
version = lti_config.version
lti_consumer = lti_config.get_lti_consumer()
elif lti_config_id:
lti_config = LtiConfiguration.objects.get(config_id=lti_config_id)
else:
usage_key = UsageKey.from_string(usage_id)
lti_config = LtiConfiguration.objects.get(location=usage_key)
except LtiConfiguration.DoesNotExist as exc:
log.warning("Error getting the LTI configuration with id %r: %s", lti_config_id, exc, exc_info=True)
version = lti_config.version
lti_consumer = lti_config.get_lti_consumer()
elif external_app and external_slug:
lti_config = get_external_config_from_filter({}, external_id)

if not lti_config:
raise ValueError("External LTI configuration not found")

version = lti_config.get("version")
# External LTI configurations don't have a get_lti_consumer method
# so we initialize the LtiConsumer1p3 class using the external config data.
lti_consumer = LtiConsumer1p3(
iss=get_lti_api_base(),
lti_oidc_url=None,
lti_launch_url=None,
client_id=lti_config.get("lti_1p3_client_id"),
deployment_id=None,
rsa_key=lti_config.get("lti_1p3_private_key"),
rsa_key_id=lti_config.get("lti_1p3_private_key_id"),
redirect_uris=None,
tool_key=lti_config.get("lti_1p3_tool_public_key"),
tool_keyset_url=lti_config.get("lti_1p3_tool_keyset_url"),
)
Agrendalath marked this conversation as resolved.
Show resolved Hide resolved
except (InvalidKeyError, LtiConfiguration.DoesNotExist, ValueError) as exc:
log.info(
"Error while retrieving access token for ID %s: %s",
usage_id or lti_config_id or external_id,
exc,
exc_info=True,
)
raise Http404 from exc

if lti_config.version != lti_config.LTI_1P3:
if version != LtiConfiguration.LTI_1P3:
return JsonResponse({"error": "invalid_lti_version"}, status=HTTP_404_NOT_FOUND)

lti_consumer = lti_config.get_lti_consumer()
try:
token = lti_consumer.access_token(
dict(urllib.parse.parse_qsl(
Expand Down
Loading
Loading