diff --git a/lti_consumer/admin.py b/lti_consumer/admin.py index f800726b..b9266783 100644 --- a/lti_consumer/admin.py +++ b/lti_consumer/admin.py @@ -11,9 +11,15 @@ LtiAgsScore, LtiConfiguration, LtiDlContentItem, + LtiXBlockConfig, ) +class LtiXBlockConfigInline(admin.TabularInline): + model = LtiXBlockConfig + extra = 0 + + @admin.register(LtiConfiguration) class LtiConfigurationAdmin(admin.ModelAdmin): """ @@ -21,7 +27,16 @@ class LtiConfigurationAdmin(admin.ModelAdmin): Makes the location field read-only to avoid issues. """ - readonly_fields = ('location', 'config_id') + readonly_fields = ('config_id',) + inlines = [LtiXBlockConfigInline] + + +@admin.register(LtiXBlockConfig) +class LtiXBlockConfigAdmin(admin.ModelAdmin): + """ + Admin view for LtiXBlockConfig models. + """ + list_display = ('location', 'lti_configuration__config_id') @admin.register(CourseAllowPIISharingInLTIFlag) diff --git a/lti_consumer/api.py b/lti_consumer/api.py index 6b278e7c..cd983390 100644 --- a/lti_consumer/api.py +++ b/lti_consumer/api.py @@ -6,28 +6,40 @@ """ import json +from typing import Any -from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.keys import CourseKey, UsageKey from lti_consumer.lti_1p3.constants import LTI_1P3_ROLE_MAP -from .models import CourseAllowPIISharingInLTIFlag, LtiConfiguration, LtiDlContentItem +from lti_consumer.lti_1p3.exceptions import Lti1p3Exception + +from .filters import get_external_config_from_filter +from .models import CourseAllowPIISharingInLTIFlag, LtiConfiguration, LtiDlContentItem, LtiXBlockConfig from .utils import ( + CONFIG_EXTERNAL, + CONFIG_ON_DB, + CONFIG_ON_XBLOCK, 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, + model_to_dict, ) -from .filters import get_external_config_from_filter -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_xblock_config( + lti_version: str, + block_location: UsageKey | str, + config_id: str | None = None, + config_store=CONFIG_ON_XBLOCK, + external_id=None, +): """ 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. + create an LtiConfiguration with the CONFIG_ON_XBLOCK config_store. Treat the lti_version argument as the source of truth for LtiConfiguration.version and override the LtiConfiguration.version with lti_version. This allows, for example, for @@ -35,7 +47,29 @@ def _get_or_create_local_lti_config(lti_version, block_location, This allows XBlock users to update the LTI version without needing to update the database. """ # 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_xblock_config, created = LtiXBlockConfig.objects.get_or_create(location=block_location) + lti_config: LtiConfiguration | None = None + if created: + if config_id: + lti_config, _ = LtiConfiguration.objects.get_or_create(config_id=config_id) + else: + lti_config = LtiConfiguration.objects.create() + lti_xblock_config.lti_configuration = lti_config + lti_xblock_config.save() + else: + lti_config = lti_xblock_config.lti_configuration + if not lti_config or (config_id and lti_config.config_id != config_id): + # This is an edge case, when an existing configuration is lost or this block is imported from another + # instance, we create a new configuration to avoid no configuration issue. + # OR + # The config_id was changed as a result of author changing the config_store type. + # In this case we create a copy of the existing configuration with the new config_id. + lti_config, _ = LtiConfiguration.objects.get_or_create( + config_id=config_id, + defaults=model_to_dict(lti_config, ['id', 'config_id', 'location', 'external_config']), + ) + lti_xblock_config.lti_configuration = lti_config + lti_xblock_config.save() lti_config.config_store = config_store lti_config.external_id = external_id @@ -45,79 +79,95 @@ def _get_or_create_local_lti_config(lti_version, block_location, lti_config.save() - return lti_config + return lti_xblock_config -def _get_config_by_config_id(config_id): +def _get_config_by_config_id(config_id) -> LtiConfiguration: """ Gets the LTI config by its UUID config ID """ return LtiConfiguration.objects.get(config_id=config_id) +def get_lti_config_by_location(location: str, **filters: dict[str, Any]) -> LtiXBlockConfig: + """ + Gets the LTI xblock config by location + """ + config = LtiXBlockConfig.objects.get( + location=location, + **filters, + ) + return config + + +def try_get_config_by_id(config_id) -> LtiConfiguration | None: + """ + Tries to get the LTI config by its UUID config ID + """ + try: + return _get_config_by_config_id(config_id) + except LtiConfiguration.DoesNotExist: + return None + + def _get_lti_config_for_block(block): """ - Retrieves or creates a LTI Configuration for a block. + Retrieves or creates a LTI Xblock Configuration for a block. - This wraps around `_get_or_create_local_lti_config` and handles the block and modulestore + This wraps around `_get_or_create_local_lti_xblock_config` and handles the block and modulestore bits of configuration. """ if block.config_type == 'database': - lti_config = _get_or_create_local_lti_config( + lti_xblock_config = _get_or_create_local_lti_xblock_config( block.lti_version, block.scope_ids.usage_id, - LtiConfiguration.CONFIG_ON_DB, + block.config_id, + 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_xblock_config = _get_or_create_local_lti_xblock_config( config.get("version"), block.scope_ids.usage_id, - LtiConfiguration.CONFIG_EXTERNAL, + block.config_id, + CONFIG_EXTERNAL, external_id=block.external_config, ) else: - lti_config = _get_or_create_local_lti_config( + lti_xblock_config = _get_or_create_local_lti_xblock_config( block.lti_version, block.scope_ids.usage_id, - LtiConfiguration.CONFIG_ON_XBLOCK, + block.config_id, + CONFIG_ON_XBLOCK, ) - return lti_config + return lti_xblock_config -def config_id_for_block(block): +def config_for_block(block): """ Returns the externally facing config_id of the LTI Configuration used by this block, creating one if required. That ID is suitable for use in launch data or get_consumer. """ - config = _get_lti_config_for_block(block) - return config.config_id - - -def get_lti_consumer(config_id): - """ - Retrieves an LTI Consumer instance for a given configuration. - - Returns an instance of LtiConsumer1p1 or LtiConsumer1p3 depending - on the configuration. - """ - # Return an instance of LTI 1.1 or 1.3 consumer, depending - # on the configuration stored in the model. - return _get_config_by_config_id(config_id).get_lti_consumer() + xblock_config = _get_lti_config_for_block(block) + return xblock_config def get_lti_1p3_launch_info( launch_data, + location: UsageKey, ): """ Retrieves the Client ID, Keyset URL and other urls used to configure a LTI tool. """ # Retrieve LTI Config and consumer - lti_config = _get_config_by_config_id(launch_data.config_id) - lti_consumer = lti_config.get_lti_consumer() + lti_xblock_config = get_lti_config_by_location( + str(location), + lti_configuration__config_id=launch_data.config_id, + ) + lti_consumer = lti_xblock_config.get_lti_consumer() # Check if deep Linking is available, if so, add some extra context: # Deep linking launch URL, and if deep linking is already configured @@ -134,19 +184,22 @@ def get_lti_1p3_launch_info( # Retrieve LTI Content Items (if any was set up) dl_content_items = LtiDlContentItem.objects.filter( - lti_configuration=lti_config + lti_xblock_config=lti_xblock_config ) # Add content item attributes to context if dl_content_items.exists(): deep_linking_content_items = [item.attributes for item in dl_content_items] + lti_config = lti_xblock_config.lti_configuration + if not lti_config: + raise Lti1p3Exception("LTI configuration not found.") config_id = lti_config.config_id client_id = lti_config.lti_1p3_client_id deployment_id = "1" # Display LTI launch information from external configuration. # if an external configuration is being used. - if lti_config.config_store == lti_config.CONFIG_EXTERNAL: + if lti_config.config_store == CONFIG_EXTERNAL and lti_config.external_id: 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') @@ -167,6 +220,7 @@ def get_lti_1p3_launch_info( def get_lti_1p3_launch_start_url( launch_data, + location: UsageKey, deep_link_launch=False, dl_content_id=None, ): @@ -174,7 +228,11 @@ def get_lti_1p3_launch_start_url( Computes and retrieves the LTI URL that starts the OIDC flow. """ # Retrieve LTI consumer - lti_consumer = get_lti_consumer(launch_data.config_id) + lti_xblock_config = get_lti_config_by_location( + str(location), + lti_configuration__config_id=launch_data.config_id, + ) + lti_consumer = lti_xblock_config.get_lti_consumer() # Include a message hint in the launch_data depending on LTI launch type # Case 1: Performs Deep Linking configuration flow. Triggered by staff users to @@ -192,6 +250,7 @@ def get_lti_1p3_launch_start_url( def get_lti_1p3_content_url( launch_data, + location: UsageKey, ): """ Computes and returns which URL the LTI consumer should launch to. @@ -204,41 +263,29 @@ def get_lti_1p3_content_url( Lti DL content in the database. """ # Retrieve LTI consumer - lti_config = _get_config_by_config_id(launch_data.config_id) + lti_xblock_config = get_lti_config_by_location( + str(location), + lti_configuration__config_id=launch_data.config_id, + ) # List content items - content_items = lti_config.ltidlcontentitem_set.all() + content_items = lti_xblock_config.ltidlcontentitem_set.all() # If there's no content items, return normal LTI launch URL if not content_items.count(): - return get_lti_1p3_launch_start_url(launch_data) + return get_lti_1p3_launch_start_url(launch_data, location) # If there's a single `ltiResourceLink` content, return the launch # url for that specific deep link if content_items.count() == 1 and content_items.get().content_type == LtiDlContentItem.LTI_RESOURCE_LINK: return get_lti_1p3_launch_start_url( launch_data, + location, dl_content_id=content_items.get().id, ) # If there's more than one content item, return content presentation URL - return get_lti_deeplinking_content_url(lti_config.id, launch_data) - - -def get_deep_linking_data(deep_linking_id, config_id): - """ - Retrieves deep linking attributes. - - Only works with a single line item, this is a limitation in the - current content presentation implementation. - """ - # Retrieve LTI Configuration - lti_config = _get_config_by_config_id(config_id) - # Only filter DL content item from content item set in the same LTI configuration. - # This avoids a malicious user to input a random LTI id and perform LTI DL - # content launches outside the scope of its configuration. - content_item = lti_config.ltidlcontentitem_set.get(pk=deep_linking_id) - return content_item.attributes + return get_lti_deeplinking_content_url(lti_xblock_config.id, launch_data) def get_lti_pii_sharing_state_for_course(course_key: CourseKey) -> bool: diff --git a/lti_consumer/filters.py b/lti_consumer/filters.py index a5275215..22b3648c 100644 --- a/lti_consumer/filters.py +++ b/lti_consumer/filters.py @@ -1,7 +1,7 @@ """ Module that contains the openedx filters for this XBlock """ -from typing import Dict +from typing import Any, Dict from openedx_filters.tooling import OpenEdxPublicFilter @@ -15,7 +15,7 @@ class LTIConfigurationListed(OpenEdxPublicFilter): filter_type = "org.openedx.xblock.lti_consumer.configuration.listed.v1" @classmethod - def run_filter(cls, context: Dict, config_id: str, configurations: Dict): + def run_filter(cls, context: Dict, config_id: str, configurations: Dict) -> tuple[Dict, str, Dict]: """ Execute the filter with the signature specified. @@ -28,7 +28,7 @@ def run_filter(cls, context: Dict, config_id: str, configurations: Dict): return data.get("context"), data.get("config_id"), data.get("configurations") -def get_external_config_from_filter(context, config_id=''): +def get_external_config_from_filter(context, config_id='') -> dict[str, Any]: """ Thin wrapper around the LTIConfigurationListed filter to get the external configuration values using a certain context and config_id. diff --git a/lti_consumer/lti_1p3/extensions/rest_framework/authentication.py b/lti_consumer/lti_1p3/extensions/rest_framework/authentication.py index ff4e8f31..63fe49b7 100644 --- a/lti_consumer/lti_1p3/extensions/rest_framework/authentication.py +++ b/lti_consumer/lti_1p3/extensions/rest_framework/authentication.py @@ -5,10 +5,9 @@ """ from django.contrib.auth.models import AnonymousUser from django.utils.translation import gettext as _ -from rest_framework import authentication -from rest_framework import exceptions +from rest_framework import authentication, exceptions -from lti_consumer.models import LtiConfiguration +from lti_consumer.models import LtiXBlockConfig class Lti1p3ApiAuthentication(authentication.BaseAuthentication): @@ -51,8 +50,8 @@ def authenticate(self, request): # Retrieve LTI configuration or fail if it doesn't exist try: - lti_configuration = LtiConfiguration.objects.get(pk=lti_config_id) - lti_consumer = lti_configuration.get_lti_consumer() + lti_xblock_config = LtiXBlockConfig.objects.get(pk=lti_config_id) + lti_consumer = lti_xblock_config.get_lti_consumer() except Exception as err: msg = _('LTI configuration not found.') raise exceptions.AuthenticationFailed(msg) from err @@ -72,7 +71,7 @@ def authenticate(self, request): # With the LTI Configuration and consumer attached to the request # the views and permissions classes can make use of the # current LTI context to retrieve settings and decode the token passed. - request.lti_configuration = lti_configuration + request.lti_xblock_config = lti_xblock_config request.lti_consumer = lti_consumer # This isn't tied to any authentication backend on Django (it's just diff --git a/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py b/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py index 953cee40..677dac15 100644 --- a/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py +++ b/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py @@ -76,7 +76,7 @@ def get_id(self, obj): return reverse( 'lti_consumer:lti-ags-view-detail', kwargs={ - 'lti_config_id': obj.lti_configuration.id, + 'lti_config_id': obj.lti_xblock_config.id, 'pk': obj.pk }, request=request, diff --git a/lti_consumer/lti_xblock.py b/lti_consumer/lti_xblock.py index f0364d42..59765da8 100644 --- a/lti_consumer/lti_xblock.py +++ b/lti_consumer/lti_xblock.py @@ -49,10 +49,10 @@ Numeric grades between 0 and 1 and text + basic HTML feedback comments are supported, via GET / PUT / DELETE HTTP methods respectively """ - import logging import re import urllib.parse +import uuid from collections import namedtuple from importlib import import_module @@ -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 @@ -74,19 +74,20 @@ from .data import Lti1p3LaunchData from .exceptions import LtiError -from .lti_1p1.consumer import LtiConsumer1p1, parse_result_json, LTI_PARAMETERS +from .lti_1p1.consumer import LTI_PARAMETERS, LtiConsumer1p1, parse_result_json from .lti_1p1.oauth import log_authorization_header from .outcomes import OutcomeService from .plugin import compat from .track import track_event from .utils import ( + EXTERNAL_ID_REGEX, _, - resolve_custom_parameter_template, - external_config_filter_enabled, - external_user_id_1p1_launches_enabled, + compare_config_type, database_config_enabled, - EXTERNAL_ID_REGEX, + external_config_filter_enabled, external_multiple_launch_urls_enabled, + external_user_id_1p1_launches_enabled, + resolve_custom_parameter_template, ) log = logging.getLogger(__name__) @@ -288,6 +289,12 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): "'Reusable Configuration' and enter it in the text field below." ) ) + config_id = String( + display_name=_("Configuration ID"), + scope=Scope.settings, + default="", + help=_("Config ID for a reusable configuration.") + ) lti_version = String( display_name=_("LTI Version"), @@ -710,6 +717,18 @@ def validate(self): ))) return validation + def save(self): + if not self.config_id: + self.config_id = str(uuid.uuid4()) + else: + from lti_consumer.api import try_get_config_by_id # pylint: disable=import-outside-toplevel + + row = try_get_config_by_id(self.config_id) + if row and not compare_config_type(self.config_type, row.config_store): + # The configuration type has been changed since it was saved. Create a new config row. + self.config_id = str(uuid.uuid4()) + super().save() + def get_settings(self): """ Get the XBlock settings bucket via the SettingsService. @@ -1115,9 +1134,9 @@ def _get_lti_consumer(self): # Runtime import since this will only run in the # Open edX LMS/Studio environments. # pylint: disable=import-outside-toplevel - from lti_consumer.api import config_id_for_block, get_lti_consumer + from lti_consumer.api import config_for_block - return get_lti_consumer(config_id_for_block(self)) + return config_for_block(self).get_lti_consumer() def extract_real_user_data(self): """ @@ -1164,6 +1183,7 @@ def _add_author_view(self, context, loader, fragment): context.update( get_lti_1p3_launch_info( launch_data, + self.scope_ids.usage_id ) ) @@ -1643,8 +1663,8 @@ def get_lti_1p3_launch_data(self): # Open edX LMS/Studio environments. # TODO: Replace this with a more appropriate API function that is intended for public use. # pylint: disable=import-outside-toplevel - from lti_consumer.api import config_id_for_block - config_id = config_id_for_block(self) + from lti_consumer.api import config_for_block + xblock_config = config_for_block(self) location = self.scope_ids.usage_id course_key = str(location.context_key) @@ -1667,7 +1687,7 @@ def get_lti_1p3_launch_data(self): launch_data = Lti1p3LaunchData( user_id=self.lms_user_id, user_role=self.role, - config_id=config_id, + config_id=xblock_config.config.config_id, resource_link_id=str(location), external_user_id=self.external_user_id, preferred_username=username, @@ -1708,6 +1728,7 @@ def _get_lti_block_launch_handler(self): from lti_consumer.api import get_lti_1p3_content_url # pylint: disable=import-outside-toplevel lti_block_launch_handler = get_lti_1p3_content_url( launch_data, + self.scope_ids.usage_id, ) return lti_block_launch_handler diff --git a/lti_consumer/migrations/0020_ltixblockconfig_ltiagslineitem_lti_xblock_config_and_more.py b/lti_consumer/migrations/0020_ltixblockconfig_ltiagslineitem_lti_xblock_config_and_more.py new file mode 100644 index 00000000..02226fc0 --- /dev/null +++ b/lti_consumer/migrations/0020_ltixblockconfig_ltiagslineitem_lti_xblock_config_and_more.py @@ -0,0 +1,51 @@ +# Generated by Django 5.2.12 on 2026-03-17 11:55 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ('lti_consumer', '0019_mariadb_uuid_conversion'), + ] + + operations = [ + migrations.CreateModel( + name='LtiXBlockConfig', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ( + 'location', + models.CharField( + db_index=True, + help_text='Normally, this is the location of xblock. But it can any string to allow it work outside of xblock context.', + max_length=255, + unique=True, + ), + ), + ( + 'lti_configuration', + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to='lti_consumer.lticonfiguration', + ), + ), + ], + ), + migrations.AddField( + model_name='ltiagslineitem', + name='lti_xblock_config', + field=models.ForeignKey( + blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='lti_consumer.ltixblockconfig' + ), + ), + migrations.AddField( + model_name='ltidlcontentitem', + name='lti_xblock_config', + field=models.ForeignKey( + blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='lti_consumer.ltixblockconfig' + ), + ), + ] diff --git a/lti_consumer/migrations/0021_migrate_config_id_to_blocks.py b/lti_consumer/migrations/0021_migrate_config_id_to_blocks.py new file mode 100644 index 00000000..7c19082f --- /dev/null +++ b/lti_consumer/migrations/0021_migrate_config_id_to_blocks.py @@ -0,0 +1,74 @@ +# 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. +""" +import uuid + +from django.db import migrations + + +def copy_config_id(apps, _): + """Copy config_id from LtiConsumer to LtiConsumerXBlock.""" + from lti_consumer.plugin.compat import load_enough_xblock, save_xblock # pylint: disable=import-outside-toplevel + + LtiConfiguration = apps.get_model("lti_consumer", "LtiConfiguration") + LtiXBlockConfig = apps.get_model("lti_consumer", "LtiXBlockConfig") + + for configuration in LtiConfiguration.objects.all(): + # Create a new unique location for cconfiguration with no location. + location = configuration.location or str(uuid.uuid4()) + LtiXBlockConfig.objects.update_or_create( + location=str(location), + defaults={ + "lti_configuration": configuration, + } + ) + try: + blockstore = load_enough_xblock(configuration.location) + blockstore.config_id = str(configuration.config_id) + blockstore.save() + save_xblock(blockstore) + except Exception as e: # pylint: disable=broad-exception-caught + print(f"Failed to copy config_id for configuration {configuration}: {e}") + + LtiAgsLineItem = apps.get_model("lti_consumer", "LtiAgsLineItem") + for line_item in LtiAgsLineItem.objects.all(): + xblock_config = LtiXBlockConfig.objects.filter( + lti_configuration=line_item.lti_configuration, + ).first() + if not xblock_config: + print(f"Invalid configuration linked to AGS line item: {line_item}.") + continue + line_item.lti_xblock_config = xblock_config + line_item.save() + + LtiDlContentItem = apps.get_model("lti_consumer", "LtiDlContentItem") + for content_item in LtiDlContentItem.objects.all(): + xblock_config = LtiXBlockConfig.objects.filter( + lti_configuration=content_item.lti_configuration, + ).first() + if not xblock_config: + print(f"Invalid configuration linked to Dl Conent Item: {content_item}.") + continue + content_item.lti_xblock_config = xblock_config + content_item.save() + + +def backwards(*_): + """Reverse the migration only for MariaDB databases.""" + + +class Migration(migrations.Migration): + + dependencies = [ + ('lti_consumer', '0020_ltixblockconfig_ltiagslineitem_lti_xblock_config_and_more'), + ] + + operations = [ + migrations.RunPython( + code=copy_config_id, + reverse_code=backwards, + ), + ] diff --git a/lti_consumer/models.py b/lti_consumer/models.py index 5a07a724..e3858355 100644 --- a/lti_consumer/models.py +++ b/lti_consumer/models.py @@ -1,40 +1,48 @@ """ LTI configuration and linking models. """ +import json import logging import uuid -import json - -from django.db import models -from django.core.validators import MinValueValidator -from django.core.exceptions import ValidationError +from typing import Any -from jsonfield import JSONField -from Cryptodome.PublicKey import RSA -from ccx_keys.locator import CCXBlockUsageLocator -from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField -from opaque_keys.edx.keys import CourseKey from config_models.models import ConfigurationModel +from Cryptodome.PublicKey import RSA +from django.core.exceptions import ValidationError +from django.core.validators import MinValueValidator +from django.db import models from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ from edx_django_utils.monitoring import function_trace +from jsonfield import JSONField +from opaque_keys import InvalidKeyError +from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField +from opaque_keys.edx.keys import CourseKey, UsageKey + from lti_consumer.filters import get_external_config_from_filter # LTI 1.1 from lti_consumer.lti_1p1.consumer import LtiConsumer1p1 + # LTI 1.3 from lti_consumer.lti_1p3.consumer import LtiAdvantageConsumer, LtiProctoringConsumer from lti_consumer.lti_1p3.key_handlers import PlatformKeyHandler from lti_consumer.plugin import compat from lti_consumer.utils import ( - get_lti_api_base, + CONFIG_EXTERNAL, + CONFIG_ON_DB, + CONFIG_ON_XBLOCK, + EXTERNAL_ID_REGEX, + LTI_ADVANTAGE_AGS_CHOICES, + LTI_ADVANTAGE_AGS_DECLARATIVE, + LTI_ADVANTAGE_AGS_DISABLED, + LTI_ADVANTAGE_AGS_PROGRAMMATIC, + choose_lti_1p3_redirect_uris, + external_multiple_launch_urls_enabled, get_lti_ags_lineitems_url, + get_lti_api_base, get_lti_deeplinking_response_url, get_lti_nrps_context_membership_url, - choose_lti_1p3_redirect_uris, - model_to_dict, - EXTERNAL_ID_REGEX, - external_multiple_launch_urls_enabled, ) log = logging.getLogger(__name__) @@ -73,14 +81,6 @@ class LtiConfiguration(models.Model): default=LTI_1P1, ) - # Configuration storage - # Initally, this only supported the configuration - # stored on the block. Now it has been expanded to - # enable storing LTI configuration in the model itself or in an external - # configuration service fetchable using openedx-filters - CONFIG_ON_XBLOCK = 'CONFIG_ON_XBLOCK' - CONFIG_ON_DB = 'CONFIG_ON_DB' - CONFIG_EXTERNAL = 'CONFIG_EXTERNAL' CONFIG_STORE_CHOICES = [ (CONFIG_ON_XBLOCK, _('Configuration Stored on XBlock fields')), (CONFIG_ON_DB, _('Configuration Stored on this model')), @@ -224,14 +224,6 @@ class LtiConfiguration(models.Model): 'use the same value as lti_1p3_launch_url.' ) - LTI_ADVANTAGE_AGS_DISABLED = 'disabled' - LTI_ADVANTAGE_AGS_DECLARATIVE = 'declarative' - LTI_ADVANTAGE_AGS_PROGRAMMATIC = 'programmatic' - LTI_ADVANTAGE_AGS_CHOICES = [ - (LTI_ADVANTAGE_AGS_DISABLED, 'Disabled'), - (LTI_ADVANTAGE_AGS_DECLARATIVE, 'Allow tools to submit grades only (declarative)'), - (LTI_ADVANTAGE_AGS_PROGRAMMATIC, 'Allow tools to manage and submit grade (programmatic)') - ] lti_advantage_ags_mode = models.CharField( "LTI Advantage Assignment and Grade Services Mode", max_length=20, @@ -251,17 +243,13 @@ class LtiConfiguration(models.Model): ) def clean(self): - if self.config_store == self.CONFIG_ON_XBLOCK and self.location is None: - 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)): + if self.config_store == 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.version == self.LTI_1P3 and self.config_store == CONFIG_ON_DB: if self.lti_1p3_tool_public_key == "" and self.lti_1p3_tool_keyset_url == "": raise ValidationError({ "config_store": _( @@ -269,55 +257,12 @@ def clean(self): "lti_1p3_tool_public_key or lti_1p3_tool_keyset_url." ), }) - if (self.version == self.LTI_1P3 and self.config_store in [self.CONFIG_ON_XBLOCK, self.CONFIG_EXTERNAL] and + if (self.version == self.LTI_1P3 and self.config_store in [CONFIG_ON_XBLOCK, CONFIG_EXTERNAL] and self.lti_1p3_proctoring_enabled): raise ValidationError({ "config_store": _("CONFIG_ON_XBLOCK and CONFIG_EXTERNAL are not supported for " "LTI 1.3 Proctoring Services."), }) - try: - consumer = self.get_lti_consumer() - except NotImplementedError: - consumer = None - if consumer is None: - raise ValidationError(_("Invalid LTI configuration.")) - - @function_trace('lti_consumer.models.LtiConfiguration.sync_configurations') - def sync_configurations(self): - """Syncronize main/children configurations. - - This method will synchronize the field values of main/children configurations. - On a configuration with a CCX location, it will copy the values from the main course configuration, - otherwise, it will try to query any children configuration and update their fields using - the current configuration values. - """ - EXCLUDED_FIELDS = ['id', 'config_id', 'location', 'external_config'] - - if isinstance(self.location, CCXBlockUsageLocator): - # Query main configuration using main location. - main_config = LtiConfiguration.objects.filter(location=self.location.to_block_locator()).first() - # Copy fields from main configuration. - for field in model_to_dict(main_config, EXCLUDED_FIELDS).items(): - setattr(self, field[0], field[1]) - else: - try: - # Query child CCX configurations using location block ID and CCX namespace. - child_configs = LtiConfiguration.objects.filter( - location__endswith=str(self.location).split('@')[-1], - ).filter( - location__startswith=CCXBlockUsageLocator.CANONICAL_NAMESPACE, - ).exclude(id=self.pk) - # Copy fields to child CCX configurations. - child_configs.update(**model_to_dict(self, EXCLUDED_FIELDS)) - except IndexError: - log.exception( - f'Failed to query children CCX LTI configurations: ' - f'Failed to parse main LTI configuration location: {self.location}', - ) - - def save(self, *args, **kwargs): - self.sync_configurations() - super().save(*args, **kwargs) def _generate_lti_1p3_keys_if_missing(self): """ @@ -376,22 +321,24 @@ def lti_1p3_public_jwk(self): return json.loads(self.lti_1p3_internal_public_jwk) @cached_property - def external_config(self): + def external_config(self) -> dict[str, Any]: """ Return the external resuable configuration. """ return get_external_config_from_filter({}, self.external_id) - def _get_lti_1p1_consumer(self): + def _get_lti_1p1_consumer(self, location: UsageKey | str | None = None): """ Return a class of LTI 1.1 consumer. """ # If LTI configuration is stored in the XBlock. - if self.config_store == self.CONFIG_ON_XBLOCK: - block = compat.load_enough_xblock(self.location) + if self.config_store == CONFIG_ON_XBLOCK: + if not location or not isinstance(location, UsageKey): + raise ValueError("Valid location is required if you are using CONFIG_ON_XBLOCK") + block = compat.load_enough_xblock(location) key, secret = block.lti_provider_key_secret launch_url = block.launch_url - elif self.config_store == self.CONFIG_EXTERNAL: + elif self.config_store == CONFIG_EXTERNAL: 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") @@ -402,138 +349,10 @@ def _get_lti_1p1_consumer(self): return LtiConsumer1p1(launch_url, key, secret) - def get_lti_advantage_ags_mode(self): - """ - Return LTI 1.3 Advantage Assignment and Grade Services mode. - """ - 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 - - def get_lti_advantage_deep_linking_enabled(self): - """ - Return whether LTI 1.3 Advantage Deep Linking is enabled. - """ - 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 - - 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_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 - - 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_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 - - def _setup_lti_1p3_ags(self, consumer): - """ - Set up LTI 1.3 Advantage Assigment and Grades Services. - """ - - try: - lti_advantage_ags_mode = self.get_lti_advantage_ags_mode() - except NotImplementedError as exc: - log.exception("Error setting up LTI 1.3 Advantage Assignment and Grade Services: %s", exc) - return - - if lti_advantage_ags_mode == self.LTI_ADVANTAGE_AGS_DISABLED: - log.info('LTI Advantage AGS is disabled for %s', self) - 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. - if not lineitem and lti_advantage_ags_mode == self.LTI_ADVANTAGE_AGS_DECLARATIVE: - try: - block = compat.load_enough_xblock(self.location) - except ValueError: # There is no location to load the block - block = None - - if block: - default_values = { - 'resource_id': self.location, - 'score_maximum': block.weight, - 'label': block.display_name, - } - if hasattr(block, 'start'): - default_values['start_date_time'] = block.start - - if hasattr(block, 'due'): - default_values['end_date_time'] = block.due - else: - # TODO find a way to make these defaults more sensible - default_values = { - 'resource_id': self.location, - 'score_maximum': 100, - 'label': 'LTI Consumer at ' + str(self.location) - } - - # create LineItem if there is none for current lti configuration - lineitem = LtiAgsLineItem.objects.create( - lti_configuration=self, - resource_link_id=self.location, - **default_values - ) - - consumer.enable_ags( - lineitems_url=get_lti_ags_lineitems_url(self.id), - lineitem_url=get_lti_ags_lineitems_url(self.id, lineitem.id) if lineitem else None, - allow_programmatic_grade_interaction=( - lti_advantage_ags_mode == self.LTI_ADVANTAGE_AGS_PROGRAMMATIC - ) - ) - - def _setup_lti_1p3_deep_linking(self, consumer): - """ - Set up LTI 1.3 Advantage Deep Linking. - """ - try: - if self.get_lti_advantage_deep_linking_enabled(): - consumer.enable_deep_linking( - self.get_lti_advantage_deep_linking_launch_url(), - get_lti_deeplinking_response_url(self.id), - ) - except NotImplementedError as exc: - log.exception("Error setting up LTI 1.3 Advantage Deep Linking: %s", exc) - - def _setup_lti_1p3_nrps(self, consumer): - """ - Set up LTI 1.3 Advantage Names and Role Provisioning Services. - """ - try: - if self.get_lti_advantage_nrps_enabled(): - consumer.enable_nrps(get_lti_nrps_context_membership_url(self.id)) - except NotImplementedError as exc: - log.exception("Error setting up LTI 1.3 Advantage Names and Role Provisioning Services: %s", exc) - - def _get_lti_1p3_consumer(self): + def _get_lti_1p3_consumer( + self, + location: UsageKey | str | None = None, + ): """ Return a class of LTI 1.3 consumer. @@ -545,11 +364,13 @@ def _get_lti_1p3_consumer(self): # NOTE: This currently prevents an LTI Consumer from supporting both the LTI 1.3 proctoring feature and the LTI # Advantage services. We plan to address this. Follow this issue: # https://github.com/openedx/xblock-lti-consumer/issues/303. - if self.lti_1p3_proctoring_enabled and self.config_store == self.CONFIG_ON_DB: + if self.lti_1p3_proctoring_enabled and self.config_store == CONFIG_ON_DB: consumer_class = LtiProctoringConsumer - if self.config_store == self.CONFIG_ON_XBLOCK: - block = compat.load_enough_xblock(self.location) + if self.config_store == CONFIG_ON_XBLOCK: + if not location or not isinstance(location, UsageKey): + raise ValueError("Valid location is required if you are using CONFIG_ON_XBLOCK") + block = compat.load_enough_xblock(location) consumer = consumer_class( iss=get_lti_api_base(), @@ -563,12 +384,12 @@ def _get_lti_1p3_consumer(self): rsa_key=self.lti_1p3_private_key, rsa_key_id=self.lti_1p3_private_key_id, # Registered redirect uris - redirect_uris=self.get_lti_1p3_redirect_uris(), + redirect_uris=self.get_lti_1p3_redirect_uris(location), # LTI 1.3 Tool key/keyset url tool_key=block.lti_1p3_tool_public_key, tool_keyset_url=block.lti_1p3_tool_keyset_url, ) - elif self.config_store == self.CONFIG_ON_DB: + elif self.config_store == CONFIG_ON_DB: consumer = consumer_class( iss=get_lti_api_base(), lti_oidc_url=self.lti_1p3_oidc_url, @@ -581,16 +402,18 @@ def _get_lti_1p3_consumer(self): rsa_key=self.lti_1p3_private_key, rsa_key_id=self.lti_1p3_private_key_id, # Registered redirect uris - redirect_uris=self.get_lti_1p3_redirect_uris(), + redirect_uris=self.get_lti_1p3_redirect_uris(location), # LTI 1.3 Tool key/keyset url tool_key=self.lti_1p3_tool_public_key, tool_keyset_url=self.lti_1p3_tool_keyset_url, ) - elif self.config_store == self.CONFIG_EXTERNAL: + elif self.config_store == CONFIG_EXTERNAL: lti_launch_url = self.external_config.get('lti_1p3_launch_url') - if external_multiple_launch_urls_enabled(self.location.course_key): - block = compat.load_enough_xblock(self.location) + if location and isinstance(location, UsageKey) and external_multiple_launch_urls_enabled( + location.course_key + ): + block = compat.load_enough_xblock(location) lti_launch_url = block.lti_1p3_launch_url or lti_launch_url @@ -603,7 +426,7 @@ def _get_lti_1p3_consumer(self): 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(), + redirect_uris=self.get_lti_1p3_redirect_uris(location), tool_key=self.external_config.get('lti_1p3_tool_public_key'), tool_keyset_url=self.external_config.get('lti_1p3_tool_keyset_url'), ) @@ -612,37 +435,32 @@ def _get_lti_1p3_consumer(self): # CONFIG_ON_XBLOCK, CONFIG_ON_DB or CONFIG_EXTERNAL. raise NotImplementedError - if isinstance(consumer, LtiAdvantageConsumer): - self._setup_lti_1p3_ags(consumer) - self._setup_lti_1p3_deep_linking(consumer) - self._setup_lti_1p3_nrps(consumer) - return consumer @function_trace('lti_consumer.models.LtiConfiguration.get_lti_consumer') - def get_lti_consumer(self): + def get_lti_consumer(self, location: UsageKey | str | None = None): """ Returns an instanced class of LTI 1.1 or 1.3 consumer. """ if self.version == self.LTI_1P3: - return self._get_lti_1p3_consumer() + return self._get_lti_1p3_consumer(location) - return self._get_lti_1p1_consumer() + return self._get_lti_1p1_consumer(location) - def get_lti_1p3_redirect_uris(self): + def get_lti_1p3_redirect_uris(self, location): """ Return pre-registered redirect uris or sensible defaults """ - if self.config_store == self.CONFIG_EXTERNAL: + if self.config_store == CONFIG_EXTERNAL: 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: + elif self.config_store == 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 else: - block = compat.load_enough_xblock(self.location) + block = compat.load_enough_xblock(location) redirect_uris = block.lti_1p3_redirect_uris launch_url = block.lti_1p3_launch_url deep_link_launch_url = block.lti_advantage_deep_linking_launch_url @@ -670,7 +488,209 @@ def pii_share_email(self, value): self.lti_config['pii_share_email'] = value # pylint: disable=unsupported-assignment-operation def __str__(self): - return f"[{self.config_store}] {self.version} - {self.location}" + return f"[{self.config_store}] {self.version} - {self.config_id}" + + class Meta: + app_label = 'lti_consumer' + + +class LtiXBlockConfig(models.Model): + """ + Modal to store xblock and lti configurations link. + """ + # Block location where the configuration is stored. + location = models.CharField( + max_length=255, + db_index=True, + unique=True, + help_text=_( + "Normally, this is the location of xblock. But it can any string " + "to allow it work outside of xblock context." + ), + ) + lti_configuration = models.ForeignKey( + LtiConfiguration, + on_delete=models.CASCADE, + ) + + def __str__(self): + return f"[{self.location}] - {self.lti_configuration}" + + @property + def location_as_usage_key(self): + """ + Try to parse the location string into a usage key + """ + try: + usage_key = UsageKey.from_string(self.location) + return usage_key + except InvalidKeyError: + return None + + def get_lti_advantage_nrps_enabled(self): + """ + Return whether LTI 1.3 Advantage Names and Role Provisioning Services is enabled. + """ + if self.lti_configuration.config_store == CONFIG_ON_DB: + return self.lti_configuration.lti_advantage_enable_nrps + elif self.lti_configuration.config_store == CONFIG_EXTERNAL: + return self.lti_configuration.external_config.get('lti_advantage_enable_nrps') + else: + block = compat.load_enough_xblock(self.location_as_usage_key) + return block.lti_1p3_enable_nrps + + def _setup_lti_1p3_nrps(self, consumer): + """ + Set up LTI 1.3 Advantage Names and Role Provisioning Services. + """ + try: + if self.get_lti_advantage_nrps_enabled(): + consumer.enable_nrps(get_lti_nrps_context_membership_url(self.id)) + except NotImplementedError as exc: + log.exception("Error setting up LTI 1.3 Advantage Names and Role Provisioning Services: %s", exc) + + def get_lti_advantage_ags_mode(self): + """ + Return LTI 1.3 Advantage Assignment and Grade Services mode. + """ + if self.lti_configuration.config_store == CONFIG_ON_DB: + return self.lti_configuration.lti_advantage_ags_mode + elif self.lti_configuration.config_store == CONFIG_EXTERNAL: + return self.lti_configuration.external_config.get('lti_advantage_ags_mode') + else: + block = compat.load_enough_xblock(self.location_as_usage_key) + return block.lti_advantage_ags_mode + + def _setup_lti_1p3_ags(self, consumer): + """ + Set up LTI 1.3 Advantage Assigment and Grades Services. + """ + + try: + lti_advantage_ags_mode = self.get_lti_advantage_ags_mode() + except NotImplementedError as exc: + log.exception("Error setting up LTI 1.3 Advantage Assignment and Grade Services: %s", exc) + return + + if lti_advantage_ags_mode == LTI_ADVANTAGE_AGS_DISABLED: + log.info('LTI Advantage AGS is disabled for %s', self) + 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. + if not lineitem and lti_advantage_ags_mode == LTI_ADVANTAGE_AGS_DECLARATIVE: + try: + if self.location_as_usage_key: + block = compat.load_enough_xblock(self.location_as_usage_key) + else: + block = None + except ValueError: # There is no location to load the block + block = None + + if block: + default_values = { + 'resource_id': self.location_as_usage_key, + 'score_maximum': block.weight, + 'label': block.display_name, + } + if hasattr(block, 'start'): + default_values['start_date_time'] = block.start + + if hasattr(block, 'due'): + default_values['end_date_time'] = block.due + else: + # TODO find a way to make these defaults more sensible + default_values = { + 'resource_id': self.location, + 'score_maximum': 100, + 'label': 'LTI Consumer at ' + str(self.location) + } + + # create LineItem if there is none for current lti configuration + lineitem = LtiAgsLineItem.objects.create( + lti_configuration=self, + resource_link_id=self.location, + **default_values + ) + + consumer.enable_ags( + lineitems_url=get_lti_ags_lineitems_url(self.id), + lineitem_url=get_lti_ags_lineitems_url(self.id, lineitem.id) if lineitem else None, + allow_programmatic_grade_interaction=( + lti_advantage_ags_mode == LTI_ADVANTAGE_AGS_PROGRAMMATIC + ) + ) + + def get_lti_advantage_deep_linking_enabled(self): + """ + Return whether LTI 1.3 Advantage Deep Linking is enabled. + """ + if self.lti_configuration.config_store == CONFIG_ON_DB: + return self.lti_configuration.lti_advantage_deep_linking_enabled + elif self.lti_configuration.config_store == CONFIG_EXTERNAL: + return self.lti_configuration.external_config.get('lti_advantage_deep_linking_enabled') + else: + block = compat.load_enough_xblock(self.location_as_usage_key) + return block.lti_advantage_deep_linking_enabled + + def get_lti_advantage_deep_linking_launch_url(self): + """ + Return the LTI 1.3 Advantage Deep Linking launch URL. + """ + if self.lti_configuration.config_store == CONFIG_ON_DB: + return self.lti_configuration.lti_advantage_deep_linking_launch_url + elif self.lti_configuration.config_store == CONFIG_EXTERNAL: + return self.lti_configuration.external_config.get('lti_advantage_deep_linking_launch_url') + else: + block = compat.load_enough_xblock(self.location_as_usage_key) + return block.lti_advantage_deep_linking_launch_url + + def _setup_lti_1p3_deep_linking(self, consumer): + """ + Set up LTI 1.3 Advantage Deep Linking. + """ + try: + if self.get_lti_advantage_deep_linking_enabled(): + consumer.enable_deep_linking( + self.get_lti_advantage_deep_linking_launch_url(), + get_lti_deeplinking_response_url(self.id), + ) + except NotImplementedError as exc: + log.exception("Error setting up LTI 1.3 Advantage Deep Linking: %s", exc) + + def _setup_advantage_conf( + self, + consumer: LtiAdvantageConsumer | LtiProctoringConsumer, + ) -> LtiAdvantageConsumer | LtiProctoringConsumer: + """ + Setup the appropriate LTI consumer class. + """ + if isinstance(consumer, LtiAdvantageConsumer): + # FIXME: move below methods to LtiXBlockConfig and use its id as lti_config_id in urls + self._setup_lti_1p3_ags(consumer) + self._setup_lti_1p3_deep_linking(consumer) + self._setup_lti_1p3_nrps(consumer) + return consumer + + @function_trace('lti_consumer.models.LtiXBlockConfig.get_lti_consumer') + def get_lti_consumer(self): + """ + Returns an instanced class of LTI 1.1 or 1.3 consumer. + """ + consumer = self.lti_configuration.get_lti_consumer(self.location_as_usage_key or self.location) + return self._setup_advantage_conf(consumer) + + def clean(self): + super().clean() + if ( + self.lti_configuration + and self.lti_configuration.config_store == CONFIG_ON_XBLOCK + and not self.location_as_usage_key + ): + raise ValidationError("Location must not be null if configuration is stored in xblock.") class Meta: app_label = 'lti_consumer' @@ -689,6 +709,16 @@ class LtiAgsLineItem(models.Model): .. no_pii: """ + # LTI xblock Configuration link + # This ties the LineItem to each tool configuration + # and allows easily retrieving LTI credentials for + # API authentication. + lti_xblock_config = models.ForeignKey( + LtiXBlockConfig, + on_delete=models.CASCADE, + null=True, + blank=True + ) # LTI Configuration link # This ties the LineItem to each tool configuration # and allows easily retrieving LTI credentials for @@ -723,10 +753,7 @@ class LtiAgsLineItem(models.Model): end_date_time = models.DateTimeField(blank=True, null=True) def __str__(self): - return "{} - {}".format( - self.resource_link_id, - self.label, - ) + return f"{self.resource_link_id} - {self.label}" class Meta: app_label = 'lti_consumer' @@ -811,11 +838,9 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) def __str__(self): - return "LineItem {line_item_id}: score {score_given} out of {score_maximum} - {grading_progress}".format( - line_item_id=self.line_item.id, - score_given=self.score_given, - score_maximum=self.score_maximum, - grading_progress=self.grading_progress + return ( + f"LineItem {self.line_item.id}: score {self.score_given} out of {self.score_maximum} -" + f" {self.grading_progress}" ) class Meta: @@ -833,6 +858,12 @@ class LtiDlContentItem(models.Model): .. no_pii: """ + lti_xblock_config = models.ForeignKey( + LtiXBlockConfig, + on_delete=models.CASCADE, + null=True, + blank=True + ) # LTI Configuration link # This ties the LineItem to each tool configuration # and allows easily retrieving LTI credentials for @@ -868,10 +899,7 @@ class LtiDlContentItem(models.Model): attributes = JSONField() def __str__(self): - return "{}: {}".format( - self.lti_configuration, - self.content_type, - ) + return f"{self.lti_configuration}: {self.content_type}" class Meta: app_label = 'lti_consumer' diff --git a/lti_consumer/plugin/compat.py b/lti_consumer/plugin/compat.py index dee1170a..6ea2ab59 100644 --- a/lti_consumer/plugin/compat.py +++ b/lti_consumer/plugin/compat.py @@ -7,11 +7,10 @@ from django.conf import settings from django.core.exceptions import ValidationError from django.forms import ModelForm -from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.keys import CourseKey, UsageKey from lti_consumer.exceptions import LtiError - log = logging.getLogger(__name__) @@ -100,15 +99,15 @@ def get_external_multiple_launch_urls_waffle_flag(): # pragma: nocover return CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.{ENABLE_EXTERNAL_MULTIPLE_LAUNCH_URLS}', __name__) -def load_enough_xblock(location): # pragma: nocover +def load_enough_xblock(location: UsageKey): # pragma: nocover """ Load enough of an xblock to read from for LTI values stored on the block. The block may or may not be bound to the user for actual use depending on what has happened in the request so far. """ # pylint: disable=import-error,import-outside-toplevel - from xmodule.modulestore.django import modulestore from openedx.core.djangoapps.xblock import api as xblock_api + from xmodule.modulestore.django import modulestore # Retrieve course block from modulestore if isinstance(location.context_key, CourseKey): @@ -118,12 +117,31 @@ def load_enough_xblock(location): # pragma: nocover return xblock_api.load_block(location, None) +def save_xblock(block): # pragma: nocover + """ + Load enough of an xblock to read from for LTI values stored on the block. + The block may or may not be bound to the user for actual use depending on + what has happened in the request so far. + """ + # pylint: disable=import-error,import-outside-toplevel + from openedx.core.djangoapps.xblock import api as xblock_api + from xmodule.modulestore.django import modulestore + + # Save course block using modulestore + if isinstance(block.scope_ids.usage_id.context_key, CourseKey): + return modulestore().update_item(block, None) + # Save library block using the XBlock API + else: + runtime = xblock_api.get_runtime(None) + return runtime.save_block(block) + + def load_block_as_user(location): # pragma: nocover """ Load a block as the current user, or load as the anonymous user if no user is available. """ # pylint: disable=import-error,import-outside-toplevel - from crum import get_current_user, get_current_request + from crum import get_current_request, get_current_user from lms.djangoapps.courseware.block_render import get_block_for_descriptor # Retrieve block from modulestore @@ -335,3 +353,27 @@ def nrps_pii_disallowed(): """ return (hasattr(settings, 'LTI_NRPS_DISALLOW_PII') and settings.LTI_NRPS_DISALLOW_PII is True) + + +def get_signal_handler(): + """ + Import the signal handler from LMS + """ + try: + # pylint: disable=import-outside-toplevel + from xmodule.modulestore.django import SignalHandler + return SignalHandler + except ImportError: + return None + + +def yield_dynamic_block_descendants(block, user_id): + """ + Import and run `yield_dynamic_block_descendants` from LMS + """ + try: + # pylint: disable=import-outside-toplevel,redefined-outer-name + from common.djangoapps.util.block_utils import yield_dynamic_block_descendants + return yield_dynamic_block_descendants(block, user_id) + except ImportError: + return None diff --git a/lti_consumer/plugin/views.py b/lti_consumer/plugin/views.py index 2f753bf3..aa3873dc 100644 --- a/lti_consumer/plugin/views.py +++ b/lti_consumer/plugin/views.py @@ -3,7 +3,7 @@ """ import logging import sys -import urllib +import urllib.parse import jwt from django.conf import settings @@ -25,32 +25,53 @@ from rest_framework.response import Response from rest_framework.status import HTTP_200_OK, HTTP_400_BAD_REQUEST, HTTP_403_FORBIDDEN, HTTP_404_NOT_FOUND -from lti_consumer.api import get_lti_pii_sharing_state_for_course, validate_lti_1p3_launch_data -from lti_consumer.exceptions import LtiError, ExternalConfigurationNotFound +from lti_consumer.api import ( + get_lti_config_by_location, + get_lti_pii_sharing_state_for_course, + validate_lti_1p3_launch_data, +) +from lti_consumer.exceptions import ExternalConfigurationNotFound, LtiError +from lti_consumer.filters import get_external_config_from_filter from lti_consumer.lti_1p3.consumer import LtiConsumer1p3, LtiProctoringConsumer -from lti_consumer.lti_1p3.exceptions import (BadJwtSignature, InvalidClaimValue, Lti1p3Exception, - LtiDeepLinkingContentTypeNotSupported, MalformedJwtToken, - MissingRequiredClaim, NoSuitableKeys, TokenSignatureExpired, - UnknownClientId, UnsupportedGrantType) +from lti_consumer.lti_1p3.exceptions import ( + BadJwtSignature, + InvalidClaimValue, + Lti1p3Exception, + LtiDeepLinkingContentTypeNotSupported, + MalformedJwtToken, + MissingRequiredClaim, + NoSuitableKeys, + TokenSignatureExpired, + UnknownClientId, + UnsupportedGrantType, +) from lti_consumer.lti_1p3.extensions.rest_framework.authentication import Lti1p3ApiAuthentication from lti_consumer.lti_1p3.extensions.rest_framework.constants import LTI_DL_CONTENT_TYPE_SERIALIZER_MAP from lti_consumer.lti_1p3.extensions.rest_framework.parsers import LineItemParser, LineItemScoreParser -from lti_consumer.lti_1p3.extensions.rest_framework.permissions import (LtiAgsPermissions, - LtiNrpsContextMembershipsPermissions) -from lti_consumer.lti_1p3.extensions.rest_framework.renderers import (LineItemRenderer, LineItemResultsRenderer, - LineItemScoreRenderer, LineItemsRenderer, - MembershipResultRenderer) -from lti_consumer.lti_1p3.extensions.rest_framework.serializers import (LtiAgsLineItemSerializer, - LtiAgsResultSerializer, LtiAgsScoreSerializer, - LtiNrpsContextMembershipBasicSerializer, - LtiNrpsContextMembershipPIISerializer) +from lti_consumer.lti_1p3.extensions.rest_framework.permissions import ( + LtiAgsPermissions, + LtiNrpsContextMembershipsPermissions, +) +from lti_consumer.lti_1p3.extensions.rest_framework.renderers import ( + LineItemRenderer, + LineItemResultsRenderer, + LineItemScoreRenderer, + LineItemsRenderer, + MembershipResultRenderer, +) +from lti_consumer.lti_1p3.extensions.rest_framework.serializers import ( + LtiAgsLineItemSerializer, + LtiAgsResultSerializer, + LtiAgsScoreSerializer, + LtiNrpsContextMembershipBasicSerializer, + LtiNrpsContextMembershipPIISerializer, +) from lti_consumer.lti_1p3.extensions.rest_framework.utils import IgnoreContentNegotiation -from lti_consumer.models import LtiAgsLineItem, LtiConfiguration, LtiDlContentItem +from lti_consumer.models import LtiAgsLineItem, LtiConfiguration, LtiDlContentItem, LtiXBlockConfig 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, get_lti_api_base -from lti_consumer.filters import get_external_config_from_filter log = logging.getLogger(__name__) @@ -217,16 +238,22 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume ) config_id = launch_data.config_id + location = launch_data.resource_link_id try: - lti_config = LtiConfiguration.objects.get( - config_id=config_id + lti_xblock_config = get_lti_config_by_location( + location, + lti_configuration__config_id=config_id, + ) + except LtiXBlockConfig.DoesNotExist as exc: + log.error( + "Invalid config_id '%s' and resource_link_id '%s' for LTI 1.3 Launch callback", + config_id, + location, ) - except (LtiConfiguration.DoesNotExist, ValidationError) as exc: - log.error("Invalid config_id '%s' for LTI 1.3 Launch callback", config_id) raise Http404 from exc - if lti_config.version != LtiConfiguration.LTI_1P3: - error_msg = f"The LTI Version of the following configuration is not LTI 1.3: {lti_config}" + if lti_xblock_config.lti_configuration.version != LtiConfiguration.LTI_1P3: + error_msg = f"The LTI Version of the following configuration is not LTI 1.3: {lti_xblock_config}" log.error(error_msg) return render( request, @@ -238,7 +265,7 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume context = {} try: - lti_consumer = lti_config.get_lti_consumer() + lti_consumer = lti_xblock_config.get_lti_consumer() # Set sub and roles claims. user_id = launch_data.external_user_id if launch_data.external_user_id else launch_data.user_id @@ -315,7 +342,7 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume # different parameters, set by instructors when running the DL configuration flow. elif deep_linking_content_item_id and lti_consumer.dl: # Retrieve Deep Linking parameters using the parameter. - content_item = lti_config.ltidlcontentitem_set.get(pk=deep_linking_content_item_id) + content_item = lti_xblock_config.lti_configuration.ltidlcontentitem_set.get(pk=deep_linking_content_item_id) # Only filter DL content item from content item set in the same LTI configuration. # This avoids a malicious user to input a random LTI id and perform LTI DL # content launches outside the scope of its configuration. @@ -361,7 +388,7 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume ) }) event = { - 'lti_version': lti_config.version, + 'lti_version': lti_xblock_config.lti_configuration.version, 'user_roles': user_role, 'launch_url': context['launch_url'] } @@ -425,12 +452,16 @@ def access_token_endpoint( version = None lti_consumer = None if usage_id: - lti_config = LtiConfiguration.objects.get(location=UsageKey.from_string(usage_id)) - version = lti_config.version + lti_config = LtiXBlockConfig.objects.get(location=UsageKey.from_string(usage_id)) + version = lti_config.lti_configuration.version lti_consumer = lti_config.get_lti_consumer() elif lti_config_id: - lti_config = LtiConfiguration.objects.get(config_id=lti_config_id) - version = lti_config.version + # It doesn't matter if multiple xblock refer to same configuration, we just need to find the first one + # as the access_token generation does not depend of any xblock data. + lti_config = LtiXBlockConfig.objects.filter(lti_configuration__config_id=lti_config_id).first() + if not lti_config: + raise LtiError("LTI configuration not found") + version = lti_config.lti_configuration.version lti_consumer = lti_config.get_lti_consumer() elif external_app and external_slug: lti_config = get_external_config_from_filter({}, external_id) @@ -453,7 +484,7 @@ def access_token_endpoint( tool_key=lti_config.get("lti_1p3_tool_public_key"), tool_keyset_url=lti_config.get("lti_1p3_tool_keyset_url"), ) - except (InvalidKeyError, LtiConfiguration.DoesNotExist, ExternalConfigurationNotFound) as exc: + except (InvalidKeyError, LtiXBlockConfig.DoesNotExist, ExternalConfigurationNotFound) as exc: log.info( "Error while retrieving access token for ID %s: %s", usage_id or lti_config_id or external_id, @@ -516,7 +547,7 @@ def deep_linking_response_endpoint(request, lti_config_id=None): """ try: # Retrieve LTI configuration - lti_config = LtiConfiguration.objects.get(id=lti_config_id) + lti_config = LtiXBlockConfig.objects.get(id=lti_config_id) # Get LTI consumer lti_consumer = lti_config.get_lti_consumer() @@ -536,7 +567,7 @@ def deep_linking_response_endpoint(request, lti_config_id=None): # verify and save each content item passed from the tool. with transaction.atomic(): # Erase older deep linking selection - LtiDlContentItem.objects.filter(lti_configuration=lti_config).delete() + LtiDlContentItem.objects.filter(lti_xblock_config=lti_config).delete() for content_item in content_items: content_type = content_item.get('type') @@ -553,7 +584,7 @@ def deep_linking_response_endpoint(request, lti_config_id=None): # Save content item LtiDlContentItem.objects.create( - lti_configuration=lti_config, + lti_xblock_config=lti_config, content_type=content_type, attributes=serializer.validated_data, ) @@ -563,8 +594,8 @@ def deep_linking_response_endpoint(request, lti_config_id=None): return render(request, 'html/lti-dl/dl_response_saved.html') # If LtiConfiguration doesn't exist, error with 404 status. - except LtiConfiguration.DoesNotExist as exc: - log.info("LtiConfiguration %r does not exist: %s", lti_config_id, exc) + except LtiXBlockConfig.DoesNotExist as exc: + log.info("LtiXBlockConfig %r does not exist: %s", lti_config_id, exc) raise Http404 from exc # If the deep linking content type is not supported except LtiDeepLinkingContentTypeNotSupported as exc: @@ -624,11 +655,21 @@ def deep_linking_content_endpoint(request, lti_config_id): ) try: # Get LTI Configuration - lti_config = LtiConfiguration.objects.get(id=lti_config_id) - except LtiConfiguration.DoesNotExist as exc: - log.info("LtiConfiguration %r does not exist: %s", lti_config_id, exc) + lti_config = LtiXBlockConfig.objects.get(id=lti_config_id) + except LtiXBlockConfig.DoesNotExist as exc: + log.info("LtiXBlockConfig %r does not exist: %s", lti_config_id, exc) raise Http404 from exc + # Currently, deep linking only works in xblock context + if not lti_config.location: + error_msg = f'Missing LTI location for LTI xblock Configuration {lti_config}' + log.warning(error_msg) + return render( + request, + 'html/lti_launch_error.html', + context={"error_msg": error_msg}, + status=HTTP_400_BAD_REQUEST + ) # check if user has proper access block = compat.load_block_as_user(lti_config.location) if not has_block_access(request.user, block, lti_config.location.course_key): @@ -640,7 +681,7 @@ def deep_linking_content_endpoint(request, lti_config_id): raise PermissionDenied # Get all LTI-DL contents - content_items = LtiDlContentItem.objects.filter(lti_configuration=lti_config) + content_items = LtiDlContentItem.objects.filter(lti_xblock_config=lti_config) # If no LTI-DL contents found for current configuration, throw 404 error if not content_items.exists(): @@ -685,7 +726,7 @@ class LtiAgsLineItemViewset(viewsets.ModelViewSet): ] def get_queryset(self): - lti_configuration = self.request.lti_configuration + lti_xblock_config = self.request.lti_xblock_config # Return all LineItems related to the LTI configuration. # TODO: @@ -693,12 +734,12 @@ def get_queryset(self): # to each resource link (block), and this filter needs # improved once we start reusing LTI configurations. return LtiAgsLineItem.objects.filter( - lti_configuration=lti_configuration + lti_xblock_config=lti_xblock_config ) def perform_create(self, serializer): - lti_configuration = self.request.lti_configuration - serializer.save(lti_configuration=lti_configuration) + lti_xblock_config = self.request.lti_xblock_config + serializer.save(lti_xblock_config=lti_xblock_config) @action( detail=True, @@ -819,7 +860,7 @@ def get_serializer_class(self): Checks if PII fields can be exposed and returns appropiate serializer. """ if (not compat.nrps_pii_disallowed() and - get_lti_pii_sharing_state_for_course(self.request.lti_configuration.location.course_key)): + get_lti_pii_sharing_state_for_course(self.request.lti_xblock_config.location.course_key)): return LtiNrpsContextMembershipPIISerializer else: return LtiNrpsContextMembershipBasicSerializer @@ -831,7 +872,7 @@ def list(self, *args, **kwargs): """ # get course key - course_key = self.request.lti_configuration.location.course_key + course_key = self.request.lti_xblock_config.location.course_key try: data = compat.get_course_members(course_key) @@ -883,8 +924,11 @@ def start_proctoring_assessment_endpoint(request): resource_link_id = decoded_jwt.get('https://purl.imsglobal.org/spec/lti/claim/resource_link', {}).get('id') try: - lti_config = LtiConfiguration.objects.get(lti_1p3_client_id=iss) - except LtiConfiguration.DoesNotExist: + lti_config = get_lti_config_by_location( + resource_link_id, + lti_configuration__lti_1p3_client_id=iss + ) + except LtiXBlockConfig.DoesNotExist: log.error("Invalid iss claim '%s' for LTI 1.3 Proctoring Services start_proctoring_assessment_endpoint" " callback", iss) return render(request, 'html/lti_proctoring_start_error.html', status=HTTP_404_NOT_FOUND) @@ -892,7 +936,8 @@ def start_proctoring_assessment_endpoint(request): lti_consumer = lti_config.get_lti_consumer() if not isinstance(lti_consumer, LtiProctoringConsumer): - log.info("Proctoring Services for LTIConfiguration with config_id %s are not enabled", lti_config.config_id) + log.info("Proctoring Services for LtiXBlockConfig with config_id %s are not enabled", + lti_config.lti_configuration.config_id) return render(request, 'html/lti_proctoring_start_error.html', status=HTTP_400_BAD_REQUEST) # Grab the data we need from the cache: launch_data and session_data. @@ -908,7 +953,7 @@ def start_proctoring_assessment_endpoint(request): log.warning( f'There was a cache miss trying to fetch the launch data during an LTI 1.3 proctoring StartAssessment ' f'launch when using the cache key {launch_data_key}. The LtiConfiguration config_id is ' - f'{lti_config.config_id} the user_id is {request.user.id}.' + f'{lti_config.lti_configuration.config_id} the user_id is {request.user.id}.' ) return render(request, 'html/lti_proctoring_start_error.html', status=HTTP_400_BAD_REQUEST) @@ -945,7 +990,7 @@ def start_proctoring_assessment_endpoint(request): # for this optional claim. log.error( "An error occurred during the handling of an LtiStartAssessment LTI lauch message for LTIConfiguration " - f"with config_id {lti_config.config_id} and resource_link_id {resource_link_id}. The " + f"with config_id {lti_config.lti_configuration.config_id} and resource_link_id {resource_link_id}. The " "end_assessment_return Tool JWT claim is not a boolean value. An LtiEndAssessment LTI launch message " "will not be sent as part of the end assessment workflow." ) diff --git a/lti_consumer/signals/signals.py b/lti_consumer/signals/signals.py index c2bfedc2..16c49873 100644 --- a/lti_consumer/signals/signals.py +++ b/lti_consumer/signals/signals.py @@ -4,13 +4,15 @@ import logging from django.db.models.signals import post_save -from django.dispatch import receiver, Signal +from django.dispatch import Signal, receiver +from openedx_events.content_authoring.data import LibraryBlockData, XBlockData +from openedx_events.content_authoring.signals import LIBRARY_BLOCK_DELETED, XBLOCK_DELETED -from lti_consumer.models import LtiAgsScore +from lti_consumer.models import LtiAgsScore, LtiConfiguration, LtiXBlockConfig from lti_consumer.plugin import compat - log = logging.getLogger(__name__) +SignalHandler = compat.get_signal_handler() @receiver(post_save, sender=LtiAgsScore, dispatch_uid='publish_grade_on_score_update') @@ -83,3 +85,63 @@ def publish_grade_on_score_update(sender, instance, **kwargs): # pylint: disabl LTI_1P3_PROCTORING_ASSESSMENT_STARTED = Signal() + + +@receiver(SignalHandler.pre_item_deleted if SignalHandler else []) +def delete_child_lti_configurations(**kwargs): + """ + Delete lti configurtion from database for this block children. + """ + usage_key = kwargs.get('usage_key') + if usage_key: + # Strip branch info + usage_key = usage_key.for_branch(None) + try: + deleted_block = compat.load_enough_xblock(usage_key) + except Exception as e: # pylint: disable=broad-exception-caught + log.warning(f"Cannot find xblock for key {usage_key}. Reason: {str(e)}. ") + return + id_list = {str(deleted_block.location)} + for block in compat.yield_dynamic_block_descendants(deleted_block, kwargs.get('user_id')): + id_list.add(str(block.location)) + + LtiXBlockConfig.objects.filter( + location__in=id_list + ).delete() + log.info(f"Deleted {len(id_list)} lti xblock configurations in modulestore") + result = LtiConfiguration.objects.filter(ltixblockconfig__isnull=True).delete() + log.info(f"Deleted {result} lti configuration objects in library") + + +@receiver(XBLOCK_DELETED) +def delete_lti_configuration(**kwargs): + """ + Delete lti configurtion from database for this block. + """ + xblock_info = kwargs.get("xblock_info", None) + if not xblock_info or not isinstance(xblock_info, XBlockData): + log.error("Received null or incorrect data for event") + return + + LtiXBlockConfig.objects.filter( + location=str(xblock_info.usage_key) + ).delete() + result = LtiConfiguration.objects.filter(ltixblockconfig__isnull=True).delete() + log.info(f"Deleted {result} lti configuration objects in library") + + +@receiver(LIBRARY_BLOCK_DELETED) +def delete_lib_lti_configuration(**kwargs): + """ + Delete lti configurtion from database for this library block. + """ + library_block = kwargs.get("library_block", None) + if not library_block or not isinstance(library_block, LibraryBlockData): + log.error("Received null or incorrect data for event") + return + + LtiXBlockConfig.objects.filter( + location=str(library_block.usage_key) + ).delete() + result = LtiConfiguration.objects.filter(ltixblockconfig__isnull=True).delete() + log.info(f"Deleted {result} lti configuration objects in library") diff --git a/lti_consumer/tests/unit/plugin/test_proctoring.py b/lti_consumer/tests/unit/plugin/test_proctoring.py index 4cd5ea3b..c2bd82e7 100644 --- a/lti_consumer/tests/unit/plugin/test_proctoring.py +++ b/lti_consumer/tests/unit/plugin/test_proctoring.py @@ -11,11 +11,16 @@ from edx_django_utils.cache import TieredCache, get_cache_key from lti_consumer.data import Lti1p3LaunchData, Lti1p3ProctoringLaunchData -from lti_consumer.lti_1p3.exceptions import (BadJwtSignature, InvalidClaimValue, MalformedJwtToken, - MissingRequiredClaim, NoSuitableKeys) +from lti_consumer.lti_1p3.exceptions import ( + BadJwtSignature, + InvalidClaimValue, + MalformedJwtToken, + MissingRequiredClaim, + NoSuitableKeys, +) from lti_consumer.lti_1p3.key_handlers import PlatformKeyHandler from lti_consumer.models import LtiConfiguration -from lti_consumer.utils import get_data_from_cache +from lti_consumer.utils import CONFIG_ON_DB, get_data_from_cache @ddt.ddt @@ -34,7 +39,7 @@ def setUp(self): self.lti_config = LtiConfiguration.objects.create( version=LtiConfiguration.LTI_1P3, lti_1p3_proctoring_enabled=True, - config_store=LtiConfiguration.CONFIG_ON_DB, + config_store=CONFIG_ON_DB, ) # Set up cached data necessary for this endpoint: launch_data and session_data. diff --git a/lti_consumer/tests/unit/plugin/test_views.py b/lti_consumer/tests/unit/plugin/test_views.py index 8a197a4c..af31ba39 100644 --- a/lti_consumer/tests/unit/plugin/test_views.py +++ b/lti_consumer/tests/unit/plugin/test_views.py @@ -2,32 +2,32 @@ Tests for LTI 1.3 endpoint views. """ import json -from unittest.mock import patch, Mock +from unittest.mock import Mock, patch import ddt import jwt +from Cryptodome.PublicKey import RSA from django.test.testcases import TestCase from django.urls import reverse from edx_django_utils.cache import TieredCache, get_cache_key from jwt.api_jwk import PyJWK - -from Cryptodome.PublicKey import RSA from opaque_keys.edx.keys import UsageKey + from lti_consumer.data import Lti1p3LaunchData, Lti1p3ProctoringLaunchData -from lti_consumer.models import LtiConfiguration, LtiDlContentItem from lti_consumer.lti_1p3.exceptions import ( - MissingRequiredClaim, MalformedJwtToken, - TokenSignatureExpired, + MissingRequiredClaim, NoSuitableKeys, + PreflightRequestValidationFailure, + TokenSignatureExpired, UnknownClientId, UnsupportedGrantType, - PreflightRequestValidationFailure, ) -from lti_consumer.lti_xblock import LtiConsumerXBlock from lti_consumer.lti_1p3.tests.utils import create_jwt +from lti_consumer.lti_xblock import LtiConsumerXBlock +from lti_consumer.models import LtiConfiguration, LtiDlContentItem from lti_consumer.tests.test_utils import make_xblock -from lti_consumer.utils import cache_lti_1p3_launch_data +from lti_consumer.utils import CONFIG_ON_DB, CONFIG_ON_XBLOCK, cache_lti_1p3_launch_data @ddt.ddt @@ -45,7 +45,7 @@ def setUp(self): # Set up LTI Configuration self.lti_config = LtiConfiguration.objects.create( version=LtiConfiguration.LTI_1P3, - config_store=LtiConfiguration.CONFIG_ON_XBLOCK, + config_store=CONFIG_ON_XBLOCK, location=UsageKey.from_string(self.location) ) @@ -150,7 +150,7 @@ def setUp(self): self.config = LtiConfiguration( version=LtiConfiguration.LTI_1P3, location=self.location, - config_store=LtiConfiguration.CONFIG_ON_DB, + config_store=CONFIG_ON_DB, lti_1p3_redirect_uris=["https://tool.example", "http://tool.example/launch"] ) self.config.save() @@ -382,7 +382,7 @@ def _setup_deep_linking(self, user_role='staff'): """ Set up deep linking for data and mocking for testing. """ - self.config.config_store = LtiConfiguration.CONFIG_ON_XBLOCK + self.config.config_store = CONFIG_ON_XBLOCK self.config.save() self.compat.get_user_role.return_value = user_role @@ -431,7 +431,7 @@ def test_launch_callback_endpoint_deep_linking_database_config(self, dl_enabled) LtiConfiguration.objects.filter(id=self.config.id).update( location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3, - config_store=LtiConfiguration.CONFIG_ON_DB, + config_store=CONFIG_ON_DB, lti_advantage_deep_linking_enabled=dl_enabled, lti_advantage_deep_linking_launch_url=url, ) @@ -669,8 +669,8 @@ class TestLti1p3AccessTokenEndpoint(TestCase): def setUp(self): super().setUp() - location = 'block-v1:course+test+2020+type@problem+block@test' - self.config = LtiConfiguration(version=LtiConfiguration.LTI_1P3, location=location) + self.location = 'block-v1:course+test+2020+type@problem+block@test' + self.config = LtiConfiguration(version=LtiConfiguration.LTI_1P3) self.config.save() self.url = reverse('lti_consumer:lti_consumer.access_token', args=[str(self.config.config_id)]) # Patch settings calls to LMS method @@ -729,7 +729,7 @@ def test_access_token_endpoint_with_location_in_url(self): url = reverse( 'lti_consumer:lti_consumer.access_token_via_location', - args=[str(self.config.location)] + args=[str(self.location)] ) body = self.get_body(create_jwt(self.key, {})) response = self.client.post(url, data=json.dumps(body), content_type='application/json') diff --git a/lti_consumer/tests/unit/test_api.py b/lti_consumer/tests/unit/test_api.py index bbd18f94..d7659eb0 100644 --- a/lti_consumer/tests/unit/test_api.py +++ b/lti_consumer/tests/unit/test_api.py @@ -3,19 +3,19 @@ """ from unittest.mock import Mock, patch from urllib.parse import parse_qs, urlparse -import ddt +import ddt from Cryptodome.PublicKey import RSA from django.test.testcases import TestCase from edx_django_utils.cache import get_cache_key from lti_consumer.api import ( _get_config_by_config_id, - _get_or_create_local_lti_config, - config_id_for_block, + _get_or_create_local_lti_xblock_config, + config_for_block, + get_deep_linking_data, get_end_assessment_return, get_lti_1p3_content_url, - get_deep_linking_data, get_lti_1p3_launch_info, get_lti_1p3_launch_start_url, validate_lti_1p3_launch_data, @@ -24,7 +24,7 @@ from lti_consumer.lti_xblock import LtiConsumerXBlock from lti_consumer.models import LtiConfiguration, LtiDlContentItem from lti_consumer.tests.test_utils import make_xblock -from lti_consumer.utils import get_data_from_cache +from lti_consumer.utils import CONFIG_EXTERNAL, CONFIG_ON_DB, CONFIG_ON_XBLOCK, get_data_from_cache # it's convenient to have this in lowercase to compare to URLs _test_config_id = "6c440bf4-face-beef-face-e8bcfb1e53bd" @@ -74,7 +74,7 @@ def _setup_lti_block(self): config_id=_test_config_id, location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3, - config_store=LtiConfiguration.CONFIG_ON_XBLOCK, + config_store=CONFIG_ON_XBLOCK, ) def _get_lti_1p3_launch_data(self): @@ -103,26 +103,26 @@ def setUp(self): def test_double_fetch(self): self.xblock.config_type = 'database' - config_id = config_id_for_block(self.xblock) + config_id = config_for_block(self.xblock) self.assertIsNotNone(config_id) config = _get_config_by_config_id(config_id) - self.assertEqual(LtiConfiguration.CONFIG_ON_DB, config.config_store) + self.assertEqual(CONFIG_ON_DB, config.config_store) # fetch again, shouldn't make a new one - second_config_id = config_id_for_block(self.xblock) + second_config_id = config_for_block(self.xblock) self.assertEqual(config_id, second_config_id) @ddt.data( - ('external', LtiConfiguration.CONFIG_EXTERNAL), - ('database', LtiConfiguration.CONFIG_ON_DB), - ('any other val', LtiConfiguration.CONFIG_ON_XBLOCK), + ('external', CONFIG_EXTERNAL), + ('database', CONFIG_ON_DB), + ('any other val', CONFIG_ON_XBLOCK), ) @patch('lti_consumer.api.get_external_config_from_filter') def test_store_types(self, mapping_pair, mock_external_config): mock_external_config.return_value = {"version": LtiConfiguration.LTI_1P3} str_store, result_store = mapping_pair self.xblock.config_type = str_store - config_id = config_id_for_block(self.xblock) + config_id = config_for_block(self.xblock) self.assertIsNotNone(config_id) config = _get_config_by_config_id(config_id) self.assertEqual(result_store, config.config_store) @@ -144,7 +144,7 @@ def test_create_lti_config_if_inexistent(self): self.assertEqual(LtiConfiguration.objects.all().count(), 0) # Call API - lti_config = _get_or_create_local_lti_config( + lti_config = _get_or_create_local_lti_xblock_config( lti_version=lti_version, block_location=location ) @@ -152,7 +152,7 @@ def test_create_lti_config_if_inexistent(self): # Check if the object was created self.assertEqual(lti_config.version, lti_version) self.assertEqual(str(lti_config.location), location) - self.assertEqual(lti_config.config_store, LtiConfiguration.CONFIG_ON_XBLOCK) + self.assertEqual(lti_config.config_store, CONFIG_ON_XBLOCK) def test_retrieve_existing(self): """ @@ -166,7 +166,7 @@ def test_retrieve_existing(self): ) # Call API - lti_config_retrieved = _get_or_create_local_lti_config( + lti_config_retrieved = _get_or_create_local_lti_xblock_config( lti_version=lti_version, block_location=location ) @@ -187,7 +187,7 @@ def test_update_lti_version(self): ) # Call API - _get_or_create_local_lti_config( + _get_or_create_local_lti_xblock_config( lti_version=LtiConfiguration.LTI_1P3, block_location=location ) @@ -196,7 +196,7 @@ def test_update_lti_version(self): lti_config.refresh_from_db() self.assertEqual(lti_config.version, LtiConfiguration.LTI_1P3) - @ddt.data(LtiConfiguration.CONFIG_ON_XBLOCK, LtiConfiguration.CONFIG_EXTERNAL, LtiConfiguration.CONFIG_ON_DB) + @ddt.data(CONFIG_ON_XBLOCK, CONFIG_EXTERNAL, CONFIG_ON_DB) def test_create_lti_config_config_store(self, config_store): """ Check if the config_store parameter to _get_or_create_local_lti_config is used to change @@ -204,7 +204,7 @@ def test_create_lti_config_config_store(self, config_store): """ location = 'block-v1:course+test+2020+type@problem+block@test' lti_version = LtiConfiguration.LTI_1P3 - lti_config = _get_or_create_local_lti_config( + lti_config = _get_or_create_local_lti_xblock_config( lti_version=lti_version, block_location=location, config_store=config_store, @@ -222,11 +222,11 @@ def test_external_config_values_are_cleared(self): lti_config = LtiConfiguration.objects.create( location=location, version=LtiConfiguration.LTI_1P3, - config_store=LtiConfiguration.CONFIG_EXTERNAL, + config_store=CONFIG_EXTERNAL, external_id="test_plugin:test-id" ) - _get_or_create_local_lti_config( + _get_or_create_local_lti_xblock_config( lti_version=lti_version, block_location=location, external_id=None @@ -235,7 +235,7 @@ def test_external_config_values_are_cleared(self): lti_config.refresh_from_db() self.assertEqual(lti_config.version, lti_version) self.assertEqual(str(lti_config.location), location) - self.assertEqual(lti_config.config_store, LtiConfiguration.CONFIG_ON_XBLOCK) + self.assertEqual(lti_config.config_store, CONFIG_ON_XBLOCK) self.assertEqual(lti_config.external_id, None) @@ -553,7 +553,7 @@ def test_launch_info_for_lti_config_with_external_configuration( lti_config = LtiConfiguration.objects.create( version=LtiConfiguration.LTI_1P3, config_id=_test_config_id, - config_store=LtiConfiguration.CONFIG_EXTERNAL, + config_store=CONFIG_EXTERNAL, external_id=external_id, ) diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py index ab81ff95..4923888c 100644 --- a/lti_consumer/tests/unit/test_lti_xblock.py +++ b/lti_consumer/tests/unit/test_lti_xblock.py @@ -18,7 +18,7 @@ from jwt.api_jwk import PyJWK, PyJWKSet from xblock.validation import Validation -from lti_consumer.api import config_id_for_block +from lti_consumer.api import config_for_block from lti_consumer.data import Lti1p3LaunchData from lti_consumer.exceptions import LtiError from lti_consumer.lti_1p3.tests.utils import create_jwt @@ -1894,7 +1894,7 @@ def test_get_lti_1p3_launch_data( expected_launch_data_kwargs = { "user_id": 1, "user_role": "instructor", - "config_id": config_id_for_block(self.xblock), + "config_id": config_for_block(self.xblock), "resource_link_id": str(self.xblock.scope_ids.usage_id), "external_user_id": "external_user_id", "launch_presentation_document_target": "iframe", diff --git a/lti_consumer/tests/unit/test_models.py b/lti_consumer/tests/unit/test_models.py index bab3504f..a244393f 100644 --- a/lti_consumer/tests/unit/test_models.py +++ b/lti_consumer/tests/unit/test_models.py @@ -3,20 +3,25 @@ """ from contextlib import contextmanager from datetime import datetime, timedelta, timezone -from unittest.mock import patch, call +from unittest.mock import patch import ddt from Cryptodome.PublicKey import RSA from django.core.exceptions import ValidationError from django.test.testcases import TestCase from edx_django_utils.cache import RequestCache -from ccx_keys.locator import CCXBlockUsageLocator from opaque_keys.edx.locator import CourseLocator from lti_consumer.lti_xblock import LtiConsumerXBlock -from lti_consumer.models import (CourseAllowPIISharingInLTIFlag, LtiAgsLineItem, LtiAgsScore, LtiConfiguration, - LtiDlContentItem) +from lti_consumer.models import ( + CourseAllowPIISharingInLTIFlag, + LtiAgsLineItem, + LtiAgsScore, + LtiConfiguration, + LtiDlContentItem, +) from lti_consumer.tests.test_utils import make_xblock +from lti_consumer.utils import CONFIG_EXTERNAL, CONFIG_ON_DB, CONFIG_ON_XBLOCK LAUNCH_URL = 'http://tool.example/launch' DEEP_LINK_URL = 'http://tool.example/deep-link/launch' @@ -68,20 +73,20 @@ def setUp(self): self.lti_1p3_config_db = LtiConfiguration.objects.create( location=self.xblock.scope_ids.usage_id, version=LtiConfiguration.LTI_1P3, - config_store=LtiConfiguration.CONFIG_ON_DB, + config_store=CONFIG_ON_DB, lti_advantage_ags_mode='programmatic', lti_advantage_deep_linking_enabled=True, ) self.lti_1p3_config_external = LtiConfiguration.objects.create( version=LtiConfiguration.LTI_1P3, - config_store=LtiConfiguration.CONFIG_EXTERNAL, + config_store=CONFIG_EXTERNAL, location=self.xblock.scope_ids.usage_id, ) self.lti_1p1_external = LtiConfiguration.objects.create( version=LtiConfiguration.LTI_1P1, - config_store=LtiConfiguration.CONFIG_EXTERNAL, + config_store=CONFIG_EXTERNAL, external_id="test-external-id" ) @@ -149,9 +154,9 @@ def test_repr(self): ) @ddt.data( - LtiConfiguration.CONFIG_ON_XBLOCK, - LtiConfiguration.CONFIG_ON_DB, - LtiConfiguration.CONFIG_EXTERNAL, + CONFIG_ON_XBLOCK, + CONFIG_ON_DB, + CONFIG_EXTERNAL, ) @patch('lti_consumer.models.get_external_config_from_filter') @patch('lti_consumer.models.external_multiple_launch_urls_enabled') @@ -186,9 +191,9 @@ def test_lti_consumer_ags_enabled(self, config_store, external_multiple_launch_u ) @ddt.data( - {'config_store': LtiConfiguration.CONFIG_ON_XBLOCK, 'expected_value': 'XBlock'}, - {'config_store': LtiConfiguration.CONFIG_ON_DB, 'expected_value': 'disabled'}, - {'config_store': LtiConfiguration.CONFIG_EXTERNAL, 'expected_value': 'external'}, + {'config_store': CONFIG_ON_XBLOCK, 'expected_value': 'XBlock'}, + {'config_store': CONFIG_ON_DB, 'expected_value': 'disabled'}, + {'config_store': CONFIG_EXTERNAL, 'expected_value': 'external'}, ) @ddt.unpack @patch('lti_consumer.models.get_external_config_from_filter') @@ -204,9 +209,9 @@ def test_get_lti_advantage_ags_mode(self, filter_mock, config_store, expected_va self.assertEqual(config.get_lti_advantage_ags_mode(), expected_value) @ddt.data( - LtiConfiguration.CONFIG_ON_XBLOCK, - LtiConfiguration.CONFIG_ON_DB, - LtiConfiguration.CONFIG_EXTERNAL, + CONFIG_ON_XBLOCK, + CONFIG_ON_DB, + CONFIG_EXTERNAL, ) @patch('lti_consumer.models.get_external_config_from_filter') @patch('lti_consumer.models.external_multiple_launch_urls_enabled') @@ -245,9 +250,9 @@ def test_lti_consumer_ags_declarative(self, config_store, external_multiple_laun ) @ddt.data( - LtiConfiguration.CONFIG_ON_XBLOCK, - LtiConfiguration.CONFIG_ON_DB, - LtiConfiguration.CONFIG_EXTERNAL, + CONFIG_ON_XBLOCK, + CONFIG_ON_DB, + CONFIG_EXTERNAL, ) @patch('lti_consumer.models.get_external_config_from_filter') @patch('lti_consumer.models.external_multiple_launch_urls_enabled') @@ -269,9 +274,9 @@ def test_lti_consumer_deep_linking_enabled(self, config_store, external_multiple self.assertTrue(consumer.dl) @ddt.data( - {'config_store': LtiConfiguration.CONFIG_ON_XBLOCK, 'expected_value': False}, - {'config_store': LtiConfiguration.CONFIG_ON_DB, 'expected_value': True}, - {'config_store': LtiConfiguration.CONFIG_EXTERNAL, 'expected_value': True}, + {'config_store': CONFIG_ON_XBLOCK, 'expected_value': False}, + {'config_store': CONFIG_ON_DB, 'expected_value': True}, + {'config_store': CONFIG_EXTERNAL, 'expected_value': True}, ) @ddt.unpack @patch('lti_consumer.models.get_external_config_from_filter') @@ -287,9 +292,9 @@ def test_get_lti_advantage_deep_linking_enabled(self, filter_mock, config_store, self.assertEqual(config.get_lti_advantage_deep_linking_enabled(), expected_value) @ddt.data( - {'config_store': LtiConfiguration.CONFIG_ON_XBLOCK, 'expected_value': 'XBlock'}, - {'config_store': LtiConfiguration.CONFIG_ON_DB, 'expected_value': 'database'}, - {'config_store': LtiConfiguration.CONFIG_EXTERNAL, 'expected_value': 'external'}, + {'config_store': CONFIG_ON_XBLOCK, 'expected_value': 'XBlock'}, + {'config_store': CONFIG_ON_DB, 'expected_value': 'database'}, + {'config_store': CONFIG_EXTERNAL, 'expected_value': 'external'}, ) @ddt.unpack @patch('lti_consumer.models.get_external_config_from_filter') @@ -305,9 +310,9 @@ def test_get_lti_advantage_deep_linking_launch_url(self, filter_mock, config_sto self.assertEqual(config.get_lti_advantage_deep_linking_launch_url(), expected_value) @ddt.data( - {'config_store': LtiConfiguration.CONFIG_ON_XBLOCK, 'expected_value': False}, - {'config_store': LtiConfiguration.CONFIG_ON_DB, 'expected_value': True}, - {'config_store': LtiConfiguration.CONFIG_EXTERNAL, 'expected_value': True}, + {'config_store': CONFIG_ON_XBLOCK, 'expected_value': False}, + {'config_store': CONFIG_ON_DB, 'expected_value': True}, + {'config_store': CONFIG_EXTERNAL, 'expected_value': True}, ) @ddt.unpack @patch('lti_consumer.models.get_external_config_from_filter') @@ -328,7 +333,7 @@ def test_generate_private_key(self): """ lti_config = LtiConfiguration.objects.create( version=LtiConfiguration.LTI_1P3, - config_store=LtiConfiguration.CONFIG_ON_XBLOCK, + config_store=CONFIG_ON_XBLOCK, location='block-v1:course+test+2020+type@problem+block@test' ) @@ -351,7 +356,7 @@ def test_generate_public_key_only(self): """ lti_config = LtiConfiguration.objects.create( version=LtiConfiguration.LTI_1P3, - config_store=LtiConfiguration.CONFIG_ON_XBLOCK, + config_store=CONFIG_ON_XBLOCK, location='block-v1:course+test+2020+type@problem+block@test' ) # Create and retrieve public keys @@ -365,13 +370,13 @@ def test_generate_public_key_only(self): self.assertEqual(regenerated_public_key, public_key) def test_clean(self): - self.lti_1p3_config.config_store = self.lti_1p3_config.CONFIG_ON_XBLOCK + self.lti_1p3_config.config_store = CONFIG_ON_XBLOCK self.lti_1p3_config.location = None with self.assertRaises(ValidationError): self.lti_1p3_config.clean() - self.lti_1p3_config.config_store = self.lti_1p3_config.CONFIG_EXTERNAL + self.lti_1p3_config.config_store = CONFIG_EXTERNAL self.lti_1p3_config.external_id = None with self.assertRaises(ValidationError): @@ -382,7 +387,7 @@ def test_clean(self): with self.assertRaises(ValidationError): self.lti_1p3_config.clean() - self.lti_1p3_config.config_store = self.lti_1p3_config.CONFIG_ON_DB + self.lti_1p3_config.config_store = CONFIG_ON_DB self.lti_1p3_config_db.lti_1p3_tool_keyset_url = '' self.lti_1p3_config_db.lti_1p3_tool_public_key = '' @@ -393,7 +398,7 @@ def test_clean(self): self.lti_1p3_config.lti_1p3_proctoring_enabled = True self.lti_1p3_config.external_id = 'test_id' - for config_store in [self.lti_1p3_config.CONFIG_ON_XBLOCK, self.lti_1p3_config.CONFIG_EXTERNAL]: + for config_store in [CONFIG_ON_XBLOCK, CONFIG_EXTERNAL]: self.lti_1p3_config.config_store = config_store with self.assertRaises(ValidationError): self.lti_1p3_config.clean() @@ -412,7 +417,9 @@ def test_get_redirect_uris_for_xblock_model_returns_expected( self.xblock.lti_advantage_deep_linking_launch_url = deep_link_url self.xblock.lti_1p3_redirect_uris = redirect_uris - assert self.lti_1p3_config.get_lti_1p3_redirect_uris() == expected + assert self.lti_1p3_config.get_lti_1p3_redirect_uris( + self.xblock.scope_ids.usage_id + ) == expected @ddt.data( (LAUNCH_URL, DEEP_LINK_URL, [], [LAUNCH_URL, DEEP_LINK_URL]), @@ -429,7 +436,9 @@ def test_get_redirect_uris_for_db_model_returns_expected( self.lti_1p3_config_db.lti_1p3_redirect_uris = redirect_uris self.lti_1p3_config_db.save() - assert self.lti_1p3_config_db.get_lti_1p3_redirect_uris() == expected + assert self.lti_1p3_config_db.get_lti_1p3_redirect_uris( + self.xblock.scope_ids.usage_id + ) == expected @patch('lti_consumer.models.choose_lti_1p3_redirect_uris', return_value=None) @patch('lti_consumer.models.get_external_config_from_filter') @@ -448,7 +457,9 @@ def test_get_redirect_uris_with_external_config( } get_external_config_from_filter_mock.return_value = external_config - self.assertEqual(self.lti_1p3_config_external.get_lti_1p3_redirect_uris(), None) + self.assertEqual(self.lti_1p3_config_external.get_lti_1p3_redirect_uris( + self.xblock.scope_ids.usage_id + ), None) get_external_config_from_filter_mock.assert_called_once_with({}, self.lti_1p3_config_external.external_id) choose_lti_1p3_redirect_uris.assert_called_once_with( external_config['lti_1p3_redirect_uris'], @@ -462,80 +473,6 @@ def test_save(self, sync_configurations_mock): self.assertEqual(self.lti_1p3_config.save(), None) sync_configurations_mock.assert_called_once_with() - @patch('lti_consumer.models.isinstance', return_value=True) - @patch.object(LtiConfiguration.objects, 'filter') - @patch('lti_consumer.models.model_to_dict') - @patch('lti_consumer.models.setattr') - def test_sync_configurations_with_ccx_location( - self, - setattr_mock, - model_to_dict_mock, - filter_mock, - isinstance_mock, - ): - """ - Test sync_configurations method with CCX location. - """ - model_to_dict_mock.return_value = {'test': 'test'} - self.lti_1p3_config.location = 'ccx-block-v1:course+test+2020+ccx@1+type@problem+block@test' - - self.assertEqual(self.lti_1p3_config.sync_configurations(), None) - isinstance_mock.assert_called_once_with(self.lti_1p3_config.location, CCXBlockUsageLocator) - filter_mock.assert_has_calls([ - call(location=self.lti_1p3_config.location.to_block_locator()), - call().first(), - ]) - model_to_dict_mock.assert_called_once_with( - filter_mock.return_value.first(), - ['id', 'config_id', 'location', 'external_config'], - ) - setattr_mock.assert_called_once_with(self.lti_1p3_config, 'test', 'test') - - @patch('lti_consumer.models.isinstance', return_value=False) - @patch.object(LtiConfiguration.objects, 'filter') - @patch('lti_consumer.models.model_to_dict') - def test_sync_configurations_with_location( - self, - model_to_dict_mock, - filter_mock, - isinstance_mock, - ): - """ - Test sync_configurations method with location. - """ - self.assertEqual(self.lti_1p3_config.sync_configurations(), None) - isinstance_mock.assert_called_once_with(self.lti_1p3_config.location, CCXBlockUsageLocator) - filter_mock.assert_has_calls([ - call(location__endswith=str(self.lti_1p3_config.location).split('@')[-1]), - call().filter(location__startswith=CCXBlockUsageLocator.CANONICAL_NAMESPACE), - call().filter().exclude(id=self.lti_1p3_config.pk), - call().filter().exclude().update(**model_to_dict_mock), - ]) - model_to_dict_mock.assert_called_once_with( - self.lti_1p3_config, - ['id', 'config_id', 'location', 'external_config'], - ) - - @patch('lti_consumer.models.isinstance', return_value=False) - @patch.object(LtiConfiguration.objects, 'filter', side_effect=IndexError()) - @patch('lti_consumer.models.log.exception') - def test_sync_configurations_with_invalid_location( - self, - log_exception_mock, - filter_mock, - isinstance_mock, - ): - """ - Test sync_configurations method with invalid location. - """ - self.assertEqual(self.lti_1p3_config.sync_configurations(), None) - isinstance_mock.assert_called_once_with(self.lti_1p3_config.location, CCXBlockUsageLocator) - filter_mock.assert_called_once_with(location__endswith=str(self.lti_1p3_config.location).split('@')[-1]) - log_exception_mock.assert_called_once_with( - f'Failed to query children CCX LTI configurations: ' - f'Failed to parse main LTI configuration location: {self.lti_1p3_config.location}' - ) - @patch('lti_consumer.models.get_external_config_from_filter') @patch('lti_consumer.models.external_multiple_launch_urls_enabled') def test_external_lti_consumer_1p3_returns_launch_url_from_block( @@ -608,7 +545,7 @@ def setUp(self): config_id='6c440bf4-face-beef-face-e8bcfb1e53bd', location=self.dummy_location, version=LtiConfiguration.LTI_1P3, - config_store=LtiConfiguration.CONFIG_ON_XBLOCK, + config_store=CONFIG_ON_XBLOCK, ) self.line_item = LtiAgsLineItem.objects.create( diff --git a/lti_consumer/utils.py b/lti_consumer/utils.py index e69cb00a..e8e950c9 100644 --- a/lti_consumer/utils.py +++ b/lti_consumer/utils.py @@ -8,21 +8,51 @@ from urllib.parse import urlencode from django.conf import settings -from edx_django_utils.cache import get_cache_key, TieredCache +from edx_django_utils.cache import TieredCache, get_cache_key +from lti_consumer.data import Lti1p3LaunchData +from lti_consumer.lti_1p3.constants import LTI_1P3_CONTEXT_TYPE +from lti_consumer.lti_1p3.exceptions import InvalidClaimValue, MissingRequiredClaim from lti_consumer.plugin.compat import ( - get_external_config_waffle_flag, - get_external_user_id_1p1_launches_waffle_flag, get_database_config_waffle_flag, + get_external_config_waffle_flag, get_external_multiple_launch_urls_waffle_flag, + get_external_user_id_1p1_launches_waffle_flag, ) -from lti_consumer.lti_1p3.constants import LTI_1P3_CONTEXT_TYPE -from lti_consumer.lti_1p3.exceptions import InvalidClaimValue, MissingRequiredClaim log = logging.getLogger(__name__) SLUG_CHARACTER_CLASS = '[-a-zA-Z0-9_]' EXTERNAL_ID_REGEX = re.compile(rf'^({SLUG_CHARACTER_CLASS}+:{SLUG_CHARACTER_CLASS}+)$') +# Configuration storage +# Initally, this only supported the configuration +# stored on the block. Now it has been expanded to +# enable storing LTI configuration in the model itself or in an external +# configuration service fetchable using openedx-filters +CONFIG_ON_XBLOCK = 'CONFIG_ON_XBLOCK' +CONFIG_ON_DB = 'CONFIG_ON_DB' +CONFIG_EXTERNAL = 'CONFIG_EXTERNAL' +LTI_ADVANTAGE_AGS_DISABLED = 'disabled' +LTI_ADVANTAGE_AGS_DECLARATIVE = 'declarative' +LTI_ADVANTAGE_AGS_PROGRAMMATIC = 'programmatic' +LTI_ADVANTAGE_AGS_CHOICES = [ + (LTI_ADVANTAGE_AGS_DISABLED, 'Disabled'), + (LTI_ADVANTAGE_AGS_DECLARATIVE, 'Allow tools to submit grades only (declarative)'), + (LTI_ADVANTAGE_AGS_PROGRAMMATIC, 'Allow tools to manage and submit grade (programmatic)') +] + + +def compare_config_type(block_field, db_field): + """ + As value of config_type stored in database and xblock differ, we use this function + compare them + """ + db_to_xblock_map = { + CONFIG_ON_XBLOCK: 'new', + CONFIG_ON_DB: 'database', + CONFIG_EXTERNAL: 'external', + } + return db_to_xblock_map[db_field] == block_field def _(text): @@ -321,7 +351,7 @@ def cache_lti_1p3_launch_data(launch_data): return launch_data_key -def get_data_from_cache(cache_key): +def get_data_from_cache(cache_key) -> Lti1p3LaunchData: """ Return data stored in the cache with the cache key, if it exists. If not, return none. diff --git a/requirements/base.in b/requirements/base.in index 97c12bc8..413a76dd 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -18,3 +18,4 @@ jsonfield django-config-models # Configuration models for Django allowing config management with auditing openedx-filters django-statici18n +openedx-events # Open edX events from Hooks extension framework (OEP-50) diff --git a/requirements/base.txt b/requirements/base.txt index c45104dc..62ec50e8 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.12 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade @@ -9,7 +9,9 @@ appdirs==1.4.4 asgiref==3.11.1 # via django attrs==25.4.0 - # via -r requirements/base.in + # via + # -r requirements/base.in + # openedx-events bleach==6.3.0 # via -r requirements/base.in cffi==2.0.0 @@ -29,6 +31,7 @@ django==5.2.12 # djangorestframework # edx-django-utils # jsonfield + # openedx-events # openedx-filters django-appconf==1.2.0 # via django-statici18n @@ -47,14 +50,21 @@ djangorestframework==3.16.1 dnspython==2.8.0 # via pymongo edx-ccx-keys==2.0.2 - # via -r requirements/base.in + # via + # -r requirements/base.in + # openedx-events edx-django-utils==8.0.1 - # via django-config-models + # via + # django-config-models + # openedx-events edx-opaque-keys[django]==3.1.0 # via # -r requirements/base.in # edx-ccx-keys + # openedx-events # openedx-filters +fastavro==1.12.1 + # via openedx-events fs==2.4.16 # via xblock jsonfield==3.2.0 @@ -75,6 +85,8 @@ markupsafe==3.0.3 # xblock oauthlib==3.3.1 # via -r requirements/base.in +openedx-events==10.5.0 + # via -r requirements/base.in openedx-filters==2.1.0 # via -r requirements/base.in psutil==7.2.2 @@ -110,7 +122,7 @@ stevedore==5.7.0 # edx-opaque-keys typing-extensions==4.15.0 # via edx-opaque-keys -web-fragments==3.1.0 +web-fragments==4.0.0 # via xblock webencodings==0.5.1 # via bleach diff --git a/requirements/ci.txt b/requirements/ci.txt index 13d794c3..fd8e8479 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.12 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade @@ -26,23 +26,29 @@ astroid==4.0.4 # pylint # pylint-celery attrs==25.4.0 - # via -r requirements/test.txt -binaryornot==0.5.0 + # via + # -r requirements/test.txt + # openedx-events +backports-tarfile==1.2.0 + # via + # -r requirements/test.txt + # jaraco-context +binaryornot==0.6.0 # via # -r requirements/test.txt # cookiecutter bleach==6.3.0 # via -r requirements/test.txt -boto3==1.42.63 +boto3==1.42.64 # via # -r requirements/test.txt # fs-s3fs -botocore==1.42.63 +botocore==1.42.64 # via # -r requirements/test.txt # boto3 # s3transfer -cachetools==7.0.3 +cachetools==7.0.5 # via # -r requirements/tox.txt # tox @@ -117,6 +123,7 @@ django==5.2.12 # djangorestframework # edx-django-utils # jsonfield + # openedx-events # openedx-filters # xblock-sdk django-appconf==1.2.0 @@ -150,19 +157,27 @@ docutils==0.22.4 # -r requirements/test.txt # readme-renderer edx-ccx-keys==2.0.2 - # via -r requirements/test.txt + # via + # -r requirements/test.txt + # openedx-events edx-django-utils==8.0.1 # via # -r requirements/test.txt # django-config-models + # openedx-events edx-lint==5.6.0 # via -r requirements/test.txt edx-opaque-keys[django]==3.1.0 # via # -r requirements/test.txt # edx-ccx-keys + # openedx-events # openedx-filters -filelock==3.25.0 +fastavro==1.12.1 + # via + # -r requirements/test.txt + # openedx-events +filelock==3.25.1 # via # -r requirements/tox.txt # python-discovery @@ -185,6 +200,10 @@ idna==3.11 # via # -r requirements/test.txt # requests +importlib-metadata==8.7.1 + # via + # -r requirements/test.txt + # keyring isort==8.0.1 # via # -r requirements/test.txt @@ -264,6 +283,8 @@ nh3==0.3.3 # readme-renderer oauthlib==3.3.1 # via -r requirements/test.txt +openedx-events==10.5.0 + # via -r requirements/test.txt openedx-filters==2.1.0 # via -r requirements/test.txt packaging==26.0 @@ -346,7 +367,7 @@ python-dateutil==2.9.0.post0 # arrow # botocore # xblock -python-discovery==1.1.1 +python-discovery==1.1.3 # via # -r requirements/tox.txt # virtualenv @@ -439,7 +460,7 @@ tomlkit==0.14.0 # via # -r requirements/test.txt # pylint -tox==4.49.0 +tox==4.49.1 # via -r requirements/tox.txt twine==6.2.0 # via -r requirements/test.txt @@ -462,11 +483,11 @@ urllib3==1.26.20 # botocore # requests # twine -virtualenv==21.1.0 +virtualenv==21.2.0 # via # -r requirements/tox.txt # tox -web-fragments==3.1.0 +web-fragments==4.0.0 # via # -r requirements/test.txt # xblock @@ -486,6 +507,10 @@ xblock==5.3.0 # xblock-sdk xblock-sdk==0.13.0 # via -r requirements/test.txt +zipp==3.23.0 + # via + # -r requirements/test.txt + # importlib-metadata # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/requirements/dev.txt b/requirements/dev.txt index 12b841df..bcd5fee2 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.12 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade @@ -13,7 +13,9 @@ asgiref==3.11.1 # -r requirements/base.txt # django attrs==25.4.0 - # via -r requirements/base.txt + # via + # -r requirements/base.txt + # openedx-events bleach==6.3.0 # via -r requirements/base.txt cffi==2.0.0 @@ -37,6 +39,7 @@ django==5.2.12 # edx-django-utils # edx-i18n-tools # jsonfield + # openedx-events # openedx-filters django-appconf==1.2.0 # via @@ -65,18 +68,26 @@ dnspython==2.8.0 # -r requirements/base.txt # pymongo edx-ccx-keys==2.0.2 - # via -r requirements/base.txt + # via + # -r requirements/base.txt + # openedx-events edx-django-utils==8.0.1 # via # -r requirements/base.txt # django-config-models + # openedx-events edx-i18n-tools==1.9.0 # via -r requirements/dev.in edx-opaque-keys[django]==3.1.0 # via # -r requirements/base.txt # edx-ccx-keys + # openedx-events # openedx-filters +fastavro==1.12.1 + # via + # -r requirements/base.txt + # openedx-events fs==2.4.16 # via # -r requirements/base.txt @@ -104,6 +115,8 @@ markupsafe==3.0.3 # xblock oauthlib==3.3.1 # via -r requirements/base.txt +openedx-events==10.5.0 + # via -r requirements/base.txt openedx-filters==2.1.0 # via -r requirements/base.txt path==16.16.0 @@ -166,7 +179,7 @@ typing-extensions==4.15.0 # via # -r requirements/base.txt # edx-opaque-keys -web-fragments==3.1.0 +web-fragments==4.0.0 # via # -r requirements/base.txt # xblock diff --git a/requirements/pip.txt b/requirements/pip.txt index 084d708e..341a251b 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.12 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade @@ -12,5 +12,5 @@ wheel==0.46.3 # The following packages are considered to be unsafe in a requirements file: pip==26.0.1 # via -r requirements/pip.in -setuptools==82.0.0 +setuptools==82.0.1 # via -r requirements/pip.in diff --git a/requirements/pip_tools.txt b/requirements/pip_tools.txt index 107789a1..c7474098 100644 --- a/requirements/pip_tools.txt +++ b/requirements/pip_tools.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.12 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade diff --git a/requirements/quality.txt b/requirements/quality.txt index 8f957cdd..f5b2ac11 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.12 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade @@ -19,14 +19,16 @@ astroid==4.0.4 # pylint # pylint-celery attrs==25.4.0 - # via -r requirements/base.txt -binaryornot==0.5.0 + # via + # -r requirements/base.txt + # openedx-events +binaryornot==0.6.0 # via cookiecutter bleach==6.3.0 # via -r requirements/base.txt -boto3==1.42.63 +boto3==1.42.64 # via fs-s3fs -botocore==1.42.63 +botocore==1.42.64 # via # boto3 # s3transfer @@ -72,6 +74,7 @@ django==5.2.12 # djangorestframework # edx-django-utils # jsonfield + # openedx-events # openedx-filters # xblock-sdk django-appconf==1.2.0 @@ -101,18 +104,26 @@ dnspython==2.8.0 # -r requirements/base.txt # pymongo edx-ccx-keys==2.0.2 - # via -r requirements/base.txt + # via + # -r requirements/base.txt + # openedx-events edx-django-utils==8.0.1 # via # -r requirements/base.txt # django-config-models + # openedx-events edx-lint==5.6.0 # via -r requirements/quality.in edx-opaque-keys[django]==3.1.0 # via # -r requirements/base.txt # edx-ccx-keys + # openedx-events # openedx-filters +fastavro==1.12.1 + # via + # -r requirements/base.txt + # openedx-events fs==2.4.16 # via # -r requirements/base.txt @@ -159,6 +170,8 @@ mdurl==0.1.2 # via markdown-it-py oauthlib==3.3.1 # via -r requirements/base.txt +openedx-events==10.5.0 + # via -r requirements/base.txt openedx-filters==2.1.0 # via -r requirements/base.txt platformdirs==4.9.4 @@ -270,7 +283,7 @@ urllib3==1.26.20 # -c requirements/constraints.txt # botocore # requests -web-fragments==3.1.0 +web-fragments==4.0.0 # via # -r requirements/base.txt # xblock diff --git a/requirements/test.txt b/requirements/test.txt index 3647d264..427981dc 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.12 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade @@ -21,14 +21,18 @@ astroid==4.0.4 # pylint # pylint-celery attrs==25.4.0 - # via -r requirements/base.txt -binaryornot==0.5.0 + # via + # -r requirements/base.txt + # openedx-events +backports-tarfile==1.2.0 + # via jaraco-context +binaryornot==0.6.0 # via cookiecutter bleach==6.3.0 # via -r requirements/base.txt -boto3==1.42.63 +boto3==1.42.64 # via fs-s3fs -botocore==1.42.63 +botocore==1.42.64 # via # boto3 # s3transfer @@ -80,6 +84,7 @@ dill==0.4.1 # djangorestframework # edx-django-utils # jsonfield + # openedx-events # openedx-filters # xblock-sdk django-appconf==1.2.0 @@ -112,18 +117,26 @@ dnspython==2.8.0 docutils==0.22.4 # via readme-renderer edx-ccx-keys==2.0.2 - # via -r requirements/base.txt + # via + # -r requirements/base.txt + # openedx-events edx-django-utils==8.0.1 # via # -r requirements/base.txt # django-config-models + # openedx-events edx-lint==5.6.0 # via -r requirements/test.in edx-opaque-keys[django]==3.1.0 # via # -r requirements/base.txt # edx-ccx-keys + # openedx-events # openedx-filters +fastavro==1.12.1 + # via + # -r requirements/base.txt + # openedx-events fs==2.4.16 # via # -r requirements/base.txt @@ -135,6 +148,8 @@ id==1.5.0 # via twine idna==3.11 # via requests +importlib-metadata==8.7.1 + # via keyring isort==8.0.1 # via pylint jaraco-classes==3.4.0 @@ -192,6 +207,8 @@ nh3==0.3.3 # via readme-renderer oauthlib==3.3.1 # via -r requirements/base.txt +openedx-events==10.5.0 + # via -r requirements/base.txt openedx-filters==2.1.0 # via -r requirements/base.txt packaging==26.0 @@ -330,7 +347,7 @@ urllib3==1.26.20 # botocore # requests # twine -web-fragments==3.1.0 +web-fragments==4.0.0 # via # -r requirements/base.txt # xblock @@ -350,6 +367,8 @@ xblock==5.3.0 # xblock-sdk xblock-sdk==0.13.0 # via -r requirements/test.in +zipp==3.23.0 + # via importlib-metadata # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/requirements/tox.txt b/requirements/tox.txt index 5aa86e57..9e105f42 100644 --- a/requirements/tox.txt +++ b/requirements/tox.txt @@ -1,16 +1,16 @@ # -# This file is autogenerated by pip-compile with Python 3.12 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade # -cachetools==7.0.3 +cachetools==7.0.5 # via tox colorama==0.4.6 # via tox distlib==0.4.0 # via virtualenv -filelock==3.25.0 +filelock==3.25.1 # via # python-discovery # tox @@ -28,11 +28,11 @@ pluggy==1.6.0 # via tox pyproject-api==1.10.0 # via tox -python-discovery==1.1.1 +python-discovery==1.1.3 # via virtualenv tomli-w==1.2.0 # via tox -tox==4.49.0 +tox==4.49.1 # via -r requirements/tox.in -virtualenv==21.1.0 +virtualenv==21.2.0 # via tox