Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
[run]
data_file = .coverage
source = lti_consumer
omit = */urls.py
omit =
*/urls.py
29 changes: 29 additions & 0 deletions lti_consumer/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,33 @@
from lti_consumer.forms import CourseAllowPIISharingInLTIAdminForm
from lti_consumer.models import (
CourseAllowPIISharingInLTIFlag,
Lti1p3Passport,
LtiAgsLineItem,
LtiAgsScore,
LtiConfiguration,
LtiDlContentItem,
)


class LtiConfigurationInline(admin.TabularInline):
"""
Inline for the LtiConfiguration models in Lti1p3Passport.
"""
model = LtiConfiguration
extra = 0
can_delete = False
fields = ('location',)

def has_change_permission(self, request, obj=None): # pragma: nocover
return False

def has_delete_permission(self, request, obj=None): # pragma: nocover
return False

def has_add_permission(self, request, obj=None): # pragma: nocover
return False


@admin.register(LtiConfiguration)
class LtiConfigurationAdmin(admin.ModelAdmin):
"""
Expand All @@ -24,6 +44,15 @@ class LtiConfigurationAdmin(admin.ModelAdmin):
readonly_fields = ('location', 'config_id')


@admin.register(Lti1p3Passport)
class Lti1p3PassportAdmin(admin.ModelAdmin):
"""
Admin view for Lti1p3Passport models.
"""
list_display = ('passport_id', 'lti_1p3_client_id')
inlines = [LtiConfigurationInline]


@admin.register(CourseAllowPIISharingInLTIFlag)
class CourseAllowPIISharingInLTIFlagAdmin(KeyedConfigurationModelAdmin):
"""
Expand Down
67 changes: 50 additions & 17 deletions lti_consumer/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,28 @@
"""

import json
import logging

from opaque_keys.edx.keys import CourseKey

from lti_consumer.lti_1p3.constants import LTI_1P3_ROLE_MAP
from .models import CourseAllowPIISharingInLTIFlag, LtiConfiguration, LtiDlContentItem

from .filters import get_external_config_from_filter
from .models import CourseAllowPIISharingInLTIFlag, Lti1p3Passport, LtiConfiguration, LtiDlContentItem
from .utils import (
get_cache_key,
get_data_from_cache,
get_lti_1p3_context_types_claim,
get_lti_deeplinking_content_url,
get_lms_lti_access_token_link,
get_lms_lti_keyset_link,
get_lms_lti_launch_link,
get_lms_lti_access_token_link,
get_lti_1p3_context_types_claim,
get_lti_deeplinking_content_url,
)
from .filters import get_external_config_from_filter

log = logging.getLogger(__name__)

def _get_or_create_local_lti_config(lti_version, block_location,
config_store=LtiConfiguration.CONFIG_ON_XBLOCK, external_id=None):

def get_or_create_local_lti_config(lti_version, block, config_store=LtiConfiguration.CONFIG_ON_XBLOCK):
"""
Retrieve the LtiConfiguration for the block described by block_location, if one exists. If one does not exist,
create an LtiConfiguration with the LtiConfiguration.CONFIG_ON_XBLOCK config_store.
Expand All @@ -34,11 +37,42 @@ def _get_or_create_local_lti_config(lti_version, block_location,
the XBlock to be the source of truth for the LTI version, which is a user-centric perspective we've adopted.
This allows XBlock users to update the LTI version without needing to update the database.
"""
from lti_consumer.plugin.compat import save_xblock # pylint: disable=import-outside-toplevel
# The create operation is only performed when there is no existing configuration for the block
lti_config, _ = LtiConfiguration.objects.get_or_create(location=block_location)
lti_config, _ = LtiConfiguration.objects.get_or_create(location=block.scope_ids.usage_id)
passport = lti_config.lti_1p3_passport
if passport and config_store == LtiConfiguration.CONFIG_ON_XBLOCK:
# Copy keyset url and public key to passport row
if passport.lticonfiguration_set.count() == 1 or (
passport.lti_1p3_tool_keyset_url == '' and passport.lti_1p3_tool_public_key == ''
):
# If the passport is only used in one LTI configuration or both fields are empty,
# we update its tool key and url to match block fields.
passport.lti_1p3_tool_public_key = str(block.lti_1p3_tool_public_key)
passport.lti_1p3_tool_keyset_url = str(block.lti_1p3_tool_keyset_url)
passport.save()
log.info("Updated LTI passport for %s to match block fields", block.scope_ids.usage_id)
elif (
block.lti_1p3_tool_key_mode == 'public_key' and
passport.lti_1p3_tool_public_key != str(block.lti_1p3_tool_public_key)
) or (
block.lti_1p3_tool_key_mode == 'keyset_url' and
passport.lti_1p3_tool_keyset_url != str(block.lti_1p3_tool_keyset_url)
):
# tool public key or url has changed, we create a new passport to avoid conflicts
# with the existing configuration
passport = Lti1p3Passport.objects.create(
lti_1p3_tool_public_key=str(block.lti_1p3_tool_public_key),
lti_1p3_tool_keyset_url=str(block.lti_1p3_tool_keyset_url),
)
block.lti_1p3_passport_id = str(passport.passport_id)
block.save()
log.info("Created new LTI passport for %s with new keyset and public key", block.scope_ids.usage_id)
save_xblock(block)

lti_config.config_store = config_store
lti_config.external_id = external_id
lti_config.external_id = block.external_config
lti_config.lti_1p3_passport = passport

if lti_config.version != lti_version:
lti_config.version = lti_version
Expand All @@ -63,26 +97,25 @@ def _get_lti_config_for_block(block):
bits of configuration.
"""
if block.config_type == 'database':
lti_config = _get_or_create_local_lti_config(
lti_config = get_or_create_local_lti_config(
block.lti_version,
block.scope_ids.usage_id,
block,
LtiConfiguration.CONFIG_ON_DB,
)
elif block.config_type == 'external':
config = get_external_config_from_filter(
{"course_key": block.scope_ids.usage_id.context_key},
block.external_config
)
lti_config = _get_or_create_local_lti_config(
lti_config = get_or_create_local_lti_config(
config.get("version"),
block.scope_ids.usage_id,
block,
LtiConfiguration.CONFIG_EXTERNAL,
external_id=block.external_config,
)
else:
lti_config = _get_or_create_local_lti_config(
lti_config = get_or_create_local_lti_config(
block.lti_version,
block.scope_ids.usage_id,
block,
LtiConfiguration.CONFIG_ON_XBLOCK,
)
return lti_config
Expand Down Expand Up @@ -140,7 +173,7 @@ def get_lti_1p3_launch_info(
if dl_content_items.exists():
deep_linking_content_items = [item.attributes for item in dl_content_items]

config_id = lti_config.config_id
config_id = lti_config.passport_id
client_id = lti_config.lti_1p3_client_id
deployment_id = "1"

Expand Down
2 changes: 1 addition & 1 deletion lti_consumer/lti_1p3/key_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def validate_and_decode(self, token):
message = jwt.decode(
token,
key,
algorithms=['RS256', 'RS512',],
algorithms=['RS256', 'RS512'],
options={
'verify_signature': True,
'verify_aud': False
Expand Down
19 changes: 16 additions & 3 deletions lti_consumer/lti_xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@
from django.conf import settings
from django.utils import timezone
from web_fragments.fragment import Fragment

from webob import Response
from xblock.core import List, Scope, String, XBlock
from xblock.fields import Boolean, Float, Integer
from xblock.validation import ValidationMessage

try:
from xblock.utils.resources import ResourceLoader
from xblock.utils.studio_editable import StudioEditableXBlockMixin
Expand Down Expand Up @@ -311,6 +311,13 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
)

# LTI 1.3 fields
lti_1p3_passport_id = String(
display_name=_("Lti 1.3 passport ID that points to Lti1p3Passport table"),
scope=Scope.settings,
default="",
help=_("Passport ID for a reusable keys.")
)

lti_1p3_launch_url = String(
display_name=_("Tool Launch URL"),
default='',
Expand Down Expand Up @@ -366,6 +373,9 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
" requests received have the signature from the tool."
"<br /><b>This is not required when doing LTI 1.3 Launches"
" without LTI Advantage nor Basic Outcomes requests.</b>"
"<br /><br /><b>Changing the public key or keyset URL will cause the client ID, block keyset URL "
"and access token URL to be regenerated if they are shared between blocks. "
"Please check and update them in the LTI tool settings if necessary.</b>"
),
)
lti_1p3_tool_public_key = String(
Expand All @@ -380,6 +390,9 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
"from the tool."
"<br /><b>This is not required when doing LTI 1.3 Launches without LTI Advantage nor "
"Basic Outcomes requests.</b>"
"<br /><br /><b>Changing the public key or keyset URL will cause the client ID, block keyset URL "
"and access token URL to be regenerated if they are shared between blocks. "
"Please check and update them in the LTI tool settings if necessary.</b>"
),
)

Expand Down Expand Up @@ -991,7 +1004,7 @@ def resource_link_id(self):
i4x-2-3-lti-31de800015cf4afb973356dbe81496df this part of resource_link_id:
makes resource_link_id to be unique among courses inside same system.
"""
return str(urllib.parse.quote(f"{settings.LMS_BASE}-{self.scope_ids.usage_id.html_id()}"))
return str(self.scope_ids.usage_id)

@property
def lis_result_sourcedid(self):
Expand Down Expand Up @@ -1668,7 +1681,7 @@ def get_lti_1p3_launch_data(self):
user_id=self.lms_user_id,
user_role=self.role,
config_id=config_id,
resource_link_id=str(location),
resource_link_id=self.resource_link_id,
external_user_id=self.external_user_id,
preferred_username=username,
name=full_name,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Generated by Django 5.2.12 on 2026-03-17 14:56

import uuid

import django.db.models.deletion
from django.db import migrations, models

import lti_consumer.models


class Migration(migrations.Migration):
dependencies = [
('lti_consumer', '0019_mariadb_uuid_conversion'),
]

operations = [
migrations.CreateModel(
name='Lti1p3Passport',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('passport_id', models.UUIDField(default=uuid.uuid4, editable=False, unique=True)),
(
'lti_1p3_internal_private_key',
models.TextField(blank=True, help_text="Platform's generated Private key. Keep this value secret."),
),
(
'lti_1p3_internal_private_key_id',
models.CharField(blank=True, help_text="Platform's generated Private key ID", max_length=255),
),
(
'lti_1p3_internal_public_jwk',
models.TextField(blank=True, help_text="Platform's generated JWK keyset."),
),
(
'lti_1p3_client_id',
models.CharField(
default=lti_consumer.models.generate_client_id,
help_text='Client ID used by LTI tool',
max_length=255,
),
),
(
'lti_1p3_tool_public_key',
models.TextField(
blank=True,
help_text="This is the LTI Tool's public key. This should be provided by the LTI Tool. One of either lti_1p3_tool_public_key or lti_1p3_tool_keyset_url must not be blank.",
verbose_name='LTI 1.3 Tool Public Key',
),
),
(
'lti_1p3_tool_keyset_url',
models.CharField(
blank=True,
help_text="This is the LTI Tool's JWK (JSON Web Key) Keyset (JWKS) URL. This should be provided by the LTI Tool. One of either lti_1p3_tool_public_key or lti_1p3_tool_keyset_url must not be blank.",
max_length=255,
verbose_name='LTI 1.3 Tool Keyset URL',
),
),
],
),
migrations.AddField(
model_name='lticonfiguration',
name='lti_1p3_passport',
field=models.ForeignKey(
blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='lti_consumer.lti1p3passport'
),
),
]
67 changes: 67 additions & 0 deletions lti_consumer/migrations/0021_create_lti_1p3_passport.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# Generated migration for copying config_id into modulestore from database (Django 6.2)
"""
This migration will copy config_id from LtiConsumer database to LtiConsumerXBlock.

This will help us link xblocks with LtiConsumer database rows without relying on the location or usage_key of xblocks.
"""
from django.db import migrations


def create_lti_1p3_passport(apps, _): # pragma: nocover
"""Copy config_id from LtiConsumer to LtiConsumerXBlock."""
from lti_consumer.plugin.compat import load_enough_xblock, save_xblock # pylint: disable=import-outside-toplevel
from lti_consumer.utils import model_to_dict # pylint: disable=import-outside-toplevel

LtiConfiguration = apps.get_model("lti_consumer", "LtiConfiguration")
Lti1p3Passport = apps.get_model("lti_consumer", "Lti1p3Passport")

for configuration in LtiConfiguration.objects.all():
try:
block = load_enough_xblock(configuration.location)
block.lti_1p3_passport_id = str(configuration.config_id)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be the lti_1p3_passport.id ? Should we also need to add a field for the LtiConfiguration to the block?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We use config_id as passport_id in migration as we want the existing blocks to work without requiring any change in the LTI tool configuration. After this PR, passport_id is included in the access_token and keyset urls instead of config_id.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, gotcha, makes sense.

block.save()
save_xblock(block)
except Exception as e: # pylint: disable=broad-exception-caught
print(f"Failed to copy passport_id for configuration {configuration}: {e}")
values = model_to_dict(
configuration,
include=[
'lti_1p3_internal_private_key',
'lti_1p3_internal_private_key_id',
'lti_1p3_internal_public_jwk',
'lti_1p3_client_id',
'lti_1p3_tool_public_key',
'lti_1p3_tool_keyset_url',
],
)
if block.config_type == "new":
# Data is stored xblock
values.update({
'lti_1p3_tool_public_key': block.lti_1p3_tool_public_key,
'lti_1p3_tool_keyset_url': block.lti_1p3_tool_keyset_url,
})
passport, _ = Lti1p3Passport.objects.update_or_create(
# Use config_id as passport_id to preserve existing urls that use it.
passport_id=configuration.config_id,
defaults=values,
)
configuration.lti_1p3_passport = passport
configuration.save()


def backwards(*_):
"""Reverse the migration only for MariaDB databases."""


class Migration(migrations.Migration):

dependencies = [
('lti_consumer', '0020_lti1p3passport_lticonfiguration_lti_1p3_passport'),
]

operations = [
migrations.RunPython(
code=create_lti_1p3_passport,
reverse_code=backwards,
),
]
Loading