diff --git a/.coveragerc b/.coveragerc index 970aebfd..f5c79646 100644 --- a/.coveragerc +++ b/.coveragerc @@ -2,4 +2,5 @@ [run] data_file = .coverage source = lti_consumer -omit = */urls.py +omit = + */urls.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a1c5e8f8..ebf6638a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,12 @@ Please See the `releases tab return current passport + + passport unshared or tool fields blank + -> update in place from block if changed + -> return passport + + passport shared + -> active tool key mode matches block + -> return passport + -> active key mode differs + -> create new passport + -> save new passport_id on block + -> return new passport + """ + passport = lti_config.lti_1p3_passport + if not passport or lti_config.config_store != LtiConfiguration.CONFIG_ON_XBLOCK: + return passport + + block_public_key = str(block.lti_1p3_tool_public_key) + block_keyset_url = str(block.lti_1p3_tool_keyset_url) + + # Update in place when passport not shared, or when key fields still empty. + if passport.lticonfiguration_set.count() == 1 or ( + not passport.lti_1p3_tool_public_key and not passport.lti_1p3_tool_keyset_url + ): + if passport.lti_1p3_tool_public_key != block_public_key or passport.lti_1p3_tool_keyset_url != block_keyset_url: + passport.lti_1p3_tool_public_key = block_public_key + passport.lti_1p3_tool_keyset_url = block_keyset_url + passport.save() + log.info("Updated LTI passport for %s", block.scope_ids.usage_id) + return passport + + # For shared passport, check only active key mode before splitting passport. + key_mismatch = ( + block.lti_1p3_tool_key_mode == 'public_key' and passport.lti_1p3_tool_public_key != block_public_key + ) or ( + block.lti_1p3_tool_key_mode == 'keyset_url' and passport.lti_1p3_tool_keyset_url != block_keyset_url + ) + + if key_mismatch: + passport = Lti1p3Passport.objects.create( + lti_1p3_tool_public_key=block_public_key, + lti_1p3_tool_keyset_url=block_keyset_url, + name=f"Passport of {block.display_name}", + context_key=block.context_id, + ) + log.info("Created new LTI passport for %s", block.scope_ids.usage_id) + + return passport - 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 - the XBlock to be the source of truth for the LTI version, which is a user-centric perspective we've adopted. - This allows XBlock users to update the LTI version without needing to update the database. + +def _get_or_create_local_lti_config(lti_version, block, config_store=LtiConfiguration.CONFIG_ON_XBLOCK): + """ + Retrieve or create an LtiConfiguration for the block. + + The lti_version parameter is treated as the source of truth, overriding + any stored version to allow XBlocks to control LTI version without DB updates. """ - # The create operation is only performed when there is no existing configuration for the block - lti_config, _ = LtiConfiguration.objects.get_or_create(location=block_location) + lti_config, _ = LtiConfiguration.objects.get_or_create(location=block.scope_ids.usage_id) - lti_config.config_store = config_store - lti_config.external_id = external_id + # Ensure passport is synced with block + passport = _ensure_lti_passport(block, lti_config) - if lti_config.version != lti_version: - lti_config.version = lti_version + # Batch updates + updates = { + 'config_store': config_store, + 'external_id': block.external_config, + 'version': lti_version, + } + if passport: + updates['lti_1p3_passport'] = passport - lti_config.save() + # Only save if changed + if any(getattr(lti_config, key) != value for key, value in updates.items()): + for key, value in updates.items(): + setattr(lti_config, key, value) + lti_config.save() return lti_config @@ -65,7 +134,7 @@ def _get_lti_config_for_block(block): if block.config_type == 'database': lti_config = _get_or_create_local_lti_config( block.lti_version, - block.scope_ids.usage_id, + block, LtiConfiguration.CONFIG_ON_DB, ) elif block.config_type == 'external': @@ -75,14 +144,13 @@ def _get_lti_config_for_block(block): ) lti_config = _get_or_create_local_lti_config( config.get("version"), - block.scope_ids.usage_id, + block, LtiConfiguration.CONFIG_EXTERNAL, - external_id=block.external_config, ) else: lti_config = _get_or_create_local_lti_config( block.lti_version, - block.scope_ids.usage_id, + block, LtiConfiguration.CONFIG_ON_XBLOCK, ) return lti_config @@ -140,7 +208,7 @@ def get_lti_1p3_launch_info( if dl_content_items.exists(): deep_linking_content_items = [item.attributes for item in dl_content_items] - config_id = lti_config.config_id + config_id = lti_config.passport_id client_id = lti_config.lti_1p3_client_id deployment_id = "1" diff --git a/lti_consumer/lti_1p1/exceptions.py b/lti_consumer/lti_1p1/exceptions.py index 80383ae8..748cfe49 100644 --- a/lti_consumer/lti_1p1/exceptions.py +++ b/lti_consumer/lti_1p1/exceptions.py @@ -1,9 +1,10 @@ """ Exceptions for the LTI1.1 Consumer. """ +from lti_consumer.exceptions import LtiError -class Lti1p1Error(Exception): +class Lti1p1Error(LtiError): """ General error class for LTI1.1 Consumer usage. """ diff --git a/lti_consumer/lti_1p3/consumer.py b/lti_consumer/lti_1p3/consumer.py index 56984ad0..9cba4e10 100644 --- a/lti_consumer/lti_1p3/consumer.py +++ b/lti_consumer/lti_1p3/consumer.py @@ -77,6 +77,15 @@ def __init__( # Extra claims - used by LTI Advantage self.extra_claims = {} + def _get_target_link_uri(self, launch_data): # pylint: disable=unused-argument + """ + Return the target_link_uri to use for the provided launch data. + + Subclasses can override this to customize the target link selection for + different message types. + """ + return self.launch_url + @staticmethod def _get_user_roles(role): """ @@ -125,12 +134,14 @@ def prepare_preflight_url( oidc_url = self.oidc_url + "?" + target_link_uri = self._get_target_link_uri(launch_data) + login_hint = user_id parameters = { "iss": self.iss, "client_id": self.client_id, "lti_deployment_id": self.deployment_id, - "target_link_uri": self.launch_url, + "target_link_uri": target_link_uri, "login_hint": login_hint, "lti_message_hint": launch_data_key, } @@ -302,6 +313,7 @@ def set_custom_parameters( def get_lti_launch_message( self, include_extra_claims=True, + target_link_uri=None, ): """ Build LTI message from class parameters @@ -312,6 +324,8 @@ def get_lti_launch_message( # Start from base message lti_message = LTI_BASE_MESSAGE.copy() + target_link_uri = target_link_uri or self.launch_url + # Add base parameters lti_message.update({ # Issuer @@ -329,7 +343,7 @@ def get_lti_launch_message( # Target Link URI: actual endpoint for the LTI resource to display # MUST be the same value as the target_link_uri passed by the platform in the OIDC login request # http://www.imsglobal.org/spec/lti/v1p3/#target-link-uri - "https://purl.imsglobal.org/spec/lti/claim/target_link_uri": self.launch_url, + "https://purl.imsglobal.org/spec/lti/claim/target_link_uri": target_link_uri, }) # Check if user data is set, then append it to lti message @@ -609,6 +623,19 @@ def lti_dl_enabled(self): else: return False + def _get_target_link_uri(self, launch_data): + """ + Use the deep linking launch URL for deep linking requests. + """ + if ( + getattr(self, 'dl', None) and + launch_data and + getattr(launch_data, 'message_type', None) == 'LtiDeepLinkingRequest' + ): + return self.dl.deep_linking_launch_url + + return super()._get_target_link_uri(launch_data) + def enable_ags( self, lineitems_url, @@ -663,12 +690,14 @@ def generate_launch_request( # Check if Deep Linking is enabled and that this is a Deep Link Launch if self.dl and launch_data.message_type == "LtiDeepLinkingRequest": + target_link_uri = self._get_target_link_uri(launch_data) # Validate preflight response self._validate_preflight_response(preflight_response) # Get LTI Launch Message lti_launch_message = self.get_lti_launch_message( include_extra_claims=False, + target_link_uri=target_link_uri, ) # Update message type to LtiDeepLinkingRequest, diff --git a/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py b/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py index 953cee40..c1d48aaa 100644 --- a/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py +++ b/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py @@ -125,7 +125,7 @@ class LtiAgsScoreSerializer(serializers.ModelSerializer): timestamp = serializers.DateTimeField(input_formats=[ISO_8601], format=ISO_8601, default_timezone=timezone.utc) scoreGiven = serializers.FloatField(source='score_given', required=False, allow_null=True, default=None) scoreMaximum = serializers.FloatField(source='score_maximum', required=False, allow_null=True, default=None) - comment = serializers.CharField(required=False, allow_null=True) + comment = serializers.CharField(required=False, allow_null=True, allow_blank=True) activityProgress = serializers.CharField(source='activity_progress') gradingProgress = serializers.CharField(source='grading_progress') userId = serializers.CharField(source='user_id') @@ -193,14 +193,19 @@ class LtiAgsResultSerializer(serializers.ModelSerializer): comment = serializers.CharField() def get_id(self, obj): + """ + Return result URL for score. Include user_id when score scoped to learner. + """ request = self.context.get('request') + kwargs = { + 'lti_config_id': obj.line_item.lti_configuration.id, + 'pk': obj.line_item.pk, + } + if obj.user_id: + kwargs['user_id'] = obj.user_id return reverse( 'lti_consumer:lti-ags-view-results', - kwargs={ - 'lti_config_id': obj.line_item.lti_configuration.id, - 'pk': obj.line_item.pk, - 'user_id': obj.user_id, - }, + kwargs=kwargs, request=request, ) diff --git a/lti_consumer/lti_1p3/key_handlers.py b/lti_consumer/lti_1p3/key_handlers.py index 5dd0d159..b8a92f64 100644 --- a/lti_consumer/lti_1p3/key_handlers.py +++ b/lti_consumer/lti_1p3/key_handlers.py @@ -112,7 +112,7 @@ def validate_and_decode(self, token): message = jwt.decode( token, key, - algorithms=['RS256', 'RS512',], + algorithms=['RS256', 'RS512'], options={ 'verify_signature': True, 'verify_aud': False diff --git a/lti_consumer/lti_1p3/tests/test_consumer.py b/lti_consumer/lti_1p3/tests/test_consumer.py index 5f2eeb25..5c810289 100644 --- a/lti_consumer/lti_1p3/tests/test_consumer.py +++ b/lti_consumer/lti_1p3/tests/test_consumer.py @@ -28,7 +28,8 @@ ISS = "http://test-platform.example/" OIDC_URL = "http://test-platform/oidc" LAUNCH_URL = "http://test-platform/launch" -REDIRECT_URIS = [LAUNCH_URL] +DEEP_LINK_LAUNCH_URL = "http://test-platform/deep_link_launch" +REDIRECT_URIS = [LAUNCH_URL, DEEP_LINK_LAUNCH_URL] CLIENT_ID = "1" DEPLOYMENT_ID = "1" NONCE = "1234" @@ -739,14 +740,14 @@ def _setup_deep_linking(self): """ Set's up deep linking class in LTI consumer. """ - self.lti_consumer.enable_deep_linking("launch-url", "return-url") + self.lti_consumer.enable_deep_linking(DEEP_LINK_LAUNCH_URL, "return-url") lti_message_hint = self._setup_lti_message_hint() # Set LTI Consumer parameters self.preflight_response = { "client_id": CLIENT_ID, - "redirect_uri": LAUNCH_URL, + "redirect_uri": DEEP_LINK_LAUNCH_URL, "nonce": NONCE, "state": STATE, "lti_message_hint": lti_message_hint, @@ -819,6 +820,40 @@ def test_deep_linking_enabled_launch_request(self): "return-url" ) + def test_deep_linking_preflight_uses_deep_link_launch_url(self): + """ + Ensure deep linking launches send the deep linking launch URL as target_link_uri during login initiation. + """ + self.lti_consumer.enable_deep_linking(DEEP_LINK_LAUNCH_URL, "return-url") + launch_data = Lti1p3LaunchData( + user_id="1", + user_role="student", + config_id="1", + resource_link_id="resource_link_id", + message_type="LtiDeepLinkingRequest", + ) + + preflight_request_data = self.lti_consumer.prepare_preflight_url(launch_data) + parameters = parse_qs(urlparse(preflight_request_data).query) + + self.assertEqual(parameters['target_link_uri'][0], DEEP_LINK_LAUNCH_URL) + + def test_deep_linking_launch_request_sets_target_link_uri(self): + """ + Ensure the ID token for deep linking launches uses the deep linking launch URL as target_link_uri. + """ + self._setup_deep_linking() + + token = self.lti_consumer.generate_launch_request( + self.preflight_response, + )['id_token'] + + decoded_token = self.lti_consumer.key_handler.validate_and_decode(token) + self.assertEqual( + decoded_token['https://purl.imsglobal.org/spec/lti/claim/target_link_uri'], + DEEP_LINK_LAUNCH_URL, + ) + def test_deep_linking_token_decode_no_dl(self): """ Check that trying to run the Deep Linking decoding fails if service is not set up. diff --git a/lti_consumer/lti_xblock.py b/lti_consumer/lti_xblock.py index f0364d42..041e95a6 100644 --- a/lti_consumer/lti_xblock.py +++ b/lti_consumer/lti_xblock.py @@ -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,19 @@ 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, 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__) @@ -311,6 +311,13 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): ) # LTI 1.3 fields + lti_1p3_passport_id = String( + display_name=_("Lti 1.3 passport ID that points to Lti1p3Passport table"), + scope=Scope.settings, + default="", + help=_("Passport ID for a reusable keys.") + ) + lti_1p3_launch_url = String( display_name=_("Tool Launch URL"), default='', @@ -366,6 +373,9 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): " requests received have the signature from the tool." "
This is not required when doing LTI 1.3 Launches" " without LTI Advantage nor Basic Outcomes requests." + "

Changing the public key or keyset URL will cause the client ID, block keyset URL " + "and access token URL to be regenerated if they are shared between blocks. " + "Please check and update them in the LTI tool settings if necessary." ), ) lti_1p3_tool_public_key = String( @@ -380,6 +390,9 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): "from the tool." "
This is not required when doing LTI 1.3 Launches without LTI Advantage nor " "Basic Outcomes requests." + "

Changing the public key or keyset URL will cause the client ID, block keyset URL " + "and access token URL to be regenerated if they are shared between blocks. " + "Please check and update them in the LTI tool settings if necessary." ), ) @@ -991,7 +1004,7 @@ def resource_link_id(self): i4x-2-3-lti-31de800015cf4afb973356dbe81496df this part of resource_link_id: makes resource_link_id to be unique among courses inside same system. """ - return str(urllib.parse.quote(f"{settings.LMS_BASE}-{self.scope_ids.usage_id.html_id()}")) + return str(self.scope_ids.usage_id) @property def lis_result_sourcedid(self): @@ -1096,7 +1109,7 @@ def is_past_due(self): close_date = due_date return close_date is not None and timezone.now() > close_date - def _get_lti_consumer(self): + def get_lti_consumer(self): """ Returns a preconfigured LTI consumer depending on the value. @@ -1269,7 +1282,7 @@ def lti_launch_handler(self, request, suffix=''): # pylint: disable=unused-argu Returns: webob.response: HTML LTI launch form """ - lti_consumer = self._get_lti_consumer() + lti_consumer = self.get_lti_consumer() # Occassionally, users try to do an LTI launch while they are unauthenticated. It is not known why this occurs. # Sometimes, it is due to a web crawlers; other times, it is due to actual users of the platform. Regardless, @@ -1372,7 +1385,7 @@ def lti_1p3_access_token(self, request, suffix=''): # pylint: disable=unused-ar # Asserting that the consumer can be created. This makes sure that the LtiConfiguration # object exists before calling the Django View - assert self._get_lti_consumer() + assert self.get_lti_consumer() # Runtime import because this can only be run in the LMS/Studio Django # environments. Importing the views on the top level will cause RuntimeErorr from lti_consumer.plugin.views import access_token_endpoint # pylint: disable=import-outside-toplevel @@ -1428,12 +1441,11 @@ def result_service_handler(self, request, suffix=''): Returns: webob.response: response to this request. See above for details. """ - lti_consumer = self._get_lti_consumer() + lti_consumer = self.get_lti_consumer() lti_consumer.set_outcome_service_url(self.outcome_service_url) if settings.DEBUG: - lti_provider_key, lti_provider_secret = self.lti_provider_key_secret - log_authorization_header(request, lti_provider_key, lti_provider_secret) + log_authorization_header(request, lti_consumer.oauth_key, lti_consumer.oauth_secret) if not self.accept_grades_past_due and self.is_past_due: return Response(status=404) # have to do 404 due to spec, but 400 is better, with error msg in body @@ -1668,7 +1680,7 @@ def get_lti_1p3_launch_data(self): user_id=self.lms_user_id, user_role=self.role, config_id=config_id, - resource_link_id=str(location), + resource_link_id=self.resource_link_id, external_user_id=self.external_user_id, preferred_username=username, name=full_name, @@ -1745,7 +1757,7 @@ def _get_context_for_template(self): # Don't pull from the Django database unless the config_type is one that stores the LTI configuration in the # database. if self.config_type in ("database", "external"): - lti_consumer = self._get_lti_consumer() + lti_consumer = self.get_lti_consumer() launch_url = self._get_lti_launch_url(lti_consumer) lti_block_launch_handler = self._get_lti_block_launch_handler() @@ -1833,3 +1845,26 @@ def index_dictionary(self): xblock_body["content_type"] = "LTI Consumer" return xblock_body + + def add_xml_to_node(self, node): + """ + The lti_1p3_passport_id XBlock field may be empty on blocks that existed before + the Lti1p3Passport model was introduced (migration 0021). Rather than backfilling + the field in the migration (which requires the XBlock runtime and can fail silently), + we read the authoritative passport_id from the DB at export time. This ensures that + when a block is duplicated or exported/imported, the receiving block's + lti_1p3_passport_id field is populated and can be used to find the shared passport + instead of creating new credentials. + """ + super().add_xml_to_node(node) + + try: + from .models import LtiConfiguration # pylint: disable=import-outside-toplevel + + configuration = LtiConfiguration.objects.select_related("lti_1p3_passport").get( + location=self.scope_ids.usage_id, + ) + if configuration.lti_1p3_passport: + node.set("lti_1p3_passport_id", str(configuration.lti_1p3_passport.passport_id)) + except LtiConfiguration.DoesNotExist: + pass diff --git a/lti_consumer/migrations/0020_lti1p3passport_lticonfiguration_lti_1p3_passport.py b/lti_consumer/migrations/0020_lti1p3passport_lticonfiguration_lti_1p3_passport.py new file mode 100644 index 00000000..4b536a61 --- /dev/null +++ b/lti_consumer/migrations/0020_lti1p3passport_lticonfiguration_lti_1p3_passport.py @@ -0,0 +1,68 @@ +# Generated by Django 5.2.12 on 2026-03-17 14:56 + +import uuid + +import django.db.models.deletion +from django.db import migrations, models + +import lti_consumer.models + + +class Migration(migrations.Migration): + dependencies = [ + ('lti_consumer', '0019_mariadb_uuid_conversion'), + ] + + operations = [ + migrations.CreateModel( + name='Lti1p3Passport', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('passport_id', models.UUIDField(default=uuid.uuid4, editable=False, unique=True)), + ( + 'lti_1p3_internal_private_key', + models.TextField(blank=True, help_text="Platform's generated Private key. Keep this value secret."), + ), + ( + 'lti_1p3_internal_private_key_id', + models.CharField(blank=True, help_text="Platform's generated Private key ID", max_length=255), + ), + ( + 'lti_1p3_internal_public_jwk', + models.TextField(blank=True, help_text="Platform's generated JWK keyset."), + ), + ( + 'lti_1p3_client_id', + models.CharField( + default=lti_consumer.models.generate_client_id, + help_text='Client ID used by LTI tool', + max_length=255, + ), + ), + ( + 'lti_1p3_tool_public_key', + models.TextField( + blank=True, + help_text="This is the LTI Tool's public key. This should be provided by the LTI Tool. One of either lti_1p3_tool_public_key or lti_1p3_tool_keyset_url must not be blank.", + verbose_name='LTI 1.3 Tool Public Key', + ), + ), + ( + 'lti_1p3_tool_keyset_url', + models.CharField( + blank=True, + help_text="This is the LTI Tool's JWK (JSON Web Key) Keyset (JWKS) URL. This should be provided by the LTI Tool. One of either lti_1p3_tool_public_key or lti_1p3_tool_keyset_url must not be blank.", + max_length=255, + verbose_name='LTI 1.3 Tool Keyset URL', + ), + ), + ], + ), + migrations.AddField( + model_name='lticonfiguration', + name='lti_1p3_passport', + field=models.ForeignKey( + blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='lti_consumer.lti1p3passport' + ), + ), + ] diff --git a/lti_consumer/migrations/0021_create_lti_1p3_passport.py b/lti_consumer/migrations/0021_create_lti_1p3_passport.py new file mode 100644 index 00000000..cb746120 --- /dev/null +++ b/lti_consumer/migrations/0021_create_lti_1p3_passport.py @@ -0,0 +1,88 @@ +# Generated migration for copying config_id to the LtiPassport table +""" +This migration will copy config_id from LtiConfiguration database to LtiPassport table. + +Existing config_id values are reused as passport_id so that the keyset and access token URLs already registered with LTI tools continue to work without reconfiguration. +""" +from django.db import migrations + +FIELD_NAMES = [ + 'lti_1p3_internal_private_key', + 'lti_1p3_internal_private_key_id', + 'lti_1p3_internal_public_jwk', + 'lti_1p3_client_id', + 'lti_1p3_tool_public_key', + 'lti_1p3_tool_keyset_url', +] + + +def create_lti_1p3_passport(apps, _): + """ + Create a new LtiPassport entry for each existing LtiConfiguration + + We use the `config_id` of the existing LtiConfiguration as the `passport_id` for the newly + created LtiPassort entries to maintain backwards compatability and to ensure urls built + from LtiPassoport passport_ids continue to work for previously created LtiConfiguration + which correspond to existing instances of the LTI Consumer XBlock. + """ + from lti_consumer.plugin.compat import load_enough_xblock # pylint: disable=import-outside-toplevel + from lti_consumer.utils import model_to_dict # pylint: disable=import-outside-toplevel + + LtiConfiguration = apps.get_model("lti_consumer", "LtiConfiguration") + Lti1p3Passport = apps.get_model("lti_consumer", "Lti1p3Passport") + + for configuration in LtiConfiguration.objects.all(): + values = model_to_dict( + configuration, + include=FIELD_NAMES, + ) + if configuration.location: + try: + block = load_enough_xblock(configuration.location) + if block.config_type == "new": + # Data is stored on the xblock + values.update({ + 'lti_1p3_tool_public_key': block.lti_1p3_tool_public_key, + 'lti_1p3_tool_keyset_url': block.lti_1p3_tool_keyset_url, + }) + except Exception as e: # pylint: disable=broad-exception-caught + print(f"Failed to load block for {configuration.location}: {e}") + passport, _ = Lti1p3Passport.objects.update_or_create( + # Use config_id as passport_id to preserve existing urls that use it. + passport_id=configuration.config_id, + defaults=values, + ) + configuration.lti_1p3_passport = passport + configuration.save() + + +def backwards(apps, _): + """Copy LTI 1.3 passport data back to configuration fields before unapplying.""" + LtiConfiguration = apps.get_model("lti_consumer", "LtiConfiguration") + + for configuration in LtiConfiguration.objects.select_related("lti_1p3_passport").all(): + passport = configuration.lti_1p3_passport + if not passport: + continue + + for field_name in FIELD_NAMES: + setattr(configuration, field_name, getattr(passport, field_name)) + + # Use config_id as passport_id to preserve existing urls that use it. + configuration.config_id = passport.passport_id + + configuration.save(update_fields=[*FIELD_NAMES, 'config_id']) + + +class Migration(migrations.Migration): + + dependencies = [ + ('lti_consumer', '0020_lti1p3passport_lticonfiguration_lti_1p3_passport'), + ] + + operations = [ + migrations.RunPython( + code=create_lti_1p3_passport, + reverse_code=backwards, + ), + ] diff --git a/lti_consumer/migrations/0022_remove_lticonfiguration_lti_1p3_client_id_and_more.py b/lti_consumer/migrations/0022_remove_lticonfiguration_lti_1p3_client_id_and_more.py new file mode 100644 index 00000000..9df18419 --- /dev/null +++ b/lti_consumer/migrations/0022_remove_lticonfiguration_lti_1p3_client_id_and_more.py @@ -0,0 +1,37 @@ +# Generated by Django 5.2.12 on 2026-03-18 11:53 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('lti_consumer', '0021_create_lti_1p3_passport'), + ] + + operations = [ + migrations.RemoveField( + model_name='lticonfiguration', + name='lti_1p3_client_id', + ), + migrations.RemoveField( + model_name='lticonfiguration', + name='lti_1p3_internal_private_key', + ), + migrations.RemoveField( + model_name='lticonfiguration', + name='lti_1p3_internal_private_key_id', + ), + migrations.RemoveField( + model_name='lticonfiguration', + name='lti_1p3_internal_public_jwk', + ), + migrations.RemoveField( + model_name='lticonfiguration', + name='lti_1p3_tool_keyset_url', + ), + migrations.RemoveField( + model_name='lticonfiguration', + name='lti_1p3_tool_public_key', + ), + ] diff --git a/lti_consumer/migrations/0023_lti1p3passport_context_key_lti1p3passport_name_and_more.py b/lti_consumer/migrations/0023_lti1p3passport_context_key_lti1p3passport_name_and_more.py new file mode 100644 index 00000000..e85dc761 --- /dev/null +++ b/lti_consumer/migrations/0023_lti1p3passport_context_key_lti1p3passport_name_and_more.py @@ -0,0 +1,63 @@ +# Generated by Django 5.2.12 on 2026-03-31 07:34 + +import opaque_keys.edx.django.models +from django.db import migrations, models + + +def set_name_and_context_key(apps, _): + """ + Copy name and context_key from Block to LTI1p3Passport. + """ + from lti_consumer.plugin.compat import load_enough_xblock # pylint: disable=import-outside-toplevel + LtiConfiguration = apps.get_model("lti_consumer", "LtiConfiguration") + for configuration in LtiConfiguration.objects.all(): + if not configuration.location: + continue + try: + block = load_enough_xblock(configuration.location) + except Exception as e: # pylint: disable=broad-exception-caught + print(f"Error loading xblock {configuration.location}: {e}") + continue + try: + passport = configuration.lti_1p3_passport + if passport and not passport.name: + passport.name = f"Passport of {block.display_name}" + passport.context_key = block.context_id + passport.save() + except Exception as e: # pylint: disable=broad-exception-caught + print(f"Error saving passport {passport}: {e}") + continue + + +def backwards(*_): + """Reverse the migration only for MariaDB databases.""" + + +class Migration(migrations.Migration): + dependencies = [ + ('lti_consumer', '0022_remove_lticonfiguration_lti_1p3_client_id_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='lti1p3passport', + name='context_key', + field=models.CharField(blank=True, max_length=255, null=True), + ), + migrations.AddField( + model_name='lti1p3passport', + name='name', + field=models.CharField(blank=True, max_length=255, null=True), + ), + migrations.AlterField( + model_name='lticonfiguration', + name='location', + field=opaque_keys.edx.django.models.UsageKeyField( + blank=True, db_index=True, max_length=255, null=True, unique=True + ), + ), + migrations.RunPython( + code=set_name_and_context_key, + reverse_code=backwards, + ), + ] diff --git a/lti_consumer/models.py b/lti_consumer/models.py index 5a07a724..02803c11 100644 --- a/lti_consumer/models.py +++ b/lti_consumer/models.py @@ -1,40 +1,41 @@ """ 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 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.edx.django.models import CourseKeyField, UsageKeyField +from opaque_keys.edx.keys import CourseKey + 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, + EXTERNAL_ID_REGEX, + 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__) @@ -47,6 +48,123 @@ def generate_client_id(): return str(uuid.uuid4()) +class Lti1p3Passport(models.Model): + """ + Model to store LTI 1.3 keys. + """ + name = models.CharField(max_length=255, null=True, blank=True) + context_key = models.CharField(max_length=255, null=True, blank=True) + passport_id = models.UUIDField(unique=True, default=uuid.uuid4, editable=False) + + lti_1p3_internal_private_key = models.TextField( + blank=True, + help_text=_("Platform's generated Private key. Keep this value secret."), + ) + + lti_1p3_internal_private_key_id = models.CharField( + max_length=255, + blank=True, + help_text=_("Platform's generated Private key ID"), + ) + + lti_1p3_internal_public_jwk = models.TextField( + blank=True, + help_text=_("Platform's generated JWK keyset."), + ) + + lti_1p3_client_id = models.CharField( + max_length=255, + default=generate_client_id, + help_text=_("Client ID used by LTI tool"), + ) + + lti_1p3_tool_public_key = models.TextField( + "LTI 1.3 Tool Public Key", + blank=True, + help_text='This is the LTI Tool\'s public key. This should be provided by the LTI Tool. ' + 'One of either lti_1p3_tool_public_key or lti_1p3_tool_keyset_url must not be blank.' + ) + + lti_1p3_tool_keyset_url = models.CharField( + "LTI 1.3 Tool Keyset URL", + max_length=255, + blank=True, + help_text='This is the LTI Tool\'s JWK (JSON Web Key) Keyset (JWKS) URL. This should be provided by the LTI ' + 'Tool. One of either lti_1p3_tool_public_key or lti_1p3_tool_keyset_url must not be blank.' + ) + + def _generate_lti_1p3_keys_if_missing(self): + """ + Generate LTI 1.3 RSA256 keys if missing. + + If either the public or private key are missing, regenerate them. + The LMS provides a keyset endpoint, so key rotations don't cause any issues + for LTI launches (as long as they have a different kid). + """ + # Generate new private key if not present + if not self.lti_1p3_internal_private_key: + # Private key + private_key = RSA.generate(2048) + self.lti_1p3_internal_private_key_id = str(uuid.uuid4()) + self.lti_1p3_internal_private_key = private_key.export_key('PEM').decode('utf-8') + + # Clear public key if any to allow regeneration + # in the code below + self.lti_1p3_internal_public_jwk = '' + + if not self.lti_1p3_internal_public_jwk: + # Public key + key_handler = PlatformKeyHandler( + key_pem=self.lti_1p3_internal_private_key, + kid=self.lti_1p3_internal_private_key_id, + ) + self.lti_1p3_internal_public_jwk = json.dumps( + key_handler.get_public_jwk() + ) + + # Doesn't do anything if model didn't change + self.save() + + @property + def lti_1p3_private_key(self): + """ + Return the platform's private key used in LTI 1.3 authentication flows. + """ + self._generate_lti_1p3_keys_if_missing() + return self.lti_1p3_internal_private_key + + @property + def lti_1p3_private_key_id(self): + """ + Return the platform's private key ID used in LTI 1.3 authentication flows. + """ + self._generate_lti_1p3_keys_if_missing() + return self.lti_1p3_internal_private_key_id + + @property + def lti_1p3_public_jwk(self): + """ + Return the platform's public keys used in LTI 1.3 authentication flows. + """ + self._generate_lti_1p3_keys_if_missing() + return json.loads(self.lti_1p3_internal_public_jwk) + + def __str__(self): + return f'Lti1p3Passport: {self.name} - {self.passport_id}' + + def clean(self): + if self.lti_1p3_tool_public_key == "" and self.lti_1p3_tool_keyset_url == "": + raise ValidationError({ + "config_store": _( + "LTI Configuration stored on the model for LTI 1.3 must have a value for one of " + "lti_1p3_tool_public_key or lti_1p3_tool_keyset_url." + ), + }) + + class Meta: + app_label = 'lti_consumer' + + class LtiConfiguration(models.Model): """ Model to store LTI Configuration for LTI 1.1 and 1.3. @@ -109,6 +227,8 @@ class LtiConfiguration(models.Model): db_index=True, null=True, blank=True, + # Duplicate fails if not unique. + unique=True, ) # This is where the configuration is stored in the model if stored on this model. @@ -138,26 +258,11 @@ class LtiConfiguration(models.Model): ) # LTI 1.3 Related variables - lti_1p3_internal_private_key = models.TextField( - blank=True, - help_text=_("Platform's generated Private key. Keep this value secret."), - ) - - lti_1p3_internal_private_key_id = models.CharField( - max_length=255, - blank=True, - help_text=_("Platform's generated Private key ID"), - ) - - lti_1p3_internal_public_jwk = models.TextField( + lti_1p3_passport = models.ForeignKey( + Lti1p3Passport, + on_delete=models.SET_NULL, + null=True, blank=True, - help_text=_("Platform's generated JWK keyset."), - ) - - lti_1p3_client_id = models.CharField( - max_length=255, - default=generate_client_id, - help_text=_("Client ID used by LTI tool"), ) lti_1p3_oidc_url = models.CharField( @@ -177,21 +282,6 @@ class LtiConfiguration(models.Model): 'when the resource is actually launched or loaded.' ) - lti_1p3_tool_public_key = models.TextField( - "LTI 1.3 Tool Public Key", - blank=True, - help_text='This is the LTI Tool\'s public key. This should be provided by the LTI Tool. ' - 'One of either lti_1p3_tool_public_key or lti_1p3_tool_keyset_url must not be blank.' - ) - - lti_1p3_tool_keyset_url = models.CharField( - "LTI 1.3 Tool Keyset URL", - max_length=255, - blank=True, - help_text='This is the LTI Tool\'s JWK (JSON Web Key) Keyset (JWKS) URL. This should be provided by the LTI ' - 'Tool. One of either lti_1p3_tool_public_key or lti_1p3_tool_keyset_url must not be blank.' - ) - lti_1p3_redirect_uris = models.JSONField( "LTI 1.3 Redirect URIs", default=list, @@ -261,14 +351,10 @@ def clean(self): 'LTI Configuration using reusable configuration needs a external ID in "x:y" format.', ), }) - if self.version == self.LTI_1P3 and self.config_store == self.CONFIG_ON_DB: - if self.lti_1p3_tool_public_key == "" and self.lti_1p3_tool_keyset_url == "": - raise ValidationError({ - "config_store": _( - "LTI Configuration stored on the model for LTI 1.3 must have a value for one of " - "lti_1p3_tool_public_key or lti_1p3_tool_keyset_url." - ), - }) + if self.version == self.LTI_1P3 and self.config_store == self.CONFIG_ON_DB and not self.lti_1p3_passport: + raise ValidationError({ + "config_store": _('LTI Configuration for 1.3 requires passport to be set.'), + }) if (self.version == self.LTI_1P3 and self.config_store in [self.CONFIG_ON_XBLOCK, self.CONFIG_EXTERNAL] and self.lti_1p3_proctoring_enabled): raise ValidationError({ @@ -315,65 +401,99 @@ def sync_configurations(self): f'Failed to parse main LTI configuration location: {self.location}', ) + def get_or_create_lti_1p3_passport(self): + """ + Create an LTI 1.3 Passport configuration for this course instance. + """ + passport = self.lti_1p3_passport + block = None + if not passport: + if self.location: + block = compat.load_enough_xblock(self.location) + if self.location and block and block.lti_1p3_passport_id: + passport, created = Lti1p3Passport.objects.get_or_create( + passport_id=block.lti_1p3_passport_id, + defaults={ + "name": f"Passport of {block.display_name}", + "context_key": block.context_id, + } + ) + if created: + log.info("Created new LTI 1.3 Passport %s for %s", passport, self) + else: + log.info("Existing LTI 1.3 Passport %s found for %s", passport, self) + else: + passport = Lti1p3Passport.objects.create() + log.info("Created new LTI 1.3 Passport %s for %s", passport, self) + if block: + passport.name = f"Passport of {block.display_name}" + passport.context_key = block.context_id + passport.save() + # We use update to avoid triggering post save signal as this function is itself + # called from the post_save signal handler, this avoids double call to same function. + LtiConfiguration.objects.filter(pk=self.pk).update(lti_1p3_passport=passport) + # Refresh block from db to get passport id. + self.refresh_from_db() + def save(self, *args, **kwargs): self.sync_configurations() super().save(*args, **kwargs) - def _generate_lti_1p3_keys_if_missing(self): + @property + def passport_id(self): """ - Generate LTI 1.3 RSA256 keys if missing. - - If either the public or private key are missing, regenerate them. - The LMS provides a keyset endpoint, so key rotations don't cause any issues - for LTI launches (as long as they have a different kid). + Return the passport ID associated with this configuration instance. """ - # Generate new private key if not present - if not self.lti_1p3_internal_private_key: - # Private key - private_key = RSA.generate(2048) - self.lti_1p3_internal_private_key_id = str(uuid.uuid4()) - self.lti_1p3_internal_private_key = private_key.export_key('PEM').decode('utf-8') - - # Clear public key if any to allow regeneration - # in the code below - self.lti_1p3_internal_public_jwk = '' - - if not self.lti_1p3_internal_public_jwk: - # Public key - key_handler = PlatformKeyHandler( - key_pem=self.lti_1p3_internal_private_key, - kid=self.lti_1p3_internal_private_key_id, - ) - self.lti_1p3_internal_public_jwk = json.dumps( - key_handler.get_public_jwk() - ) - - # Doesn't do anything if model didn't change - self.save() + self.get_or_create_lti_1p3_passport() + return self.lti_1p3_passport.passport_id @property def lti_1p3_private_key(self): """ Return the platform's private key used in LTI 1.3 authentication flows. """ - self._generate_lti_1p3_keys_if_missing() - return self.lti_1p3_internal_private_key + self.get_or_create_lti_1p3_passport() + return self.lti_1p3_passport.lti_1p3_private_key @property def lti_1p3_private_key_id(self): """ Return the platform's private key ID used in LTI 1.3 authentication flows. """ - self._generate_lti_1p3_keys_if_missing() - return self.lti_1p3_internal_private_key_id + self.get_or_create_lti_1p3_passport() + return self.lti_1p3_passport.lti_1p3_private_key_id @property def lti_1p3_public_jwk(self): """ Return the platform's public keys used in LTI 1.3 authentication flows. """ - self._generate_lti_1p3_keys_if_missing() - return json.loads(self.lti_1p3_internal_public_jwk) + self.get_or_create_lti_1p3_passport() + return self.lti_1p3_passport.lti_1p3_public_jwk + + @property + def lti_1p3_client_id(self): + """ + Return platform client id from passport + """ + self.get_or_create_lti_1p3_passport() + return self.lti_1p3_passport.lti_1p3_client_id + + @property + def lti_1p3_tool_keyset_url(self): + """ + Return tool keyset url from passport + """ + self.get_or_create_lti_1p3_passport() + return self.lti_1p3_passport.lti_1p3_tool_keyset_url + + @property + def lti_1p3_tool_public_key(self): + """ + Return tool public key from passport + """ + self.get_or_create_lti_1p3_passport() + return self.lti_1p3_passport.lti_1p3_tool_public_key @cached_property def external_config(self): diff --git a/lti_consumer/outcomes.py b/lti_consumer/outcomes.py index 72bc759c..7de7587b 100644 --- a/lti_consumer/outcomes.py +++ b/lti_consumer/outcomes.py @@ -6,10 +6,12 @@ """ import logging -from xml.sax.saxutils import escape from urllib.parse import unquote +from xml.sax.saxutils import escape +from django.conf import settings from lxml import etree + try: from xblock.utils.resources import ResourceLoader except ModuleNotFoundError: # For backward compatibility with releases older than Quince. @@ -176,7 +178,13 @@ def handle_request(self, request): return response_xml_template.format(**failure_values) # Verify OAuth signing. - __, secret = self.xblock.lti_provider_key_secret + secret = self.xblock.get_lti_consumer().oauth_secret + if settings.DEBUG: + log.debug( + "[LTI]: verifying OAuth body signature for outcomes. service_url=%s request.url=%s", + self.xblock.outcome_service_url, + request.url, + ) try: verify_oauth_body_signature(request, secret, self.xblock.outcome_service_url) except (ValueError, LtiError) as ex: diff --git a/lti_consumer/plugin/compat.py b/lti_consumer/plugin/compat.py index dee1170a..82544565 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): @@ -335,3 +334,27 @@ def nrps_pii_disallowed(): """ return (hasattr(settings, 'LTI_NRPS_DISALLOW_PII') and settings.LTI_NRPS_DISALLOW_PII is True) + + +def get_signal_handler(): # pragma: nocover + """ + 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): # pragma: nocover + """ + 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/urls.py b/lti_consumer/plugin/urls.py index a6117b71..f837c8a4 100644 --- a/lti_consumer/plugin/urls.py +++ b/lti_consumer/plugin/urls.py @@ -22,7 +22,7 @@ app_name = 'lti_consumer' urlpatterns = [ path( - 'lti_consumer/v1/public_keysets/', + 'lti_consumer/v1/public_keysets/', public_keyset_endpoint, name='lti_consumer.public_keyset_endpoint' ), @@ -44,7 +44,7 @@ name='lti_consumer.launch_gate' ), path( - 'lti_consumer/v1/token/', + 'lti_consumer/v1/token/', access_token_endpoint, name='lti_consumer.access_token' ), diff --git a/lti_consumer/plugin/views.py b/lti_consumer/plugin/views.py index 2f753bf3..1c7f3b3e 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 @@ -26,31 +26,48 @@ 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.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 Lti1p3Passport, LtiAgsLineItem, LtiConfiguration, LtiDlContentItem 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__) @@ -88,7 +105,7 @@ def has_block_access(user, block, course_key): def public_keyset_endpoint( request, usage_id=None, - lti_config_id=None, + passport_id=None, external_app=None, external_slug=None, ): @@ -100,7 +117,7 @@ def public_keyset_endpoint( and run the proper handler. Arguments: - lti_config_id (UUID): config_id of the LtiConfiguration + passport_id (UUID): passport_id of the Lti1p3Passport usage_id (UsageKey): location of the Block external_app (str): App name of the external LTI configuration external_slug (str): Slug of the external LTI configuration. @@ -117,10 +134,13 @@ def public_keyset_endpoint( lti_config = LtiConfiguration.objects.get(location=UsageKey.from_string(usage_id)) version = lti_config.version public_jwk = lti_config.lti_1p3_public_jwk - elif lti_config_id: - lti_config = LtiConfiguration.objects.get(config_id=lti_config_id) + elif passport_id: + lti_passport = Lti1p3Passport.objects.get(passport_id=passport_id) + public_jwk = lti_passport.lti_1p3_public_jwk + # TODO: move version inside passport from config + # We just need any lti_config that is using this passport to check version + lti_config = LtiConfiguration.objects.filter(lti_1p3_passport=lti_passport).first() version = lti_config.version - public_jwk = lti_config.lti_1p3_public_jwk elif external_app and external_slug: lti_config = get_external_config_from_filter({}, external_id) @@ -141,10 +161,16 @@ def public_keyset_endpoint( response = JsonResponse(public_jwk) response['Content-Disposition'] = 'attachment; filename=keyset.json' return response - except (InvalidKeyError, LtiConfiguration.DoesNotExist, ExternalConfigurationNotFound, LtiError) as exc: + except ( + InvalidKeyError, + LtiConfiguration.DoesNotExist, + Lti1p3Passport.DoesNotExist, + ExternalConfigurationNotFound, + LtiError, + ) as exc: log.info( "Error while retrieving keyset for ID %s: %s", - usage_id or lti_config_id or external_id, + usage_id or passport_id or external_id, exc, exc_info=True, ) @@ -396,7 +422,7 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume @require_http_methods(["POST"]) def access_token_endpoint( request, - lti_config_id=None, + passport_id=None, usage_id=None, external_app=None, external_slug=None, @@ -407,7 +433,7 @@ def access_token_endpoint( This endpoint is only valid when a LTI 1.3 tool is being used. Arguments: - lti_config_id (UUID): config_id of the LtiConfiguration + passport_id (UUID): passport_id of the Lti1p3Passport usage_id (UsageKey): location of the Block external_app (str): App name of the external LTI configuration external_slug (str): Slug of the external LTI configuration. @@ -428,10 +454,24 @@ def access_token_endpoint( lti_config = LtiConfiguration.objects.get(location=UsageKey.from_string(usage_id)) version = lti_config.version lti_consumer = lti_config.get_lti_consumer() - elif lti_config_id: - lti_config = LtiConfiguration.objects.get(config_id=lti_config_id) + elif passport_id: + lti_passport = Lti1p3Passport.objects.get(passport_id=passport_id) + # TODO: move version inside passport from config + # We just need any lti_config that is using this passport to check version + lti_config = LtiConfiguration.objects.filter(lti_1p3_passport=lti_passport).first() version = lti_config.version - lti_consumer = lti_config.get_lti_consumer() + lti_consumer = LtiConsumer1p3( + iss=get_lti_api_base(), + lti_oidc_url=None, + lti_launch_url=None, + client_id=lti_passport.lti_1p3_client_id, + deployment_id=None, + rsa_key=lti_passport.lti_1p3_private_key, + rsa_key_id=lti_passport.lti_1p3_private_key_id, + redirect_uris=None, + tool_key=lti_passport.lti_1p3_tool_public_key, + tool_keyset_url=lti_passport.lti_1p3_tool_keyset_url, + ) elif external_app and external_slug: lti_config = get_external_config_from_filter({}, external_id) @@ -453,10 +493,15 @@ 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, + LtiConfiguration.DoesNotExist, + Lti1p3Passport.DoesNotExist, + ExternalConfigurationNotFound, + ) as exc: log.info( "Error while retrieving access token for ID %s: %s", - usage_id or lti_config_id or external_id, + usage_id or passport_id or external_id, exc, exc_info=True, ) @@ -703,7 +748,7 @@ def perform_create(self, serializer): @action( detail=True, methods=['GET'], - url_path='results/(?P[^/.]+)?', + url_path=r'results(?:/(?P[^/.]+))?/?', renderer_classes=[LineItemResultsRenderer], content_negotiation_class=IgnoreContentNegotiation, ) @@ -883,10 +928,15 @@ 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) + lti_config = LtiConfiguration.objects.get( + lti_1p3_passport__lti_1p3_client_id=iss, + location=resource_link_id + ) except LtiConfiguration.DoesNotExist: - log.error("Invalid iss claim '%s' for LTI 1.3 Proctoring Services start_proctoring_assessment_endpoint" - " callback", iss) + log.error( + "Invalid resource_link id '%s' for LTI 1.3 Proctoring Services start_proctoring_assessment_endpoint", + resource_link_id, + ) return render(request, 'html/lti_proctoring_start_error.html', status=HTTP_404_NOT_FOUND) lti_consumer = lti_config.get_lti_consumer() diff --git a/lti_consumer/signals/signals.py b/lti_consumer/signals/signals.py index c2bfedc2..26e477f6 100644 --- a/lti_consumer/signals/signals.py +++ b/lti_consumer/signals/signals.py @@ -2,15 +2,19 @@ LTI Consumer related Signal handlers """ import logging +import uuid 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 DuplicatedXBlockData, LibraryBlockData, XBlockData +from openedx_events.content_authoring.signals import LIBRARY_BLOCK_DELETED, XBLOCK_DELETED, XBLOCK_DUPLICATED -from lti_consumer.models import LtiAgsScore +from lti_consumer.models import Lti1p3Passport, LtiAgsScore, LtiConfiguration from lti_consumer.plugin import compat - +from lti_consumer.utils import model_to_dict log = logging.getLogger(__name__) +SignalHandler = compat.get_signal_handler() @receiver(post_save, sender=LtiAgsScore, dispatch_uid='publish_grade_on_score_update') @@ -30,7 +34,7 @@ def publish_grade_on_score_update(sender, instance, **kwargs): # pylint: disabl # have permissions to. # TODO: This security mechanism will need to be reworked once we enable LTI 1.3 # reusability to allow one configuration to save scores on multiple placements, - # but still locking down access to the items that are using the LTI configurtion. + # but still locking down access to the items that are using the LTI configuration. if line_item.resource_link_id != lti_config.location: log.warning( "LTI tool tried publishing score %r to block %s (outside allowed scope of: %s).", @@ -82,4 +86,115 @@ def publish_grade_on_score_update(sender, instance, **kwargs): # pylint: disabl raise exc +@receiver(post_save, sender=LtiConfiguration, dispatch_uid='create_lti_1p3_passport') +def create_lti_1p3_passport(sender, instance: LtiConfiguration, **kwargs): # pylint: disable=unused-argument + instance.get_or_create_lti_1p3_passport() + + +@receiver(SignalHandler.pre_item_delete if SignalHandler else []) +def delete_child_lti_configurations(**kwargs): + """ + Delete lti configuration 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 + block_locations = {str(deleted_block.location)} + for block in compat.yield_dynamic_block_descendants(deleted_block, kwargs.get('user_id')): + block_locations.add(str(block.location)) + + LtiConfiguration.objects.filter( + location__in=block_locations + ).delete() + log.info(f"Deleted {len(block_locations)} LTI configurations for block and its children in modulestore") + result = Lti1p3Passport.objects.filter(lticonfiguration__isnull=True).delete() + log.info(f"Deleted {result} lti 1.3 passport objects in library") + + +@receiver(XBLOCK_DELETED) +def delete_lti_configuration(**kwargs): + """ + Delete lti configuration 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 + + LtiConfiguration.objects.filter( + location=str(xblock_info.usage_key) + ).delete() + result = Lti1p3Passport.objects.filter(lticonfiguration__isnull=True).delete() + log.info(f"Deleted {result} lti 1.3 passport objects in library") + + +@receiver(LIBRARY_BLOCK_DELETED) +def delete_lib_lti_configuration(**kwargs): + """ + Delete lti configuration 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 + + LtiConfiguration.objects.filter( + location=str(library_block.usage_key) + ).delete() + result = Lti1p3Passport.objects.filter(lticonfiguration__isnull=True).delete() + log.info(f"Deleted {result} lti 1.3 passport objects in library") + + +@receiver(XBLOCK_DUPLICATED) +def duplicate_xblock_lti_configuration(**kwargs): + """ + Duplicate LTI configuration from the source to the target xblock. + """ + xblock_data = kwargs.get("xblock_info", None) + if not xblock_data or not isinstance(xblock_data, DuplicatedXBlockData): + log.error("Received null or incorrect data for event") + return + + source_usage_key = str(xblock_data.source_usage_key) + target_usage_key = str(xblock_data.usage_key) + + src_lti_config = LtiConfiguration.objects.filter(location=source_usage_key).first() + if not src_lti_config: + log.warning("No LTI configuration found to duplicate. source=%s", source_usage_key) + return + + if LtiConfiguration.objects.filter(location=target_usage_key).exists(): + log.info( + "Target already has LTI configuration. Skipping duplicate. target=%s source=%s", + target_usage_key, + source_usage_key, + ) + return + + # Convert the LTI configuration to a dictionary and set the new location and config_id + # to copy lticonfiguration data to the new location and config_id + payload = model_to_dict( + src_lti_config, + # Include all unique fields and generated ones. + exclude=["id", "pk", "location", "config_id"], + ) + payload["location"] = target_usage_key + payload["config_id"] = uuid.uuid4() + + try: + LtiConfiguration.objects.create(**payload) + except Exception: # pylint: disable=broad-exception-caught + log.exception( + "Failed duplicating LTI configuration. source=%s target=%s", + source_usage_key, + target_usage_key, + ) + + LTI_1P3_PROCTORING_ASSESSMENT_STARTED = Signal() diff --git a/lti_consumer/tests/test_utils.py b/lti_consumer/tests/test_utils.py index abe0b2bd..10e8f82b 100644 --- a/lti_consumer/tests/test_utils.py +++ b/lti_consumer/tests/test_utils.py @@ -2,9 +2,10 @@ Utility functions used within unit tests """ -from unittest.mock import Mock import urllib +from unittest.mock import Mock, patch +from django.test.testcases import TestCase from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LocalId from webob import Request @@ -12,7 +13,6 @@ from xblock.fields import ScopeIds from xblock.runtime import DictKeyValueStore, KvsFieldData - FAKE_USER_ID = 'fake_user_id' @@ -108,3 +108,24 @@ def get_mock_lti_configuration(editable): return_value=editable ) return lti_configuration + + +class TestBaseWithPatch(TestCase): + """Base test class that mocks xblock imports""" + + def setUp(self): + """Set up mock patches for xblock imports""" + self.mock_load_xblock = Mock() + self.mock_load_xblock.return_value = getattr(self, 'xblock', None) + + # Patch the imports + self.patcher_load = patch('lti_consumer.plugin.compat.load_enough_xblock', self.mock_load_xblock) + + self.patcher_load.start() + + super().setUp() + + def tearDown(self): + """Clean up patches""" + self.patcher_load.stop() + super().tearDown() diff --git a/lti_consumer/tests/unit/plugin/test_proctoring.py b/lti_consumer/tests/unit/plugin/test_proctoring.py index 4cd5ea3b..0882a18e 100644 --- a/lti_consumer/tests/unit/plugin/test_proctoring.py +++ b/lti_consumer/tests/unit/plugin/test_proctoring.py @@ -7,19 +7,24 @@ import ddt from Cryptodome.PublicKey import RSA from django.contrib.auth import get_user_model -from django.test.testcases import TestCase 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.tests.test_utils import TestBaseWithPatch from lti_consumer.utils import get_data_from_cache @ddt.ddt -class TestLti1p3ProctoringStartProctoringAssessmentEndpoint(TestCase): +class TestLti1p3ProctoringStartProctoringAssessmentEndpoint(TestBaseWithPatch): """Tests for the start_proctoring_assessment_endpoint endpoint.""" def setUp(self): @@ -30,8 +35,10 @@ def setUp(self): # Set up user. self._setup_user() + self.resource_link_id = "block-v1:course+test+2020+type@problem+block@test" # Set up an LtiConfiguration instance for the integration. self.lti_config = LtiConfiguration.objects.create( + location=self.resource_link_id, version=LtiConfiguration.LTI_1P3, lti_1p3_proctoring_enabled=True, config_store=LtiConfiguration.CONFIG_ON_DB, @@ -45,7 +52,7 @@ def setUp(self): self.private_key = RSA.generate(2048) self.public_key = self.private_key.publickey().export_key().decode() - self.lti_config.lti_1p3_tool_public_key = self.public_key + self.lti_config.lti_1p3_passport.lti_1p3_tool_public_key = self.public_key self.lti_config.save() def _setup_user(self): @@ -61,7 +68,7 @@ def _setup_cached_data(self): self.common_cache_key_arguments = { "app": "lti", "user_id": self.user.id, - "resource_link_id": "resource_link_id", + "resource_link_id": self.resource_link_id, } # Cache session_data. @@ -77,7 +84,7 @@ def _setup_cached_data(self): user_id="1", user_role=None, config_id=self.lti_config.config_id, - resource_link_id="resource_link_id", + resource_link_id=self.resource_link_id, proctoring_launch_data=proctoring_launch_data, context_id="course-v1:testU+DemoX+Demo_Course", context_title="http://localhost:2000", @@ -105,7 +112,7 @@ def create_tool_jwt_token(self, **kwargs): "https://purl.imsglobal.org/spec/lti/claim/message_type": "LtiStartAssessment", "https://purl.imsglobal.org/spec/lti/claim/version": "1.3.0", "https://purl.imsglobal.org/spec/lti-ap/claim/session_data": "session_data", - "https://purl.imsglobal.org/spec/lti/claim/resource_link": {"id": "resource_link_id"}, + "https://purl.imsglobal.org/spec/lti/claim/resource_link": {"id": self.resource_link_id}, "https://purl.imsglobal.org/spec/lti-ap/claim/attempt_number": 2, } @@ -273,7 +280,7 @@ def test_lti_1p3_proctoring_assessment_started_signal(self, mock_assessment_star expected_call_args = call( sender=None, attempt_number=2, - resource_link={'id': 'resource_link_id'}, + resource_link={'id': self.resource_link_id}, user_id=self.user.id, ) self.assertEqual(mock_assessment_started_signal.call_args, expected_call_args) diff --git a/lti_consumer/tests/unit/plugin/test_views.py b/lti_consumer/tests/unit/plugin/test_views.py index 8a197a4c..410b3d58 100644 --- a/lti_consumer/tests/unit/plugin/test_views.py +++ b/lti_consumer/tests/unit/plugin/test_views.py @@ -2,36 +2,36 @@ Tests for LTI 1.3 endpoint views. """ import json -from unittest.mock import patch, Mock +import urllib.parse +from unittest.mock import Mock, patch import ddt import jwt -from django.test.testcases import TestCase +from Cryptodome.PublicKey import RSA 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.tests.test_utils import make_xblock +from lti_consumer.lti_xblock import LtiConsumerXBlock +from lti_consumer.models import LtiConfiguration, LtiDlContentItem +from lti_consumer.tests.test_utils import TestBaseWithPatch, make_xblock from lti_consumer.utils import cache_lti_1p3_launch_data @ddt.ddt -class TestLti1p3KeysetEndpoint(TestCase): +class TestLti1p3KeysetEndpoint(TestBaseWithPatch): """ Test `public_keyset_endpoint` method. """ @@ -102,7 +102,7 @@ def test_public_keyset_endpoint_using_lti_config_id_in_url(self): """ Check that the endpoint is accessible using the ID of the LTI Config object """ - response = self.client.get(f'/lti_consumer/v1/public_keysets/{self.lti_config.config_id}') + response = self.client.get(f'/lti_consumer/v1/public_keysets/{self.lti_config.passport_id}') # Check response self.assertEqual(response.status_code, 200) @@ -137,7 +137,7 @@ def test_public_keyset_endpoint_using_external_id_in_url(self, get_external_conf @ddt.ddt -class TestLti1p3LaunchGateEndpoint(TestCase): +class TestLti1p3LaunchGateEndpoint(TestBaseWithPatch): """ Tests for the `launch_gate_endpoint` method. """ @@ -661,7 +661,7 @@ def test_launch_non_existing_custom_parameters(self, mock_set_custom_parameters) @ddt.ddt -class TestLti1p3AccessTokenEndpoint(TestCase): +class TestLti1p3AccessTokenEndpoint(TestBaseWithPatch): """ Test `access_token_endpoint` method. """ @@ -672,15 +672,7 @@ def setUp(self): location = 'block-v1:course+test+2020+type@problem+block@test' self.config = LtiConfiguration(version=LtiConfiguration.LTI_1P3, location=location) self.config.save() - self.url = reverse('lti_consumer:lti_consumer.access_token', args=[str(self.config.config_id)]) - # Patch settings calls to LMS method - self.mock_client = Mock() - get_lti_consumer_patcher = patch( - 'lti_consumer.plugin.views.LtiConfiguration.get_lti_consumer', - return_value=self.mock_client - ) - self.addCleanup(get_lti_consumer_patcher.stop) - self._mock_xblock_handler = get_lti_consumer_patcher.start() + self.url = reverse('lti_consumer:lti_consumer.access_token', args=[str(self.config.passport_id)]) # Generate RSA and save exports rsa_key = RSA.generate(2048) algo_obj = jwt.get_algorithm_by_name('RS256') @@ -689,6 +681,8 @@ def setUp(self): private_jwk['kid'] = 1 self.key = PyJWK.from_dict(private_jwk) self.public_key = rsa_key.public_key().export_key('PEM') + self.config.lti_1p3_passport.lti_1p3_tool_public_key = self.public_key.decode() + self.config.lti_1p3_passport.save() def get_body(self, token, **overrides): """ @@ -706,40 +700,28 @@ def test_access_token_endpoint(self): """ Check that the access_token generated by the lti_consumer is returned. """ - token = {"access_token": "test-token"} - self.mock_client.access_token.return_value = token + body = urllib.parse.urlencode(self.get_body(create_jwt(self.key, {}))) + response = self.client.post(self.url, data=body, content_type='application/json') - body = self.get_body(create_jwt(self.key, {})) - response = self.client.post(self.url, data=json.dumps(body), content_type='application/json') - self.mock_client.access_token.assert_called_once() - called_args = self.mock_client.access_token.call_args[0] - actual_arg = called_args[0] - actual_dict = json.loads(next(iter(actual_arg.keys()))) - - self.assertEqual(actual_dict, body) self.assertEqual(response.status_code, 200) - self.assertEqual(response.json(), token) + self.assertContains(response, 'access_token') - def test_access_token_endpoint_with_location_in_url(self): + @patch('lti_consumer.plugin.views.LtiConfiguration.get_lti_consumer') + def test_access_token_endpoint_with_location_in_url(self, mock_client): """ Check that the access_token generated by the lti_consumer is returned. """ token = {"access_token": "test-token"} - self.mock_client.access_token.return_value = token + mock_client().access_token.return_value = token url = reverse( 'lti_consumer:lti_consumer.access_token_via_location', args=[str(self.config.location)] ) - body = self.get_body(create_jwt(self.key, {})) - response = self.client.post(url, data=json.dumps(body), content_type='application/json') - - self.mock_client.access_token.assert_called_once() - called_args = self.mock_client.access_token.call_args[0] - actual_arg = called_args[0] - actual_dict = json.loads(next(iter(actual_arg.keys()))) + body = urllib.parse.urlencode(self.get_body(create_jwt(self.key, {}))) + response = self.client.post(url, data=body, content_type='application/json') - self.assertEqual(actual_dict, body) + mock_client().access_token.assert_called_once() self.assertEqual(response.status_code, 200) self.assertEqual(response.json(), token) @@ -825,48 +807,52 @@ def test_verify_lti_version_is_1p3(self): self.config.version = LtiConfiguration.LTI_1P3 self.config.save() - def test_missing_required_claim(self): + @patch('lti_consumer.lti_1p3.consumer.LtiConsumer1p3.access_token') + def test_missing_required_claim(self, access_token_mock): """ Check that 400 is returned when required attributes are missing in the request """ - self.mock_client.access_token = Mock(side_effect=MissingRequiredClaim()) + access_token_mock.side_effect = MissingRequiredClaim() response = self.client.post(self.url) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), {'error': 'invalid_request'}) - def test_token_errors(self): + @patch('lti_consumer.lti_1p3.consumer.LtiConsumer1p3.access_token') + def test_token_errors(self, access_token_mock): """ Check invalid_grant error is returned when the token is invalid """ - self.mock_client.access_token = Mock(side_effect=MalformedJwtToken()) + access_token_mock.side_effect = MalformedJwtToken() response = self.client.post(self.url) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), {'error': 'invalid_grant'}) - self.mock_client.access_token = Mock(side_effect=TokenSignatureExpired()) + access_token_mock.side_effect = TokenSignatureExpired() response = self.client.post(self.url) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), {'error': 'invalid_grant'}) - def test_client_credential_errors(self): + @patch('lti_consumer.lti_1p3.consumer.LtiConsumer1p3.access_token') + def test_client_credential_errors(self, access_token_mock): """ Check invalid_client error is returned when the client credentials are wrong """ - self.mock_client.access_token = Mock(side_effect=NoSuitableKeys()) + access_token_mock.side_effect = NoSuitableKeys() response = self.client.post(self.url) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), {'error': 'invalid_client'}) - self.mock_client.access_token = Mock(side_effect=UnknownClientId()) + access_token_mock.side_effect = UnknownClientId() response = self.client.post(self.url) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), {'error': 'invalid_client'}) - def test_unsupported_grant_error(self): + @patch('lti_consumer.lti_1p3.consumer.LtiConsumer1p3.access_token') + def test_unsupported_grant_error(self, access_token_mock): """ Check unsupported_grant_type is returned when the grant type is wrong """ - self.mock_client.access_token = Mock(side_effect=UnsupportedGrantType()) + access_token_mock.side_effect = UnsupportedGrantType() response = self.client.post(self.url) self.assertEqual(response.status_code, 400) self.assertEqual(response.json(), {'error': 'unsupported_grant_type'}) diff --git a/lti_consumer/tests/unit/plugin/test_views_lti_ags.py b/lti_consumer/tests/unit/plugin/test_views_lti_ags.py index 0bb24bb6..8bb376ca 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_ags.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_ags.py @@ -13,10 +13,10 @@ from lti_consumer.lti_xblock import LtiConsumerXBlock from lti_consumer.models import LtiAgsLineItem, LtiAgsScore, LtiConfiguration -from lti_consumer.tests.test_utils import make_xblock +from lti_consumer.tests.test_utils import TestBaseWithPatch, make_xblock -class LtiAgsLineItemViewSetTestCase(APITransactionTestCase): +class LtiAgsLineItemViewSetTestCase(APITransactionTestCase, TestBaseWithPatch): """ Test `LtiAgsLineItemViewset` Class. """ @@ -177,10 +177,7 @@ def test_lti_ags_list(self): response.data, [ { - 'id': 'http://testserver/lti_consumer/v1/lti/{}/lti-ags/{}'.format( - self.lti_config.id, - line_item.id - ), + 'id': f'http://testserver/lti_consumer/v1/lti/{self.lti_config.id}/lti-ags/{line_item.id}', 'resourceId': 'test', 'scoreMaximum': 100, 'label': 'test label', @@ -221,10 +218,7 @@ def test_lti_ags_retrieve(self): self.assertEqual( response.data, { - 'id': 'http://testserver/lti_consumer/v1/lti/{}/lti-ags/{}'.format( - self.lti_config.id, - line_item.id - ), + 'id': f'http://testserver/lti_consumer/v1/lti/{self.lti_config.id}/lti-ags/{line_item.id}', 'resourceId': 'test', 'scoreMaximum': 100, 'label': 'test label', @@ -401,6 +395,43 @@ def test_create_score(self): self.assertEqual(score.grading_progress, LtiAgsScore.FULLY_GRADED) self.assertEqual(score.user_id, self.primary_user_id) + @ddt.data(None, "") + def test_create_score_without_comment(self, comment): + """ + Test the LTI AGS LineItem Score Creation when comment is omitted or blank. + """ + self._set_lti_token('https://purl.imsglobal.org/spec/lti-ags/scope/score') + + data = { + "timestamp": self.early_timestamp, + "scoreGiven": 83, + "scoreMaximum": 100, + "activityProgress": LtiAgsScore.COMPLETED, + "gradingProgress": LtiAgsScore.FULLY_GRADED, + "userId": self.primary_user_id, + } + if comment is not None: + data["comment"] = comment + + response = self.client.post( + self.scores_endpoint, + data=json.dumps(data), + content_type="application/vnd.ims.lis.v1.score+json", + ) + + self.assertEqual(response.status_code, 201) + self.assertEqual(LtiAgsScore.objects.filter( + line_item=self.line_item, + user_id=self.primary_user_id + ).count(), 1) + + score = LtiAgsScore.objects.get(line_item=self.line_item, user_id=self.primary_user_id) + if comment is None: + self.assertIsNone(score.comment) + else: + self.assertEqual(score.comment, comment) + score.delete() + def _post_lti_score(self, override_data=None): """ Helper method to post a LTI score @@ -936,11 +967,14 @@ def test_retrieve_results(self): # Create Score response = self.client.get(self.results_endpoint) + response_with_trailing_slash = self.client.get(self.results_endpoint + '/') self.assertEqual(response.status_code, 200) + self.assertEqual(response_with_trailing_slash.status_code, 200) # There should be 2 results (not include the empty score user's result) self.assertEqual(len(response.data), 2) + self.assertEqual(len(response_with_trailing_slash.data), 2) # Check the data primary_user_results_endpoint = reverse( @@ -983,7 +1017,7 @@ def test_retrieve_results(self): def test_retrieve_results_for_user_id(self): """ - Test the LTI AGS LineItem Resul Retrieval for a single user. + Test the LTI AGS LineItem Result Retrieval for a single user. """ self._set_lti_token('https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly') @@ -996,14 +1030,23 @@ def test_retrieve_results_for_user_id(self): } ) + results_user_endpoint_with_trailing_slash = results_user_endpoint + '/' + # Request results with userId response = self.client.get(results_user_endpoint, data={"userId": self.secondary_user_id}) + response_with_trailing_slash = self.client.get( + results_user_endpoint_with_trailing_slash, + data={"userId": self.secondary_user_id}, + ) self.assertEqual(response.status_code, 200) + self.assertEqual(response_with_trailing_slash.status_code, 200) # There should be 1 result for that user self.assertEqual(len(response.data), 1) + self.assertEqual(len(response_with_trailing_slash.data), 1) self.assertEqual(response.data[0]['userId'], self.secondary_user_id) + self.assertEqual(response_with_trailing_slash.data[0]['userId'], self.secondary_user_id) def test_retrieve_results_with_limit(self): """ @@ -1023,3 +1066,30 @@ def test_retrieve_results_with_limit(self): # `primary_user_id` was assigned to the record with the `late_timestamp` self.assertEqual(len(response.data), 1) self.assertEqual(response.data[0]['userId'], self.primary_user_id) + + def test_results_serializer_id_includes_user_id_separator(self): + """ + Test that the results serializer builds a valid URL for a user-specific result. + """ + self._set_lti_token('https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly') + + results_user_endpoint = reverse( + 'lti_consumer:lti-ags-view-results', + kwargs={ + "lti_config_id": self.lti_config.id, + "pk": self.line_item.id, + "user_id": self.secondary_user_id, + } + ) + + response = self.client.get(results_user_endpoint, data={"userId": self.secondary_user_id}) + + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), 1) + self.assertEqual( + response.data[0]['id'], + ( + f'http://testserver/lti_consumer/v1/lti/{self.lti_config.id}/lti-ags' + f'/{self.line_item.id}/results/{self.secondary_user_id}' + ), + ) diff --git a/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py b/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py index 3c131fe6..f31094ab 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_deep_linking.py @@ -1,23 +1,22 @@ """ Tests for LTI Advantage Assignments and Grades Service views. """ -from unittest.mock import patch, Mock - import re +from unittest.mock import Mock, patch + import ddt from Cryptodome.PublicKey import RSA -from rest_framework.test import APITransactionTestCase from rest_framework.exceptions import ValidationError - +from rest_framework.test import APITransactionTestCase from lti_consumer.data import Lti1p3LaunchData -from lti_consumer.utils import cache_lti_1p3_launch_data 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.tests.test_utils import TestBaseWithPatch, make_xblock +from lti_consumer.utils import cache_lti_1p3_launch_data -class LtiDeepLinkingTestCase(APITransactionTestCase): +class LtiDeepLinkingTestCase(TestBaseWithPatch, APITransactionTestCase): """ Test `LtiAgsLineItemViewset` endpoint. """ diff --git a/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py b/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py index e6ae56cf..2b0c3509 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py @@ -2,14 +2,15 @@ Tests for LTI Names and Role Provisioning Service views. """ from unittest.mock import Mock, patch + from Cryptodome.PublicKey import RSA -from rest_framework.test import APITransactionTestCase from rest_framework.reverse import reverse +from rest_framework.test import APITransactionTestCase from lti_consumer.exceptions import LtiError from lti_consumer.lti_xblock import LtiConsumerXBlock from lti_consumer.models import LtiConfiguration -from lti_consumer.tests.test_utils import make_xblock +from lti_consumer.tests.test_utils import TestBaseWithPatch, make_xblock def generate_mock_members(num, role='student'): @@ -21,9 +22,9 @@ def generate_mock_members(num, role='student'): for i in range(num): member = { 'id': i, - 'username': 'user_{}'.format(i), - 'email': 'user{}@test.com'.format(i), - 'name': 'User {}'.format(i) + 'username': f'user_{i}', + 'email': f'user{i}@test.com', + 'name': f'User {i}' } if role == 'student': @@ -103,7 +104,7 @@ def _get_memberships(course_key): # pylint: disable=unused-argument return _get_memberships -class LtiNrpsTestCase(APITransactionTestCase): +class LtiNrpsTestCase(APITransactionTestCase, TestBaseWithPatch): # noqa: F821 """ Test LtiNrpsViewSet actions """ @@ -170,7 +171,7 @@ def _set_lti_token(self, scopes=None): "scopes": scopes, }) self.client.credentials( - HTTP_AUTHORIZATION="Bearer {}".format(token) + HTTP_AUTHORIZATION=f"Bearer {token}" ) def _parse_link_headers(self, links): @@ -240,7 +241,7 @@ def test_get_without_pii(self, expose_pii_fields_patcher): """ self._set_lti_token('https://purl.imsglobal.org/spec/lti-nrps/scope/contextmembership.readonly') response = self.client.get(self.context_membership_endpoint) - self.assertEqual(response.data['id'], 'http://testserver{}'.format(self.context_membership_endpoint)) + self.assertEqual(response.data['id'], f'http://testserver{self.context_membership_endpoint}') self.assertEqual(len(response.data['members']), 4) self.assertEqual(response.has_header('Link'), False) @@ -268,7 +269,7 @@ def test_get_with_pii(self, expose_pii_fields_patcher): self._set_lti_token('https://purl.imsglobal.org/spec/lti-nrps/scope/contextmembership.readonly') response = self.client.get(self.context_membership_endpoint) - self.assertEqual(response.data['id'], 'http://testserver{}'.format(self.context_membership_endpoint)) + self.assertEqual(response.data['id'], f'http://testserver{self.context_membership_endpoint}') self.assertEqual(len(response.data['members']), 4) self.assertEqual(response.has_header('Link'), False) diff --git a/lti_consumer/tests/unit/test_api.py b/lti_consumer/tests/unit/test_api.py index bbd18f94..8c0c8f76 100644 --- a/lti_consumer/tests/unit/test_api.py +++ b/lti_consumer/tests/unit/test_api.py @@ -3,34 +3,35 @@ """ 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 opaque_keys.edx.keys import UsageKey from lti_consumer.api import ( _get_config_by_config_id, - _get_or_create_local_lti_config, config_id_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, + _get_or_create_local_lti_config, validate_lti_1p3_launch_data, ) from lti_consumer.data import Lti1p3LaunchData, Lti1p3ProctoringLaunchData 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.models import Lti1p3Passport, LtiConfiguration, LtiDlContentItem +from lti_consumer.tests.test_utils import TestBaseWithPatch, make_xblock from lti_consumer.utils import get_data_from_cache # it's convenient to have this in lowercase to compare to URLs _test_config_id = "6c440bf4-face-beef-face-e8bcfb1e53bd" -class Lti1P3TestCase(TestCase): +class Lti1P3TestCase(TestBaseWithPatch): """ Reusable test case for testing LTI 1.3 configurations. """ @@ -40,15 +41,7 @@ def setUp(self): """ self._setup_lti_block() - # Patch compat method to avoid calls to modulestore - patcher = patch( - 'lti_consumer.plugin.compat.load_enough_xblock', - ) - self.addCleanup(patcher.stop) - self._load_block_patch = patcher.start() - self._load_block_patch.return_value = self.xblock - - return super().setUp() + super().setUp() def _setup_lti_block(self): """ @@ -68,26 +61,12 @@ def _setup_lti_block(self): 'lti_1p3_tool_public_key': public_key, } self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, xblock_attributes) - - # Create lti configuration - self.lti_config = LtiConfiguration.objects.create( - config_id=_test_config_id, - location=self.xblock.scope_ids.usage_id, - version=LtiConfiguration.LTI_1P3, - config_store=LtiConfiguration.CONFIG_ON_XBLOCK, - ) - - def _get_lti_1p3_launch_data(self): - return Lti1p3LaunchData( - user_id="1", - user_role="student", - config_id=self.lti_config.config_id, - resource_link_id="resource_link_id", - ) + self.location = self.xblock.scope_ids.usage_id + self.other_location = UsageKey.from_string("block-v1:course+101+2024+type@lti_consumer+block@test") @ddt.ddt -class TestConfigIdForBlock(TestCase): +class TestConfigIdForBlock(Lti1P3TestCase): """ Test config ID for block which is either a simple lookup or creates the config if it hasn't existed before. Config @@ -129,7 +108,7 @@ def test_store_types(self, mapping_pair, mock_external_config): @ddt.ddt -class TestGetOrCreateLocalLtiConfiguration(TestCase): +class TestGetOrCreateLocalLtiConfiguration(Lti1P3TestCase): """ Unit tests for _get_or_create_local_lti_config API method. """ @@ -137,38 +116,37 @@ def test_create_lti_config_if_inexistent(self): """ Check if the API creates a model if no object matching properties is found. """ - location = 'block-v1:course+test+2020+type@problem+block@test' lti_version = LtiConfiguration.LTI_1P3 - # Check that there's nothing in the models + # Check that there's 1 in the models self.assertEqual(LtiConfiguration.objects.all().count(), 0) # Call API lti_config = _get_or_create_local_lti_config( lti_version=lti_version, - block_location=location + block=self.xblock ) + self.assertEqual(LtiConfiguration.objects.all().count(), 1) # Check if the object was created self.assertEqual(lti_config.version, lti_version) - self.assertEqual(str(lti_config.location), location) + self.assertEqual(str(lti_config.location), str(self.location)) self.assertEqual(lti_config.config_store, LtiConfiguration.CONFIG_ON_XBLOCK) def test_retrieve_existing(self): """ Check if the API retrieves a model if the configuration matches. """ - location = 'block-v1:course+test+2020+type@problem+block@test' lti_version = LtiConfiguration.LTI_1P1 lti_config = LtiConfiguration.objects.create( - location=location + location=self.location ) # Call API lti_config_retrieved = _get_or_create_local_lti_config( lti_version=lti_version, - block_location=location + block=self.xblock ) # Check if the object was created @@ -179,17 +157,15 @@ def test_update_lti_version(self): """ Check if the API retrieves the config and updates the API version. """ - location = 'block-v1:course+test+2020+type@problem+block@test' - lti_config = LtiConfiguration.objects.create( - location=location, + location=self.location, version=LtiConfiguration.LTI_1P1 ) # Call API _get_or_create_local_lti_config( lti_version=LtiConfiguration.LTI_1P3, - block_location=location + block=self.xblock ) # Check if the object was created @@ -202,11 +178,10 @@ 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 the config_store field of the LtiConfiguration model appropriately. """ - location = 'block-v1:course+test+2020+type@problem+block@test' lti_version = LtiConfiguration.LTI_1P3 lti_config = _get_or_create_local_lti_config( lti_version=lti_version, - block_location=location, + block=self.xblock, config_store=config_store, ) @@ -216,11 +191,10 @@ def test_external_config_values_are_cleared(self): """ Check if the API clears external configuration values when external id is none """ - location = 'block-v1:course+test+2020+type@problem+block@test' lti_version = LtiConfiguration.LTI_1P3 lti_config = LtiConfiguration.objects.create( - location=location, + location=self.location, version=LtiConfiguration.LTI_1P3, config_store=LtiConfiguration.CONFIG_EXTERNAL, external_id="test_plugin:test-id" @@ -228,16 +202,234 @@ def test_external_config_values_are_cleared(self): _get_or_create_local_lti_config( lti_version=lti_version, - block_location=location, - external_id=None + block=self.xblock, ) lti_config.refresh_from_db() self.assertEqual(lti_config.version, lti_version) - self.assertEqual(str(lti_config.location), location) + self.assertEqual(str(lti_config.location), str(self.location)) self.assertEqual(lti_config.config_store, LtiConfiguration.CONFIG_ON_XBLOCK) self.assertEqual(lti_config.external_id, None) + def test_passport_created_on_first_call(self): + """ + Check if a passport is created when none exists. + """ + lti_version = LtiConfiguration.LTI_1P3 + + lti_config = _get_or_create_local_lti_config( + lti_version=lti_version, + block=self.xblock + ) + + self.assertIsNotNone(lti_config.lti_1p3_passport) + + def test_passport_updated_when_single_use(self): + """ + Check if passport is updated when it's only used by one config. + """ + passport = Lti1p3Passport.objects.create( + lti_1p3_tool_public_key='old_key', + lti_1p3_tool_keyset_url='old_url' + ) + LtiConfiguration.objects.create( + location=self.location, + lti_1p3_passport=passport + ) + + self.xblock.lti_1p3_tool_public_key = 'new_key' + self.xblock.lti_1p3_tool_keyset_url = 'new_url' + + _get_or_create_local_lti_config( + lti_version=LtiConfiguration.LTI_1P3, + block=self.xblock + ) + + passport.refresh_from_db() + self.assertEqual(passport.lti_1p3_tool_public_key, 'new_key') + self.assertEqual(passport.lti_1p3_tool_keyset_url, 'new_url') + + def test_passport_not_updated_when_config_store_not_on_xblock(self): + """ + Ensure that when the existing LtiConfiguration is not stored on the XBlock + (e.g., CONFIG_ON_DB), the passport is not updated even if block key values differ. + """ + passport = Lti1p3Passport.objects.create( + lti_1p3_tool_public_key='shared_key', + lti_1p3_tool_keyset_url='shared_url' + ) + # Create a configuration that uses the passport but is stored on DB + LtiConfiguration.objects.create( + location=self.location, + lti_1p3_passport=passport, + config_store=LtiConfiguration.CONFIG_ON_DB, + ) + + # Block has different keys, but since the config is not on XBlock, + # _ensure_lti_passport should not update the shared passport. + self.xblock.lti_1p3_tool_public_key = 'new_key' + self.xblock.lti_1p3_tool_keyset_url = 'new_url' + + _get_or_create_local_lti_config( + lti_version=LtiConfiguration.LTI_1P3, + block=self.xblock, + config_store=LtiConfiguration.CONFIG_ON_XBLOCK, + ) + + passport.refresh_from_db() + # passport should remain unchanged + self.assertEqual(passport.lti_1p3_tool_public_key, 'shared_key') + self.assertEqual(passport.lti_1p3_tool_keyset_url, 'shared_url') + + def test_passport_not_saved_when_no_changes(self): + """ + Ensure that passport.save() is not called when passport fields already match the + XBlock values and no update is required. + """ + passport = Lti1p3Passport.objects.create( + lti_1p3_tool_public_key='matching_key', + lti_1p3_tool_keyset_url='matching_url' + ) + LtiConfiguration.objects.create( + location=self.location, + lti_1p3_passport=passport + ) + + # Make block keys match the passport + self.xblock.lti_1p3_tool_public_key = 'matching_key' + self.xblock.lti_1p3_tool_keyset_url = 'matching_url' + + with patch.object(Lti1p3Passport, 'save') as mock_save: + _get_or_create_local_lti_config( + lti_version=LtiConfiguration.LTI_1P3, + block=self.xblock + ) + mock_save.assert_not_called() + + def test_config_not_saved_when_no_changes(self): + """ + Ensure that LtiConfiguration.save() is not called when no fields change. + """ + # Create a config that already matches the block's current values + LtiConfiguration.objects.create( + location=self.location, + version=LtiConfiguration.LTI_1P3, + config_store=LtiConfiguration.CONFIG_ON_XBLOCK, + ) + + # Ensure block has default external_config of None and LTI version matches + with patch.object(LtiConfiguration, 'save') as mock_save: + _get_or_create_local_lti_config( + lti_version=LtiConfiguration.LTI_1P3, + block=self.xblock, + config_store=LtiConfiguration.CONFIG_ON_XBLOCK, + ) + mock_save.assert_not_called() + + def test_new_passport_created_on_key_conflict(self): + """ + Check if a new passport is created when block key differs and passport is shared. + """ + passport = Lti1p3Passport.objects.create( + lti_1p3_tool_public_key='shared_key', + lti_1p3_tool_keyset_url='shared_url' + ) + # Create two configs sharing the passport + LtiConfiguration.objects.create(location=self.location, lti_1p3_passport=passport) + LtiConfiguration.objects.create(location=self.other_location, lti_1p3_passport=passport) + + self.xblock.lti_1p3_tool_key_mode = 'public_key' + self.xblock.lti_1p3_tool_public_key = 'new_key' + + _get_or_create_local_lti_config( + lti_version=LtiConfiguration.LTI_1P3, + block=self.xblock + ) + + # Original passport unchanged + passport.refresh_from_db() + self.assertEqual(passport.lti_1p3_tool_public_key, 'shared_key') + + # Block has new passport + self.assertNotEqual(self.xblock.lti_1p3_passport_id, str(passport.passport_id)) + + def test_passport_unchanged_when_keys_match(self): + """ + Check if passport is not updated when block keys already match. + """ + passport = Lti1p3Passport.objects.create( + lti_1p3_tool_public_key='matching_key', + lti_1p3_tool_keyset_url='matching_url' + ) + lti_config = LtiConfiguration.objects.create( + location=self.location, + lti_1p3_passport=passport + ) + + self.xblock.lti_1p3_tool_public_key = 'matching_key' + self.xblock.lti_1p3_tool_keyset_url = 'matching_url' + + original_passport_id = passport.passport_id + + _get_or_create_local_lti_config( + lti_version=LtiConfiguration.LTI_1P3, + block=self.xblock + ) + + # Same passport used + self.assertEqual(Lti1p3Passport.objects.count(), 1) + self.assertEqual(str(lti_config.lti_1p3_passport.passport_id), str(original_passport_id)) + + @ddt.data( + ('public_key', 'lti_1p3_tool_public_key'), + ('keyset_url', 'lti_1p3_tool_keyset_url') + ) + @ddt.unpack + def test_new_passport_on_key_mode_change(self, key_mode, key_field): + """ + Check if a new passport is created when key_mode-specific field changes. + """ + passport = Lti1p3Passport.objects.create( + lti_1p3_tool_public_key='old_key', + lti_1p3_tool_keyset_url='old_url' + ) + LtiConfiguration.objects.create(location=self.location, lti_1p3_passport=passport) + LtiConfiguration.objects.create(location=self.other_location, lti_1p3_passport=passport) + + self.xblock.lti_1p3_tool_key_mode = key_mode + setattr(self.xblock, key_field, 'new_value') + + _get_or_create_local_lti_config( + lti_version=LtiConfiguration.LTI_1P3, + block=self.xblock + ) + + self.assertEqual(Lti1p3Passport.objects.count(), 2) + + def test_passport_updated_when_fields_empty(self): + """ + Check if passport is updated when both key fields are empty. + """ + passport = Lti1p3Passport.objects.create( + lti_1p3_tool_public_key='', + lti_1p3_tool_keyset_url='' + ) + LtiConfiguration.objects.create( + location=self.location, + lti_1p3_passport=passport + ) + + self.xblock.lti_1p3_tool_public_key = 'new_key' + self.xblock.lti_1p3_tool_keyset_url = 'new_url' + + _get_or_create_local_lti_config( + lti_version=LtiConfiguration.LTI_1P3, + block=self.xblock + ) + + passport.refresh_from_db() + self.assertEqual(passport.lti_1p3_tool_public_key, 'new_key') + @ddt.ddt class TestValidateLti1p3LaunchData(TestCase): @@ -416,11 +608,12 @@ def test_required_start_assessment_url_for_start_proctoring_message_type(self, s ) -class TestGetLti1p3LaunchInfo(TestCase): +class TestGetLti1p3LaunchInfo(Lti1P3TestCase): """ Unit tests for get_lti_1p3_launch_info API method. """ def setUp(self): + self.maxDiff = 30000 # Patch internal method to avoid calls to modulestore patcher = patch( 'lti_consumer.models.LtiConfiguration.get_lti_consumer', @@ -446,9 +639,8 @@ def test_retrieve_with_id(self): """ Check if the API retrieves the launch with id. """ - location = 'block-v1:course+test+2020+type@problem+block@test' lti_config = LtiConfiguration.objects.create( - location=location, + location=self.location, config_id=_test_config_id, ) @@ -468,7 +660,7 @@ def test_retrieve_with_block(self): # Create LTI Config and Deep linking object lti_config = LtiConfiguration.objects.create( - location='block-v1:course+test+2020+type@problem+block@test', + location=self.location, config_id=_test_config_id, ) LtiDlContentItem.objects.create( @@ -486,14 +678,10 @@ def test_retrieve_with_block(self): launch_info, { 'client_id': lti_config.lti_1p3_client_id, - 'keyset_url': 'https://example.com/api/lti_consumer/v1/public_keysets/{}'.format( - lti_config.config_id - ), + 'keyset_url': f'https://example.com/api/lti_consumer/v1/public_keysets/{lti_config.passport_id}', 'deployment_id': '1', 'oidc_callback': 'https://example.com/api/lti_consumer/v1/launch/', - 'token_url': 'https://example.com/api/lti_consumer/v1/token/{}'.format( - lti_config.config_id - ), + 'token_url': f'https://example.com/api/lti_consumer/v1/token/{lti_config.passport_id}', 'deep_linking_launch_url': 'http://example.com', 'deep_linking_content_items': @@ -523,14 +711,10 @@ def test_launch_info_for_lti_config_without_location(self): launch_info, { 'client_id': lti_config.lti_1p3_client_id, - 'keyset_url': 'https://example.com/api/lti_consumer/v1/public_keysets/{}'.format( - lti_config.config_id - ), + 'keyset_url': f'https://example.com/api/lti_consumer/v1/public_keysets/{lti_config.passport_id}', 'deployment_id': '1', 'oidc_callback': 'https://example.com/api/lti_consumer/v1/launch/', - 'token_url': 'https://example.com/api/lti_consumer/v1/token/{}'.format( - lti_config.config_id - ), + 'token_url': f'https://example.com/api/lti_consumer/v1/token/{lti_config.passport_id}', 'deep_linking_launch_url': 'http://example.com', 'deep_linking_content_items': @@ -582,6 +766,27 @@ class TestGetLti1p3LaunchUrl(Lti1P3TestCase): """ Unit tests for LTI 1.3 launch API methods. """ + def setUp(self): + """ + Method to set up test data. + """ + super().setUp() + # Create lti configuration + self.lti_config = LtiConfiguration.objects.create( + config_id=_test_config_id, + location=self.xblock.scope_ids.usage_id, + version=LtiConfiguration.LTI_1P3, + config_store=LtiConfiguration.CONFIG_ON_XBLOCK, + ) + + def _get_lti_1p3_launch_data(self): + return Lti1p3LaunchData( + user_id="1", + user_role="student", + config_id=self.lti_config.config_id, + resource_link_id="resource_link_id", + ) + def test_get_normal_lti_launch_url(self): """ Check if the correct launch url is retrieved for a normal LTI 1.3 launch. @@ -638,6 +843,27 @@ class TestGetLti1p3ContentUrl(Lti1P3TestCase): """ Unit tests for get_lti_1p3_launch_start_url API method. """ + def setUp(self): + """ + Method to set up test data. + """ + super().setUp() + # Create lti configuration + self.lti_config = LtiConfiguration.objects.create( + config_id=_test_config_id, + location=self.xblock.scope_ids.usage_id, + version=LtiConfiguration.LTI_1P3, + config_store=LtiConfiguration.CONFIG_ON_XBLOCK, + ) + + def _get_lti_1p3_launch_data(self): + return Lti1p3LaunchData( + user_id="1", + user_role="student", + config_id=self.lti_config.config_id, + resource_link_id="resource_link_id", + ) + @patch("lti_consumer.api.get_lti_1p3_launch_start_url") def test_lti_content_presentation(self, mock_get_launch_url): """ @@ -704,7 +930,7 @@ def test_lti_content_presentation_multiple_links(self): ) -class TestGetLtiDlContentItemData(TestCase): +class TestGetLtiDlContentItemData(Lti1P3TestCase): """ Unit tests for get_deep_linking_data API method. """ @@ -712,12 +938,11 @@ def setUp(self): """ Set up an empty block configuration. """ + super().setUp() self.lti_config = LtiConfiguration.objects.create( location='block-v1:course+test+2020+type@problem+block@test', ) - return super().setUp() - def test_lti_retrieve_content_item(self): """ Check if the API return the right content item. diff --git a/lti_consumer/tests/unit/test_filters.py b/lti_consumer/tests/unit/test_filters.py index f4afc5c5..40c42e32 100644 --- a/lti_consumer/tests/unit/test_filters.py +++ b/lti_consumer/tests/unit/test_filters.py @@ -2,14 +2,11 @@ Tests for the LTI consumer filters. """ from unittest.mock import patch -from django.test import TestCase, override_settings - +from django.test import TestCase, override_settings from openedx_filters import PipelineStep -from lti_consumer.filters import ( - LTIConfigurationListed, - get_external_config_from_filter -) + +from lti_consumer.filters import LTIConfigurationListed, get_external_config_from_filter class MyTestPipelineStep(PipelineStep): diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py index ab81ff95..0f5fdcd0 100644 --- a/lti_consumer/tests/unit/test_lti_xblock.py +++ b/lti_consumer/tests/unit/test_lti_xblock.py @@ -7,6 +7,7 @@ from datetime import timedelta from itertools import product from unittest.mock import Mock, PropertyMock, patch +from xml.etree.ElementTree import Element import ddt import jwt @@ -23,9 +24,11 @@ from lti_consumer.exceptions import LtiError from lti_consumer.lti_1p3.tests.utils import create_jwt from lti_consumer.lti_xblock import LtiConsumerXBlock, parse_handler_suffix, valid_config_type_values +from lti_consumer.models import Lti1p3Passport, LtiConfiguration from lti_consumer.tests import test_utils from lti_consumer.tests.test_utils import ( FAKE_USER_ID, + TestBaseWithPatch, get_mock_lti_configuration, make_jwt_request, make_request, @@ -40,7 +43,7 @@ HTML_IFRAME = '= 1 + + @data( + None, + UsageKey.from_string("block-v1:course+test+2020+type@problem+block@parent").for_branch("branch"), + ) + def test_delete_child_lti_configurations_invalid_usage_key(self, usage_key): + """Test with None or missing usage_key.""" + mocks, _, _ = self._setup_mocks() + + if usage_key is None: + delete_child_lti_configurations(usage_key=None) + else: + # Test branch stripping + delete_child_lti_configurations(usage_key=usage_key, user_id="test_user") + mocks['compat'].load_enough_xblock.assert_called_once_with(self.usage_key.for_branch(None)) + + @data( + Exception("Block not found"), + ValueError("Invalid block"), + RuntimeError("Load failed"), + ) + def test_delete_child_lti_configurations_load_block_fails(self, error): + """Test when load_enough_xblock raises exceptions.""" + mocks, _, _ = self._setup_mocks(load_error=error) + + delete_child_lti_configurations(usage_key=self.usage_key, user_id="test_user") + + # Verify warning logged with error details + mocks['log'].warning.assert_called_once() + warning_msg = mocks['log'].warning.call_args[0][0] + assert "Cannot find xblock for key" in warning_msg + assert str(error) in warning_msg + + # Verify no deletion attempted + mocks['lti_config'].objects.filter.assert_not_called() + + def test_delete_child_lti_configurations_no_usage_key(self): + """Test when usage_key is not provided.""" + with patch("lti_consumer.signals.signals.log") as mock_log: + delete_child_lti_configurations(usage_key=None) + + # Should return early without logging + mock_log.warning.assert_not_called() + mock_log.info.assert_not_called() + + +@ddt +class TestDeleteLibLtiConfiguration(TestCase): + """Tests for delete_lib_lti_configuration function.""" + + def setUp(self): + """Set up test fixtures.""" + self.library_block = Mock(spec=LibraryBlockData) + self.library_block.usage_key = UsageKey.from_string( + "lb:TestOrg:TestLibrary:problem:test_problem" + ) + + def _setup_mocks(self, passport_count=0): + """Helper to setup common mock patches.""" + patches = { + 'lti_config': patch("lti_consumer.signals.signals.LtiConfiguration"), + 'passport': patch("lti_consumer.signals.signals.Lti1p3Passport"), + 'log': patch("lti_consumer.signals.signals.log"), + } + + mocks = {name: p.start() for name, p in patches.items()} + self.addCleanup(lambda: [p.stop() for p in patches.values()]) + + mocks['lti_config'].objects.filter.return_value.delete.return_value = None + mocks['passport'].objects.filter.return_value.delete.return_value = ( + passport_count, + {'Lti1p3Passport': passport_count} if passport_count else {} + ) + + return mocks + + @data(0, 1, 5, 3) + def test_delete_lib_lti_configuration_success(self, passport_count): + """Test successful deletion with various passport counts.""" + mocks = self._setup_mocks(passport_count) + + delete_lib_lti_configuration(library_block=self.library_block) + + # Verify LtiConfiguration filter called with correct location + mocks['lti_config'].objects.filter.assert_called_once_with( + location=str(self.library_block.usage_key) + ) + + # Verify orphaned passports deleted + mocks['passport'].objects.filter.assert_called_once_with(lticonfiguration__isnull=True) + + # Verify info logged with passport count + mocks['log'].info.assert_called_once() + log_msg = mocks['log'].info.call_args[0][0] + assert str(passport_count) in log_msg + if passport_count > 0: + assert "lti 1.3 passport" in log_msg + + @data( + None, + "invalid_string", + {"usage_key": "test"}, + 123, + Mock(), # Mock without LibraryBlockData spec + ) + def test_delete_lib_lti_configuration_invalid_input(self, library_block): + """Test with invalid library_block inputs.""" + mocks = self._setup_mocks() + + delete_lib_lti_configuration(library_block=library_block) + + mocks['log'].error.assert_called_once_with("Received null or incorrect data for event") + mocks['lti_config'].objects.filter.assert_not_called() + + def test_delete_lib_lti_configuration_missing_library_block(self): + """Test when library_block kwarg is missing.""" + mocks = self._setup_mocks() + + delete_lib_lti_configuration() + + mocks['log'].error.assert_called_once_with("Received null or incorrect data for event") + + def test_delete_lib_lti_configuration_extra_kwargs_ignored(self): + """Test that extra kwargs are safely ignored.""" + mocks = self._setup_mocks(passport_count=2) + + delete_lib_lti_configuration( + library_block=self.library_block, + extra_param="ignored", + another_param=123 + ) + + mocks['lti_config'].objects.filter.assert_called_once() + mocks['log'].info.assert_called_once() + + +class TestDuplicateXblockLtiConfiguration(TestCase): + """Tests for duplicate_xblock_lti_configuration function.""" + + def setUp(self): + self.source_usage_key = UsageKey.from_string( + "block-v1:course+101+2024+type@lti_consumer+block@source" + ) + self.target_usage_key = UsageKey.from_string( + "block-v1:course+101+2024+type@lti_consumer+block@target" + ) + self.xblock_data = Mock(spec=DuplicatedXBlockData) + self.xblock_data.source_usage_key = self.source_usage_key + self.xblock_data.usage_key = self.target_usage_key + + @patch('lti_consumer.signals.signals.model_to_dict') + @patch('lti_consumer.signals.signals.LtiConfiguration') + def test_duplicate_success(self, mock_lti_config, mock_model_to_dict): + """Duplicate creates copy when source exists and target empty.""" + source_config = Mock() + mock_lti_config.objects.filter.return_value.first.side_effect = [source_config] + mock_lti_config.objects.filter.return_value.exists.return_value = False + mock_model_to_dict.return_value = { + 'field_a': 'value_a', + 'field_b': 'value_b', + } + + duplicate_xblock_lti_configuration(xblock_info=self.xblock_data) + + mock_lti_config.objects.filter.assert_any_call(location=str(self.source_usage_key)) + mock_lti_config.objects.filter.assert_any_call(location=str(self.target_usage_key)) + mock_model_to_dict.assert_called_once_with( + source_config, + exclude=["id", "pk", "location", "config_id"], + ) + mock_lti_config.objects.create.assert_called_once() + create_kwargs = mock_lti_config.objects.create.call_args.kwargs + assert create_kwargs['field_a'] == 'value_a' + assert create_kwargs['field_b'] == 'value_b' + assert create_kwargs['location'] == str(self.target_usage_key) + assert create_kwargs['config_id'] is not None + + @patch('lti_consumer.signals.signals.log') + @patch('lti_consumer.signals.signals.LtiConfiguration') + def test_duplicate_source_missing(self, mock_lti_config, mock_log): + """No source config triggers warning and exits.""" + mock_lti_config.objects.filter.return_value.first.return_value = None + mock_lti_config.objects.filter.return_value.exists.return_value = False + + duplicate_xblock_lti_configuration(xblock_info=self.xblock_data) + + mock_log.warning.assert_called_once() + mock_lti_config.objects.create.assert_not_called() + + @patch('lti_consumer.signals.signals.log') + @patch('lti_consumer.signals.signals.LtiConfiguration') + def test_duplicate_target_exists(self, mock_lti_config, mock_log): + """Existing target config skips duplication.""" + mock_lti_config.objects.filter.return_value.first.return_value = Mock() + mock_lti_config.objects.filter.return_value.exists.return_value = True + + duplicate_xblock_lti_configuration(xblock_info=self.xblock_data) + + mock_log.info.assert_called_once() + mock_lti_config.objects.create.assert_not_called() + + @patch('lti_consumer.signals.signals.log') + def test_duplicate_invalid_input(self, mock_log): + """Invalid xblock_info logs error.""" + duplicate_xblock_lti_configuration(xblock_info=None) + mock_log.error.assert_called_once_with("Received null or incorrect data for event") diff --git a/lti_consumer/tests/unit/test_utils.py b/lti_consumer/tests/unit/test_utils.py index 45cf1535..aadf8164 100644 --- a/lti_consumer/tests/unit/test_utils.py +++ b/lti_consumer/tests/unit/test_utils.py @@ -5,17 +5,17 @@ import ddt from django.test.testcases import TestCase - from opaque_keys.edx.locator import CourseLocator + from lti_consumer.lti_1p3.constants import LTI_1P3_CONTEXT_TYPE from lti_consumer.utils import ( + cache_lti_1p3_launch_data, choose_lti_1p3_redirect_uris, + external_multiple_launch_urls_enabled, + get_data_from_cache, get_lti_1p3_context_types_claim, get_lti_1p3_launch_data_cache_key, - cache_lti_1p3_launch_data, - get_data_from_cache, model_to_dict, - external_multiple_launch_urls_enabled, ) LAUNCH_URL = "http://tool.launch" diff --git a/lti_consumer/utils.py b/lti_consumer/utils.py index e69cb00a..5b25e39e 100644 --- a/lti_consumer/utils.py +++ b/lti_consumer/utils.py @@ -353,7 +353,7 @@ def check_token_claim(token, claim_key, expected_value=None, invalid_claim_error raise InvalidClaimValue(msg) -def model_to_dict(model_object, exclude=None): +def model_to_dict(model_object, exclude: list[str] | None = None, include: list[str] | None = None): """ Get dictionary from model object. @@ -369,7 +369,7 @@ def model_to_dict(model_object, exclude=None): # Remove private and excluded fields. for key in list(object_fields): - if key.startswith('_') or key in exclude: + if key.startswith('_') or key in exclude or (include and key not in include): object_fields.pop(key, None) return object_fields diff --git a/requirements/base.in b/requirements/base.in index 97c12bc8..15fc9330 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 b175ed5d..3e75f1d5 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -9,7 +9,9 @@ appdirs==1.4.4 asgiref==3.11.1 # via django attrs==26.1.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.13 # djangorestframework # edx-django-utils # jsonfield + # openedx-events # openedx-filters django-appconf==1.2.0 # via django-statici18n @@ -47,25 +50,32 @@ djangorestframework==3.17.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]==4.0.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 # via -r requirements/base.in lazy==1.6 # via -r requirements/base.in -lxml==6.0.4 +lxml==6.1.0 # via # -r requirements/base.in # xblock -mako==1.3.10 +mako==1.3.11 # via # -r requirements/base.in # xblock @@ -75,6 +85,8 @@ markupsafe==3.0.3 # xblock oauthlib==3.3.1 # via -r requirements/base.in +openedx-events==11.1.1 + # via -r requirements/base.in openedx-filters==3.1.0 # via -r requirements/base.in psutil==7.2.2 @@ -95,7 +107,7 @@ pytz==2026.1.post1 # via xblock pyyaml==6.0.3 # via xblock -simplejson==3.20.2 +simplejson==4.0.1 # via xblock six==1.17.0 # via diff --git a/requirements/ci.txt b/requirements/ci.txt index f010cd1f..b72da466 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -26,18 +26,20 @@ astroid==4.0.4 # pylint # pylint-celery attrs==26.1.0 - # via -r requirements/test.txt + # via + # -r requirements/test.txt + # openedx-events binaryornot==0.6.0 # via # -r requirements/test.txt # cookiecutter bleach==6.3.0 # via -r requirements/test.txt -boto3==1.42.89 +boto3==1.42.91 # via # -r requirements/test.txt # fs-s3fs -botocore==1.42.89 +botocore==1.42.91 # via # -r requirements/test.txt # boto3 @@ -117,6 +119,7 @@ django==5.2.13 # djangorestframework # edx-django-utils # jsonfield + # openedx-events # openedx-filters # xblock-sdk django-appconf==1.2.0 @@ -150,19 +153,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==6.0.0 # via -r requirements/test.txt edx-opaque-keys[django]==4.0.0 # via # -r requirements/test.txt # edx-ccx-keys + # openedx-events # openedx-filters -filelock==3.25.2 +fastavro==1.12.1 + # via + # -r requirements/test.txt + # openedx-events +filelock==3.29.0 # via # -r requirements/tox.txt # python-discovery @@ -224,12 +235,12 @@ keyring==25.7.0 # twine lazy==1.6 # via -r requirements/test.txt -lxml==6.0.4 +lxml==6.1.0 # via # -r requirements/test.txt # xblock # xblock-sdk -mako==1.3.10 +mako==1.3.11 # via # -r requirements/test.txt # xblock @@ -264,9 +275,11 @@ nh3==0.3.4 # readme-renderer oauthlib==3.3.1 # via -r requirements/test.txt +openedx-events==11.1.1 + # via -r requirements/test.txt openedx-filters==3.1.0 # via -r requirements/test.txt -packaging==26.0 +packaging==26.1 # via # -r requirements/test.txt # -r requirements/tox.txt @@ -405,7 +418,7 @@ shellingham==1.5.4 # via # -r requirements/test.txt # typer -simplejson==3.20.2 +simplejson==4.0.1 # via # -r requirements/test.txt # xblock @@ -463,7 +476,7 @@ urllib3==1.26.20 # botocore # requests # twine -virtualenv==21.2.3 +virtualenv==21.2.4 # via # -r requirements/tox.txt # tox diff --git a/requirements/dev.txt b/requirements/dev.txt index 32c3ce7a..717f7141 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -13,7 +13,9 @@ asgiref==3.11.1 # -r requirements/base.txt # django attrs==26.1.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.13 # 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==2.0.0 # via -r requirements/dev.in edx-opaque-keys[django]==4.0.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 @@ -85,7 +96,7 @@ jsonfield==3.2.0 # via -r requirements/base.txt lazy==1.6 # via -r requirements/base.txt -lxml[html-clean]==6.0.4 +lxml[html-clean]==6.1.0 # via # -r requirements/base.txt # edx-i18n-tools @@ -93,7 +104,7 @@ lxml[html-clean]==6.0.4 # xblock lxml-html-clean==0.4.4 # via lxml -mako==1.3.10 +mako==1.3.11 # via # -r requirements/base.txt # xblock @@ -104,6 +115,8 @@ markupsafe==3.0.3 # xblock oauthlib==3.3.1 # via -r requirements/base.txt +openedx-events==11.1.1 + # via -r requirements/base.txt openedx-filters==3.1.0 # via -r requirements/base.txt path==16.16.0 @@ -143,7 +156,7 @@ pyyaml==6.0.3 # -r requirements/base.txt # edx-i18n-tools # xblock -simplejson==3.20.2 +simplejson==4.0.1 # via # -r requirements/base.txt # xblock diff --git a/requirements/pip.txt b/requirements/pip.txt index c87fe302..b76333d3 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -4,7 +4,7 @@ # # make upgrade # -packaging==26.0 +packaging==26.1 # via wheel wheel==0.46.3 # via -r requirements/pip.in diff --git a/requirements/pip_tools.txt b/requirements/pip_tools.txt index 81dcfcd5..fa009aa9 100644 --- a/requirements/pip_tools.txt +++ b/requirements/pip_tools.txt @@ -8,7 +8,7 @@ build==1.4.3 # via pip-tools click==8.3.2 # via pip-tools -packaging==26.0 +packaging==26.1 # via # build # wheel diff --git a/requirements/quality.txt b/requirements/quality.txt index 9e8f4ea7..35d1881b 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -19,14 +19,16 @@ astroid==4.0.4 # pylint # pylint-celery attrs==26.1.0 - # via -r requirements/base.txt + # 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.89 +boto3==1.42.91 # via fs-s3fs -botocore==1.42.89 +botocore==1.42.91 # via # boto3 # s3transfer @@ -72,6 +74,7 @@ django==5.2.13 # 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==6.0.0 # via -r requirements/quality.in edx-opaque-keys[django]==4.0.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 @@ -136,12 +147,12 @@ jsonfield==3.2.0 # via -r requirements/base.txt lazy==1.6 # via -r requirements/base.txt -lxml==6.0.4 +lxml==6.1.0 # via # -r requirements/base.txt # xblock # xblock-sdk -mako==1.3.10 +mako==1.3.11 # via # -r requirements/base.txt # xblock @@ -159,6 +170,8 @@ mdurl==0.1.2 # via markdown-it-py oauthlib==3.3.1 # via -r requirements/base.txt +openedx-events==11.1.1 + # via -r requirements/base.txt openedx-filters==3.1.0 # via -r requirements/base.txt platformdirs==4.9.6 @@ -232,7 +245,7 @@ rich==15.0.0 # via cookiecutter s3transfer==0.16.0 # via boto3 -simplejson==3.20.2 +simplejson==4.0.1 # via # -r requirements/base.txt # xblock diff --git a/requirements/test.txt b/requirements/test.txt index 990e8447..b059357d 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -21,14 +21,16 @@ astroid==4.0.4 # pylint # pylint-celery attrs==26.1.0 - # via -r requirements/base.txt + # 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.89 +boto3==1.42.91 # via fs-s3fs -botocore==1.42.89 +botocore==1.42.91 # via # boto3 # s3transfer @@ -80,6 +82,7 @@ dill==0.4.1 # djangorestframework # edx-django-utils # jsonfield + # openedx-events # openedx-filters # xblock-sdk django-appconf==1.2.0 @@ -112,18 +115,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==6.0.0 # via -r requirements/test.in edx-opaque-keys[django]==4.0.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 @@ -161,12 +172,12 @@ keyring==25.7.0 # via twine lazy==1.6 # via -r requirements/base.txt -lxml==6.0.4 +lxml==6.1.0 # via # -r requirements/base.txt # xblock # xblock-sdk -mako==1.3.10 +mako==1.3.11 # via # -r requirements/base.txt # xblock @@ -192,9 +203,11 @@ nh3==0.3.4 # via readme-renderer oauthlib==3.3.1 # via -r requirements/base.txt +openedx-events==11.1.1 + # via -r requirements/base.txt openedx-filters==3.1.0 # via -r requirements/base.txt -packaging==26.0 +packaging==26.1 # via twine platformdirs==4.9.6 # via pylint @@ -287,7 +300,7 @@ secretstorage==3.5.0 # via keyring shellingham==1.5.4 # via typer -simplejson==3.20.2 +simplejson==4.0.1 # via # -r requirements/base.txt # xblock diff --git a/requirements/tox.txt b/requirements/tox.txt index 1609ba19..0999dd66 100644 --- a/requirements/tox.txt +++ b/requirements/tox.txt @@ -10,12 +10,12 @@ colorama==0.4.6 # via tox distlib==0.4.0 # via virtualenv -filelock==3.25.2 +filelock==3.29.0 # via # python-discovery # tox # virtualenv -packaging==26.0 +packaging==26.1 # via # pyproject-api # tox @@ -36,5 +36,5 @@ tomli-w==1.2.0 # via tox tox==4.53.0 # via -r requirements/tox.in -virtualenv==21.2.3 +virtualenv==21.2.4 # via tox