From a52eef76c967c4790b99b846f6eb9f1300d81717 Mon Sep 17 00:00:00 2001 From: Andy Shultz <ashultz@edx.org> Date: Wed, 12 Oct 2022 12:17:54 -0400 Subject: [PATCH] feat: update lti API to pick config via launch data Note that this uses config_id (the UUID) not config.id (the int) This required a way to get config_id if we only have the block. And it means that we are more likely to go through load_block_as_user because we have not created the config off the block even when calling from the block (since the block has to go through that config ID). A lot of tests had to be updated to have more complete configuration setup because config_id is now load bearing. --- CHANGELOG.rst | 16 +- lti_consumer/__init__.py | 2 +- lti_consumer/api.py | 103 ++++----- lti_consumer/lti_xblock.py | 14 +- .../templatetags/get_dl_lti_launch_url.py | 1 - lti_consumer/tests/unit/test_api.py | 211 ++++++++---------- lti_consumer/tests/unit/test_lti_xblock.py | 8 +- 7 files changed, 170 insertions(+), 185 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2584786..0f67cf4 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,13 +15,25 @@ Please See the [releases tab](https://github.com/edx/xblock-lti-consumer/release Unreleased ~~~~~~~~~~ +6.0.0 - 2022-10-24 +------------------ +BREAKING CHANGE: + +Please note that additional breaking changes will be forthcoming in future versions of this library. + +* Modified Python API methods to use config_id (the UUID field) exclusively rather than config.id or block. + + * For the functions changed in 5.0.0 the config_id is available in the launch_data. + * Other functions had config.id changed to config_id and block removed as an argument. + * The new function config_id_for_block gets that config ID if all you have is a block. + 5.0.1 - 2022-10-17 ------------------ * Fixed a bug that prevented LTI 1.3 launches from occurring in the browser due to Django's clickjacking protection. * Added the xframe_options_exempt view decorator to launch_gate_endpoint to allow loading response in an <iframe> tags * Fixed a bug in the URL used for an LTI 1.3 launch; the library now sends LTI 1.3 launches to the redirect_uri provided - by the Tool in the authentication request, instead of the preregistered target_link_uri. + by the Tool in the authentication request, instead of the preregistered target_link_uri. 5.0.0 - 2022-10-12 ------------------ @@ -107,7 +119,7 @@ Please note that additional breaking changes will be forthcoming in future versi 3.4.1 - 2022-02-01 ------------------ -* Fix the target_link_uri parameter on OIDC login preflight url parameter so it matches +* Fix the target_link_uri parameter on OIDC login preflight url parameter so it matches claim message definition of the field. See docs at https://www.imsglobal.org/spec/lti/v1p3#target-link-uri diff --git a/lti_consumer/__init__.py b/lti_consumer/__init__.py index 45f4905..0e810af 100644 --- a/lti_consumer/__init__.py +++ b/lti_consumer/__init__.py @@ -4,4 +4,4 @@ Runtime will load the XBlock class from here. from .apps import LTIConsumerApp from .lti_xblock import LtiConsumerXBlock -__version__ = '5.0.1' +__version__ = '6.0.0' diff --git a/lti_consumer/api.py b/lti_consumer/api.py index 3a4ff70..dc1eed0 100644 --- a/lti_consumer/api.py +++ b/lti_consumer/api.py @@ -10,7 +10,6 @@ import json from opaque_keys.edx.keys import CourseKey from lti_consumer.lti_1p3.constants import LTI_1P3_ROLE_MAP -from .exceptions import LtiError from .models import CourseAllowPIISharingInLTIFlag, LtiConfiguration, LtiDlContentItem from .utils import ( get_lti_1p3_context_types_claim, @@ -47,50 +46,61 @@ def _get_or_create_local_lti_config(lti_version, block_location, return lti_config -def _get_lti_config(config_id=None, block=None): +def _get_config_by_config_id(config_id): """ - Retrieves or creates a LTI Configuration using either block or LTI Config ID. + Gets the LTI config by its UUID config ID + """ + return LtiConfiguration.objects.get(config_id=config_id) + + +def _get_lti_config_for_block(block): + """ + Retrieves or creates a LTI Configuration for a block. This wraps around `_get_or_create_local_lti_config` and handles the block and modulestore bits of configuration. """ - if config_id: - lti_config = LtiConfiguration.objects.get(pk=config_id) - elif block: - if block.config_type == 'database': - lti_config = _get_or_create_local_lti_config( - block.lti_version, - block.location, - LtiConfiguration.CONFIG_ON_DB, - ) - elif block.config_type == 'external': - config = get_external_config_from_filter( - {"course_key": block.location.course_key}, - block.external_config - ) - lti_config = _get_or_create_local_lti_config( - config.get("version"), - block.location, - LtiConfiguration.CONFIG_EXTERNAL, - external_id=block.external_config, - ) - else: - lti_config = _get_or_create_local_lti_config( - block.lti_version, - block.location, - LtiConfiguration.CONFIG_ON_XBLOCK, - ) - - # Since the block was passed, preload it to avoid - # having to instance the modulestore and fetch it again. - lti_config.block = block + if block.config_type == 'database': + lti_config = _get_or_create_local_lti_config( + block.lti_version, + block.location, + LtiConfiguration.CONFIG_ON_DB, + ) + elif block.config_type == 'external': + config = get_external_config_from_filter( + {"course_key": block.location.course_key}, + block.external_config + ) + lti_config = _get_or_create_local_lti_config( + config.get("version"), + block.location, + LtiConfiguration.CONFIG_EXTERNAL, + external_id=block.external_config, + ) else: - raise LtiError('Either a config_id or block is required to get or create an LTI Configuration.') + lti_config = _get_or_create_local_lti_config( + block.lti_version, + block.location, + LtiConfiguration.CONFIG_ON_XBLOCK, + ) + + # Since the block was passed, preload it to avoid + # having to instance the modulestore and fetch it again. + lti_config.block = block return lti_config -def get_lti_consumer(config_id=None, block=None): +def config_id_for_block(block): + """ + Returns the externally facing config_id of the LTI Configuration used by this block, + creating one if required. That ID is suitable for use in launch data or get_consumer. + """ + config = _get_lti_config_for_block(block) + return config.config_id + + +def get_lti_consumer(config_id): """ Retrieves an LTI Consumer instance for a given configuration. @@ -99,19 +109,17 @@ def get_lti_consumer(config_id=None, block=None): """ # Return an instance of LTI 1.1 or 1.3 consumer, depending # on the configuration stored in the model. - return _get_lti_config(config_id, block).get_lti_consumer() + return _get_config_by_config_id(config_id).get_lti_consumer() def get_lti_1p3_launch_info( launch_data, - config_id=None, - block=None, ): """ Retrieves the Client ID, Keyset URL and other urls used to configure a LTI tool. """ # Retrieve LTI Config and consumer - lti_config = _get_lti_config(config_id, block) + lti_config = _get_config_by_config_id(launch_data.config_id) lti_consumer = lti_config.get_lti_consumer() # Check if deep Linking is available, if so, add some extra context: @@ -152,8 +160,6 @@ def get_lti_1p3_launch_info( def get_lti_1p3_launch_start_url( launch_data, - config_id=None, - block=None, deep_link_launch=False, dl_content_id=None, ): @@ -161,8 +167,7 @@ def get_lti_1p3_launch_start_url( Computes and retrieves the LTI URL that starts the OIDC flow. """ # Retrieve LTI consumer - lti_config = _get_lti_config(config_id, block) - lti_consumer = lti_config.get_lti_consumer() + lti_consumer = get_lti_consumer(launch_data.config_id) # Include a message hint in the launch_data depending on LTI launch type # Case 1: Performs Deep Linking configuration flow. Triggered by staff users to @@ -180,8 +185,6 @@ def get_lti_1p3_launch_start_url( def get_lti_1p3_content_url( launch_data, - config_id=None, - block=None, ): """ Computes and returns which URL the LTI consumer should launch to. @@ -194,22 +197,20 @@ def get_lti_1p3_content_url( Lti DL content in the database. """ # Retrieve LTI consumer - lti_config = _get_lti_config(config_id, block) + lti_config = _get_config_by_config_id(launch_data.config_id) # List content items content_items = lti_config.ltidlcontentitem_set.all() # If there's no content items, return normal LTI launch URL if not content_items.count(): - return get_lti_1p3_launch_start_url(launch_data, config_id=config_id, block=block) + return get_lti_1p3_launch_start_url(launch_data) # If there's a single `ltiResourceLink` content, return the launch # url for that specific deep link if content_items.count() == 1 and content_items.get().content_type == LtiDlContentItem.LTI_RESOURCE_LINK: return get_lti_1p3_launch_start_url( launch_data, - config_id=config_id, - block=block, dl_content_id=content_items.get().id, ) @@ -217,7 +218,7 @@ def get_lti_1p3_content_url( return get_lti_deeplinking_content_url(lti_config.id, launch_data) -def get_deep_linking_data(deep_linking_id, config_id=None, block=None): +def get_deep_linking_data(deep_linking_id, config_id): """ Retrieves deep linking attributes. @@ -225,7 +226,7 @@ def get_deep_linking_data(deep_linking_id, config_id=None, block=None): current content presentation implementation. """ # Retrieve LTI Configuration - lti_config = _get_lti_config(config_id, block) + lti_config = _get_config_by_config_id(config_id) # Only filter DL content item from content item set in the same LTI configuration. # This avoids a malicious user to input a random LTI id and perform LTI DL # content launches outside the scope of its configuration. diff --git a/lti_consumer/lti_xblock.py b/lti_consumer/lti_xblock.py index 29c2752..9973e43 100644 --- a/lti_consumer/lti_xblock.py +++ b/lti_consumer/lti_xblock.py @@ -973,9 +973,9 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): # Runtime import since this will only run in the # Open edX LMS/Studio environments. # pylint: disable=import-outside-toplevel - from lti_consumer.api import get_lti_consumer + from lti_consumer.api import config_id_for_block, get_lti_consumer - return get_lti_consumer(block=self) + return get_lti_consumer(config_id_for_block(self)) def extract_real_user_data(self): """ @@ -1034,8 +1034,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): launch_data = self.get_lti_1p3_launch_data() context.update( get_lti_1p3_launch_info( - launch_data=launch_data, - block=self + launch_data, ) ) @@ -1423,8 +1422,8 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): # Open edX LMS/Studio environments. # TODO: Replace this with a more appropriate API function that is intended for public use. # pylint: disable=import-outside-toplevel - from lti_consumer.api import _get_lti_config - lti_config = _get_lti_config(block=self) + from lti_consumer.api import config_id_for_block + config_id = config_id_for_block(self) location = self.location # pylint: disable=no-member course_key = str(location.course_key) @@ -1432,7 +1431,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): launch_data = Lti1p3LaunchData( user_id=self.external_user_id, user_role=self.role, - config_id=lti_config.config_id, + config_id=config_id, resource_link_id=str(location), launch_presentation_document_target="iframe", context_id=course_key, @@ -1473,7 +1472,6 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): from lti_consumer.api import get_lti_1p3_content_url # pylint: disable=import-outside-toplevel lti_block_launch_handler = get_lti_1p3_content_url( launch_data, - block=self, ) return lti_block_launch_handler diff --git a/lti_consumer/templatetags/get_dl_lti_launch_url.py b/lti_consumer/templatetags/get_dl_lti_launch_url.py index d49f585..b56d606 100644 --- a/lti_consumer/templatetags/get_dl_lti_launch_url.py +++ b/lti_consumer/templatetags/get_dl_lti_launch_url.py @@ -20,6 +20,5 @@ def get_dl_lti_launch_url(content_item, launch_data): """ return get_lti_1p3_launch_start_url( launch_data, - config_id=content_item.lti_configuration.id, dl_content_id=content_item.id, ) diff --git a/lti_consumer/tests/unit/test_api.py b/lti_consumer/tests/unit/test_api.py index 94633a9..54c8e0f 100644 --- a/lti_consumer/tests/unit/test_api.py +++ b/lti_consumer/tests/unit/test_api.py @@ -8,16 +8,16 @@ 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.locations import Location from lti_consumer.api import ( + _get_config_by_config_id, _get_or_create_local_lti_config, + config_id_for_block, get_lti_1p3_content_url, get_deep_linking_data, get_lti_1p3_launch_info, get_lti_1p3_launch_start_url, - get_lti_consumer, - validate_lti_1p3_launch_data + validate_lti_1p3_launch_data, ) from lti_consumer.data import Lti1p3LaunchData from lti_consumer.lti_xblock import LtiConsumerXBlock @@ -25,6 +25,9 @@ from lti_consumer.models import LtiConfiguration, LtiDlContentItem from lti_consumer.tests.test_utils import make_xblock from lti_consumer.utils import get_data_from_cache +# it's convenient to have this in lowercase to compare to URLs +_test_config_id = "6c440bf4-face-beef-face-e8bcfb1e53bd" + class Lti1P3TestCase(TestCase): """ @@ -34,8 +37,15 @@ class Lti1P3TestCase(TestCase): """ Set up an empty block configuration. """ - self.xblock = None - self.lti_config = None + self._setup_lti_block() + + # Patch compat method to avoid calls to modulestore + patcher = patch( + 'lti_consumer.plugin.compat.load_block_as_user', + ) + self.addCleanup(patcher.stop) + self._load_block_patch = patcher.start() + self._load_block_patch.return_value = self.xblock return super().setUp() @@ -48,7 +58,7 @@ class Lti1P3TestCase(TestCase): public_key = rsa_key.publickey().export_key() xblock_attributes = { - 'lti_version': 'lti_1p3', + 'lti_version': LtiConfiguration.LTI_1P3, 'lti_1p3_launch_url': 'http://tool.example/launch', 'lti_1p3_oidc_url': 'http://tool.example/oidc', # We need to set the values below because they are not automatically @@ -60,19 +70,62 @@ class Lti1P3TestCase(TestCase): # Create lti configuration self.lti_config = LtiConfiguration.objects.create( - location=self.xblock.location # pylint: disable=no-member + config_id=_test_config_id, + location=self.xblock.location, # pylint: disable=no-member + block=self.xblock, + version=LtiConfiguration.LTI_1P3, + config_store=LtiConfiguration.CONFIG_ON_XBLOCK, ) - @staticmethod - def _get_lti_1p3_launch_data(): + def _get_lti_1p3_launch_data(self): return Lti1p3LaunchData( user_id="1", user_role="student", - config_id="1", + config_id=self.lti_config.config_id, resource_link_id="resource_link_id", ) +@ddt.ddt +class TestConfigIdForBlock(TestCase): + """ + Test config ID for block which is either a simple lookup + or creates the config if it hasn't existed before. Config + creation forks on store type. + """ + def setUp(self): + xblock_attributes = { + 'lti_version': LtiConfiguration.LTI_1P1, + } + self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, xblock_attributes) + + def test_double_fetch(self): + self.xblock.config_type = 'database' + config_id = config_id_for_block(self.xblock) + self.assertIsNotNone(config_id) + config = _get_config_by_config_id(config_id) + self.assertEqual(LtiConfiguration.CONFIG_ON_DB, config.config_store) + + # fetch again, shouldn't make a new one + second_config_id = config_id_for_block(self.xblock) + self.assertEqual(config_id, second_config_id) + + @ddt.data( + ('external', LtiConfiguration.CONFIG_EXTERNAL), + ('database', LtiConfiguration.CONFIG_ON_DB), + ('any other val', LtiConfiguration.CONFIG_ON_XBLOCK), + ) + @patch('lti_consumer.api.get_external_config_from_filter') + def test_store_types(self, mapping_pair, mock_external_config): + mock_external_config.return_value = {"version": LtiConfiguration.LTI_1P3} + str_store, result_store = mapping_pair + self.xblock.config_type = str_store + config_id = config_id_for_block(self.xblock) + self.assertIsNotNone(config_id) + config = _get_config_by_config_id(config_id) + self.assertEqual(result_store, config.config_store) + + @ddt.ddt class TestGetOrCreateLocalLtiConfiguration(TestCase): """ @@ -184,63 +237,6 @@ class TestGetOrCreateLocalLtiConfiguration(TestCase): self.assertEqual(lti_config.external_id, None) -class TestGetLtiConsumer(TestCase): - """ - Unit tests for get_lti_consumer API method. - """ - def test_retrieve_with_block(self): - """ - Check if the API creates a model if no object matching properties is found. - """ - block = Mock() - block.location = 'block-v1:course+test+2020+type@problem+block@test' - block.lti_version = LtiConfiguration.LTI_1P3 - LtiConfiguration.objects.create(location=block.location) - - # Call API - with patch("lti_consumer.models.LtiConfiguration.get_lti_consumer") as mock_get_lti_consumer: - get_lti_consumer(block=block) - mock_get_lti_consumer.assert_called_once() - - # Check that there's just a single LTI Config in the models - self.assertEqual(LtiConfiguration.objects.all().count(), 1) - - def test_retrieve_with_id(self): - """ - Check if the API retrieves a model if the configuration matches. - """ - location = 'block-v1:course+test+2020+type@problem+block@test' - lti_config = LtiConfiguration.objects.create(location=location) - - # Call API - with patch("lti_consumer.models.LtiConfiguration.get_lti_consumer") as mock_get_lti_consumer: - get_lti_consumer(config_id=lti_config.id) - mock_get_lti_consumer.assert_called_once() - - def test_retrieve_from_external_configuration(self): - """ - Check if the API creates a model from the external configuration ID - """ - external_id = 'my-plugin:my-lti-tool' - - block = Mock() - block.config_type = 'external' - block.location = Location('edx', 'Demo_Course', '2020', 'T2', 'UNIV') - block.external_config = external_id - block.lti_version = LtiConfiguration.LTI_1P1 - - # Call API - with patch("lti_consumer.models.LtiConfiguration.get_lti_consumer") as mock_get_lti_consumer, \ - patch("lti_consumer.api.get_external_config_from_filter") as mock_get_from_filter: - mock_get_from_filter.return_value = {"version": "lti_1p1"} - get_lti_consumer(block=block) - mock_get_lti_consumer.assert_called_once() - mock_get_from_filter.assert_called_once_with({"course_key": block.location.course_key}, external_id) - - # Check that there's just a single LTI Config in the models - self.assertEqual(LtiConfiguration.objects.all().count(), 1) - - class TestValidateLti1p3LaunchData(TestCase): """ Unit tests for validate_lti_1p3_launch_data API method. @@ -273,7 +269,7 @@ class TestValidateLti1p3LaunchData(TestCase): launch_data = Lti1p3LaunchData( user_id="1", user_role="student", - config_id="1", + config_id=_test_config_id, resource_link_id="resource_link_id", context_id="1", context_type=["course_offering"], @@ -292,7 +288,7 @@ class TestValidateLti1p3LaunchData(TestCase): launch_data = Lti1p3LaunchData( user_id="1", user_role="student", - config_id="1", + config_id=_test_config_id, resource_link_id="resource_link_id", ) @@ -319,7 +315,7 @@ class TestValidateLti1p3LaunchData(TestCase): launch_data = Lti1p3LaunchData( user_id="1", user_role=user_role, - config_id="1", + config_id=_test_config_id, resource_link_id="resource_link_id", ) @@ -341,7 +337,7 @@ class TestValidateLti1p3LaunchData(TestCase): launch_data = Lti1p3LaunchData( user_id="1", user_role="student", - config_id="1", + config_id=_test_config_id, resource_link_id="resource_link_id", context_id="1", context_type=context_type, @@ -378,28 +374,24 @@ class TestGetLti1p3LaunchInfo(TestCase): return Lti1p3LaunchData( user_id="1", user_role="student", - config_id="1", + config_id=_test_config_id, resource_link_id="resource_link_id", ) - def test_no_parameters(self): - """ - Check if the API creates a model if no object matching properties is found. - """ - with self.assertRaises(Exception): - get_lti_1p3_launch_info({}) - 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) + lti_config = LtiConfiguration.objects.create( + location=location, + config_id=_test_config_id, + ) launch_data = self._get_lti_1p3_launch_data() # Call and check returns - launch_info = get_lti_1p3_launch_info(launch_data, config_id=lti_config.id) + launch_info = get_lti_1p3_launch_info(launch_data) # Not checking all data here, there's a test specific for that self.assertEqual(launch_info['client_id'], lti_config.lti_1p3_client_id) @@ -415,7 +407,10 @@ class TestGetLti1p3LaunchInfo(TestCase): launch_data = self._get_lti_1p3_launch_data() # Create LTI Config and Deep linking object - lti_config = LtiConfiguration.objects.create(location=block.location) + lti_config = LtiConfiguration.objects.create( + location=block.location, + config_id=_test_config_id, + ) LtiDlContentItem.objects.create( lti_configuration=lti_config, content_type=LtiDlContentItem.IMAGE, @@ -423,7 +418,7 @@ class TestGetLti1p3LaunchInfo(TestCase): ) # Call API - launch_info = get_lti_1p3_launch_info(launch_data, block=block) + launch_info = get_lti_1p3_launch_info(launch_data) # Retrieve created config and check full launch info data lti_config = LtiConfiguration.objects.get() @@ -451,7 +446,10 @@ class TestGetLti1p3LaunchInfo(TestCase): Check if the API can return launch info for LtiConfiguration objects without specified block location. """ - lti_config = LtiConfiguration.objects.create(version=LtiConfiguration.LTI_1P3) + lti_config = LtiConfiguration.objects.create( + version=LtiConfiguration.LTI_1P3, + config_id=_test_config_id, + ) LtiDlContentItem.objects.create( lti_configuration=lti_config, content_type=LtiDlContentItem.IMAGE, @@ -460,7 +458,7 @@ class TestGetLti1p3LaunchInfo(TestCase): launch_data = self._get_lti_1p3_launch_data() - launch_info = get_lti_1p3_launch_info(launch_data, config_id=lti_config.id) + launch_info = get_lti_1p3_launch_info(launch_data) self.assertEqual( launch_info, { @@ -483,25 +481,17 @@ class TestGetLti1p3LaunchInfo(TestCase): class TestGetLti1p3LaunchUrl(Lti1P3TestCase): """ - Unit tests for get_lti_1p3_launch_start_url API method. + Unit tests for LTI 1.3 launch API methods. """ - def test_no_parameters(self): - """ - Check if the API creates a model if no object matching properties is found. - """ - with self.assertRaises(Exception): - get_lti_1p3_launch_start_url({}) - def test_get_normal_lti_launch_url(self): """ Check if the correct launch url is retrieved for a normal LTI 1.3 launch. """ - self._setup_lti_block() launch_data = self._get_lti_1p3_launch_data() # Call API for normal LTI launch initiation. - launch_url = get_lti_1p3_launch_start_url(launch_data, block=self.xblock) + launch_url = get_lti_1p3_launch_start_url(launch_data) parameters = parse_qs(urlparse(launch_url).query) launch_data = get_data_from_cache(parameters.get("lti_message_hint")[0]) @@ -513,15 +503,11 @@ class TestGetLti1p3LaunchUrl(Lti1P3TestCase): """ Check if the correct launch url is retrieved for a deep linking LTI 1.3 launch. """ - self._setup_lti_block() launch_data = self._get_lti_1p3_launch_data() # Call API for normal LTI launch initiation. - launch_url = get_lti_1p3_launch_start_url(launch_data, block=self.xblock) - - parameters = parse_qs(urlparse(launch_url).query) - launch_url = get_lti_1p3_launch_start_url(launch_data, block=self.xblock, deep_link_launch=True) + launch_url = get_lti_1p3_launch_start_url(launch_data, deep_link_launch=True) parameters = parse_qs(urlparse(launch_url).query) launch_data = get_data_from_cache(parameters.get("lti_message_hint")[0]) @@ -533,15 +519,14 @@ class TestGetLti1p3LaunchUrl(Lti1P3TestCase): """ Check if the correct launch url is retrieved for a deep linking content item LTI 1.3 launch. """ - self._setup_lti_block() launch_data = self._get_lti_1p3_launch_data() # Call API for normal LTI launch initiation. - launch_url = get_lti_1p3_launch_start_url(launch_data, block=self.xblock) + launch_url = get_lti_1p3_launch_start_url(launch_data) parameters = parse_qs(urlparse(launch_url).query) - launch_url = get_lti_1p3_launch_start_url(launch_data, block=self.xblock, dl_content_id="1") + launch_url = get_lti_1p3_launch_start_url(launch_data, dl_content_id="1") parameters = parse_qs(urlparse(launch_url).query) launch_data = get_data_from_cache(parameters.get("lti_message_hint")[0]) @@ -559,18 +544,17 @@ class TestGetLti1p3ContentUrl(Lti1P3TestCase): """ Check if the correct LTI content presentation is returned on a normal LTI Launch. """ + launch_data = self._get_lti_1p3_launch_data() mock_get_launch_url.return_value = 'test_url' - self._setup_lti_block() - self.assertEqual(get_lti_1p3_content_url(launch_data, block=self.xblock), 'test_url') + self.assertEqual(get_lti_1p3_content_url(launch_data), 'test_url') def test_lti_content_presentation_single_link(self): """ Check if the correct LTI content presentation is returned if a `ltiResourceLink` content type is present. """ - self._setup_lti_block() launch_data = self._get_lti_1p3_launch_data() @@ -582,7 +566,7 @@ class TestGetLti1p3ContentUrl(Lti1P3TestCase): ) # Call API to retrieve content item URL - launch_url = get_lti_1p3_content_url(launch_data, block=self.xblock) + launch_url = get_lti_1p3_content_url(launch_data) parameters = parse_qs(urlparse(launch_url).query) launch_data = get_data_from_cache(parameters.get("lti_message_hint")[0]) @@ -595,7 +579,6 @@ class TestGetLti1p3ContentUrl(Lti1P3TestCase): Check if the correct LTI content presentation is returned if multiple LTI DL content items are set up. """ - self._setup_lti_block() launch_data = self._get_lti_1p3_launch_data() @@ -618,7 +601,7 @@ class TestGetLti1p3ContentUrl(Lti1P3TestCase): self.assertIn( # Checking for the content presentation URL f"/api/lti_consumer/v1/lti/{self.lti_config.id}/lti-dl/content?launch_data_key={launch_data_key}", - get_lti_1p3_content_url(launch_data, block=self.xblock), + get_lti_1p3_content_url(launch_data), ) @@ -646,10 +629,7 @@ class TestGetLtiDlContentItemData(TestCase): attributes={"test": "test"}, ) - data = get_deep_linking_data( - deep_linking_id=content_item.id, - config_id=self.lti_config.id, - ) + data = get_deep_linking_data(content_item.id, self.lti_config.config_id) self.assertEqual( data, @@ -668,7 +648,4 @@ class TestGetLtiDlContentItemData(TestCase): ) with self.assertRaises(Exception): - get_deep_linking_data( - deep_linking_id=content_item.id, - config_id=self.lti_config.id, - ) + get_deep_linking_data(content_item.id, self.lti_config.config_id) diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py index 59c27f4..b960b20 100644 --- a/lti_consumer/tests/unit/test_lti_xblock.py +++ b/lti_consumer/tests/unit/test_lti_xblock.py @@ -18,7 +18,7 @@ from jwkest.jwk import RSAKey, KEYS from lti_consumer.exceptions import LtiError -from lti_consumer.api import _get_lti_config +from lti_consumer.api import config_id_for_block from lti_consumer.data import Lti1p3LaunchData from lti_consumer.lti_xblock import LtiConsumerXBlock, parse_handler_suffix, valid_config_type_values from lti_consumer.lti_1p3.tests.utils import create_jwt @@ -598,7 +598,7 @@ class TestGetLti1p1Consumer(TestLtiConsumerXBlock): self.xblock.lti_id = provider type(mock_course).lti_passports = PropertyMock(return_value=[f"{provider}:{key}:{secret}"]) - with patch('lti_consumer.models.LtiConfiguration.block', return_value=self.xblock): + with patch('lti_consumer.plugin.compat.load_block_as_user', return_value=self.xblock): self.xblock._get_lti_consumer() # pylint: disable=protected-access mock_lti_consumer.assert_called_with(self.xblock.launch_url, key, secret) @@ -1483,13 +1483,11 @@ class TestLtiConsumer1p3XBlock(TestCase): launch_data = self.xblock.get_lti_1p3_launch_data() - lti_config = _get_lti_config(block=self.xblock) - course_key = str(self.xblock.location.course_key) # pylint: disable=no-member expected_launch_data = Lti1p3LaunchData( user_id="external_user_id", user_role="instructor", - config_id=lti_config.config_id, + config_id=config_id_for_block(self.xblock), resource_link_id=str(self.xblock.location), # pylint: disable=no-member launch_presentation_document_target="iframe", message_type="LtiResourceLinkRequest", -- GitLab