From f7b9d401749f1f58ed933589e6b46c6a22e1b9ed Mon Sep 17 00:00:00 2001 From: michaelroytman <mroytman@edx.org> Date: Thu, 22 Sep 2022 16:36:37 -0400 Subject: [PATCH] feat!: decouple LTI 1.3 launch from LtiConsumerXBlock Purpose ------- The purpose of these changes is to decouple the LTI 1.3 launch from the LtiConsumerXBlock. It is in accordance with the ADR "0007 Decouple LTI 1.3 Launch from XBlock and edX Platform", which is currently under review. The pull request for the ADR is here: https://github.com/openedx/xblock-lti-consumer/pull/281. The general premise of these changes is to shift the responsibility of defining key launch claims to users of the library. Such claims include user ID, user role, resource link ID, etc. Prior to this change, this context was defined directly in the launch view by referencing XBlock fields and functions, thereby tying the LTI 1.3 launch to the XBlock. By shifting the responsibility out of the view, we will be able to genericize the launch and make it functional in more contexts than just the XBlock and the XBlock runtime. In short, the key launch claims are encoded in an instance of a data class Lti1p3LaunchData. Users of the library will instantiate this class with necessary launch data to it and pass the instance to various methods of the Python API to communicate the data to the library. Please see the aforementioned ADR for more details about this decoupling strategy. Note that the majority of these changes affect only the basic LTI 1.3 launch. There have largely been no changes to LTI 1.3 Advantage Services. The one exception is the Deep Linking content launch endpoint. This is because this launch is implemented in the basic LTI 1.3 launch, and it was necessary to make the same changes to the deep linking content launch to ensure that it works properly. Otherwise, LTI 1.3 Advantage Services are out of scope of these changes. Change Summary for Developers ----------------------------- Below is a summary of changes contained in this pull request. * added an Lti1p3LaunchData data class * added caching for Lti1p3LaunchData to limit data sent in request query or form parameters * BREAKING CHANGE: modified Python API methods to take Lti1p3LaunchData as a required argument ** get_lti_1p3_launch_info ** get_lti_1p3_launch_start_url ** get_lti_1p3_content_url * replaced references to LtiConsumerXBlock.location with Lti1p3LaunchData.config_id * removed definition of key LTI 1.3 claims from the launch_gate_endpoint and instantiated Lti1p3LaunchData from within the LtiConsumerXBlock instead * added a required launch_data_key request query parameter to the deep_linking_content_endpoint and refactored associated templates and template tags to pass this parameter in the request to the view Change Summary for Course Staff and Instructors ----------------------------------------------- The only changes relevant for course staff and instructors is that the access token and keyset URLs displayed in Studio have changed in format. The old format was: Access Token URL: https://courses.edx.org/api/lti_consumer/v1/token/block-v1:edX+999+2022Q3+type@lti_consumer+block@714c10a5e4df452da9d058788acb56be Keyset URL: https://courses.edx.org/api/lti_consumer/v1/public_keysets/block-v1:edX+999+2022Q3+type@lti_consumer+block@714c10a5e4df452da9d058788acb56be The new format is: Access Token URL: https://courses.edx.org/api/lti_consumer/v1/token/c3f6af60-dbf2-4f85-8974-4ff870068d43 Keyset URL: https://courses.edx.org/api/lti_consumer/v1/public_keysets/c3f6af60-dbf2-4f85-8974-4ff870068d43 The difference is in the slug at the end of the URL. In the old format, the slug was the UsageKey of the XBlock associated with the LTI integration. In the new format, the slug is the config_id of the LtiConfiguration associated with the LTI integration. This is an iterative step toward decoupling the access_token_endpoint and the public_keyset_endpoint views from the XBlock location field. The XBlock location field appears as the usage_key parameter to both views. We cannot simply remove the usage_key parameter from the views, because existing LTI 1.3 integrations may have been created using the old format, and we need to maintain backwards compatibility. This change, however, prevents new integrations from being created that are coupled to the XBlock. In the future, we may address integrations that use the old format to fully decouple the XBlock from the views. Testing ------- Unit tests were added for all changes. In addition, manual testing was performed using the instructions in the documents listed below. * https://github.com/openedx/xblock-lti-consumer#lti-13 * https://openedx.atlassian.net/wiki/spaces/COMM/pages/1858601008/How+to+run+the+LTI+Validation+test Resources --------- JIRA: MST-1603: https://2u-internal.atlassian.net/browse/MST-1603 BREAKING CHANGE --- CHANGELOG.rst | 21 ++ lti_consumer/__init__.py | 2 +- lti_consumer/api.py | 95 +++++-- lti_consumer/data.py | 46 +++ lti_consumer/lti_1p1/tests/test_consumer.py | 2 +- lti_consumer/lti_1p1/tests/test_oauth.py | 2 +- lti_consumer/lti_1p3/consumer.py | 89 ++++-- lti_consumer/lti_1p3/exceptions.py | 4 + lti_consumer/lti_1p3/tests/test_consumer.py | 134 +++++++-- lti_consumer/lti_xblock.py | 70 ++++- lti_consumer/plugin/compat.py | 19 -- lti_consumer/plugin/views.py | 162 ++++++----- .../html/lti-dl/render_dl_content.html | 2 +- .../html/lti-dl/render_lti_resource_link.html | 4 +- .../templatetags/get_dl_lti_launch_url.py | 17 +- lti_consumer/tests/test_utils.py | 96 +++++++ lti_consumer/tests/unit/plugin/test_views.py | 133 +++++++-- .../tests/unit/plugin/test_views_lti_ags.py | 25 +- .../plugin/test_views_lti_deep_linking.py | 68 ++++- .../tests/unit/plugin/test_views_lti_nrps.py | 7 +- lti_consumer/tests/unit/test_api.py | 263 +++++++++++++++--- lti_consumer/tests/unit/test_lti_xblock.py | 120 ++++++-- lti_consumer/tests/unit/test_models.py | 26 +- lti_consumer/tests/unit/test_outcomes.py | 2 +- lti_consumer/tests/unit/test_utils.py | 176 +++++++----- lti_consumer/utils.py | 112 +++++++- requirements/base.in | 1 + requirements/base.txt | 2 + requirements/ci.txt | 12 +- requirements/dev.txt | 2 + requirements/quality.txt | 2 + requirements/test.txt | 9 +- 32 files changed, 1314 insertions(+), 411 deletions(-) create mode 100644 lti_consumer/data.py create mode 100644 lti_consumer/tests/test_utils.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e0a76ab..39be796 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,27 @@ Please See the [releases tab](https://github.com/edx/xblock-lti-consumer/release Unreleased ~~~~~~~~~~ +5.0.0 - 2022-10-12 +------------------ +BREAKING CHANGE: + +Please note that additional breaking changes will be forthcoming in future versions of this library. + +* Modified Python API methods to take Lti1p3LaunchData as a required argument +** get_lti_1p3_launch_info +** get_lti_1p3_launch_start_url +** get_lti_1p3_content_url + +* Added an Lti1p3LaunchData data class +* Added caching for Lti1p3LaunchData to limit data sent in request query or form parameters +* Replaced references to LtiConsumerXBlock.location with Lti1p3LaunchData.config_id +* Removed definition of key LTI 1.3 claims from the launch_gate_endpoint and instantiated Lti1p3LaunchData from within + the LtiConsumerXBlock instead +* Added a required launch_data_key request query parameter to the deep_linking_content_endpoint and refactored + associated templates and template tags to pass this parameter in the request to the view +* Changed the access token URL and Keyset URL to use the LtiConfiguration.config_id in the URL instead of the + LtiConfiguration.location + 4.4.0 - 2022-08-17 ------------------ * Move the LTI 1.3 Access Token and Launch Callback endpoint logic from the XBlock to the Django views diff --git a/lti_consumer/__init__.py b/lti_consumer/__init__.py index 0bcf3e4..0e15340 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__ = '4.5.0' +__version__ = '5.0.0' diff --git a/lti_consumer/api.py b/lti_consumer/api.py index d03be16..3a4ff70 100644 --- a/lti_consumer/api.py +++ b/lti_consumer/api.py @@ -9,9 +9,11 @@ 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, get_lti_deeplinking_content_url, get_lms_lti_keyset_link, get_lms_lti_launch_link, @@ -24,7 +26,7 @@ def _get_or_create_local_lti_config(lti_version, block_location, config_store=LtiConfiguration.CONFIG_ON_XBLOCK, external_id=None): """ Retrieve the LtiConfiguration for the block described by block_location, if one exists. If one does not exist, - create an LtiConfiguration with the LtiConfiguration.CONFIG_ON_DB config_store. + create an LtiConfiguration with the LtiConfiguration.CONFIG_ON_XBLOCK config_store. Treat the lti_version argument as the source of truth for LtiConfiguration.version and override the LtiConfiguration.version with lti_version. This allows, for example, for @@ -100,7 +102,11 @@ def get_lti_consumer(config_id=None, block=None): return _get_lti_config(config_id, block).get_lti_consumer() -def get_lti_1p3_launch_info(config_id=None, block=None): +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. """ @@ -116,9 +122,9 @@ def get_lti_1p3_launch_info(config_id=None, block=None): deep_linking_enabled = lti_consumer.lti_dl_enabled() if deep_linking_enabled: + launch_data.message_type = "LtiDeepLinkingRequest" deep_linking_launch_url = lti_consumer.prepare_preflight_url( - hint=lti_config.location, - lti_hint="deep_linking_launch" + launch_data, ) # Retrieve LTI Content Items (if any was set up) @@ -129,22 +135,28 @@ def get_lti_1p3_launch_info(config_id=None, block=None): if dl_content_items.exists(): deep_linking_content_items = [item.attributes for item in dl_content_items] - link_location = lti_config.location if lti_config.location else lti_config.config_id + config_id = lti_config.config_id # Return LTI launch information for end user configuration return { 'client_id': lti_config.lti_1p3_client_id, - 'keyset_url': get_lms_lti_keyset_link(link_location), + 'keyset_url': get_lms_lti_keyset_link(config_id), 'deployment_id': '1', 'oidc_callback': get_lms_lti_launch_link(), - 'token_url': get_lms_lti_access_token_link(link_location), + 'token_url': get_lms_lti_access_token_link(config_id), 'deep_linking_launch_url': deep_linking_launch_url, 'deep_linking_content_items': json.dumps(deep_linking_content_items, indent=4) if deep_linking_content_items else None, } -def get_lti_1p3_launch_start_url(config_id=None, block=None, deep_link_launch=False, dl_content_id=None, hint=""): +def get_lti_1p3_launch_start_url( + launch_data, + config_id=None, + block=None, + deep_link_launch=False, + dl_content_id=None, +): """ Computes and retrieves the LTI URL that starts the OIDC flow. """ @@ -152,25 +164,25 @@ def get_lti_1p3_launch_start_url(config_id=None, block=None, deep_link_launch=Fa lti_config = _get_lti_config(config_id, block) lti_consumer = lti_config.get_lti_consumer() - # Change LTI hint depending on LTI launch type - lti_hint = "" + # 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 # configure tool options and select content to be presented. if deep_link_launch: - lti_hint = "deep_linking_launch" + launch_data.message_type = "LtiDeepLinkingRequest" # Case 2: Perform a LTI Launch for `ltiResourceLink` content types, since they # need to use the launch mechanism from the callback view. elif dl_content_id: - lti_hint = f"deep_linking_content_launch:{dl_content_id}" + launch_data.deep_linking_content_item_id = dl_content_id # Prepare and return OIDC flow start url - return lti_consumer.prepare_preflight_url( - hint=hint, - lti_hint=lti_hint - ) + return lti_consumer.prepare_preflight_url(launch_data) -def get_lti_1p3_content_url(config_id=None, block=None, hint=""): +def get_lti_1p3_content_url( + launch_data, + config_id=None, + block=None, +): """ Computes and returns which URL the LTI consumer should launch to. @@ -189,20 +201,20 @@ def get_lti_1p3_content_url(config_id=None, block=None, hint=""): # If there's no content items, return normal LTI launch URL if not content_items.count(): - return get_lti_1p3_launch_start_url(config_id, block, hint=hint) + return get_lti_1p3_launch_start_url(launch_data, config_id=config_id, block=block) # If there's a single `ltiResourceLink` content, return the launch - # url for that specif deep link + # 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( - config_id, - block, + launch_data, + config_id=config_id, + block=block, dl_content_id=content_items.get().id, - hint=hint, ) # If there's more than one content item, return content presentation URL - return get_lti_deeplinking_content_url(lti_config.id) + return get_lti_deeplinking_content_url(lti_config.id, launch_data) def get_deep_linking_data(deep_linking_id, config_id=None, block=None): @@ -232,3 +244,40 @@ def get_lti_pii_sharing_state_for_course(course_key: CourseKey) -> bool: bool: The state of PII sharing for this course for LTI. """ return CourseAllowPIISharingInLTIFlag.current(course_key).enabled + + +def validate_lti_1p3_launch_data(launch_data): + """ + Validate that the data in Lti1p3LaunchData are valid and raise an LtiMessageHintValidationFailure exception if they + are not. + + The initializer of the Lti1p3LaunchData takes care of ensuring that required data is provided to the class. This + utility method verifies that other requirements of the data are met. + """ + validation_messages = [] + + # The context claim is an object that composes properties about the context. The claim itself is optional, but if it + # is provided, then the id property is required. Ensure that if any other of the other optional properties are + # provided that the id property is also provided. + if ((launch_data.context_type or launch_data.context_title or launch_data.context_label) and not + launch_data.context_id): + validation_messages.append( + "The context_id attribute is required in the launch data if any optional context properties are provided." + ) + + if launch_data.user_role not in LTI_1P3_ROLE_MAP: + validation_messages.append(f"The user_role attribute {launch_data.user_role} is not a valid user_role.") + + context_type = launch_data.context_type + if context_type: + try: + get_lti_1p3_context_types_claim(context_type) + except ValueError: + validation_messages.append( + f"The context_type attribute {context_type} in the launch data is not a valid context_type." + ) + + if validation_messages: + return False, validation_messages + else: + return True, [] diff --git a/lti_consumer/data.py b/lti_consumer/data.py new file mode 100644 index 0000000..7d2bfcf --- /dev/null +++ b/lti_consumer/data.py @@ -0,0 +1,46 @@ +""" +This modules provides public data structures to represent LTI 1.3 launch data that can be used within this library and +by users of this library. +""" + +from attrs import define, field + + +@define +class Lti1p3LaunchData: + """ + The Lti1p3LaunchData class contains data necessary and related to an LTI 1.3 launch. It is a mechanism to share + launch data between apps. Applications using this library should use the Lti1p3LaunchData class to supply + contextually defined or stored launch data to the generic LTI 1.3 launch. + + * user_id (required): the user's unique identifier + * user_role (required): the user's role as one of the keys in LTI_1P3_ROLE_MAP: staff, instructor, student, or + guest + * config_id (required): the config_id field of an LtiConfiguration to use for the launch + * resource_link_id (required): a unique identifier that is guaranteed to be unique for each placement of the LTI + link + * launch_presentation_document_target (optional): the document_target property of the launch_presentation claim; it + describes the kind of browser window or frame from which the user launched inside the message sender's system; + it is one of frame, iframe, or window; it defaults to iframe + * message_type (optional): the message type of the eventual LTI launch; defaults to LtiResourceLinkRequest + * context_id (conditionally required): the id property of the context claim; the stable, unique identifier for the + context; if any of the context properties are provided, context_id is required + * context_type (optional): the type property of the context claim; a list of some combination of the following valid + context_types: group, course_offering, course_section, or course_template + * context_title (optional): the title proerty of the context claim; a short, descriptive name for the context + * context_label (optional): the label property of the context claim; a full, descriptive name for the context + * deep_linking_context_item_id (optional): the database id of the LtiDlContentItem that should be used for the LTI + 1.3 Deep Linking launch; this is used when the LTI 1.3 Deep Linking launch is a regular LTI resource link + request using a content item that was configured via a previous LTI 1.3 Deep Linking request + """ + user_id = field() + user_role = field() + config_id = field() + resource_link_id = field() + launch_presentation_document_target = field(default="iframe") + message_type = field(default="LtiResourceLinkRequest") + context_id = field(default=None) + context_type = field(default=None) + context_title = field(default=None) + context_label = field(default=None) + deep_linking_content_item_id = field(default=None) diff --git a/lti_consumer/lti_1p1/tests/test_consumer.py b/lti_consumer/lti_1p1/tests/test_consumer.py index 88482b1..0ae9d04 100644 --- a/lti_consumer/lti_1p1/tests/test_consumer.py +++ b/lti_consumer/lti_1p1/tests/test_consumer.py @@ -8,7 +8,7 @@ from unittest.mock import Mock, patch from lti_consumer.lti_1p1.exceptions import Lti1p1Error from lti_consumer.lti_1p1.consumer import LtiConsumer1p1, parse_result_json -from lti_consumer.tests.unit.test_utils import make_request +from lti_consumer.tests.test_utils import make_request INVALID_JSON_INPUTS = [ ([ diff --git a/lti_consumer/lti_1p1/tests/test_oauth.py b/lti_consumer/lti_1p1/tests/test_oauth.py index 619dd6a..a0899b4 100644 --- a/lti_consumer/lti_1p1/tests/test_oauth.py +++ b/lti_consumer/lti_1p1/tests/test_oauth.py @@ -10,7 +10,7 @@ from lti_consumer.lti_1p1.exceptions import Lti1p1Error from lti_consumer.lti_1p1.oauth import (get_oauth_request_signature, log_authorization_header, verify_oauth_body_signature) -from lti_consumer.tests.unit.test_utils import make_request +from lti_consumer.tests.test_utils import make_request OAUTH_PARAMS = [ ('oauth_nonce', '80966668944732164491378916897'), diff --git a/lti_consumer/lti_1p3/consumer.py b/lti_consumer/lti_1p3/consumer.py index f2b2a4f..c45a608 100644 --- a/lti_consumer/lti_1p3/consumer.py +++ b/lti_consumer/lti_1p3/consumer.py @@ -1,8 +1,11 @@ """ LTI 1.3 Consumer implementation """ +import logging from urllib.parse import urlencode +from lti_consumer.utils import cache_lti_1p3_launch_data, get_data_from_cache + from . import constants, exceptions from .constants import ( LTI_1P3_ROLE_MAP, @@ -16,6 +19,8 @@ from .ags import LtiAgs from .deep_linking import LtiDeepLinking from .nprs import LtiNrps +log = logging.getLogger(__name__) + class LtiConsumer1p3: """ @@ -53,6 +58,7 @@ class LtiConsumer1p3: # IMS LTI Claim data self.lti_claim_user_data = None + self.lti_claim_resource_link = None self.lti_claim_launch_presentation = None self.lti_claim_context = None self.lti_claim_custom_parameters = None @@ -90,20 +96,31 @@ class LtiConsumer1p3: def prepare_preflight_url( self, - hint="hint", - lti_hint="lti_hint" + launch_data, ): """ Generates OIDC url with parameters """ + user_id = launch_data.user_id + + # Set the launch_data in the cache. An LTI 1.3 launch involves two "legs" - the third party initiated + # login request (the preflight request) and the actual launch -, and this information must be shared between + # the two requests. A simple example is the intended LTI launch message of the LTI launch. This value is + # known at the time that preflight request is made, but it is not accessible when the tool responds to the + # preflight request and the platform must craft a launch request. This library stores the launch_data in the + # cache and includes the cache key as the lti_message_hint query or form parameter to retrieve it later. + launch_data_key = cache_lti_1p3_launch_data(launch_data) + oidc_url = self.oidc_url + "?" + + 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, - "login_hint": hint, - "lti_message_hint": lti_hint + "login_hint": login_hint, + "lti_message_hint": launch_data_key, } return oidc_url + urlencode(parameters) @@ -143,6 +160,37 @@ class LtiConsumer1p3: "email": email_address, }) + def set_resource_link_claim( + self, + resource_link_id, + description=None, + title=None, + ): + """ + Set resource_link claim. The resource link must be stable and unique to each deployment_id. This value MUST + change if the link is copied or exported from one system or context and imported into another system or context + + https://www.imsglobal.org/spec/lti/v1p3#resource-link-claim + + Arguments: + * id (string): opaque, unique value identifying the placement of an LTI resource link + * description (string): description for the placement of an LTI resource link + * title (string): title for the placement of an LTI resource link + """ + resource_link_claim_data = { + "id": resource_link_id, + } + + if description: + resource_link_claim_data["description"] = description + + if title: + resource_link_claim_data["title"] = title + + self.lti_claim_resource_link = { + "https://purl.imsglobal.org/spec/lti/claim/resource_link": resource_link_claim_data + } + def set_launch_presentation_claim( self, document_target="iframe" @@ -233,7 +281,6 @@ class LtiConsumer1p3: def get_lti_launch_message( self, - resource_link, include_extra_claims=True, ): """ @@ -263,17 +310,6 @@ class LtiConsumer1p3: # 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, - - # Resource link: stable and unique to each deployment_id - # This value MUST change if the link is copied or exported from one system or - # context and imported into another system or context - # http://www.imsglobal.org/spec/lti/v1p3/#resource-link-claim - "https://purl.imsglobal.org/spec/lti/claim/resource_link": { - "id": resource_link, - # Optional claims - # "title": "Introduction Assignment" - # "description": "Assignment to introduce who you are", - }, }) # Check if user data is set, then append it to lti message @@ -283,6 +319,13 @@ class LtiConsumer1p3: else: raise ValueError("Required user data isn't set.") + # Check if the resource_link claim has been set and append it to the LTI message if it has. + # The resource_link claim is required, so raise an exception if it has not been set. + if self.lti_claim_resource_link: + lti_message.update(self.lti_claim_resource_link) + else: + raise ValueError("Required resource_link data isn't set.") + # Only used when doing normal LTI launches if include_extra_claims: # Set optional claims @@ -307,7 +350,6 @@ class LtiConsumer1p3: def generate_launch_request( self, preflight_response, - resource_link ): """ Build LTI message from class parameters @@ -319,7 +361,7 @@ class LtiConsumer1p3: self._validate_preflight_response(preflight_response) # Get LTI Launch Message - lti_launch_message = self.get_lti_launch_message(resource_link=resource_link) + lti_launch_message = self.get_lti_launch_message() # Nonce from OIDC preflight launch request lti_launch_message.update({ @@ -551,21 +593,25 @@ class LtiAdvantageConsumer(LtiConsumer1p3): def generate_launch_request( self, preflight_response, - resource_link ): """ Build LTI message for Deep linking launches. Overrides method from LtiConsumer1p3 to allow handling LTI Deep linking messages """ + lti_message_hint = preflight_response.get('lti_message_hint') + launch_data = get_data_from_cache(lti_message_hint) + + if not launch_data: + log.warning(f'There was a cache miss during an LTI 1.3 launch when using the cache_key {lti_message_hint}.') + # Check if Deep Linking is enabled and that this is a Deep Link Launch - if self.dl and preflight_response.get("lti_message_hint") == "deep_linking_launch": + if self.dl and launch_data.message_type == "LtiDeepLinkingRequest": # Validate preflight response self._validate_preflight_response(preflight_response) # Get LTI Launch Message lti_launch_message = self.get_lti_launch_message( - resource_link=resource_link, include_extra_claims=False, ) @@ -598,7 +644,6 @@ class LtiAdvantageConsumer(LtiConsumer1p3): # set up or this isn't a Deep Link Launch return super().generate_launch_request( preflight_response, - resource_link ) def check_and_decode_deep_linking_token(self, token): diff --git a/lti_consumer/lti_1p3/exceptions.py b/lti_consumer/lti_1p3/exceptions.py index bc9551f..7ef835b 100644 --- a/lti_consumer/lti_1p3/exceptions.py +++ b/lti_consumer/lti_1p3/exceptions.py @@ -62,6 +62,10 @@ class PreflightRequestValidationFailure(Lti1p3Exception): message = "The preflight response is not valid." +class LtiLaunchDataValidationFailure(Lti1p3Exception): + message = "The Lti1p3LaunchData is not valid." + + class LtiAdvantageServiceNotSetUp(Lti1p3Exception): message = "The LTI Advantage Service is not set up." diff --git a/lti_consumer/lti_1p3/tests/test_consumer.py b/lti_consumer/lti_1p3/tests/test_consumer.py index 77881b6..423c414 100644 --- a/lti_consumer/lti_1p3/tests/test_consumer.py +++ b/lti_consumer/lti_1p3/tests/test_consumer.py @@ -9,9 +9,11 @@ from urllib.parse import parse_qs, urlparse import ddt from Cryptodome.PublicKey import RSA from django.test.testcases import TestCase +from edx_django_utils.cache import get_cache_key, TieredCache from jwkest.jwk import load_jwks from jwkest.jws import JWS +from lti_consumer.data import Lti1p3LaunchData from lti_consumer.lti_1p3 import exceptions from lti_consumer.lti_1p3.ags import LtiAgs from lti_consumer.lti_1p3.deep_linking import LtiDeepLinking @@ -54,23 +56,22 @@ class TestLti1p3Consumer(TestCase): tool_key=RSA_KEY ) - def _setup_lti_user(self): + def _setup_lti_launch_data(self): """ Set up a minimal LTI message with only required parameters. - Currently, the only required parameters are the user data, - but using a helper function to keep the usage consistent accross - all tests. + Currently, the only required parameters are the user data and resource_link_data. """ self.lti_consumer.set_user_data( user_id="1", role="student", ) + self.lti_consumer.set_resource_link_claim("resource_link_id") + def _get_lti_message( self, preflight_response=None, - resource_link="link" ): """ Retrieves a base LTI message with fixed test parameters. @@ -88,7 +89,6 @@ class TestLti1p3Consumer(TestCase): return self.lti_consumer.generate_launch_request( preflight_response, - resource_link ) def _decode_token(self, token): @@ -149,11 +149,17 @@ class TestLti1p3Consumer(TestCase): """ Check if preflight request is properly formed and has all required keys. """ - preflight_request_data = self.lti_consumer.prepare_preflight_url( - hint="test-hint", - lti_hint="test-lti-hint" + user_id = "1" + resource_link_id = "resource_link_id" + launch_data = Lti1p3LaunchData( + user_id=user_id, + user_role="student", + config_id="1", + resource_link_id=resource_link_id, ) + preflight_request_data = self.lti_consumer.prepare_preflight_url(launch_data) + # Extract and check parameters from OIDC launch request url parameters = parse_qs(urlparse(preflight_request_data).query) self.assertCountEqual( @@ -169,11 +175,18 @@ class TestLti1p3Consumer(TestCase): ) self.assertEqual(parameters['iss'][0], ISS) self.assertEqual(parameters['client_id'][0], CLIENT_ID) - self.assertEqual(parameters['login_hint'][0], "test-hint") - self.assertEqual(parameters['lti_message_hint'][0], "test-lti-hint") + self.assertEqual(parameters['login_hint'][0], user_id) self.assertEqual(parameters['lti_deployment_id'][0], DEPLOYMENT_ID) self.assertEqual(parameters['target_link_uri'][0], LAUNCH_URL) + launch_data_key = get_cache_key( + app="lti", + key="launch_data", + user_id=user_id, + resource_link_id=resource_link_id + ) + self.assertEqual(parameters['lti_message_hint'][0], launch_data_key) + @ddt.data( # User with no roles ( @@ -203,7 +216,6 @@ class TestLti1p3Consumer(TestCase): "email": "jonh@example.com" } ), - ) @ddt.unpack def test_set_user_data(self, data, expected_output): @@ -216,6 +228,15 @@ class TestLti1p3Consumer(TestCase): expected_output ) + def test_check_no_user_data_error(self): + """ + Check if the launch request fails if no user data is set. + """ + with self.assertRaises(ValueError) as context_manager: + self._get_lti_message() + + self.assertEqual(str(context_manager.exception), "Required user data isn't set.") + @ddt.data( "iframe", "frame", @@ -225,7 +246,7 @@ class TestLti1p3Consumer(TestCase): """ Check if setting presentation claim data works """ - self._setup_lti_user() + self._setup_lti_launch_data() self.lti_consumer.set_launch_presentation_claim(document_target=target) self.assertEqual( self.lti_consumer.lti_claim_launch_presentation, @@ -266,7 +287,7 @@ class TestLti1p3Consumer(TestCase): """ Check if setting context claim data works """ - self._setup_lti_user() + self._setup_lti_launch_data() self.lti_consumer.set_context_claim( "context_id", context_types=[context_type], @@ -306,7 +327,7 @@ class TestLti1p3Consumer(TestCase): """ Check if setting invalid context claim type omits type attribute """ - self._setup_lti_user() + self._setup_lti_launch_data() self.lti_consumer.set_context_claim( "context_id", context_types=["invalid"], @@ -345,7 +366,7 @@ class TestLti1p3Consumer(TestCase): """ Check if setting no context claim type works """ - self._setup_lti_user() + self._setup_lti_launch_data() self.lti_consumer.set_context_claim( "context_id" ) @@ -375,19 +396,56 @@ class TestLti1p3Consumer(TestCase): expected_claim_data ) - def test_check_no_user_data_error(self): + @ddt.data( + ( + {"resource_link_id": "id"}, + { + "https://purl.imsglobal.org/spec/lti/claim/resource_link": { + "id": "id", + } + } + ), + ( + {"resource_link_id": "id", "description": "description", "title": "title"}, + { + "https://purl.imsglobal.org/spec/lti/claim/resource_link": { + "id": "id", + "description": "description", + "title": "title", + } + } + ), + ) + @ddt.unpack + def test_set_resource_link_claim(self, data, expected_output): """ - Check if the launch request fails if no user data is set. + Test that setting the lti_consumer.lti_claim_resource_link attribute with + the lti_consumer.set_resource_link_claim method works correctly. """ - with self.assertRaises(ValueError): + self.lti_consumer.set_resource_link_claim(**data) + self.assertEqual( + self.lti_consumer.lti_claim_resource_link, + expected_output + ) + + def test_check_no_resource_link_claim_error(self): + """ + Check if the launch request fails if no resource_link data is set. + """ + # In order to satisfy the user data check, set some user data. + self.lti_consumer.set_user_data("1", "student") + + with self.assertRaises(ValueError) as context_manager: self._get_lti_message() + self.assertEqual(str(context_manager.exception), "Required resource_link data isn't set.") + @patch('time.time', return_value=1000) def test_launch_request(self, mock_time): """ Check if the launch request works if user data is set. """ - self._setup_lti_user() + self._setup_lti_launch_data() launch_request = self._get_lti_message() self.assertEqual(mock_time.call_count, 2) @@ -405,7 +463,7 @@ class TestLti1p3Consumer(TestCase): "custom": "parameter", } - self._setup_lti_user() + self._setup_lti_launch_data() self.lti_consumer.set_custom_parameters(custom_parameters) launch_request = self._get_lti_message() @@ -522,7 +580,7 @@ class TestLti1p3Consumer(TestCase): """ Check if extra claims are correctly added to the LTI message """ - self._setup_lti_user() + self._setup_lti_launch_data() self.lti_consumer.set_extra_claim({"fake_claim": "test"}) # Retrieve launch message @@ -571,21 +629,50 @@ class TestLtiAdvantageConsumer(TestCase): self.preflight_response = {} + def _setup_lti_message_hint(self): + """ + Instantiate Lti1p3LaunchData with the appropriate launch data and store it in the cache. + + Return the cache key that was used to store the Lti1p3LaunchData. + """ + user_id = "1" + resource_link_id = "resource_link_id" + launch_data = Lti1p3LaunchData( + user_id=user_id, + user_role="student", + config_id="1", + resource_link_id=resource_link_id, + message_type="LtiDeepLinkingRequest", + ) + + launch_data_key = get_cache_key( + app="lti", + key="launch_data", + user_id=user_id, + resource_link_id=resource_link_id, + ) + TieredCache.set_all_tiers(launch_data_key, launch_data) + + return launch_data_key + def _setup_deep_linking(self): """ Set's up deep linking class in LTI consumer. """ self.lti_consumer.enable_deep_linking("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, "nonce": NONCE, "state": STATE, - "lti_message_hint": "deep_linking_launch", + "lti_message_hint": lti_message_hint, } self.lti_consumer.set_user_data("1", "student") + self.lti_consumer.set_resource_link_claim("resource_link_id") def test_enable_ags(self): """ @@ -639,7 +726,6 @@ class TestLtiAdvantageConsumer(TestCase): # Retrieve LTI Deep Link Launch Message token = self.lti_consumer.generate_launch_request( self.preflight_response, - "resourceLink" )['id_token'] # Decode and check diff --git a/lti_consumer/lti_xblock.py b/lti_consumer/lti_xblock.py index be30ff3..c39387a 100644 --- a/lti_consumer/lti_xblock.py +++ b/lti_consumer/lti_xblock.py @@ -68,14 +68,15 @@ from xblock.validation import ValidationMessage from xblockutils.resources import ResourceLoader from xblockutils.studio_editable import StudioEditableXBlockMixin +from .data import Lti1p3LaunchData from .exceptions import LtiError from .lti_1p1.consumer import LtiConsumer1p1, parse_result_json, LTI_PARAMETERS from .lti_1p1.oauth import log_authorization_header from .outcomes import OutcomeService +from .plugin import compat from .track import track_event from .utils import _, resolve_custom_parameter_template, external_config_filter_enabled, database_config_enabled - log = logging.getLogger(__name__) DOCS_ANCHOR_TAG_OPEN = ( @@ -87,7 +88,7 @@ DOCS_ANCHOR_TAG_OPEN = ( "'>" ) RESULT_SERVICE_SUFFIX_PARSER = re.compile(r"^user/(?P<anon_id>\w+)", re.UNICODE) -ROLE_MAP = { +LTI_1P1_ROLE_MAP = { 'student': 'Student,Learner', 'staff': 'Administrator', 'instructor': 'Instructor', @@ -743,14 +744,13 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): @property def role(self): """ - Get system user role and convert it to LTI role. + Get system user role. """ user = self.runtime.service(self, 'user').get_current_user() if not user.opt_attrs["edx-platform.is_authenticated"]: raise LtiError(self.ugettext("Could not get user data for current request")) - role = user.opt_attrs.get('edx-platform.user_role', 'student') - return ROLE_MAP.get(role, 'Student,Learner') + return user.opt_attrs.get('edx-platform.user_role', 'student') @property def course(self): @@ -1031,7 +1031,13 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): from lti_consumer.api import get_lti_1p3_launch_info # Retrieve LTI 1.3 Launch information - context.update(get_lti_1p3_launch_info(block=self)) + launch_data = self.get_lti_1p3_launch_data() + context.update( + get_lti_1p3_launch_info( + launch_data=launch_data, + block=self + ) + ) # Render template fragment = Fragment() @@ -1097,6 +1103,10 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): real_user_data = self.extract_real_user_data() user_id = self.user_id role = self.role + + # Convert the LMS role into an LTI 1.1 role. + role = LTI_1P1_ROLE_MAP.get(role, 'Student,Learner') + result_sourcedid = self.lis_result_sourcedid except LtiError: loader = ResourceLoader(__name__) @@ -1405,6 +1415,47 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): return launch_url + def get_lti_1p3_launch_data(self): + """ + Return an instance of Lti1p3LaunchData, containing necessary data for an LTI 1.3 launch. + """ + # Runtime import since this will only run in the + # 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) + + location = self.location # pylint: disable=no-member + course_key = str(location.course_key) + + launch_data = Lti1p3LaunchData( + user_id=self.external_user_id, + user_role=self.role, + config_id=lti_config.config_id, + resource_link_id=str(location), + launch_presentation_document_target="iframe", + context_id=course_key, + context_type=["course_offering"], + context_title=self.get_context_title(), + context_label=course_key, + ) + + return launch_data + + def get_context_title(self): + """ + Return the title attribute of the context_claim for LTI 1.3 launches. This information is included in the + launch_data query or form parameter of the LTI 1.3 third-party login initiation request. + """ + course_key = self.location.course_key # pylint: disable=no-member + course = compat.get_course_by_id(course_key) + + return " - ".join([ + course.display_name_with_default, + course.display_org_with_default + ]) + def _get_lti_block_launch_handler(self): """ Return the LTI block launch handler. @@ -1415,13 +1466,14 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): if self.lti_version == 'lti_1p1' or self.config_type == "external": lti_block_launch_handler = self.runtime.handler_url(self, 'lti_launch_handler').rstrip('/?') else: - # Runtime import since this will only run in the Open edX LMS/Studio environments. - from lti_consumer.api import get_lti_1p3_content_url # pylint: disable=import-outside-toplevel + launch_data = self.get_lti_1p3_launch_data() # Retrieve and set LTI 1.3 Launch start URL + # Runtime import since this will only run in the Open edX LMS/Studio environments. + 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, - hint=str(self.location) # pylint: disable=no-member ) return lti_block_launch_handler diff --git a/lti_consumer/plugin/compat.py b/lti_consumer/plugin/compat.py index 5f3d1ee..e7a1b15 100644 --- a/lti_consumer/plugin/compat.py +++ b/lti_consumer/plugin/compat.py @@ -258,22 +258,3 @@ def get_event_tracker(): return tracker except ModuleNotFoundError: return None - - -def get_user_role(user, course_key: CourseKey) -> str: - """ - Import the get_user_role from LMS and return the value. - """ - # pylint: disable=import-error,import-outside-toplevel - from lms.djangoapps.courseware.access import get_user_role as get_role - return get_role(user, course_key) - - -def get_external_id_for_user(user) -> str: - """ - Import and run `get_external_id_for_user` from LMS - """ - # pylint: disable=import-error,import-outside-toplevel - from openedx.core.djangoapps.external_user_ids.models import ExternalId - ext_user, _ = ExternalId.add_new_user_id(user, 'lti') - return str(ext_user.external_user_id) diff --git a/lti_consumer/plugin/views.py b/lti_consumer/plugin/views.py index 7818bff..09c0bc5 100644 --- a/lti_consumer/plugin/views.py +++ b/lti_consumer/plugin/views.py @@ -5,7 +5,7 @@ import logging import urllib from django.contrib.auth import get_user_model -from django.core.exceptions import ObjectDoesNotExist, PermissionDenied +from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError from django.http import JsonResponse, Http404 from django.db import transaction from django.views.decorators.csrf import csrf_exempt @@ -20,7 +20,7 @@ from rest_framework.decorators import action from rest_framework.response import Response from rest_framework.status import HTTP_400_BAD_REQUEST, HTTP_403_FORBIDDEN, HTTP_404_NOT_FOUND -from lti_consumer.api import get_lti_pii_sharing_state_for_course +from lti_consumer.api import get_lti_pii_sharing_state_for_course, validate_lti_1p3_launch_data from lti_consumer.exceptions import LtiError from lti_consumer.models import ( LtiConfiguration, @@ -28,7 +28,6 @@ from lti_consumer.models import ( LtiDlContentItem, ) -from lti_consumer.lti_1p3.consumer import LTI_1P3_CONTEXT_TYPE from lti_consumer.lti_1p3.exceptions import ( Lti1p3Exception, LtiDeepLinkingContentTypeNotSupported, @@ -65,7 +64,7 @@ from lti_consumer.lti_1p3.extensions.rest_framework.parsers import ( ) from lti_consumer.lti_1p3.extensions.rest_framework.utils import IgnoreContentNegotiation from lti_consumer.plugin import compat -from lti_consumer.utils import _ +from lti_consumer.utils import _, get_lti_1p3_context_types_claim, get_data_from_cache from lti_consumer.track import track_event @@ -143,32 +142,44 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume """ Gate endpoint that triggers LTI launch endpoint XBlock handler - This uses the location key from the "login_hint" query parameter + This uses the config_id key of the "lti_message_hint" query parameter to identify the LtiConfiguration and its consumer to generate the LTI 1.3 Launch Form. """ - usage_id = request.GET.get('login_hint') - if not usage_id: - log.info('The `login_hint` query param in the request is missing or empty.') + # pylint: disable=too-many-statements + request_params = request.GET if request.method == 'GET' else request.POST + + lti_message_hint = request_params.get('lti_message_hint') + if not lti_message_hint: + log.info('The lti_message_hint query param in the request is missing or empty.') return render(request, 'html/lti_launch_error.html', status=HTTP_400_BAD_REQUEST) - try: - usage_key = UsageKey.from_string(usage_id) - except InvalidKeyError as exc: + login_hint = request_params.get('login_hint') + if not login_hint: + log.info('The login_hint query param in the request is missing or empty.') + return render(request, 'html/lti_launch_error.html', status=HTTP_400_BAD_REQUEST) + + launch_data = get_data_from_cache(lti_message_hint) + if not launch_data: + log.warning(f'There was a cache miss during an LTI 1.3 launch when using the cache_key {lti_message_hint}.') + return render(request, 'html/lti_launch_error.html', status=HTTP_400_BAD_REQUEST) + + # Validate the Lti1p3LaunchData. + is_valid, validation_messages = validate_lti_1p3_launch_data(launch_data) + if not is_valid: + validation_message = " ".join(validation_messages) log.error( - "The login_hint: %s is not a valid block location. Error: %s", - usage_id, - exc, - exc_info=True + f"The Lti1p3LaunchData is not valid. {validation_message}" ) - return render(request, 'html/lti_launch_error.html', status=HTTP_404_NOT_FOUND) + return render(request, 'html/lti_launch_error.html', status=HTTP_400_BAD_REQUEST) + config_id = launch_data.config_id try: lti_config = LtiConfiguration.objects.get( - location=usage_key + config_id=config_id ) - except LtiConfiguration.DoesNotExist as exc: - log.error("Invalid usage_id '%s' for LTI 1.3 Launch callback", usage_id) + except (LtiConfiguration.DoesNotExist, ValidationError) as exc: + log.error("Invalid config_id '%s' for LTI 1.3 Launch callback", config_id) raise Http404 from exc if lti_config.version != LtiConfiguration.LTI_1P3: @@ -177,50 +188,58 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume context = {} - course_key = usage_key.course_key - course = compat.get_course_by_id(course_key) - user_role = compat.get_user_role(request.user, course_key) - external_user_id = compat.get_external_id_for_user(request.user) - lti_consumer = lti_config.get_lti_consumer() - try: - # Pass user data - # Pass django user role to library - lti_consumer.set_user_data(user_id=external_user_id, role=user_role) - - # Set launch context - # Hardcoded for now, but we need to translate from - # self.launch_target to one of the LTI compliant names, - # either `iframe`, `frame` or `window` - # This is optional though - lti_consumer.set_launch_presentation_claim('iframe') - - # Set context claim - # This is optional - context_title = " - ".join([ - course.display_name_with_default, - course.display_org_with_default - ]) - # Course ID is the context ID for the LTI for now. This can be changed to be - # more specific in the future for supporting other tools like discussions, etc. + lti_consumer = lti_config.get_lti_consumer() + + # Set sub and roles claims. + user_id = launch_data.user_id + user_role = launch_data.user_role + lti_consumer.set_user_data( + user_id=user_id, + role=user_role, + ) + + # Set resource_link claim. + lti_consumer.set_resource_link_claim(launch_data.resource_link_id) + + # Set launch_presentation claim. + launch_presentation_target = launch_data.launch_presentation_document_target + if launch_presentation_target: + lti_consumer.set_launch_presentation_claim(launch_presentation_target) + + # Set optional context claim, if supplied. + context_type = launch_data.context_type + context_types_claim = None + + if context_type: + try: + context_types_claim = get_lti_1p3_context_types_claim(context_type) + except ValueError: + log.error( + "The context_type key %s in the launch data does not represent a valid context_type.", + context_type + ) + return render(request, 'html/lti_launch_error.html', status=HTTP_400_BAD_REQUEST) + lti_consumer.set_context_claim( - str(course_key), - context_types=[LTI_1P3_CONTEXT_TYPE.course_offering], - context_title=context_title, - context_label=str(course_key) + launch_data.context_id, + context_types_claim, + launch_data.context_title, + launch_data.context_label, ) - # Retrieve preflight response - preflight_response = request.GET.dict() - lti_message_hint = preflight_response.get('lti_message_hint', '') + # Retrieve preflight response. + preflight_response = request_params.dict() - # Set LTI Launch URL + # Set LTI Launch URL. context.update({'launch_url': lti_consumer.launch_url}) - # Modify LTI Launch URL dependind on launch type + # Modify LTI Launch URL depending on launch type. # Deep Linking Launch - Configuration flow launched by # course creators to set up content. - if lti_consumer.dl and lti_message_hint == 'deep_linking_launch': + deep_linking_content_item_id = launch_data.deep_linking_content_item_id + + if lti_consumer.dl and launch_data.message_type == 'LtiDeepLinkingRequest': # Check if the user is staff before LTI doing deep linking launch. # If not, raise exception and display error page if user_role not in ['instructor', 'staff']: @@ -231,10 +250,9 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume # Deep Linking ltiResourceLink content presentation # When content type is `ltiResourceLink`, the tool will be launched with # different parameters, set by instructors when running the DL configuration flow. - elif lti_consumer.dl and 'deep_linking_content_launch' in lti_message_hint: - # Retrieve Deep Linking parameters using lti_message_hint parameter. - deep_linking_id = lti_message_hint.split(':')[1] - content_item = lti_config.ltidlcontentitem_set.get(pk=deep_linking_id) + elif lti_consumer.dl and deep_linking_content_item_id: + # Retrieve Deep Linking parameters using the parameter. + content_item = lti_config.ltidlcontentitem_set.get(pk=deep_linking_content_item_id) # Only filter DL content item from content item set in the same LTI configuration. # This avoids a malicious user to input a random LTI id and perform LTI DL # content launches outside the scope of its configuration. @@ -250,8 +268,7 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume context.update({ "preflight_response": preflight_response, "launch_request": lti_consumer.generate_launch_request( - resource_link=usage_id, - preflight_response=preflight_response + preflight_response=preflight_response, ) }) event = { @@ -263,18 +280,20 @@ def launch_gate_endpoint(request, suffix=None): # pylint: disable=unused-argume return render(request, 'html/lti_1p3_launch.html', context) except Lti1p3Exception as exc: + resource_link_id = launch_data.resource_link_id log.warning( - "Error preparing LTI 1.3 launch for block %r: %s", - usage_id, + "Error preparing LTI 1.3 launch for resource with resource_link_id %r: %s", + resource_link_id, exc, exc_info=True ) return render(request, 'html/lti_launch_error.html', context, status=HTTP_400_BAD_REQUEST) except AssertionError as exc: + resource_link_id = launch_data.resource_link_id log.warning( - "Permission on LTI block %r denied for user %r: %s", - usage_id, - external_user_id, + "Permission on resource with resource_link_id %r denied for user %r: %s", + resource_link_id, + user_id, exc, exc_info=True ) @@ -438,10 +457,20 @@ def deep_linking_response_endpoint(request, lti_config_id=None): @require_http_methods(['GET']) @xframe_options_sameorigin -def deep_linking_content_endpoint(request, lti_config_id=None): +def deep_linking_content_endpoint(request, lti_config_id): """ Deep Linking endpoint for rendering Deep Linking Content Items. """ + launch_data_key = request.GET.get("launch_data_key") + if not launch_data_key: + log.info('The launch_data_key query param in the request is missing or empty.') + return render(request, 'html/lti_launch_error.html', status=HTTP_400_BAD_REQUEST) + + launch_data = get_data_from_cache(launch_data_key) + if not launch_data: + log.warning(f'There was a cache miss during an LTI 1.3 launch when using the cache_key {launch_data_key}.') + return render(request, 'html/lti_launch_error.html', status=HTTP_400_BAD_REQUEST) + try: # Get LTI Configuration lti_config = LtiConfiguration.objects.get(id=lti_config_id) @@ -470,6 +499,7 @@ def deep_linking_content_endpoint(request, lti_config_id=None): return render(request, 'html/lti-dl/render_dl_content.html', { 'content_items': content_items, 'block': lti_config.block, + 'launch_data': launch_data, }) diff --git a/lti_consumer/templates/html/lti-dl/render_dl_content.html b/lti_consumer/templates/html/lti-dl/render_dl_content.html index 0afa083..ef333b8 100644 --- a/lti_consumer/templates/html/lti-dl/render_dl_content.html +++ b/lti_consumer/templates/html/lti-dl/render_dl_content.html @@ -8,7 +8,7 @@ {% for item in content_items %} <div> {% if item.content_type == 'ltiResourceLink' %} - {% include "html/lti-dl/render_lti_resource_link.html" with item=item attrs=item.attributes %} + {% include "html/lti-dl/render_lti_resource_link.html" with item=item attrs=item.attributes launch_data=launch_data %} {% elif item.content_type == 'link' %} {% include "html/lti-dl/render_link.html" with item=item attrs=item.attributes %} {% elif item.content_type == 'html' %} diff --git a/lti_consumer/templates/html/lti-dl/render_lti_resource_link.html b/lti_consumer/templates/html/lti-dl/render_lti_resource_link.html index 8e5b10a..9b6baf4 100644 --- a/lti_consumer/templates/html/lti-dl/render_lti_resource_link.html +++ b/lti_consumer/templates/html/lti-dl/render_lti_resource_link.html @@ -3,7 +3,9 @@ {% if attrs.title %}<h2>{{ attrs.title }}</h2>{% endif %} {% if attrs.text %}<p>{{ attrs.text }}</p>{% endif %} -<a href="{{ item | get_dl_lti_launch_url }}" target="_blank"> +{% get_dl_lti_launch_url item launch_data as launch_url %} + +<a href="{{ launch_url }}" target="_blank"> Click here to launch the LTI content in a new tab <i class="icon fa fa-external-link"></i> </a> diff --git a/lti_consumer/templatetags/get_dl_lti_launch_url.py b/lti_consumer/templatetags/get_dl_lti_launch_url.py index 1dd97c9..d49f585 100644 --- a/lti_consumer/templatetags/get_dl_lti_launch_url.py +++ b/lti_consumer/templatetags/get_dl_lti_launch_url.py @@ -4,23 +4,22 @@ Template tags and helper functions for sanitizing html. from django import template from lti_consumer.api import get_lti_1p3_launch_start_url - register = template.Library() -@register.filter() -def get_dl_lti_launch_url(content_item): +@register.simple_tag +def get_dl_lti_launch_url(content_item, launch_data): """ - Template tag to retrive the LTI launch URL for `ltiResourceLink` content. + Template tag to retrieve the LTI launch URL for `ltiResourceLink` content. - This uses the LTI Consumer API, but hardcodes the `hint` parameter to the - block id (used when LTI launches are tied to XBlocks). + This uses the LTI Consumer API to generate the LTI 1.3 third party login initiation + URL to start the LTI 1.3 launch flow. - TODO: Refactor `hint` to use generic ID once LTI launches out of XBlocks are - supported. + This template tag requires content_item and launch_data arguments, which are passed to + get_lti_1p3_launch_start_url to encode information necessary to the eventual LTI 1.3 launch. """ return get_lti_1p3_launch_start_url( + launch_data, config_id=content_item.lti_configuration.id, dl_content_id=content_item.id, - hint=str(content_item.lti_configuration.location), ) diff --git a/lti_consumer/tests/test_utils.py b/lti_consumer/tests/test_utils.py new file mode 100644 index 0000000..192505b --- /dev/null +++ b/lti_consumer/tests/test_utils.py @@ -0,0 +1,96 @@ +""" +Utility functions used within unit tests +""" + +from unittest.mock import Mock +import urllib + +from opaque_keys.edx.keys import UsageKey +from webob import Request +from workbench.runtime import WorkbenchRuntime +from xblock.fields import ScopeIds +from xblock.runtime import DictKeyValueStore, KvsFieldData + + +FAKE_USER_ID = 'fake_user_id' + + +def make_xblock(xblock_name, xblock_cls, attributes): + """ + Helper to construct XBlock objects + """ + runtime = WorkbenchRuntime() + key_store = DictKeyValueStore() + db_model = KvsFieldData(key_store) + ids = generate_scope_ids(runtime, xblock_name) + xblock = xblock_cls(runtime, db_model, scope_ids=ids) + xblock.category = Mock() + + xblock.location = UsageKey.from_string( + 'block-v1:edX+DemoX+Demo_Course+type@problem+block@466f474fa4d045a8b7bde1b911e095ca' + ) + + xblock.runtime = Mock( + hostname='localhost', + ) + xblock.course_id = 'course-v1:edX+DemoX+Demo_Course' + for key, value in attributes.items(): + setattr(xblock, key, value) + return xblock + + +def generate_scope_ids(runtime, block_type): + """ + Helper to generate scope IDs for an XBlock + """ + def_id = runtime.id_generator.create_definition(block_type) + usage_id = runtime.id_generator.create_usage(def_id) + return ScopeIds('user', block_type, def_id, usage_id) + + +def make_request(body, method='POST'): + """ + Helper to make a request + """ + request = Request.blank('/') + request.method = 'POST' + request.body = body.encode('utf-8') + request.method = method + return request + + +def make_jwt_request(token, **overrides): + """ + Builds a Request with a JWT body. + """ + body = { + "grant_type": "client_credentials", + "client_assertion_type": "something", + "client_assertion": token, + "scope": "", + } + request = make_request(urllib.parse.urlencode({**body, **overrides}), 'POST') + request.content_type = 'application/x-www-form-urlencoded' + return request + + +def dummy_processor(_xblock): + """ + A dummy LTI parameter processor. + """ + return { + 'custom_author_email': 'author@example.com', + 'custom_author_country': '', + } + + +def defaulting_processor(_xblock): + """ + A dummy LTI parameter processor with default params. + """ + + +defaulting_processor.lti_xblock_default_params = { + 'custom_name': 'Lex', + 'custom_country': '', +} diff --git a/lti_consumer/tests/unit/plugin/test_views.py b/lti_consumer/tests/unit/plugin/test_views.py index 5a5b5d6..17db1dc 100644 --- a/lti_consumer/tests/unit/plugin/test_views.py +++ b/lti_consumer/tests/unit/plugin/test_views.py @@ -12,6 +12,7 @@ from django.urls import reverse from Cryptodome.PublicKey import RSA from jwkest.jwk import RSAKey from opaque_keys.edx.keys import UsageKey +from lti_consumer.data import Lti1p3LaunchData from lti_consumer.models import LtiConfiguration, LtiDlContentItem from lti_consumer.lti_1p3.exceptions import ( MissingRequiredClaim, @@ -24,7 +25,8 @@ from lti_consumer.lti_1p3.exceptions import ( ) from lti_consumer.lti_xblock import LtiConsumerXBlock from lti_consumer.lti_1p3.tests.utils import create_jwt -from lti_consumer.tests.unit.test_utils import make_xblock +from lti_consumer.tests.test_utils import make_xblock +from lti_consumer.utils import cache_lti_1p3_launch_data class TestLti1p3KeysetEndpoint(TestCase): @@ -122,8 +124,14 @@ class TestLti1p3LaunchGateEndpoint(TestCase): 'lti_1p3_oidc_url': 'http://tool.example/oidc', } self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, self.xblock_attributes) - # Set dummy location so that UsageKey lookup is valid - self.xblock.location = self.location + + self.launch_data = Lti1p3LaunchData( + user_id="1", + user_role="student", + config_id=self.config.config_id, + resource_link_id="resource_link_id", + ) + self.launch_data_key = cache_lti_1p3_launch_data(self.launch_data) self.compat_patcher = patch("lti_consumer.plugin.views.compat") self.compat = self.compat_patcher.start() @@ -139,13 +147,6 @@ class TestLti1p3LaunchGateEndpoint(TestCase): model_compat.load_block_as_anonymous_user.return_value = self.xblock self.addCleanup(model_compat_patcher.stop) - def test_invalid_usage_key(self): - """ - Check that passing a invalid login_hint yields HTTP code 404. - """ - response = self.client.get(self.url, {"login_hint": "useless-location"}) - self.assertEqual(response.status_code, 404) - def test_invalid_lti_version(self): """ Check that a LTI 1.1 tool accessing this endpoint is returned a 404. @@ -153,7 +154,13 @@ class TestLti1p3LaunchGateEndpoint(TestCase): self.config.version = LtiConfiguration.LTI_1P1 self.config.save() - response = self.client.get(self.url, {"login_hint": self.location}) + response = self.client.get( + self.url, + { + "login_hint": self.launch_data.user_id, + "lti_message_hint": self.launch_data_key + } + ) self.assertEqual(response.status_code, 404) # Rollback @@ -164,19 +171,78 @@ class TestLti1p3LaunchGateEndpoint(TestCase): """ Check that a 404 is returned when LtiConfiguration for a location doesn't exist """ - response = self.client.get(self.url, {"login_hint": self.location + "extra"}) + self.launch_data.config_id = "1" + response = self.client.get( + self.url, + { + "login_hint": self.launch_data.user_id, + "lti_message_hint": self.launch_data_key + } + ) self.assertEqual(response.status_code, 404) + def test_missing_required_lti_message_hint_param(self): + """ + Check that a 400 error is returned when required lti_message_hint query parameter is not provided. + """ + response = self.client.post(self.url, {"login_hint": "login_hint"}) + self.assertEqual(response.status_code, 400) + + def test_missing_required_login_hint_param(self): + """ + Check that a 400 error is returned when required login_hint query parameter is not provided. + """ + response = self.client.post(self.url, {"lti_message_hint": "lti_message_hint"}) + self.assertEqual(response.status_code, 400) + + def test_missing_launch_data(self): + """ + Check that a 400 error is returned when required lti_message_hint query parameter is not associated with + launch_data in the cache. + """ + response = self.client.post(self.url, {"lti_message_hint": "lti_message_hint", "login_hint": "login_hint"}) + self.assertEqual(response.status_code, 400) + + @patch('lti_consumer.api.validate_lti_1p3_launch_data') + @patch('lti_consumer.utils.get_data_from_cache') + def test_invalid_launch_data(self, mock_get_data_from_cache, mock_validate_launch_data): + """ + Check that a 400 error is returned when the launch_data stored in the cache is not valid. + """ + # Mock getting the launch_data from the cache. + mock_get_data_from_cache.return_value = {} + + # Mock checking the launch_data for validity. + mock_validate_launch_data.return_value = (False, []) + + response = self.client.post(self.url, {"lti_message_hint": "lti_message_hint", "login_hint": "login_hint"}) + self.assertEqual(response.status_code, 400) + + @patch('lti_consumer.api.validate_lti_1p3_launch_data') + @patch('lti_consumer.utils.get_data_from_cache') + def test_invalid_context_type(self, mock_get_data_from_cache, mock_validate_launch_data): + # Mock getting the launch_data from the cache. + mock_launch_data = Mock() + mock_launch_data.context_type = "invalid_context_type" + mock_get_data_from_cache.return_value = mock_launch_data + + # Mock checking the launch_data for validity. + mock_validate_launch_data.return_value = (True, []) + + response = self.client.post(self.url, {"lti_message_hint": "lti_message_hint", "login_hint": "login_hint"}) + self.assertEqual(response.status_code, 400) + def test_lti_launch_response(self): """ Check that the right launch response is generated """ params = { - "login_hint": self.location, "nonce": "nonce-value", "state": "hello-world", "redirect_uri": "https://tool.example", - "client_id": self.config.lti_1p3_client_id + "client_id": self.config.lti_1p3_client_id, + "login_hint": self.launch_data.user_id, + "lti_message_hint": self.launch_data_key } response = self.client.get(self.url, params) self.assertEqual(response.status_code, 200) @@ -207,8 +273,8 @@ class TestLti1p3LaunchGateEndpoint(TestCase): "client_id": self.config.lti_1p3_client_id, "redirect_ur": "http://tool.example/launch", "state": "state_test_123", - "nonce": "nonce", - "login_hint": self.location + "login_hint": self.launch_data.user_id, + "lti_message_hint": self.launch_data_key, } response = self.client.get(self.url, params) self.assertEqual(response.status_code, 400) @@ -225,6 +291,8 @@ class TestLti1p3LaunchGateEndpoint(TestCase): mock_user_service.get_external_user_id.return_value = 2 self.xblock.runtime.service.return_value = mock_user_service + self.launch_data.user_role = user_role + self.xblock.course.display_name_with_default = 'course_display_name' self.xblock.course.display_org_with_default = 'course_display_org' @@ -242,8 +310,8 @@ class TestLti1p3LaunchGateEndpoint(TestCase): "redirect_uri": "http://tool.example/launch", "state": "state_test_123", "nonce": "nonce", - "login_hint": self.location, - "lti_message_hint": "deep_linking_launch" + "login_hint": self.launch_data.user_id, + "lti_message_hint": self.launch_data_key, } response = self.client.get(self.url, params) @@ -262,7 +330,7 @@ class TestLti1p3LaunchGateEndpoint(TestCase): self.xblock.config_type = 'database' LtiConfiguration.objects.filter(id=self.config.id).update( - location=self.xblock.location, + location=self.xblock.location, # pylint: disable=no-member version=LtiConfiguration.LTI_1P3, config_store=LtiConfiguration.CONFIG_ON_DB, lti_advantage_deep_linking_enabled=dl_enabled, @@ -270,14 +338,15 @@ class TestLti1p3LaunchGateEndpoint(TestCase): ) if dl_enabled: self.xblock.lti_advantage_deep_linking_launch_url = url + self.launch_data.message_type = "LtiDeepLinkingRequest" params = { "client_id": self.config.lti_1p3_client_id, "redirect_uri": "http://tool.example/launch", "state": "state_test_123", "nonce": "nonce", - "login_hint": self.location, - "lti_message_hint": "deep_linking_launch" + "login_hint": self.launch_data.user_id, + "lti_message_hint": self.launch_data_key, } response = self.client.get(self.url, params) @@ -299,14 +368,15 @@ class TestLti1p3LaunchGateEndpoint(TestCase): self._setup_deep_linking(user_role='student') # Enable deep linking self.xblock.lti_advantage_deep_linking_enabled = True + self.launch_data.message_type = "LtiDeepLinkingRequest" params = { "client_id": self.config.lti_1p3_client_id, "redirect_uri": "http://tool.example/launch", "state": "state_test_123", "nonce": "nonce", - "login_hint": self.location, - "lti_message_hint": "deep_linking_launch" + "login_hint": self.launch_data.user_id, + "lti_message_hint": self.launch_data_key, } response = self.client.get(self.url, params) @@ -332,13 +402,17 @@ class TestLti1p3LaunchGateEndpoint(TestCase): }, } + # We need to re-cache the launch_data with a new cache key that is specific to the content item. + self.launch_data.deep_linking_content_item_id = "1" + launch_data_key = cache_lti_1p3_launch_data(self.launch_data) + params = { "client_id": self.config.lti_1p3_client_id, "redirect_uri": "http://tool.example/launch", "state": "state_test_123", "nonce": "nonce", - "login_hint": self.location, - "lti_message_hint": "deep_linking_content_launch:1" + "login_hint": self.launch_data.user_id, + "lti_message_hint": launch_data_key, } response = self.client.get(self.url, params) content = response.content.decode('utf-8') @@ -363,13 +437,18 @@ class TestLti1p3LaunchGateEndpoint(TestCase): content_type="link", attributes={"parameter": "custom"} ) + + # We need to re-cache the launch_data with a new cache key that is specific to the content item. + self.launch_data.deep_linking_content_item_id = dl_item.id + launch_data_key = cache_lti_1p3_launch_data(self.launch_data) + params = { "client_id": self.config.lti_1p3_client_id, "redirect_uri": "http://tool.example/launch", "state": "state_test_123", "nonce": "nonce", - "login_hint": self.location, - "lti_message_hint": f"deep_linking_content_launch:{dl_item.id}" + "login_hint": self.launch_data.user_id, + "lti_message_hint": launch_data_key, } response = self.client.get(self.url, params) # Check response 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 085816b..7a3e7f9 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_ags.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_ags.py @@ -15,7 +15,7 @@ from rest_framework.test import APITransactionTestCase from lti_consumer.lti_xblock import LtiConsumerXBlock from lti_consumer.models import LtiConfiguration, LtiAgsLineItem, LtiAgsScore -from lti_consumer.tests.unit.test_utils import make_xblock +from lti_consumer.tests.test_utils import make_xblock class LtiAgsLineItemViewSetTestCase(APITransactionTestCase): @@ -49,12 +49,9 @@ class LtiAgsLineItemViewSetTestCase(APITransactionTestCase): } self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, self.xblock_attributes) - # Set dummy location so that UsageKey lookup is valid - self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test' - # Create configuration self.lti_config = LtiConfiguration.objects.create( - location=str(self.xblock.location), + location=self.xblock.location, # pylint: disable=no-member version=LtiConfiguration.LTI_1P3, ) # Preload XBlock to avoid calls to modulestore @@ -177,7 +174,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): line_item = LtiAgsLineItem.objects.create( lti_configuration=self.lti_config, resource_id="test", - resource_link_id=self.xblock.location, + resource_link_id=self.xblock.location, # pylint: disable=no-member label="test label", score_maximum=100 ) @@ -198,7 +195,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): 'scoreMaximum': 100, 'label': 'test label', 'tag': '', - 'resourceLinkId': self.xblock.location, + 'resourceLinkId': str(self.xblock.location), # pylint: disable=no-member 'startDateTime': None, 'endDateTime': None, } @@ -215,7 +212,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): line_item = LtiAgsLineItem.objects.create( lti_configuration=self.lti_config, resource_id="test", - resource_link_id=self.xblock.location, + resource_link_id=self.xblock.location, # pylint: disable=no-member label="test label", score_maximum=100 ) @@ -242,7 +239,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): 'scoreMaximum': 100, 'label': 'test label', 'tag': '', - 'resourceLinkId': self.xblock.location, + 'resourceLinkId': str(self.xblock.location), # pylint: disable=no-member 'startDateTime': None, 'endDateTime': None, } @@ -262,7 +259,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): 'scoreMaximum': 100, 'label': 'test', 'tag': 'score', - 'resourceLinkId': self.xblock.location, + 'resourceLinkId': str(self.xblock.location), # pylint: disable=no-member }), content_type="application/vnd.ims.lis.v2.lineitem+json", ) @@ -276,7 +273,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): 'scoreMaximum': 100, 'label': 'test', 'tag': 'score', - 'resourceLinkId': self.xblock.location, + 'resourceLinkId': str(self.xblock.location), # pylint: disable=no-member 'startDateTime': None, 'endDateTime': None, } @@ -287,7 +284,7 @@ class LtiAgsViewSetLineItemTests(LtiAgsLineItemViewSetTestCase): self.assertEqual(line_item.score_maximum, 100) self.assertEqual(line_item.label, 'test') self.assertEqual(line_item.tag, 'score') - self.assertEqual(str(line_item.resource_link_id), self.xblock.location) + self.assertEqual(str(line_item.resource_link_id), str(self.xblock.location)) # pylint: disable=no-member def test_create_lineitem_invalid_resource_link_id(self): """ @@ -324,7 +321,7 @@ class LtiAgsViewSetScoresTests(LtiAgsLineItemViewSetTestCase): self.line_item = LtiAgsLineItem.objects.create( lti_configuration=self.lti_config, resource_id="test", - resource_link_id=self.xblock.location, + resource_link_id=self.xblock.location, # pylint: disable=no-member label="test label", score_maximum=100 ) @@ -856,7 +853,7 @@ class LtiAgsViewSetResultsTests(LtiAgsLineItemViewSetTestCase): self.line_item = LtiAgsLineItem.objects.create( lti_configuration=self.lti_config, resource_id="test", - resource_link_id=self.xblock.location, + resource_link_id=self.xblock.location, # pylint: disable=no-member label="test label", score_maximum=100 ) 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 77505f9..b19794d 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 @@ -3,6 +3,7 @@ Tests for LTI Advantage Assignments and Grades Service views. """ from unittest.mock import patch, PropertyMock, Mock +import re import ddt from Cryptodome.PublicKey import RSA from jwkest.jwk import RSAKey @@ -10,9 +11,11 @@ from rest_framework.test import APITransactionTestCase from rest_framework.exceptions import ValidationError +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.unit.test_utils import make_xblock +from lti_consumer.tests.test_utils import make_xblock class LtiDeepLinkingTestCase(APITransactionTestCase): @@ -22,11 +25,13 @@ class LtiDeepLinkingTestCase(APITransactionTestCase): def setUp(self): super().setUp() - self.location = 'block-v1:course+test+2020+type@problem+block@test' + # We define the XBlock first in order to create an LtiConfiguration instance, which is used to generate + # LTI 1.3 keys. Later, we set the necessary XBlock attributes. + self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, {}) # Create configuration self.lti_config = LtiConfiguration.objects.create( - location=self.location, + location=self.xblock.location, # pylint: disable=no-member version=LtiConfiguration.LTI_1P3, ) @@ -35,7 +40,7 @@ class LtiDeepLinkingTestCase(APITransactionTestCase): self.key = RSAKey( # Using the same key ID as client id # This way we can easily serve multiple public - # keys on teh same endpoint and keep all + # keys on the same endpoint and keep all # LTI 1.3 blocks working kid=self.lti_config.lti_1p3_private_key_id, key=rsa_key @@ -54,10 +59,9 @@ class LtiDeepLinkingTestCase(APITransactionTestCase): 'lti_advantage_deep_linking_enabled': True, 'lti_advantage_deep_linking_launch_url': 'http://tool.example/deep_link_launch', } - self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, self.xblock_attributes) - # Set dummy location so that UsageKey lookup is valid - self.xblock.location = self.location + for key, value in self.xblock_attributes.items(): + setattr(self.xblock, key, value) # Preload XBlock to avoid calls to modulestore self.lti_config.block = self.xblock @@ -406,7 +410,18 @@ class LtiDeepLinkingContentEndpointTestCase(LtiDeepLinkingTestCase): def setUp(self): super().setUp() - self.url = '/lti_consumer/v1/lti/{}/lti-dl/content'.format(self.lti_config.id) + + self.launch_data = Lti1p3LaunchData( + user_id="1", + user_role="student", + config_id=self.lti_config.config_id, + resource_link_id="resource_link_id", + ) + self.launch_data_key = cache_lti_1p3_launch_data(self.launch_data) + + self.url = '/lti_consumer/v1/lti/{}/lti-dl/content?launch_data_key={}'.format( + self.lti_config.id, self.launch_data_key + ) @patch('lti_consumer.plugin.views.has_block_access', return_value=False) def test_forbidden_access(self, has_block_access_patcher): # pylint: disable=unused-argument @@ -420,9 +435,35 @@ class LtiDeepLinkingContentEndpointTestCase(LtiDeepLinkingTestCase): """ Test if throws 404 when lti configuration not found. """ - resp = self.client.get('/lti_consumer/v1/lti/200/lti-dl/content') + resp = self.client.get( + '/lti_consumer/v1/lti/200/lti-dl/content?launch_data_key={}'.format(self.launch_data_key) + ) self.assertEqual(resp.status_code, 404) + def test_missing_required_launch_data_key_param(self): + """ + Check that a 400 error is returned when required launch_data_key query parameter is not provided. + """ + # Use a new URL instead of self.url so that we do not include a launch_data_key. + url = '/lti_consumer/v1/lti/{}/lti-dl/content'.format( + self.lti_config.id + ) + response = self.client.get(url, {}) + self.assertEqual(response.status_code, 400) + + def test_missing_launch_data(self): + """ + Check that a 400 error is returned when required launch_data_key query parameter is not associated with + launch_data in the cache. + """ + # Use a new URL instead of self.url so that we include a launch_data_key that is not associated with any + # launch_data in the cache. + url = '/lti_consumer/v1/lti/{}/lti-dl/content'.format( + self.lti_config.id + ) + response = self.client.get(url, {"launch_data_key": "launch_data_key"}) + self.assertEqual(response.status_code, 400) + @patch('lti_consumer.plugin.views.has_block_access', return_value=True) def test_no_dl_contents(self, has_block_access_patcher): # pylint: disable=unused-argument """ @@ -630,9 +671,12 @@ class LtiDeepLinkingContentEndpointTestCase(LtiDeepLinkingTestCase): resp = self.client.get(self.url) self.assertEqual(resp.status_code, 200) - # Check that there's three LTI Resource links presented - for item in content_items: + # Check that there's three LTI Resource links presented with the correct launch_data_key query parameter and the + # correct deep_linking_content_item_id parameter in the launch_data. + lti_message_hints = re.findall('lti_message_hint=(\\w*)', str(resp.content)) + + for lti_message_hint in lti_message_hints: self.assertContains( resp, - f"lti_message_hint=deep_linking_content_launch%3A{item.id}" + f"lti_message_hint={lti_message_hint}" ) 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 4a9eaba..2a40014 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py @@ -10,7 +10,7 @@ from rest_framework.reverse import reverse from lti_consumer.exceptions import LtiError from lti_consumer.lti_xblock import LtiConsumerXBlock from lti_consumer.models import LtiConfiguration -from lti_consumer.tests.unit.test_utils import make_xblock +from lti_consumer.tests.test_utils import make_xblock def generate_mock_members(num, role='student'): @@ -135,12 +135,9 @@ class LtiNrpsTestCase(APITransactionTestCase): self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, self.xblock_attributes) - # Set dummy location so that UsageKey lookup is valid - self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test' - # Create configuration self.lti_config = LtiConfiguration.objects.create( - location=str(self.xblock.location), + location=self.xblock.location, # pylint: disable=no-member version=LtiConfiguration.LTI_1P3, ) # Preload XBlock to avoid calls to modulestore diff --git a/lti_consumer/tests/unit/test_api.py b/lti_consumer/tests/unit/test_api.py index c034a49..94633a9 100644 --- a/lti_consumer/tests/unit/test_api.py +++ b/lti_consumer/tests/unit/test_api.py @@ -2,10 +2,12 @@ Tests for LTI API. """ from unittest.mock import Mock, patch +from urllib.parse import parse_qs, urlparse 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 ( @@ -14,11 +16,14 @@ from lti_consumer.api import ( get_deep_linking_data, get_lti_1p3_launch_info, get_lti_1p3_launch_start_url, - get_lti_consumer + get_lti_consumer, + validate_lti_1p3_launch_data ) +from lti_consumer.data import Lti1p3LaunchData from lti_consumer.lti_xblock import LtiConsumerXBlock from lti_consumer.models import LtiConfiguration, LtiDlContentItem -from lti_consumer.tests.unit.test_utils import make_xblock +from lti_consumer.tests.test_utils import make_xblock +from lti_consumer.utils import get_data_from_cache class Lti1P3TestCase(TestCase): @@ -52,11 +57,19 @@ class Lti1P3TestCase(TestCase): 'lti_1p3_tool_public_key': public_key, } self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, xblock_attributes) - # Set dummy location so that UsageKey lookup is valid - self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test' + # Create lti configuration self.lti_config = LtiConfiguration.objects.create( - location=self.xblock.location + location=self.xblock.location # pylint: disable=no-member + ) + + @staticmethod + def _get_lti_1p3_launch_data(): + return Lti1p3LaunchData( + user_id="1", + user_role="student", + config_id="1", + resource_link_id="resource_link_id", ) @@ -228,9 +241,124 @@ class TestGetLtiConsumer(TestCase): self.assertEqual(LtiConfiguration.objects.all().count(), 1) +class TestValidateLti1p3LaunchData(TestCase): + """ + Unit tests for validate_lti_1p3_launch_data API method. + """ + def setUp(self): + # Patch internal method to avoid calls to modulestore + super().setUp() + patcher = patch( + 'lti_consumer.models.LtiConfiguration.get_lti_consumer', + ) + self.addCleanup(patcher.stop) + + def _assert_required_context_id_message(self, validation_messages): + """ + Assert that validation_messages is the correct list of validation_messages for the required context_id + attribute. + + Arguments: + validation_messages (list): a list of strings representing validation messages + """ + self.assertEqual( + validation_messages, + ["The context_id attribute is required in the launch data if any optional context properties are provided."] + ) + + def test_valid(self): + """ + Ensure that valid instances of Lti1p3LaunchData are appropriately validated. + """ + launch_data = Lti1p3LaunchData( + user_id="1", + user_role="student", + config_id="1", + resource_link_id="resource_link_id", + context_id="1", + context_type=["course_offering"], + ) + + is_valid, validation_messages = validate_lti_1p3_launch_data(launch_data) + + self.assertEqual(is_valid, True) + self.assertEqual(validation_messages, []) + + def test_invalid_context_values_context_id_required(self): + """ + Ensure that instances of Lti1p3LaunchData that are instantiated with optional context_* attributes also are + instantiated with the context_id attribute. + """ + launch_data = Lti1p3LaunchData( + user_id="1", + user_role="student", + config_id="1", + resource_link_id="resource_link_id", + ) + + launch_data.context_type = ["course_offering"] + is_valid, validation_messages = validate_lti_1p3_launch_data(launch_data) + self.assertEqual(is_valid, False) + self._assert_required_context_id_message(validation_messages) + + launch_data.context_title = "context_title" + is_valid, validation_messages = validate_lti_1p3_launch_data(launch_data) + self.assertEqual(is_valid, False) + self._assert_required_context_id_message(validation_messages) + + launch_data.context_label = "context_label" + is_valid, validation_messages = validate_lti_1p3_launch_data(launch_data) + self.assertEqual(is_valid, False) + self._assert_required_context_id_message(validation_messages) + + def test_invalid_user_role(self): + """ + Ensure that instances of Lti1p3LaunchData are instantiated with a user_role that is in the LTI_1P3_ROLE_MAP. + """ + user_role = "cat" + launch_data = Lti1p3LaunchData( + user_id="1", + user_role=user_role, + config_id="1", + resource_link_id="resource_link_id", + ) + + is_valid, validation_messages = validate_lti_1p3_launch_data(launch_data) + + self.assertEqual(is_valid, False) + self.assertEqual( + validation_messages, + [f"The user_role attribute {user_role} is not a valid user_role."] + ) + + def test_invalid_context_type(self): + """ + Ensure that instances of Lti1p3LaunchData are instantiated with a context_type that is one of group, + course_offering, course_section, or course_template. + """ + context_type = "invalid_context" + + launch_data = Lti1p3LaunchData( + user_id="1", + user_role="student", + config_id="1", + resource_link_id="resource_link_id", + context_id="1", + context_type=context_type, + ) + + is_valid, validation_messages = validate_lti_1p3_launch_data(launch_data) + + self.assertEqual(is_valid, False) + self.assertEqual( + validation_messages, + [f"The context_type attribute {context_type} in the launch data is not a valid context_type."] + ) + + class TestGetLti1p3LaunchInfo(TestCase): """ - Unit tests for get_lti_consumer API method. + Unit tests for get_lti_1p3_launch_info API method. """ def setUp(self): # Patch internal method to avoid calls to modulestore @@ -245,12 +373,21 @@ class TestGetLti1p3LaunchInfo(TestCase): return super().setUp() + @staticmethod + def _get_lti_1p3_launch_data(): + return Lti1p3LaunchData( + user_id="1", + user_role="student", + config_id="1", + 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() + get_lti_1p3_launch_info({}) def test_retrieve_with_id(self): """ @@ -259,8 +396,10 @@ class TestGetLti1p3LaunchInfo(TestCase): location = 'block-v1:course+test+2020+type@problem+block@test' lti_config = LtiConfiguration.objects.create(location=location) + launch_data = self._get_lti_1p3_launch_data() + # Call and check returns - launch_info = get_lti_1p3_launch_info(config_id=lti_config.id) + launch_info = get_lti_1p3_launch_info(launch_data, config_id=lti_config.id) # Not checking all data here, there's a test specific for that self.assertEqual(launch_info['client_id'], lti_config.lti_1p3_client_id) @@ -273,6 +412,8 @@ class TestGetLti1p3LaunchInfo(TestCase): block.location = 'block-v1:course+test+2020+type@problem+block@test' block.lti_version = LtiConfiguration.LTI_1P3 + launch_data = self._get_lti_1p3_launch_data() + # Create LTI Config and Deep linking object lti_config = LtiConfiguration.objects.create(location=block.location) LtiDlContentItem.objects.create( @@ -282,7 +423,7 @@ class TestGetLti1p3LaunchInfo(TestCase): ) # Call API - launch_info = get_lti_1p3_launch_info(block=block) + launch_info = get_lti_1p3_launch_info(launch_data, block=block) # Retrieve created config and check full launch info data lti_config = LtiConfiguration.objects.get() @@ -291,12 +432,12 @@ class TestGetLti1p3LaunchInfo(TestCase): { 'client_id': lti_config.lti_1p3_client_id, 'keyset_url': 'https://example.com/api/lti_consumer/v1/public_keysets/{}'.format( - lti_config.location + lti_config.config_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.location + lti_config.config_id ), 'deep_linking_launch_url': 'http://example.com', @@ -316,7 +457,10 @@ class TestGetLti1p3LaunchInfo(TestCase): content_type=LtiDlContentItem.IMAGE, attributes={"test": "this is a test attribute"} ) - launch_info = get_lti_1p3_launch_info(config_id=lti_config.id) + + launch_data = self._get_lti_1p3_launch_data() + + launch_info = get_lti_1p3_launch_info(launch_data, config_id=lti_config.id) self.assertEqual( launch_info, { @@ -346,22 +490,64 @@ class TestGetLti1p3LaunchUrl(Lti1P3TestCase): Check if the API creates a model if no object matching properties is found. """ with self.assertRaises(Exception): - get_lti_1p3_launch_start_url() + 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) + + parameters = parse_qs(urlparse(launch_url).query) + launch_data = get_data_from_cache(parameters.get("lti_message_hint")[0]) + + self.assertEqual(launch_data.message_type, "LtiResourceLinkRequest") + self.assertEqual(launch_data.deep_linking_content_item_id, None) - def test_retrieve_url(self): + def test_get_deep_linking_lti_launch_url(self): """ - Check if the correct launch url is retrieved + Check if the correct launch url is retrieved for a deep linking LTI 1.3 launch. """ self._setup_lti_block() - # Call API for normal LTI launch initiation - launch_url = get_lti_1p3_launch_start_url(block=self.xblock, hint="test_hint") - self.assertIn('login_hint=test_hint', launch_url) - self.assertIn('lti_message_hint=', launch_url) + launch_data = self._get_lti_1p3_launch_data() - # Call API for deep link launch - launch_url = get_lti_1p3_launch_start_url(block=self.xblock, deep_link_launch=True) - self.assertIn('lti_message_hint=deep_linking_launch', launch_url) + # 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) + + parameters = parse_qs(urlparse(launch_url).query) + launch_data = get_data_from_cache(parameters.get("lti_message_hint")[0]) + + self.assertEqual(launch_data.message_type, "LtiDeepLinkingRequest") + self.assertEqual(launch_data.deep_linking_content_item_id, None) + + def test_get_deep_linking_content_item_launch_url(self): + """ + 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) + + parameters = parse_qs(urlparse(launch_url).query) + launch_url = get_lti_1p3_launch_start_url(launch_data, block=self.xblock, dl_content_id="1") + + parameters = parse_qs(urlparse(launch_url).query) + launch_data = get_data_from_cache(parameters.get("lti_message_hint")[0]) + + self.assertEqual(launch_data.message_type, "LtiResourceLinkRequest") + self.assertEqual(launch_data.deep_linking_content_item_id, "1") class TestGetLti1p3ContentUrl(Lti1P3TestCase): @@ -373,9 +559,11 @@ 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(block=self.xblock), 'test_url') + self.assertEqual(get_lti_1p3_content_url(launch_data, block=self.xblock), 'test_url') def test_lti_content_presentation_single_link(self): """ @@ -384,6 +572,8 @@ class TestGetLti1p3ContentUrl(Lti1P3TestCase): """ self._setup_lti_block() + launch_data = self._get_lti_1p3_launch_data() + # Create LTI DL content items lti_content = LtiDlContentItem.objects.create( lti_configuration=self.lti_config, @@ -392,13 +582,13 @@ class TestGetLti1p3ContentUrl(Lti1P3TestCase): ) # Call API to retrieve content item URL - launch_url = get_lti_1p3_content_url(block=self.xblock) - self.assertIn( - # Checking for `deep_linking_content_launch:<content_item_id>` - # URL Encoded `:` is `%3A` - f'lti_message_hint=deep_linking_content_launch%3A{lti_content.id}', - launch_url, - ) + launch_url = get_lti_1p3_content_url(launch_data, block=self.xblock) + + parameters = parse_qs(urlparse(launch_url).query) + launch_data = get_data_from_cache(parameters.get("lti_message_hint")[0]) + + self.assertEqual(lti_content.id, launch_data.deep_linking_content_item_id) + self.assertEqual(launch_data.message_type, "LtiResourceLinkRequest") def test_lti_content_presentation_multiple_links(self): """ @@ -407,6 +597,15 @@ class TestGetLti1p3ContentUrl(Lti1P3TestCase): """ self._setup_lti_block() + launch_data = self._get_lti_1p3_launch_data() + + launch_data_key = get_cache_key( + app="lti", + key="launch_data", + user_id=launch_data.user_id, + resource_link_id=launch_data.resource_link_id + ) + # Create LTI DL content items for _ in range(3): LtiDlContentItem.objects.create( @@ -418,8 +617,8 @@ class TestGetLti1p3ContentUrl(Lti1P3TestCase): # Call API to retrieve content item URL self.assertIn( # Checking for the content presentation URL - f"/api/lti_consumer/v1/lti/{self.lti_config.id}/lti-dl/content", - get_lti_1p3_content_url(block=self.xblock), + 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), ) diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py index 242924b..7f535df 100644 --- a/lti_consumer/tests/unit/test_lti_xblock.py +++ b/lti_consumer/tests/unit/test_lti_xblock.py @@ -15,14 +15,15 @@ from django.test import override_settings from django.test.testcases import TestCase from django.utils import timezone from jwkest.jwk import RSAKey, KEYS -from opaque_keys.edx.keys import UsageKey from lti_consumer.exceptions import LtiError +from lti_consumer.api import _get_lti_config +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 -from lti_consumer.tests.unit import test_utils -from lti_consumer.tests.unit.test_utils import FAKE_USER_ID, make_jwt_request, make_request, make_xblock +from lti_consumer.tests import test_utils +from lti_consumer.tests.test_utils import FAKE_USER_ID, make_jwt_request, make_request, make_xblock from lti_consumer.utils import resolve_custom_parameter_template HTML_PROBLEM_PROGRESS = '<div class="problem-progress">' @@ -150,28 +151,28 @@ class TestProperties(TestLtiConsumerXBlock): 'edx-platform.is_authenticated': True, } self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user) - self.assertEqual(self.xblock.role, 'Student,Learner') + self.assertEqual(self.xblock.role, 'student') fake_user.opt_attrs = { 'edx-platform.user_role': 'guest', 'edx-platform.is_authenticated': True, } self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user) - self.assertEqual(self.xblock.role, 'Student,Learner') + self.assertEqual(self.xblock.role, 'guest') fake_user.opt_attrs = { 'edx-platform.user_role': 'staff', 'edx-platform.is_authenticated': True, } self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user) - self.assertEqual(self.xblock.role, 'Administrator') + self.assertEqual(self.xblock.role, 'staff') fake_user.opt_attrs = { 'edx-platform.user_role': 'instructor', 'edx-platform.is_authenticated': True, } self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user) - self.assertEqual(self.xblock.role, 'Instructor') + self.assertEqual(self.xblock.role, 'instructor') fake_user.opt_attrs = { 'edx-platform.user_role': 'student', @@ -276,13 +277,29 @@ class TestProperties(TestLtiConsumerXBlock): with self.assertRaises(LtiError): __ = self.xblock.user_id + def test_external_user_id(self): + """ + Test `external_user_id` returns the correct external user ID. + """ + external_user_id = "external_user_id" + self.xblock.runtime.service(self, 'user').get_external_user_id = Mock(return_value=external_user_id) + self.assertEqual(self.xblock.external_user_id, external_user_id) + + def test_external_user_id_none(self): + """ + Test `external_user_id` raises LtiError when the external user ID cannot be returned. + """ + self.xblock.runtime.service(self, 'user').get_external_user_id = Mock(return_value=None) + with self.assertRaises(LtiError): + __ = self.xblock.external_user_id + def test_resource_link_id(self): """ Test `resource_link_id` returns appropriate string """ self.assertEqual( self.xblock.resource_link_id, - f"{self.xblock.runtime.hostname}-{self.xblock.location.html_id()}" + f"{self.xblock.runtime.hostname}-{self.xblock.location.html_id()}" # pylint: disable=no-member ) @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.context_id') @@ -579,7 +596,6 @@ class TestGetLti1p1Consumer(TestLtiConsumerXBlock): key = 'test' secret = 'secret' self.xblock.lti_id = provider - self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test' type(mock_course).lti_passports = PropertyMock(return_value=[f"{provider}:{key}:{secret}"]) with patch('lti_consumer.models.LtiConfiguration.block', return_value=self.xblock): @@ -1275,7 +1291,8 @@ class TestGetContext(TestLtiConsumerXBlock): @ddt.data('lti_1p1', 'lti_1p3') @patch('lti_consumer.api.get_lti_1p3_content_url') - def test_context_keys(self, lti_version, lti_api_patch): + @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.get_lti_1p3_launch_data') + def test_context_keys(self, lti_version, lti_api_patch, mock_get_lti_1p3_launch_data): """ Test `_get_context_for_template` returns dict with correct keys """ @@ -1287,6 +1304,11 @@ class TestGetContext(TestLtiConsumerXBlock): 'modal_vertical_offset', 'modal_horizontal_offset', 'modal_width', 'accept_grades_past_due' ) + + # This test isn't testing the value of any of the above keys. Calling _get_lti_block_launch_handler raises an + # error because the mocked XBlock location attribute does not act like a UsageKey, so mock out + # get_lti_1p3_launch_data to avoid accessing it. + mock_get_lti_1p3_launch_data.return_value = None context = self.xblock._get_context_for_template() # pylint: disable=protected-access for key in context_keys: @@ -1332,8 +1354,9 @@ class TestGetContext(TestLtiConsumerXBlock): context = self.xblock._get_context_for_template() # pylint: disable=protected-access self.assertEqual(context['launch_url'], lti_launch_url) + @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.get_lti_1p3_launch_data') @patch('lti_consumer.api.get_lti_1p3_content_url') - def test_context_correct_origin_1p3(self, mock_get_lti_1p3_content_url): + def test_context_correct_origin_1p3(self, mock_get_lti_1p3_content_url, mock_get_lti_1p3_launch_data): """ Test that certain context keys relevant to 1.3 integrations that can be stored on different types of config_stores are pulled from the appropriate config_store. @@ -1348,6 +1371,10 @@ class TestGetContext(TestLtiConsumerXBlock): type(mock_lti_consumer).launch_url = PropertyMock(return_value=lti_1p3_launch_url) self.xblock._get_lti_consumer = Mock(return_value=mock_lti_consumer) # pylint: disable=protected-access + # Calling _get_lti_block_launch_handler raises an error because the mocked XBlock location attribute does not + # act like a UsageKey, so mock out get_lti_1p3_launch_data to avoid accessing it. + mock_get_lti_1p3_launch_data.return_value = None + context = self.xblock._get_context_for_template() # pylint: disable=protected-access self.assertEqual(context['lti_1p3_launch_url'], lti_1p3_launch_url) @@ -1358,7 +1385,7 @@ class TestProcessorSettings(TestLtiConsumerXBlock): Unit tests for the adding custom LTI parameters. """ settings = { - 'parameter_processors': ['lti_consumer.tests.unit.test_utils:dummy_processor'] + 'parameter_processors': ['lti_consumer.tests.test_utils:dummy_processor'] } def test_no_processors_by_default(self): @@ -1390,7 +1417,7 @@ class TestProcessorSettings(TestLtiConsumerXBlock): }, { # Non-existent processor 'parameter_processors': [ - 'lti_consumer.tests.unit.test_utils:non_existent', + 'lti_consumer.tests.test_utils:non_existent', ], }) @patch('lti_consumer.lti_xblock.log') @@ -1430,8 +1457,6 @@ class TestLtiConsumer1p3XBlock(TestCase): 'lti_1p3_oidc_url': 'http://tool.example/oidc', } self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, self.xblock_attributes) - # Set dummy location so that UsageKey lookup is valid - self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test' self.mock_filter_enabled_patcher = patch("lti_consumer.lti_xblock.external_config_filter_enabled") self.mock_database_config_enabled_patcher = patch("lti_consumer.lti_xblock.database_config_enabled") @@ -1440,21 +1465,72 @@ class TestLtiConsumer1p3XBlock(TestCase): self.addCleanup(self.mock_filter_enabled_patcher.stop) self.addCleanup(self.mock_database_config_enabled_patcher.stop) + def test_get_lti_1p3_launch_data(self): + """ + Test that get_lti_1p3_launch_data returns an instance of Lti1p3LaunchData with the correct data. + """ + # Mock out the user role and external_user_id properties. + fake_user = Mock() + fake_user.opt_attrs = { + 'edx-platform.user_role': 'instructor', + 'edx-platform.is_authenticated': True, + } + self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user) + self.xblock.runtime.service(self, 'user').get_external_user_id = Mock(return_value="external_user_id") + + # Mock out get_context_title to avoid calling into the compatability layer. + self.xblock.get_context_title = Mock(return_value="context_title") + + 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, + resource_link_id=str(self.xblock.location), # pylint: disable=no-member + launch_presentation_document_target="iframe", + message_type="LtiResourceLinkRequest", + context_id=course_key, + context_type=["course_offering"], + context_title="context_title", + context_label=course_key, + ) + + self.assertEqual( + launch_data, + expected_launch_data + ) + + @patch('lti_consumer.plugin.compat.get_course_by_id') + def test_get_context_title(self, mock_get_course_by_id): + """ + Test that get_context_title returns the correct context title + """ + mock_course = Mock() + mock_course.display_name_with_default = "DemoX" + mock_course.display_org_with_default = "edX" + + mock_get_course_by_id.return_value = mock_course + + self.assertEqual(self.xblock.get_context_title(), "DemoX - edX") + def test_studio_view(self): """ Test that the studio settings view load the custom js. """ - self.xblock.location = Mock() - self.xblock.location.__get__ = 'block-v1:course+test+2020+type@problem+block@test' - self.xblock.location.course_key = 'course-v1:Demo_Course' response = self.xblock.studio_view({}) self.assertEqual(response.js_init_fn, 'LtiConsumerXBlockInitStudio') + @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.get_lti_1p3_launch_data') @patch('lti_consumer.api.get_lti_1p3_launch_info') - def test_author_view(self, mock_get_launch_info): + def test_author_view(self, mock_get_launch_info, mock_lti_get_1p3_launch_data): """ Test that the studio view loads LTI 1.3 view. """ + mock_lti_get_1p3_launch_data.return_value = None mock_get_launch_info.return_value = { 'client_id': "mock-client_id", 'keyset_url': "mock-keyset_url", @@ -1497,8 +1573,7 @@ class TestLti1p3AccessTokenEndpoint(TestLtiConsumerXBlock): 'lti_1p3_tool_public_key': self.public_key, } self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, self.xblock_attributes) - # Set dummy location so that UsageKey lookup is valid - self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test' + patcher = patch( 'lti_consumer.models.compat', **{'load_block_as_anonymous_user.return_value': self.xblock} @@ -1669,8 +1744,6 @@ class TestLti1p3AccessTokenJWK(TestCase): 'lti_1p3_oidc_url': 'http://tool.example/oidc', 'lti_1p3_tool_keyset_url': "http://tool.example/keyset", }) - self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test' - self.xblock.save() self.key = RSAKey(key=RSA.generate(2048), kid="1") @@ -1753,7 +1826,6 @@ class TestSubmitStudioEditsHandler(TestLtiConsumerXBlock): def setUp(self): super().setUp() - self.xblock.location = UsageKey.from_string('block-v1:course+test+2020+type@problem+block@test') self.xblock.lti_version = "lti_1p3" db_config_waffle_patcher = patch('lti_consumer.lti_xblock.database_config_enabled', return_value=True) diff --git a/lti_consumer/tests/unit/test_models.py b/lti_consumer/tests/unit/test_models.py index 58a2ccc..aa06952 100644 --- a/lti_consumer/tests/unit/test_models.py +++ b/lti_consumer/tests/unit/test_models.py @@ -3,7 +3,7 @@ Unit tests for LTI models. """ from contextlib import contextmanager from datetime import datetime, timedelta -from unittest.mock import patch, Mock, PropertyMock +from unittest.mock import patch import ddt from Cryptodome.PublicKey import RSA @@ -22,7 +22,7 @@ from lti_consumer.models import ( LtiConfiguration, LtiDlContentItem, ) -from lti_consumer.tests.unit.test_utils import make_xblock +from lti_consumer.tests.test_utils import make_xblock @ddt.ddt @@ -54,22 +54,20 @@ class TestLtiConfigurationModel(TestCase): 'lti_advantage_deep_linking_enabled': True, } self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, self.xblock_attributes) - # Set dummy location so that UsageKey lookup is valid - self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test' # Creates an LTI configuration objects for testing self.lti_1p1_config = LtiConfiguration.objects.create( - location=str(self.xblock.location), + location=self.xblock.location, # pylint: disable=no-member version=LtiConfiguration.LTI_1P1 ) self.lti_1p3_config = LtiConfiguration.objects.create( - location=str(self.xblock.location), + location=self.xblock.location, # pylint: disable=no-member version=LtiConfiguration.LTI_1P3 ) self.lti_1p3_config_db = LtiConfiguration.objects.create( - location=str(self.xblock.location), + location=self.xblock.location, # pylint: disable=no-member version=LtiConfiguration.LTI_1P3, config_store=LtiConfiguration.CONFIG_ON_DB, lti_advantage_ags_mode='programmatic', @@ -92,7 +90,7 @@ class TestLtiConfigurationModel(TestCase): Helper function to create a LtiConfiguration object with specific attributes """ return LtiConfiguration.objects.create( - location=str(self.xblock.location), + location=self.xblock.location, # pylint: disable=no-member version=LtiConfiguration.LTI_1P3, **kwargs ) @@ -385,10 +383,6 @@ class TestLtiConfigurationModel(TestCase): self.lti_1p3_config.config_store = self.lti_1p3_config.CONFIG_ON_DB self.lti_1p3_config.block = self.xblock - self.xblock.location = Mock() - course_key_mock = PropertyMock(return_value='course-v1:edX+DemoX+Demo_Course') - type(self.xblock.location).course_key = course_key_mock - self.lti_1p3_config_db.block = self.xblock with patch("lti_consumer.models.database_config_enabled", return_value=False),\ @@ -495,11 +489,9 @@ class TestLtiDlContentItemModel(TestCase): self.xblock_attributes = {'lti_version': 'lti_1p3'} self.xblock = make_xblock('lti_consumer', LtiConsumerXBlock, self.xblock_attributes) - # Set dummy location so that UsageKey lookup is valid - self.xblock.location = 'block-v1:course+test+2020+type@problem+block@test' self.lti_1p3_config = LtiConfiguration.objects.create( - location=str(self.xblock.location), + location=self.xblock.location, # pylint: disable=no-member version=LtiConfiguration.LTI_1P3 ) @@ -507,6 +499,7 @@ class TestLtiDlContentItemModel(TestCase): """ Test String representation of model. """ + content_item = LtiDlContentItem.objects.create( lti_configuration=self.lti_1p3_config, content_type=LtiDlContentItem.IMAGE, @@ -514,7 +507,8 @@ class TestLtiDlContentItemModel(TestCase): ) self.assertEqual( str(content_item), - "[CONFIG_ON_XBLOCK] lti_1p3 - block-v1:course+test+2020+type@problem+block@test: image" + "[CONFIG_ON_XBLOCK] lti_1p3 - " + "block-v1:edX+DemoX+Demo_Course+type@problem+block@466f474fa4d045a8b7bde1b911e095ca: image" ) diff --git a/lti_consumer/tests/unit/test_outcomes.py b/lti_consumer/tests/unit/test_outcomes.py index 9fe0337..d97d926 100644 --- a/lti_consumer/tests/unit/test_outcomes.py +++ b/lti_consumer/tests/unit/test_outcomes.py @@ -11,7 +11,7 @@ from unittest.mock import Mock, PropertyMock, patch from lti_consumer.exceptions import LtiError from lti_consumer.outcomes import OutcomeService, parse_grade_xml_body from lti_consumer.tests.unit.test_lti_xblock import TestLtiConsumerXBlock -from lti_consumer.tests.unit.test_utils import make_request +from lti_consumer.tests.test_utils import make_request REQUEST_BODY_TEMPLATE_VALID = textwrap.dedent(""" <?xml version="1.0" encoding="UTF-8"?> diff --git a/lti_consumer/tests/unit/test_utils.py b/lti_consumer/tests/unit/test_utils.py index 214db16..4393772 100644 --- a/lti_consumer/tests/unit/test_utils.py +++ b/lti_consumer/tests/unit/test_utils.py @@ -1,91 +1,115 @@ """ -Utility functions used within unit tests +Unit tests for lti_consumer.utils module """ +from unittest.mock import Mock, patch -from unittest.mock import Mock -import urllib -from webob import Request -from workbench.runtime import WorkbenchRuntime -from xblock.fields import ScopeIds -from xblock.runtime import DictKeyValueStore, KvsFieldData +import ddt +from django.test.testcases import TestCase -FAKE_USER_ID = 'fake_user_id' +from lti_consumer.lti_1p3.constants import LTI_1P3_CONTEXT_TYPE +from lti_consumer.utils import ( + get_lti_1p3_context_types_claim, + get_lti_1p3_launch_data_cache_key, + cache_lti_1p3_launch_data, + get_data_from_cache, +) -def make_xblock(xblock_name, xblock_cls, attributes): +@ddt.ddt +class TestGetLti1p3ContextTypesClaim(TestCase): """ - Helper to construct XBlock objects + Tests for the get_lti_1p3_context_types_claim function of the utils module. """ - runtime = WorkbenchRuntime() - key_store = DictKeyValueStore() - db_model = KvsFieldData(key_store) - ids = generate_scope_ids(runtime, xblock_name) - xblock = xblock_cls(runtime, db_model, scope_ids=ids) - xblock.category = Mock() - xblock.location = Mock( - html_id=Mock(return_value='sample_element_id'), + + @ddt.data( + (["course_offering"], [LTI_1P3_CONTEXT_TYPE.course_offering]), + (["course_offering", "group"], [LTI_1P3_CONTEXT_TYPE.course_offering, LTI_1P3_CONTEXT_TYPE.group]), ) - xblock.runtime = Mock( - hostname='localhost', + @ddt.unpack + def test_get_lti_1p3_context_types_claim(self, argument, expected_output): + """ + Test that get_lti_1p3_context_types_claim returns the correct context_types. + """ + lti_context_types_claims = get_lti_1p3_context_types_claim(argument) + + self.assertEqual(lti_context_types_claims, expected_output) + + @ddt.data( + ["course_offering", "nonsense"], + ["nonsense"], ) - xblock.course_id = 'course-v1:edX+DemoX+Demo_Course' - for key, value in attributes.items(): - setattr(xblock, key, value) - return xblock - - -def generate_scope_ids(runtime, block_type): - """ - Helper to generate scope IDs for an XBlock - """ - def_id = runtime.id_generator.create_definition(block_type) - usage_id = runtime.id_generator.create_usage(def_id) - return ScopeIds('user', block_type, def_id, usage_id) - - -def make_request(body, method='POST'): - """ - Helper to make a request - """ - request = Request.blank('/') - request.method = 'POST' - request.body = body.encode('utf-8') - request.method = method - return request - - -def make_jwt_request(token, **overrides): - """ - Builds a Request with a JWT body. - """ - body = { - "grant_type": "client_credentials", - "client_assertion_type": "something", - "client_assertion": token, - "scope": "", - } - request = make_request(urllib.parse.urlencode({**body, **overrides}), 'POST') - request.content_type = 'application/x-www-form-urlencoded' - return request - - -def dummy_processor(_xblock): - """ - A dummy LTI parameter processor. - """ - return { - 'custom_author_email': 'author@example.com', - 'custom_author_country': '', - } + def test_get_lti_1p3_context_types_claim_invalid(self, argument): + """ + Test that get_lti_1p3_context_types_claim if any of the context_types are invalid. + """ + with self.assertRaises(ValueError): + get_lti_1p3_context_types_claim(argument) -def defaulting_processor(_xblock): +@ddt.ddt +class TestCacheUtilities(TestCase): """ - A dummy LTI parameter processor with default params. + Tests for the cache utilities in the utils module. """ - -defaulting_processor.lti_xblock_default_params = { - 'custom_name': 'Lex', - 'custom_country': '', -} + @patch('lti_consumer.utils.get_cache_key') + @ddt.data(None, "1") + def test_get_lti_1p3_launch_data_cache_key(self, deep_linking_content_item_id, mock_get_cache_key): + """ + Test that get_lti_1p3_launch_data_cache_key calls the get_cache_key function with the correct arguments. + """ + mock_launch_data = Mock() + mock_launch_data.user_id = "1" + mock_launch_data.resource_link_id = "1" + mock_launch_data.deep_linking_content_item_id = deep_linking_content_item_id + + get_lti_1p3_launch_data_cache_key(mock_launch_data) + + get_cache_key_kwargs = { + "app": "lti", + "key": "launch_data", + "user_id": "1", + "resource_link_id": "1" + } + + if deep_linking_content_item_id: + get_cache_key_kwargs['deep_linking_content_item_id'] = deep_linking_content_item_id + + mock_get_cache_key.assert_called_with( + **get_cache_key_kwargs + ) + + @patch('lti_consumer.utils.TieredCache.set_all_tiers') + @patch('lti_consumer.utils.get_lti_1p3_launch_data_cache_key') + def test_cache_lti_1p3_launch_data(self, mock_get_cache_key, mock_set_all_tiers): + """ + Test that cache_lti_1p3_launch_data caches the launch_data and returns the cache key. + """ + mock_launch_data = Mock() + + mock_get_cache_key.return_value = "launch_data_cache_key" + + cache_lti_1p3_launch_data(mock_launch_data) + + mock_get_cache_key.assert_called_with(mock_launch_data) + mock_set_all_tiers.assert_called_with("launch_data_cache_key", mock_launch_data, django_cache_timeout=600) + + @patch('lti_consumer.utils.TieredCache.get_cached_response') + @ddt.data(True, False) + def test_get_data_from_cache(self, is_found, mock_get_cached_response): + """ + Test that get_data_from_cache returns the data from the cache correctly or returns None if the data + is not in the cache. + """ + mock_cached_data = Mock() + mock_cached_data.value = "value" + mock_cached_data.is_found = is_found + + mock_get_cached_response.return_value = mock_cached_data + + value = get_data_from_cache("key") + + if is_found: + self.assertEqual(value, "value") + else: + self.assertIsNone(value) diff --git a/lti_consumer/utils.py b/lti_consumer/utils.py index c29eb45..eba6abd 100644 --- a/lti_consumer/utils.py +++ b/lti_consumer/utils.py @@ -3,10 +3,13 @@ Utility functions for LTI Consumer block """ import logging from importlib import import_module +from urllib.parse import urlencode from django.conf import settings +from edx_django_utils.cache import get_cache_key, TieredCache from lti_consumer.plugin.compat import get_external_config_waffle_flag, get_database_config_waffle_flag +from lti_consumer.lti_1p3.constants import LTI_1P3_CONTEXT_TYPE log = logging.getLogger(__name__) @@ -30,15 +33,15 @@ def get_lms_base(): return settings.LMS_ROOT_URL -def get_lms_lti_keyset_link(location): +def get_lms_lti_keyset_link(config_id): """ Returns an LMS link to LTI public keyset endpoint - :param location: the location or config_id of the LtiConfiguration object + :param config_id: the config_id of the LtiConfiguration object """ - return "{lms_base}/api/lti_consumer/v1/public_keysets/{location}".format( + return "{lms_base}/api/lti_consumer/v1/public_keysets/{config_id}".format( lms_base=get_lms_base(), - location=str(location), + config_id=str(config_id), ) @@ -53,15 +56,15 @@ def get_lms_lti_launch_link(): ) -def get_lms_lti_access_token_link(location): +def get_lms_lti_access_token_link(config_id): """ Returns an LMS link to LTI Launch endpoint - :param location: the location or config_id of the LtiConfiguration object + :param config_id: the config_id of the LtiConfiguration object """ - return "{lms_base}/api/lti_consumer/v1/token/{location}".format( + return "{lms_base}/api/lti_consumer/v1/token/{config_id}".format( lms_base=get_lms_base(), - location=str(location), + config_id=str(config_id), ) @@ -97,16 +100,26 @@ def get_lti_deeplinking_response_url(lti_config_id): ) -def get_lti_deeplinking_content_url(lti_config_id): +def get_lti_deeplinking_content_url(lti_config_id, launch_data): """ Return the LTI Deep Linking content presentation endpoint :param lti_config_id: LTI configuration id + :param launch_data: (lti_consumer.data.Lti1p3LaunchData): a class containing data necessary for an LTI 1.3 launch """ - return "{lms_base}/api/lti_consumer/v1/lti/{lti_config_id}/lti-dl/content".format( + url = "{lms_base}/api/lti_consumer/v1/lti/{lti_config_id}/lti-dl/content".format( lms_base=get_lms_base(), lti_config_id=str(lti_config_id), ) + url += "?" + + launch_data_key = cache_lti_1p3_launch_data(launch_data) + + url += urlencode({ + "launch_data_key": launch_data_key, + }) + + return url def get_lti_nrps_context_membership_url(lti_config_id): @@ -172,3 +185,82 @@ def database_config_enabled(course_key): return False if it is not enabled. """ return get_database_config_waffle_flag().is_enabled(course_key) + + +def get_lti_1p3_context_types_claim(context_types): + """ + Return the LTI 1.3 context_type claim based on the context_type slug provided as an argument. + + Arguments: + context_type (list): a list of context_types + """ + lti_context_types = [] + + for context_type in context_types: + if context_type == "group": + lti_context_types.append(LTI_1P3_CONTEXT_TYPE.group) + elif context_type == "course_offering": + lti_context_types.append(LTI_1P3_CONTEXT_TYPE.course_offering) + elif context_type == "course_section": + lti_context_types.append(LTI_1P3_CONTEXT_TYPE.course_section) + elif context_type == "course_template": + lti_context_types.append(LTI_1P3_CONTEXT_TYPE.course_template) + else: + raise ValueError("context_type is not a valid type.") + + return lti_context_types + + +def get_lti_1p3_launch_data_cache_key(launch_data): + """ + Return the cache key for the instance of Lti1p3LaunchData. + + Arugments: + launch_data (lti_consumer.data.Lti1p3LaunchData): a class containing data necessary for an LTI 1.3 launch + """ + kwargs = { + "app": "lti", + "key": "launch_data", + "user_id": launch_data.user_id, + "resource_link_id": launch_data.resource_link_id + } + + # If the LTI 1.3 launch is a deep linking launch to a particular content item, then the launch data should be cached + # per content item to properly do an LTI 1.3 deep linking launch. Otherwise, deep linking launches to different + # items will not work via the deep_linking_content_endpoint view, because launch data is cached at the time that the + # preflight URL is generated. + content_item_id = launch_data.deep_linking_content_item_id + if content_item_id: + kwargs["deep_linking_content_item_id"] = content_item_id + + return get_cache_key(**kwargs) + + +def cache_lti_1p3_launch_data(launch_data): + """ + Insert the launch_data into the cache and return the cache key. + + Arguments: + launch_data (lti_consumer.data.Lti1p3LaunchData): a class containing data necessary for an LTI 1.3 launch + """ + launch_data_key = get_lti_1p3_launch_data_cache_key(launch_data) + + # Insert the data into the cache with a 600 second timeout. + TieredCache.set_all_tiers(launch_data_key, launch_data, django_cache_timeout=600) + + return launch_data_key + + +def get_data_from_cache(cache_key): + """ + Return data stored in the cache with the cache key, if it exists. If not, return none. + + Arguments: + cache_key: the key for the data in the cache + """ + cached_data = TieredCache.get_cached_response(cache_key) + + if cached_data.is_found: + return cached_data.value + + return None diff --git a/requirements/base.in b/requirements/base.in index 2cf6882..f946ff4 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -1,6 +1,7 @@ # Core requirements for using this package -c constraints.txt +attrs lxml bleach django diff --git a/requirements/base.txt b/requirements/base.txt index 771f6e8..eaa9a19 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -8,6 +8,8 @@ appdirs==1.4.4 # via fs asgiref==3.5.2 # via django +attrs==22.1.0 + # via -r requirements/base.in bleach==5.0.1 # via -r requirements/base.in certifi==2022.9.24 diff --git a/requirements/ci.txt b/requirements/ci.txt index 4fe8d2a..81bcc0a 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -21,6 +21,8 @@ astroid==2.12.10 # -r requirements/test.txt # pylint # pylint-celery +attrs==22.1.0 + # via -r requirements/test.txt binaryornot==0.4.4 # via # -r requirements/test.txt @@ -49,7 +51,6 @@ certifi==2022.9.24 cffi==1.15.1 # via # -r requirements/test.txt - # cryptography # pynacl chardet==5.0.0 # via @@ -189,11 +190,6 @@ jaraco-classes==3.2.3 # via # -r requirements/test.txt # keyring -jeepney==0.8.0 - # via - # -r requirements/test.txt - # keyring - # secretstorage jinja2==3.1.2 # via # -r requirements/test.txt @@ -391,10 +387,6 @@ s3transfer==0.6.0 # via # -r requirements/test.txt # boto3 -secretstorage==3.3.3 - # via - # -r requirements/test.txt - # keyring simplejson==3.17.6 # via # -r requirements/test.txt diff --git a/requirements/dev.txt b/requirements/dev.txt index e89ce09..772c680 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -12,6 +12,8 @@ asgiref==3.5.2 # via # -r requirements/base.txt # django +attrs==22.1.0 + # via -r requirements/base.txt bleach==5.0.1 # via -r requirements/base.txt certifi==2022.9.24 diff --git a/requirements/quality.txt b/requirements/quality.txt index 2125e0a..08d7165 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -18,6 +18,8 @@ astroid==2.12.10 # via # pylint # pylint-celery +attrs==22.1.0 + # via -r requirements/base.txt binaryornot==0.4.4 # via cookiecutter bleach==5.0.1 diff --git a/requirements/test.txt b/requirements/test.txt index 392b3cf..4252b81 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -18,6 +18,8 @@ astroid==2.12.10 # via # pylint # pylint-celery +attrs==22.1.0 + # via -r requirements/base.txt binaryornot==0.4.4 # via cookiecutter bleach==5.0.1 @@ -39,7 +41,6 @@ certifi==2022.9.24 cffi==1.15.1 # via # -r requirements/base.txt - # cryptography # pynacl chardet==5.0.0 # via binaryornot @@ -144,10 +145,6 @@ isort==5.10.1 # via pylint jaraco-classes==3.2.3 # via keyring -jeepney==0.8.0 - # via - # keyring - # secretstorage jinja2==3.1.2 # via # code-annotations @@ -293,8 +290,6 @@ rich==12.5.1 # via twine s3transfer==0.6.0 # via boto3 -secretstorage==3.3.3 - # via keyring simplejson==3.17.6 # via # -r requirements/base.txt -- GitLab