diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 258478610176e6aa71b97badd2ebc4203cb64986..0f67cf480d3c79d94ec02b045d082e43263dec76 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 45f490566f8c7fedf76ac50f74a2f412e428d747..0e810af59c09127ad16a744f219d15bd300706e3 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 3a4ff706a5449ea6d3f2a3470689dd4a675b2a3b..dc1eed0776ea016eabf7bfbcff4999b21bf348d6 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 c39387a09f4ab5299e6d6c9f4ca55238a622db3c..9973e4395917bec2af8bcea551442e282b3c4557 100644
--- a/lti_consumer/lti_xblock.py
+++ b/lti_consumer/lti_xblock.py
@@ -932,13 +932,13 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
custom_parameters['custom_component_display_name'] = str(self.display_name)
- if self.due: # pylint: disable=no-member
+ if self.due:
custom_parameters.update({
- 'custom_component_due_date': self.due.strftime('%Y-%m-%d %H:%M:%S') # pylint: disable=no-member
+ 'custom_component_due_date': self.due.strftime('%Y-%m-%d %H:%M:%S')
})
- if self.graceperiod: # pylint: disable=no-member
+ if self.graceperiod:
custom_parameters.update({
- 'custom_component_graceperiod': str(self.graceperiod.total_seconds()) # pylint: disable=no-member
+ 'custom_component_graceperiod': str(self.graceperiod.total_seconds())
})
return custom_parameters
@@ -947,9 +947,9 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock):
"""
Is it now past this problem's due date, including grace period?
"""
- due_date = self.due # pylint: disable=no-member
- if self.graceperiod is not None and due_date: # pylint: disable=no-member
- close_date = due_date + self.graceperiod # pylint: disable=no-member
+ due_date = self.due
+ if self.graceperiod is not None and due_date:
+ close_date = due_date + self.graceperiod
else:
close_date = due_date
return close_date is not None and timezone.now() > close_date
@@ -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/models.py b/lti_consumer/models.py
index e8736d2ea6148743ee4cfcff7471453381d12935..acf8eb267b3e0cac38f6a7fd398f3f2ddf908dc5 100644
--- a/lti_consumer/models.py
+++ b/lti_consumer/models.py
@@ -264,7 +264,7 @@ class LtiConfiguration(models.Model):
if block is None:
if self.location is None:
raise ValueError(_("Block location not set, it's not possible to retrieve the block."))
- block = self._block = compat.load_block_as_anonymous_user(self.location)
+ block = self._block = compat.load_block_as_user(self.location)
return block
@block.setter
diff --git a/lti_consumer/plugin/compat.py b/lti_consumer/plugin/compat.py
index e7a1b1561841072bd9fc2035c9059a800da0d8a7..1ce76a0fb4b3d0559520f7fe1ea5928053feb145 100644
--- a/lti_consumer/plugin/compat.py
+++ b/lti_consumer/plugin/compat.py
@@ -60,27 +60,43 @@ def get_database_config_waffle_flag():
return CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.{ENABLE_DATABASE_CONFIG}', __name__)
-def run_xblock_handler(*args, **kwargs):
+def load_block_as_user(location): # pragma: nocover
"""
- Import and run `handle_xblock_callback` from LMS
+ Load a block as the current user, or load as the anonymous user if no user is available.
"""
# pylint: disable=import-error,import-outside-toplevel
- from lms.djangoapps.courseware.module_render import handle_xblock_callback
- return handle_xblock_callback(*args, **kwargs)
-
+ from crum import get_current_user, get_current_request
+ from xmodule.modulestore.django import modulestore
+ from lms.djangoapps.courseware.module_render import get_module_for_descriptor_internal
+ from openedx.core.lib.xblock_utils import request_token
-def run_xblock_handler_noauth(*args, **kwargs):
- """
- Import and run `handle_xblock_callback_noauth` from LMS
- """
- # pylint: disable=import-error,import-outside-toplevel
- from lms.djangoapps.courseware.module_render import handle_xblock_callback_noauth
- return handle_xblock_callback_noauth(*args, **kwargs)
+ # Retrieve descriptor from modulestore
+ descriptor = modulestore().get_item(location)
+ user = get_current_user()
+ request = get_current_request()
+ if user and request:
+ # If we're in request scope, the descriptor may already be a block bound to a user
+ # and we don't need to do any more loading
+ if descriptor.scope_ids.user_id is not None and user.id == descriptor.scope_ids.user_id:
+ return descriptor
+
+ # If not load this block to bind it onto the user
+ get_module_for_descriptor_internal(
+ user=user,
+ descriptor=descriptor,
+ student_data=None,
+ course_id=location.course_key,
+ track_function=None,
+ request_token=request_token(request),
+ )
+ return descriptor
+ else:
+ return _load_block_as_anonymous_user(location, descriptor)
-def load_block_as_anonymous_user(location):
+def _load_block_as_anonymous_user(location, descriptor): # pragma: nocover
"""
- Load a block as anonymous user.
+ Load a block as the anonymous user because no user is available.
This uses a few internal courseware methods to retrieve the descriptor
and bind an AnonymousUser to it, in a similar fashion as a `noauth` XBlock
@@ -89,12 +105,8 @@ def load_block_as_anonymous_user(location):
# pylint: disable=import-error,import-outside-toplevel
from crum import impersonate
from django.contrib.auth.models import AnonymousUser
- from xmodule.modulestore.django import modulestore
from lms.djangoapps.courseware.module_render import get_module_for_descriptor_internal
- # Retrieve descriptor from modulestore
- descriptor = modulestore().get_item(location)
-
# ensure `crum.get_current_user` returns AnonymousUser. It returns None when outside
# of request scope which causes error during block loading.
user = AnonymousUser()
@@ -112,7 +124,7 @@ def load_block_as_anonymous_user(location):
return descriptor
-def get_user_from_external_user_id(external_user_id):
+def get_user_from_external_user_id(external_user_id): # pragma: nocover
"""
Import ExternalId model and find user by external_user_id
"""
@@ -130,7 +142,8 @@ def get_user_from_external_user_id(external_user_id):
raise LtiError('Invalid userID') from exception
-def publish_grade(block, user, score, possible, only_if_higher=False, score_deleted=None, comment=None):
+def publish_grade(block, user, score, possible,
+ only_if_higher=False, score_deleted=None, comment=None): # pragma: nocover
"""
Import grades signals and publishes score by triggering SCORE_PUBLISHED signal.
"""
@@ -150,7 +163,7 @@ def publish_grade(block, user, score, possible, only_if_higher=False, score_dele
)
-def user_has_access(*args, **kwargs):
+def user_has_access(*args, **kwargs): # pragma: nocover
"""
Import and run `has_access` from LMS
"""
@@ -159,7 +172,7 @@ def user_has_access(*args, **kwargs):
return has_access(*args, **kwargs)
-def user_has_studio_write_access(*args, **kwargs):
+def user_has_studio_write_access(*args, **kwargs): # pragma: nocover
"""
Import and run `has_studio_write_access` from common modules.
@@ -171,7 +184,7 @@ def user_has_studio_write_access(*args, **kwargs):
return has_studio_write_access(*args, **kwargs)
-def get_course_by_id(course_key):
+def get_course_by_id(course_key): # pragma: nocover
"""
Import and run `get_course_by_id` from LMS
@@ -188,7 +201,7 @@ def get_course_by_id(course_key):
return lms_get_course_by_id(course_key)
-def user_course_access(*args, **kwargs):
+def user_course_access(*args, **kwargs): # pragma: nocover
"""
Import and run `check_course_access` from LMS
"""
@@ -197,7 +210,7 @@ def user_course_access(*args, **kwargs):
return check_course_access(*args, **kwargs)
-def batch_get_or_create_externalids(users):
+def batch_get_or_create_externalids(users): # pragma: nocover
"""
Given a list of user, returns corresponding external id's
@@ -212,7 +225,7 @@ def batch_get_or_create_externalids(users):
return ExternalId.batch_get_or_create_user_ids(users, 'lti')
-def get_course_members(course_key):
+def get_course_members(course_key): # pragma: nocover
"""
Returns a dict containing all users associated with the given course
"""
@@ -239,7 +252,7 @@ def request_cached(func) -> Callable[[Callable], Callable]:
return func
-def clean_course_id(model_form: ModelForm) -> CourseKey:
+def clean_course_id(model_form: ModelForm) -> CourseKey: # pragma: nocover
"""
Import and run `clean_course_id` from LMS
"""
@@ -248,7 +261,7 @@ def clean_course_id(model_form: ModelForm) -> CourseKey:
return lms_clean_course_id(model_form)
-def get_event_tracker():
+def get_event_tracker(): # pragma: nocover
"""
Import and return LMS event tracking function
"""
diff --git a/lti_consumer/signals.py b/lti_consumer/signals.py
index b239349326834701c53b176b6faa7ee6c9f2a72a..e9d928e9faea722b96f05febe3f48d4cb1cd6082 100644
--- a/lti_consumer/signals.py
+++ b/lti_consumer/signals.py
@@ -31,7 +31,7 @@ def publish_grade_on_score_update(sender, instance, **kwargs): # pylint: disabl
and instance.score_given:
try:
# Load block using LMS APIs and check if the block is graded and still accept grades.
- block = compat.load_block_as_anonymous_user(instance.line_item.resource_link_id)
+ block = compat.load_block_as_user(instance.line_item.resource_link_id)
if block.has_score and (not block.is_past_due() or block.accept_grades_past_due):
# Map external ID to platform user
user = compat.get_user_from_external_user_id(instance.user_id)
diff --git a/lti_consumer/templatetags/get_dl_lti_launch_url.py b/lti_consumer/templatetags/get_dl_lti_launch_url.py
index d49f5858d6234fed14822b6430679ac54dd2f91c..b56d6065ce15e5a69273d31583bdc0e80858f653 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/plugin/test_views.py b/lti_consumer/tests/unit/plugin/test_views.py
index 17db1dcfbd32600ada6a8aba2fdb96c94e268bee..4d7e9e99818fae94675a3ce2d9514d58d4e19d19 100644
--- a/lti_consumer/tests/unit/plugin/test_views.py
+++ b/lti_consumer/tests/unit/plugin/test_views.py
@@ -144,7 +144,7 @@ class TestLti1p3LaunchGateEndpoint(TestCase):
self.compat.get_external_id_for_user.return_value = "12345"
model_compat_patcher = patch("lti_consumer.models.compat")
model_compat = model_compat_patcher.start()
- model_compat.load_block_as_anonymous_user.return_value = self.xblock
+ model_compat.load_block_as_user.return_value = self.xblock
self.addCleanup(model_compat_patcher.stop)
def test_invalid_lti_version(self):
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 7a3e7f94441eb56327d7f93735752bd4547ce8c1..856419b9bfc9e73f5639ee4fbc090538867f3e4e 100644
--- a/lti_consumer/tests/unit/plugin/test_views_lti_ags.py
+++ b/lti_consumer/tests/unit/plugin/test_views_lti_ags.py
@@ -71,7 +71,7 @@ class LtiAgsLineItemViewSetTestCase(APITransactionTestCase):
self.addCleanup(compat_mock.stop)
self._compat_mock = compat_mock.start()
self._compat_mock.get_user_from_external_user_id.return_value = self._mock_user
- self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock
+ self._compat_mock.load_block_as_user.return_value = self.xblock
def _set_lti_token(self, scopes=None):
"""
@@ -431,7 +431,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase):
"gradingProgress": grading_progress,
})
- self._compat_mock.load_block_as_anonymous_user.assert_not_called()
+ self._compat_mock.load_block_as_user.assert_not_called()
self._compat_mock.get_user_from_external_user_id.assert_not_called()
def test_xblock_grade_publish_on_score_save(self):
@@ -439,7 +439,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase):
Test that the grade is submitted when gradingProgress is `FullyGraded`.
"""
# Set up LMS mocks
- self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock
+ self._compat_mock.load_block_as_user.return_value = self.xblock
self._compat_mock.get_user_from_external_user_id.return_value = 'user_mock'
self.xblock.set_user_module_score = Mock()
@@ -452,7 +452,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase):
# Check if publish grade was called
self.xblock.set_user_module_score.assert_called_once()
self._compat_mock.get_user_from_external_user_id.assert_called_once()
- self._compat_mock.load_block_as_anonymous_user.assert_called_once()
+ self._compat_mock.load_block_as_user.assert_called_once()
call_args = self.xblock.set_user_module_score.call_args.args
self.assertEqual(call_args, ('user_mock', 0.83, 1, 'This is exceptional work.'))
@@ -462,7 +462,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase):
Test when given score is bigger than maximum score.
"""
# Return block bypassing LMS API
- self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock
+ self._compat_mock.load_block_as_user.return_value = self.xblock
self._compat_mock.get_user_from_external_user_id.return_value = 'user_mock'
self.xblock.set_user_module_score = Mock()
@@ -494,7 +494,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase):
self.xblock.runtime.publish.side_effect = LmsException
# Return block bypassing LMS API
- self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock
+ self._compat_mock.load_block_as_user.return_value = self.xblock
# Check that the except statement catches the exception
with self.assertRaises(LmsException):
@@ -511,7 +511,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase):
self._post_lti_score()
# Check that the block wasn't set if due date is past
- self._compat_mock.load_block_as_anonymous_user.assert_called_once()
+ self._compat_mock.load_block_as_user.assert_called_once()
self._compat_mock.get_user_from_external_user_id.assert_not_called()
self.xblock.set_user_module_score.assert_not_called()
@@ -521,7 +521,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase):
Test grade publish after due date when accept_grades_past_due is True. Grade should publish.
"""
# Return block bypassing LMS API
- self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock
+ self._compat_mock.load_block_as_user.return_value = self.xblock
self._compat_mock.get_user_from_external_user_id.return_value = 'user_mock'
self.xblock.set_user_module_score = Mock()
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 b19794d6bc831b93663b26ed9f7033299572ccd3..445730f038f58697d8a8bbb5559019eaa80a86f6 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
@@ -80,7 +80,7 @@ class LtiDeepLinkingTestCase(APITransactionTestCase):
self.addCleanup(compat_mock.stop)
self._compat_mock = compat_mock.start()
self._compat_mock.get_user_from_external_user_id.return_value = self._mock_user
- self._compat_mock.load_block_as_anonymous_user.return_value = self.xblock
+ self._compat_mock.load_block_as_user.return_value = self.xblock
@ddt.ddt
diff --git a/lti_consumer/tests/unit/test_api.py b/lti_consumer/tests/unit/test_api.py
index 94633a97ff28773dccde9c3309cc505fb8965fc6..54c8e0fad143f0086af49bfae4295ee21a095a2f 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 7f535df62952f7db0cc11cf9ebe6393bc40a3a48..b960b2073f21daf5ac33af705032c5dbd6096f55 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",
@@ -1576,7 +1574,7 @@ class TestLti1p3AccessTokenEndpoint(TestLtiConsumerXBlock):
patcher = patch(
'lti_consumer.models.compat',
- **{'load_block_as_anonymous_user.return_value': self.xblock}
+ **{'load_block_as_user.return_value': self.xblock}
)
patcher.start()
self.addCleanup(patcher.stop)
@@ -1751,7 +1749,7 @@ class TestLti1p3AccessTokenJWK(TestCase):
self.request = make_jwt_request(jwt)
patcher = patch(
'lti_consumer.models.compat',
- **{'load_block_as_anonymous_user.return_value': self.xblock}
+ **{'load_block_as_user.return_value': self.xblock}
)
patcher.start()
self.addCleanup(patcher.stop)
diff --git a/lti_consumer/tests/unit/test_models.py b/lti_consumer/tests/unit/test_models.py
index aa06952c8ea2ed1fa7115ec9cb3621cc4873c528..f1562bc4989cc126e8e64b23b3c8770b6694e463 100644
--- a/lti_consumer/tests/unit/test_models.py
+++ b/lti_consumer/tests/unit/test_models.py
@@ -318,7 +318,7 @@ class TestLtiConfigurationModel(TestCase):
"""
Check if a block is properly loaded when calling the `block` property.
"""
- compat_mock.load_block_as_anonymous_user.return_value = self.xblock
+ compat_mock.load_block_as_user.return_value = self.xblock
block = self.lti_1p3_config.block
self.assertEqual(block, self.xblock)
@@ -435,7 +435,7 @@ class TestLtiAgsScoreModel(TestCase):
compat_mock = patch("lti_consumer.signals.compat")
self.addCleanup(compat_mock.stop)
self._compat_mock = compat_mock.start()
- self._compat_mock.load_block_as_anonymous_user.return_value = make_xblock(
+ self._compat_mock.load_block_as_user.return_value = make_xblock(
'lti_consumer', LtiConsumerXBlock, {
'due': datetime.now(timezone.utc),
'graceperiod': timedelta(days=2),