diff --git a/docs/decisions/0008-lti-1p3-role-mapping-update.rst b/docs/decisions/0008-lti-1p3-role-mapping-update.rst new file mode 100644 index 00000000..cdfcc601 --- /dev/null +++ b/docs/decisions/0008-lti-1p3-role-mapping-update.rst @@ -0,0 +1,166 @@ +LTI 1.3 roles mapping update +---------------------------- + +Status +====== + +Provisional + +Context +======= + +Open edX LTI 1.3 launch code historically mapped course roles to institution and system role URIs. +This caused interoperability problems with tools that expect context role URIs in LTI launch and NRPS +membership responses. + +LTI 1.3 roles claim allows role URIs from published LIS vocabularies, including: + +* system roles +* institution roles +* context roles + +In practice, LTI launches in Open edX happen in course context, so context roles are most relevant. +Open edX also has more explicit role values than ``staff``, ``instructor``, ``student``, and ``guest``. +These include course roles such as ``limited_staff``, ``finance_admin``, ``sales_admin``, +``beta_testers``, ``library_user``, ``ccx_coach``, and ``data_researcher``. +Open edX also defines discussion roles such as ``Administrator``, ``Moderator``, +``Group Moderator``, ``Community TA``, and ``Student``. + +This ADR records updated mapping used for: + +* LTI 1.3 launch roles claim +* LTI NRPS membership container member roles + +This ADR supersedes Roles section from ``0002-lti-1p3-variables.rst``. + +Decisions +========= + +Launch roles claim +~~~~~~~~~~~~~~~~~~ + +For LTI launches, Open edX includes context role URIs plus neutral system and institution role URIs: + +* ``http://purl.imsglobal.org/vocab/lis/v2/system/person#None`` +* ``http://purl.imsglobal.org/vocab/lis/v2/institution/person#None`` + +Context role mapping is shown below. + +.. list-table:: + :widths: auto + :header-rows: 1 + + * - Open edX role + - LTI 1.3 roles included + - Reasoning + * - instructor + - ``system/person#None`` + ``institution/person#None`` + ``membership#Administrator`` + ``membership#Instructor`` + - Course admin role in Open edX maps to highest course-context privilege. + * - staff + - ``system/person#None`` + ``institution/person#None`` + ``membership#Instructor`` + - Course staff should have instructor-level context access, but not course admin role. + * - limited_staff + - ``system/person#None`` + ``institution/person#None`` + ``membership#Instructor`` + - Limited staff derives from staff and should expose same course-context role to tools. + * - student + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - Standard learner mapping. + * - guest + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - Guest launch paths use learner-compatible mapping for interoperability. + * - finance_admin + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - No stronger LTI-specific course-context privilege required. + * - sales_admin + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - No stronger LTI-specific course-context privilege required. + * - beta_testers + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - No stronger LTI-specific course-context privilege required. + * - library_user + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - No stronger LTI-specific course-context privilege required. + * - ccx_coach + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - No stronger LTI-specific course-context privilege required. + * - data_researcher + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - No stronger LTI-specific course-context privilege required. + * - org_course_creator_group + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - Org-scoped role does not imply elevated privilege in specific course launch context. + * - course_creator_group + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - Platform-wide role does not imply elevated privilege in specific course launch context. + * - support + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - Support role should not expose elevated course-context privilege to tools by default. + * - Administrator, Moderator, Student + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + - Discussion roles do not map cleanly to elevated LTI course-context privilege, so default to learner. + * - Group Moderator, Community TA + - ``system/person#None`` + ``institution/person#None`` + ``membership#Learner`` + ``membership#TeachingAssistant`` + - These discussion roles align with teaching-assistant style participation in course context. + +NRPS membership roles +~~~~~~~~~~~~~~~~~~~~~ + +For NRPS membership responses, Open edX includes context roles only. + +.. list-table:: + :widths: auto + :header-rows: 1 + + * - Open edX role + - NRPS roles included + * - instructor + - ``membership#Administrator`` ``membership#Instructor`` + * - staff + - ``membership#Instructor`` + * - limited_staff + - ``membership#Instructor`` + * - Group Moderator, Community TA + - ``membership#Learner`` ``membership#TeachingAssistant`` + * - all other explicitly mapped roles + - ``membership#Learner`` + +Consequences +============ + +* Tool compatibility improves because context role URIs are now present in launch and NRPS flows. +* Open edX role handling becomes explicit for current known course and org role values. +* Older documentation in ``0002-lti-1p3-variables.rst`` remains historical and should not be treated as current source of truth for roles mapping. diff --git a/lti_consumer/data.py b/lti_consumer/data.py index 0c92f61c..e2bfc126 100644 --- a/lti_consumer/data.py +++ b/lti_consumer/data.py @@ -52,8 +52,9 @@ class Lti1p3LaunchData: attribute external_user_id is provided, user_id will only be used internally and will not be shared externally. If external_user_id is not provided, user_id will be shared externally, and then it must be stable to the issuer. - * user_role (required): The user's role as one of the keys in LTI_1P3_ROLE_MAP: staff, instructor, student, or - guest. It can also be None if the empty list should be sent in the LTI launch message. + * user_role (required): The user's role as one of the keys in LTI_1P3_ROLE_MAP, including Open edX course and + org role names explicitly supported by this package. It can also be None if the empty list should be sent in + the LTI launch message. * 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. diff --git a/lti_consumer/lti_1p1/exceptions.py b/lti_consumer/lti_1p1/exceptions.py index 80383ae8..748cfe49 100644 --- a/lti_consumer/lti_1p1/exceptions.py +++ b/lti_consumer/lti_1p1/exceptions.py @@ -1,9 +1,10 @@ """ Exceptions for the LTI1.1 Consumer. """ +from lti_consumer.exceptions import LtiError -class Lti1p1Error(Exception): +class Lti1p1Error(LtiError): """ General error class for LTI1.1 Consumer usage. """ diff --git a/lti_consumer/lti_1p3/constants.py b/lti_consumer/lti_1p3/constants.py index 3d992c2f..6ecf523f 100644 --- a/lti_consumer/lti_1p3/constants.py +++ b/lti_consumer/lti_1p3/constants.py @@ -7,7 +7,6 @@ """ from enum import Enum - LTI_BASE_MESSAGE = { # Claim type: fixed key with value `LtiResourceLinkRequest` # http://www.imsglobal.org/spec/lti/v1p3/#message-type-claim @@ -18,34 +17,74 @@ "https://purl.imsglobal.org/spec/lti/claim/version": "1.3.0", } +# Role mappings +# Values follow proposed mapping in role matrix. +LTI_1P3_ROLE_NONE = [ + 'http://purl.imsglobal.org/vocab/lis/v2/system/person#None', + 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#None', +] + +LTI_1P3_CONTEXT_ROLE_INSTRUCTOR = [ + 'http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor', +] + +LTI_1P3_CONTEXT_ROLE_ADMINISTRATOR = [ + 'http://purl.imsglobal.org/vocab/lis/v2/membership#Administrator', + 'http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor', +] + +LTI_1P3_CONTEXT_ROLE_LEARNER = [ + 'http://purl.imsglobal.org/vocab/lis/v2/membership#Learner', +] + +LTI_1P3_CONTEXT_ROLE_TEACHING_ASSISTANT = [ + 'http://purl.imsglobal.org/vocab/lis/v2/membership/Instructor#TeachingAssistant', +] + LTI_1P3_ROLE_MAP = { - 'staff': [ - 'http://purl.imsglobal.org/vocab/lis/v2/system/person#Administrator', - 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Instructor', - ], - 'instructor': [ - 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Instructor', - ], - 'student': [ - 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Student' - ], - 'guest': [ - 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Student' - ], + 'staff': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_INSTRUCTOR, + 'limited_staff': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_INSTRUCTOR, + 'instructor': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_ADMINISTRATOR, + 'student': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'guest': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'finance_admin': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'sales_admin': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'beta_testers': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'library_user': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'ccx_coach': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'data_researcher': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'org_course_creator_group': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'course_creator_group': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'support': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'Administrator': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'Moderator': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, + 'Group Moderator': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER + LTI_1P3_CONTEXT_ROLE_TEACHING_ASSISTANT, + 'Community TA': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER + LTI_1P3_CONTEXT_ROLE_TEACHING_ASSISTANT, + 'Student': LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER, } -# Context membership roles +# Context membership roles (kept for callers using context map directly) # https://www.imsglobal.org/spec/lti/v1p3/#lis-vocabulary-for-context-roles LTI_1P3_CONTEXT_ROLE_MAP = { - 'staff': [ - 'http://purl.imsglobal.org/vocab/lis/v2/membership#Administrator', - ], - 'instructor': [ - 'http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor', - ], - 'student': [ - 'http://purl.imsglobal.org/vocab/lis/v2/membership#Learner', - ], + 'staff': LTI_1P3_CONTEXT_ROLE_INSTRUCTOR, + 'limited_staff': LTI_1P3_CONTEXT_ROLE_INSTRUCTOR, + 'instructor': LTI_1P3_CONTEXT_ROLE_ADMINISTRATOR, + 'student': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'guest': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'finance_admin': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'sales_admin': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'beta_testers': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'library_user': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'ccx_coach': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'data_researcher': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'org_course_creator_group': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'course_creator_group': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'support': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'Administrator': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'Moderator': LTI_1P3_CONTEXT_ROLE_LEARNER, + 'Group Moderator': LTI_1P3_CONTEXT_ROLE_LEARNER + LTI_1P3_CONTEXT_ROLE_TEACHING_ASSISTANT, + 'Community TA': LTI_1P3_CONTEXT_ROLE_LEARNER + LTI_1P3_CONTEXT_ROLE_TEACHING_ASSISTANT, + 'Student': LTI_1P3_CONTEXT_ROLE_LEARNER, } LTI_1P3_ACCESS_TOKEN_REQUIRED_CLAIMS = { diff --git a/lti_consumer/lti_1p3/consumer.py b/lti_consumer/lti_1p3/consumer.py index 56984ad0..4c60ec37 100644 --- a/lti_consumer/lti_1p3/consumer.py +++ b/lti_consumer/lti_1p3/consumer.py @@ -77,6 +77,15 @@ def __init__( # Extra claims - used by LTI Advantage self.extra_claims = {} + def _get_target_link_uri(self, launch_data): # pylint: disable=unused-argument + """ + Return the target_link_uri to use for the provided launch data. + + Subclasses can override this to customize the target link selection for + different message types. + """ + return self.launch_url + @staticmethod def _get_user_roles(role): """ @@ -85,10 +94,11 @@ def _get_user_roles(role): Used in roles claim: should return array of URI values for roles that the user has within the message's context. - Supported roles: - * Core - Administrator - * Institution - Instructor (non-core role) - * Institution - Student + Supported mappings: + * instructor -> membership Administrator + Instructor, system None, institution None + * staff/limited_staff -> membership Instructor, system None, institution None + * student/guest, discussion roles, and other explicit Open edX roles -> membership Learner, + system None, institution None Reference: http://www.imsglobal.org/spec/lti/v1p3/#roles-claim Role vocabularies: http://www.imsglobal.org/spec/lti/v1p3/#role-vocabularies @@ -125,12 +135,14 @@ def prepare_preflight_url( oidc_url = self.oidc_url + "?" + target_link_uri = self._get_target_link_uri(launch_data) + login_hint = user_id parameters = { "iss": self.iss, "client_id": self.client_id, "lti_deployment_id": self.deployment_id, - "target_link_uri": self.launch_url, + "target_link_uri": target_link_uri, "login_hint": login_hint, "lti_message_hint": launch_data_key, } @@ -302,6 +314,7 @@ def set_custom_parameters( def get_lti_launch_message( self, include_extra_claims=True, + target_link_uri=None, ): """ Build LTI message from class parameters @@ -312,6 +325,8 @@ def get_lti_launch_message( # Start from base message lti_message = LTI_BASE_MESSAGE.copy() + target_link_uri = target_link_uri or self.launch_url + # Add base parameters lti_message.update({ # Issuer @@ -329,7 +344,7 @@ def get_lti_launch_message( # Target Link URI: actual endpoint for the LTI resource to display # MUST be the same value as the target_link_uri passed by the platform in the OIDC login request # http://www.imsglobal.org/spec/lti/v1p3/#target-link-uri - "https://purl.imsglobal.org/spec/lti/claim/target_link_uri": self.launch_url, + "https://purl.imsglobal.org/spec/lti/claim/target_link_uri": target_link_uri, }) # Check if user data is set, then append it to lti message @@ -609,6 +624,19 @@ def lti_dl_enabled(self): else: return False + def _get_target_link_uri(self, launch_data): + """ + Use the deep linking launch URL for deep linking requests. + """ + if ( + getattr(self, 'dl', None) and + launch_data and + getattr(launch_data, 'message_type', None) == 'LtiDeepLinkingRequest' + ): + return self.dl.deep_linking_launch_url + + return super()._get_target_link_uri(launch_data) + def enable_ags( self, lineitems_url, @@ -663,12 +691,14 @@ def generate_launch_request( # Check if Deep Linking is enabled and that this is a Deep Link Launch if self.dl and launch_data.message_type == "LtiDeepLinkingRequest": + target_link_uri = self._get_target_link_uri(launch_data) # Validate preflight response self._validate_preflight_response(preflight_response) # Get LTI Launch Message lti_launch_message = self.get_lti_launch_message( include_extra_claims=False, + target_link_uri=target_link_uri, ) # Update message type to LtiDeepLinkingRequest, diff --git a/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py b/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py index 953cee40..c1d48aaa 100644 --- a/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py +++ b/lti_consumer/lti_1p3/extensions/rest_framework/serializers.py @@ -125,7 +125,7 @@ class LtiAgsScoreSerializer(serializers.ModelSerializer): timestamp = serializers.DateTimeField(input_formats=[ISO_8601], format=ISO_8601, default_timezone=timezone.utc) scoreGiven = serializers.FloatField(source='score_given', required=False, allow_null=True, default=None) scoreMaximum = serializers.FloatField(source='score_maximum', required=False, allow_null=True, default=None) - comment = serializers.CharField(required=False, allow_null=True) + comment = serializers.CharField(required=False, allow_null=True, allow_blank=True) activityProgress = serializers.CharField(source='activity_progress') gradingProgress = serializers.CharField(source='grading_progress') userId = serializers.CharField(source='user_id') @@ -193,14 +193,19 @@ class LtiAgsResultSerializer(serializers.ModelSerializer): comment = serializers.CharField() def get_id(self, obj): + """ + Return result URL for score. Include user_id when score scoped to learner. + """ request = self.context.get('request') + kwargs = { + 'lti_config_id': obj.line_item.lti_configuration.id, + 'pk': obj.line_item.pk, + } + if obj.user_id: + kwargs['user_id'] = obj.user_id return reverse( 'lti_consumer:lti-ags-view-results', - kwargs={ - 'lti_config_id': obj.line_item.lti_configuration.id, - 'pk': obj.line_item.pk, - 'user_id': obj.user_id, - }, + kwargs=kwargs, request=request, ) diff --git a/lti_consumer/lti_1p3/tests/test_consumer.py b/lti_consumer/lti_1p3/tests/test_consumer.py index 5f2eeb25..7969de15 100644 --- a/lti_consumer/lti_1p3/tests/test_consumer.py +++ b/lti_consumer/lti_1p3/tests/test_consumer.py @@ -2,33 +2,41 @@ Unit tests for LTI 1.3 consumer implementation """ +import uuid from unittest.mock import patch from urllib.parse import parse_qs, urlparse -import uuid import ddt import jwt from Cryptodome.PublicKey import RSA from django.conf import settings from django.test.testcases import TestCase -from edx_django_utils.cache import get_cache_key, TieredCache +from edx_django_utils.cache import TieredCache, get_cache_key from jwt.api_jwk import PyJWKSet 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 -from lti_consumer.lti_1p3.nprs import LtiNrps -from lti_consumer.lti_1p3.constants import LTI_1P3_CONTEXT_TYPE, LTI_PROCTORING_DATA_KEYS +from lti_consumer.lti_1p3.constants import ( + LTI_1P3_CONTEXT_ROLE_ADMINISTRATOR, + LTI_1P3_CONTEXT_ROLE_INSTRUCTOR, + LTI_1P3_CONTEXT_ROLE_LEARNER, + LTI_1P3_CONTEXT_ROLE_TEACHING_ASSISTANT, + LTI_1P3_CONTEXT_TYPE, + LTI_1P3_ROLE_NONE, + LTI_PROCTORING_DATA_KEYS, +) from lti_consumer.lti_1p3.consumer import LtiAdvantageConsumer, LtiConsumer1p3, LtiProctoringConsumer +from lti_consumer.lti_1p3.deep_linking import LtiDeepLinking from lti_consumer.lti_1p3.exceptions import InvalidClaimValue, MissingRequiredClaim - +from lti_consumer.lti_1p3.nprs import LtiNrps # Variables required for testing and verification ISS = "http://test-platform.example/" OIDC_URL = "http://test-platform/oidc" LAUNCH_URL = "http://test-platform/launch" -REDIRECT_URIS = [LAUNCH_URL] +DEEP_LINK_LAUNCH_URL = "http://test-platform/deep_link_launch" +REDIRECT_URIS = [LAUNCH_URL, DEEP_LINK_LAUNCH_URL] CLIENT_ID = "1" DEPLOYMENT_ID = "1" NONCE = "1234" @@ -153,17 +161,25 @@ def test_preflight_validation(self, preflight_response, success): return self.lti_consumer._validate_preflight_response(preflight_response) # pylint: disable=protected-access @ddt.data( - ( - 'student', - ['http://purl.imsglobal.org/vocab/lis/v2/institution/person#Student'] - ), - ( - 'staff', - [ - 'http://purl.imsglobal.org/vocab/lis/v2/system/person#Administrator', - 'http://purl.imsglobal.org/vocab/lis/v2/institution/person#Instructor', - ] - ) + ('student', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('staff', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_INSTRUCTOR), + ('instructor', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_ADMINISTRATOR), + ('guest', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('limited_staff', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_INSTRUCTOR), + ('finance_admin', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('sales_admin', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('beta_testers', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('library_user', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('ccx_coach', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('data_researcher', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('org_course_creator_group', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('course_creator_group', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('support', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('Administrator', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('Moderator', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), + ('Group Moderator', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER + LTI_1P3_CONTEXT_ROLE_TEACHING_ASSISTANT), + ('Community TA', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER + LTI_1P3_CONTEXT_ROLE_TEACHING_ASSISTANT), + ('Student', LTI_1P3_ROLE_NONE + LTI_1P3_CONTEXT_ROLE_LEARNER), ) @ddt.unpack def test_get_user_roles(self, role, expected_output): @@ -237,7 +253,9 @@ def test_prepare_preflight_url(self): { "sub": "1", "https://purl.imsglobal.org/spec/lti/claim/roles": [ - "http://purl.imsglobal.org/vocab/lis/v2/institution/person#Student" + "http://purl.imsglobal.org/vocab/lis/v2/system/person#None", + "http://purl.imsglobal.org/vocab/lis/v2/institution/person#None", + "http://purl.imsglobal.org/vocab/lis/v2/membership#Learner", ] } ), @@ -266,10 +284,14 @@ def test_set_user_data(self, data, expected_output): Check if setting user data works """ self.lti_consumer.set_user_data(**data) - self.assertEqual( - self.lti_consumer.lti_claim_user_data, - expected_output - ) + + actual_output = self.lti_consumer.lti_claim_user_data.copy() + expected_output = expected_output.copy() + actual_roles = actual_output.pop("https://purl.imsglobal.org/spec/lti/claim/roles") + expected_roles = expected_output.pop("https://purl.imsglobal.org/spec/lti/claim/roles") + + self.assertEqual(actual_output, expected_output) + self.assertCountEqual(actual_roles, expected_roles) def test_check_no_user_data_error(self): """ @@ -739,14 +761,14 @@ def _setup_deep_linking(self): """ Set's up deep linking class in LTI consumer. """ - self.lti_consumer.enable_deep_linking("launch-url", "return-url") + self.lti_consumer.enable_deep_linking(DEEP_LINK_LAUNCH_URL, "return-url") lti_message_hint = self._setup_lti_message_hint() # Set LTI Consumer parameters self.preflight_response = { "client_id": CLIENT_ID, - "redirect_uri": LAUNCH_URL, + "redirect_uri": DEEP_LINK_LAUNCH_URL, "nonce": NONCE, "state": STATE, "lti_message_hint": lti_message_hint, @@ -819,6 +841,40 @@ def test_deep_linking_enabled_launch_request(self): "return-url" ) + def test_deep_linking_preflight_uses_deep_link_launch_url(self): + """ + Ensure deep linking launches send the deep linking launch URL as target_link_uri during login initiation. + """ + self.lti_consumer.enable_deep_linking(DEEP_LINK_LAUNCH_URL, "return-url") + launch_data = Lti1p3LaunchData( + user_id="1", + user_role="student", + config_id="1", + resource_link_id="resource_link_id", + message_type="LtiDeepLinkingRequest", + ) + + preflight_request_data = self.lti_consumer.prepare_preflight_url(launch_data) + parameters = parse_qs(urlparse(preflight_request_data).query) + + self.assertEqual(parameters['target_link_uri'][0], DEEP_LINK_LAUNCH_URL) + + def test_deep_linking_launch_request_sets_target_link_uri(self): + """ + Ensure the ID token for deep linking launches uses the deep linking launch URL as target_link_uri. + """ + self._setup_deep_linking() + + token = self.lti_consumer.generate_launch_request( + self.preflight_response, + )['id_token'] + + decoded_token = self.lti_consumer.key_handler.validate_and_decode(token) + self.assertEqual( + decoded_token['https://purl.imsglobal.org/spec/lti/claim/target_link_uri'], + DEEP_LINK_LAUNCH_URL, + ) + def test_deep_linking_token_decode_no_dl(self): """ Check that trying to run the Deep Linking decoding fails if service is not set up. diff --git a/lti_consumer/lti_xblock.py b/lti_consumer/lti_xblock.py index 1adfbc34..c8ebff7d 100644 --- a/lti_consumer/lti_xblock.py +++ b/lti_consumer/lti_xblock.py @@ -939,6 +939,27 @@ def external_user_id(self): raise LtiError(self.ugettext("Could not get user id for current request")) return str(user_id) + def _get_lti_1p3_user_role(self): + """ + Return effective LTI 1.3 user role, including supported forum roles. + """ + role = self.role + # Keep privileged course roles unchanged. + # `staff`, `instructor`, and `limited_staff` already map to stronger LTI roles + # than forum roles like `Community TA` or `Group Moderator`, so forum role + # should only override learner-like base roles. + if role in {'staff', 'instructor', 'limited_staff'}: + return role + + forum_role = compat.get_user_course_forum_role( + self.lms_user_id, + self.scope_ids.usage_id.context_key, + ) + if forum_role in {'Community TA', 'Group Moderator'}: + return forum_role + + return role + def get_lti_1p1_user_id(self): """ Returns the user ID to send to an LTI tool during an LTI 1.1/2.0 launch. If the @@ -1109,7 +1130,7 @@ def is_past_due(self): close_date = due_date return close_date is not None and timezone.now() > close_date - def _get_lti_consumer(self): + def get_lti_consumer(self): """ Returns a preconfigured LTI consumer depending on the value. @@ -1282,7 +1303,7 @@ def lti_launch_handler(self, request, suffix=''): # pylint: disable=unused-argu Returns: webob.response: HTML LTI launch form """ - lti_consumer = self._get_lti_consumer() + lti_consumer = self.get_lti_consumer() # Occassionally, users try to do an LTI launch while they are unauthenticated. It is not known why this occurs. # Sometimes, it is due to a web crawlers; other times, it is due to actual users of the platform. Regardless, @@ -1385,7 +1406,7 @@ def lti_1p3_access_token(self, request, suffix=''): # pylint: disable=unused-ar # Asserting that the consumer can be created. This makes sure that the LtiConfiguration # object exists before calling the Django View - assert self._get_lti_consumer() + assert self.get_lti_consumer() # Runtime import because this can only be run in the LMS/Studio Django # environments. Importing the views on the top level will cause RuntimeErorr from lti_consumer.plugin.views import access_token_endpoint # pylint: disable=import-outside-toplevel @@ -1441,12 +1462,11 @@ def result_service_handler(self, request, suffix=''): Returns: webob.response: response to this request. See above for details. """ - lti_consumer = self._get_lti_consumer() + lti_consumer = self.get_lti_consumer() lti_consumer.set_outcome_service_url(self.outcome_service_url) if settings.DEBUG: - lti_provider_key, lti_provider_secret = self.lti_provider_key_secret - log_authorization_header(request, lti_provider_key, lti_provider_secret) + log_authorization_header(request, lti_consumer.oauth_key, lti_consumer.oauth_secret) if not self.accept_grades_past_due and self.is_past_due: return Response(status=404) # have to do 404 due to spec, but 400 is better, with error msg in body @@ -1679,7 +1699,7 @@ def get_lti_1p3_launch_data(self): launch_data = Lti1p3LaunchData( user_id=self.lms_user_id, - user_role=self.role, + user_role=self._get_lti_1p3_user_role(), config_id=config_id, resource_link_id=self.resource_link_id, external_user_id=self.external_user_id, @@ -1758,7 +1778,7 @@ def _get_context_for_template(self): # Don't pull from the Django database unless the config_type is one that stores the LTI configuration in the # database. if self.config_type in ("database", "external"): - lti_consumer = self._get_lti_consumer() + lti_consumer = self.get_lti_consumer() launch_url = self._get_lti_launch_url(lti_consumer) lti_block_launch_handler = self._get_lti_block_launch_handler() diff --git a/lti_consumer/outcomes.py b/lti_consumer/outcomes.py index 72bc759c..7de7587b 100644 --- a/lti_consumer/outcomes.py +++ b/lti_consumer/outcomes.py @@ -6,10 +6,12 @@ """ import logging -from xml.sax.saxutils import escape from urllib.parse import unquote +from xml.sax.saxutils import escape +from django.conf import settings from lxml import etree + try: from xblock.utils.resources import ResourceLoader except ModuleNotFoundError: # For backward compatibility with releases older than Quince. @@ -176,7 +178,13 @@ def handle_request(self, request): return response_xml_template.format(**failure_values) # Verify OAuth signing. - __, secret = self.xblock.lti_provider_key_secret + secret = self.xblock.get_lti_consumer().oauth_secret + if settings.DEBUG: + log.debug( + "[LTI]: verifying OAuth body signature for outcomes. service_url=%s request.url=%s", + self.xblock.outcome_service_url, + request.url, + ) try: verify_oauth_body_signature(request, secret, self.xblock.outcome_service_url) except (ValueError, LtiError) as ex: diff --git a/lti_consumer/plugin/compat.py b/lti_consumer/plugin/compat.py index 922659b3..c7d13ab0 100644 --- a/lti_consumer/plugin/compat.py +++ b/lti_consumer/plugin/compat.py @@ -345,6 +345,47 @@ def get_event_tracker(): # pragma: nocover return None +def get_user_course_forum_role(user_id, course_id): # pragma: nocover + """ + Return exact forum access role for user in course when it maps to LTI 1.3 forum roles. + """ + target_roles = {'Community TA', 'Group Moderator'} + + try: + # pylint: disable=import-outside-toplevel + from django.contrib.auth import get_user_model + from lms.djangoapps.discussion.django_comment_client.utils import get_user_role_names + + user = get_user_model().objects.get(id=user_id) + forum_roles = set(get_user_role_names(user, course_id)) + if 'Group Moderator' in forum_roles: + return 'Group Moderator' + if 'Community TA' in forum_roles: + return 'Community TA' + return None + except ImportError: + try: + # pylint: disable=import-outside-toplevel + from openedx.core.djangoapps.django_comment_common.models import Role + + user_forum_roles = set( + Role.objects.filter( + users__id=user_id, + course_id=course_id, + name__in=target_roles, + ).values_list('name', flat=True) + ) + + if 'Group Moderator' in user_forum_roles: + return 'Group Moderator' + if 'Community TA' in user_forum_roles: + return 'Community TA' + except ImportError: + return None + + return None + + def nrps_pii_disallowed(): """ Check if platform disallows sharing pii over NRPS diff --git a/lti_consumer/plugin/views.py b/lti_consumer/plugin/views.py index 13e8a019..1c7f3b3e 100644 --- a/lti_consumer/plugin/views.py +++ b/lti_consumer/plugin/views.py @@ -748,7 +748,7 @@ def perform_create(self, serializer): @action( detail=True, methods=['GET'], - url_path='results/(?P[^/.]+)?', + url_path=r'results(?:/(?P[^/.]+))?/?', renderer_classes=[LineItemResultsRenderer], content_negotiation_class=IgnoreContentNegotiation, ) 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 08e4acf8..8bb376ca 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_ags.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_ags.py @@ -177,10 +177,7 @@ def test_lti_ags_list(self): response.data, [ { - 'id': 'http://testserver/lti_consumer/v1/lti/{}/lti-ags/{}'.format( - self.lti_config.id, - line_item.id - ), + 'id': f'http://testserver/lti_consumer/v1/lti/{self.lti_config.id}/lti-ags/{line_item.id}', 'resourceId': 'test', 'scoreMaximum': 100, 'label': 'test label', @@ -221,10 +218,7 @@ def test_lti_ags_retrieve(self): self.assertEqual( response.data, { - 'id': 'http://testserver/lti_consumer/v1/lti/{}/lti-ags/{}'.format( - self.lti_config.id, - line_item.id - ), + 'id': f'http://testserver/lti_consumer/v1/lti/{self.lti_config.id}/lti-ags/{line_item.id}', 'resourceId': 'test', 'scoreMaximum': 100, 'label': 'test label', @@ -401,6 +395,43 @@ def test_create_score(self): self.assertEqual(score.grading_progress, LtiAgsScore.FULLY_GRADED) self.assertEqual(score.user_id, self.primary_user_id) + @ddt.data(None, "") + def test_create_score_without_comment(self, comment): + """ + Test the LTI AGS LineItem Score Creation when comment is omitted or blank. + """ + self._set_lti_token('https://purl.imsglobal.org/spec/lti-ags/scope/score') + + data = { + "timestamp": self.early_timestamp, + "scoreGiven": 83, + "scoreMaximum": 100, + "activityProgress": LtiAgsScore.COMPLETED, + "gradingProgress": LtiAgsScore.FULLY_GRADED, + "userId": self.primary_user_id, + } + if comment is not None: + data["comment"] = comment + + response = self.client.post( + self.scores_endpoint, + data=json.dumps(data), + content_type="application/vnd.ims.lis.v1.score+json", + ) + + self.assertEqual(response.status_code, 201) + self.assertEqual(LtiAgsScore.objects.filter( + line_item=self.line_item, + user_id=self.primary_user_id + ).count(), 1) + + score = LtiAgsScore.objects.get(line_item=self.line_item, user_id=self.primary_user_id) + if comment is None: + self.assertIsNone(score.comment) + else: + self.assertEqual(score.comment, comment) + score.delete() + def _post_lti_score(self, override_data=None): """ Helper method to post a LTI score @@ -936,11 +967,14 @@ def test_retrieve_results(self): # Create Score response = self.client.get(self.results_endpoint) + response_with_trailing_slash = self.client.get(self.results_endpoint + '/') self.assertEqual(response.status_code, 200) + self.assertEqual(response_with_trailing_slash.status_code, 200) # There should be 2 results (not include the empty score user's result) self.assertEqual(len(response.data), 2) + self.assertEqual(len(response_with_trailing_slash.data), 2) # Check the data primary_user_results_endpoint = reverse( @@ -983,7 +1017,7 @@ def test_retrieve_results(self): def test_retrieve_results_for_user_id(self): """ - Test the LTI AGS LineItem Resul Retrieval for a single user. + Test the LTI AGS LineItem Result Retrieval for a single user. """ self._set_lti_token('https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly') @@ -996,14 +1030,23 @@ def test_retrieve_results_for_user_id(self): } ) + results_user_endpoint_with_trailing_slash = results_user_endpoint + '/' + # Request results with userId response = self.client.get(results_user_endpoint, data={"userId": self.secondary_user_id}) + response_with_trailing_slash = self.client.get( + results_user_endpoint_with_trailing_slash, + data={"userId": self.secondary_user_id}, + ) self.assertEqual(response.status_code, 200) + self.assertEqual(response_with_trailing_slash.status_code, 200) # There should be 1 result for that user self.assertEqual(len(response.data), 1) + self.assertEqual(len(response_with_trailing_slash.data), 1) self.assertEqual(response.data[0]['userId'], self.secondary_user_id) + self.assertEqual(response_with_trailing_slash.data[0]['userId'], self.secondary_user_id) def test_retrieve_results_with_limit(self): """ @@ -1023,3 +1066,30 @@ def test_retrieve_results_with_limit(self): # `primary_user_id` was assigned to the record with the `late_timestamp` self.assertEqual(len(response.data), 1) self.assertEqual(response.data[0]['userId'], self.primary_user_id) + + def test_results_serializer_id_includes_user_id_separator(self): + """ + Test that the results serializer builds a valid URL for a user-specific result. + """ + self._set_lti_token('https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly') + + results_user_endpoint = reverse( + 'lti_consumer:lti-ags-view-results', + kwargs={ + "lti_config_id": self.lti_config.id, + "pk": self.line_item.id, + "user_id": self.secondary_user_id, + } + ) + + response = self.client.get(results_user_endpoint, data={"userId": self.secondary_user_id}) + + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), 1) + self.assertEqual( + response.data[0]['id'], + ( + f'http://testserver/lti_consumer/v1/lti/{self.lti_config.id}/lti-ags' + f'/{self.line_item.id}/results/{self.secondary_user_id}' + ), + ) diff --git a/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py b/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py index 2b0c3509..bf343956 100644 --- a/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py +++ b/lti_consumer/tests/unit/plugin/test_views_lti_nrps.py @@ -8,6 +8,11 @@ from rest_framework.test import APITransactionTestCase from lti_consumer.exceptions import LtiError +from lti_consumer.lti_1p3.constants import ( + LTI_1P3_CONTEXT_ROLE_ADMINISTRATOR, + LTI_1P3_CONTEXT_ROLE_INSTRUCTOR, + LTI_1P3_CONTEXT_ROLE_LEARNER, +) from lti_consumer.lti_xblock import LtiConsumerXBlock from lti_consumer.models import LtiConfiguration from lti_consumer.tests.test_utils import TestBaseWithPatch, make_xblock @@ -283,6 +288,40 @@ def test_get_with_pii(self, expose_pii_fields_patcher): self.assertIn('email', member_fields) self.assertIn('name', member_fields) + @patch('lti_consumer.plugin.views.get_lti_pii_sharing_state_for_course', return_value=False) + @patch( + 'lti_consumer.plugin.views.compat.get_course_members', + Mock(side_effect=patch_get_memberships({ + 'student': 1, + 'instructor': 1, + 'staff': 1, + })), + ) + def test_get_membership_roles_mapping(self, expose_pii_fields_patcher): + """ + Test context membership endpoint returns mapped LTI context role URIs. + """ + self._set_lti_token('https://purl.imsglobal.org/spec/lti-nrps/scope/contextmembership.readonly') + response = self.client.get(self.context_membership_endpoint) + + expose_pii_fields_patcher.assert_called() + self.assertEqual(len(response.data['members']), 3) + + actual_roles = [set(member['roles']) for member in response.data['members']] + + self.assertIn( + set(LTI_1P3_CONTEXT_ROLE_LEARNER), + actual_roles, + ) + self.assertIn( + set(LTI_1P3_CONTEXT_ROLE_INSTRUCTOR), + actual_roles, + ) + self.assertIn( + set(LTI_1P3_CONTEXT_ROLE_ADMINISTRATOR), + actual_roles, + ) + @patch('lti_consumer.plugin.views.get_lti_pii_sharing_state_for_course', Mock(return_value=False)) @patch( 'lti_consumer.plugin.views.compat.get_course_members', diff --git a/lti_consumer/tests/unit/test_lti_xblock.py b/lti_consumer/tests/unit/test_lti_xblock.py index a97138b3..a120c9c0 100644 --- a/lti_consumer/tests/unit/test_lti_xblock.py +++ b/lti_consumer/tests/unit/test_lti_xblock.py @@ -74,6 +74,7 @@ def setUp(self): self.compat.get_course_by_id.return_value = course self.compat.get_user_role.return_value = "student" self.compat.get_external_id_for_user.return_value = "12345" + self.compat.get_user_course_forum_role.return_value = None class TestAddXmlToNode(TestCase): @@ -349,6 +350,41 @@ def test_role(self): with self.assertRaises(LtiError): _ = self.xblock.role + @ddt.data( + ('student', None, 'student'), + ('guest', 'Community TA', 'Community TA'), + ('student', 'Group Moderator', 'Group Moderator'), + ('staff', 'Community TA', 'staff'), + ('limited_staff', 'Group Moderator', 'limited_staff'), + ('instructor', 'Community TA', 'instructor'), + ) + @ddt.unpack + def test_get_lti_1p3_user_role(self, base_role, forum_role, expected_role): + """ + Test effective LTI 1.3 role includes supported forum roles for non-staff users. + """ + fake_user = Mock() + fake_user.opt_attrs = { + 'edx-platform.user_id': FAKE_USER_ID, + 'edx-platform.user_role': base_role, + 'edx-platform.is_authenticated': True, + } + self.xblock.runtime.service(self, 'user').get_current_user = Mock(return_value=fake_user) + self.compat.get_user_course_forum_role.return_value = forum_role + + # pylint: disable=protected-access + self.assertEqual(self.xblock._get_lti_1p3_user_role(), expected_role) + + if base_role in {'staff', 'instructor', 'limited_staff'}: + self.compat.get_user_course_forum_role.assert_not_called() + else: + self.compat.get_user_course_forum_role.assert_called_once_with( + FAKE_USER_ID, + self.xblock.scope_ids.usage_id.context_key, + ) + + self.compat.get_user_course_forum_role.reset_mock() + def test_user_is_staff(self): """ Test `user_is_staff` returns the correct status from the user service @@ -1196,12 +1232,15 @@ def setUp(self): @override_settings(DEBUG=True) @patch('lti_consumer.lti_xblock.log_authorization_header') - @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.lti_provider_key_secret') - def test_runtime_debug_true(self, mock_lti_provider_key_secret, mock_log_auth_header): + @patch('lti_consumer.lti_xblock.LtiConsumerXBlock._get_lti_consumer') + def test_runtime_debug_true(self, mock_get_lti_consumer, mock_log_auth_header): """ Test `log_authorization_header` is called when settings.DEBUG is True """ - mock_lti_provider_key_secret.__get__ = Mock(return_value=(self.lti_provider_key, self.lti_provider_secret)) + mock_get_lti_consumer.return_value = Mock( + oauth_key=self.lti_provider_key, + oauth_secret=self.lti_provider_secret, + ) request = make_request('', 'GET') self.xblock.result_service_handler(request) @@ -1958,6 +1997,34 @@ def test_get_lti_1p3_launch_data( ) self.xblock.get_lti_1p3_custom_parameters.assert_called_once_with() + @patch('lti_consumer.lti_xblock.compat.get_user_course_forum_role') + def test_get_lti_1p3_launch_data_uses_forum_role(self, mock_get_user_course_forum_role): + """ + Test that get_lti_1p3_launch_data uses forum role for supported non-staff users. + """ + fake_user = Mock() + fake_user.opt_attrs = { + 'edx-platform.user_id': 1, + 'edx-platform.user_role': 'student', + 'edx-platform.is_authenticated': True, + 'edx-platform.username': 'fake_username', + } + + 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") + self.xblock.get_context_title = Mock(return_value="context_title") + self.xblock.get_pii_sharing_enabled = Mock(return_value=False) + self.xblock.get_lti_1p3_custom_parameters = Mock(return_value={}) + mock_get_user_course_forum_role.return_value = 'Community TA' + + launch_data = self.xblock.get_lti_1p3_launch_data() + + self.assertEqual(launch_data.user_role, 'Community TA') + mock_get_user_course_forum_role.assert_called_once_with( + 1, + self.xblock.scope_ids.usage_id.context_key, + ) + @patch('lti_consumer.plugin.compat.get_course_by_id') def test_get_context_title(self, mock_get_course_by_id): """ diff --git a/lti_consumer/tests/unit/test_outcomes.py b/lti_consumer/tests/unit/test_outcomes.py index 8008f9f7..c5854d3c 100644 --- a/lti_consumer/tests/unit/test_outcomes.py +++ b/lti_consumer/tests/unit/test_outcomes.py @@ -348,7 +348,7 @@ def setUp(self): self.mock_get_user_id_patcher_enabled.return_value = mock_user @patch('lti_consumer.outcomes.verify_oauth_body_signature', Mock(return_value=True)) - @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.lti_provider_key_secret', PropertyMock(return_value=('t', 's'))) + @patch('lti_consumer.lti_xblock.LtiConsumerXBlock._get_lti_consumer', Mock(return_value=Mock(oauth_secret='s'))) @patch('lti_consumer.outcomes.parse_grade_xml_body', Mock(return_value=('', '', 0.5, 'replaceResultRequest'))) def test_handle_replace_result_success(self): """ @@ -407,7 +407,7 @@ def test_xml_parse_lti_error(self, mock_parse): self.assertIn('Request body XML parsing error', response) @patch('lti_consumer.outcomes.verify_oauth_body_signature') - @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.lti_provider_key_secret', PropertyMock(return_value=('t', 's'))) + @patch('lti_consumer.lti_xblock.LtiConsumerXBlock._get_lti_consumer', Mock(return_value=Mock(oauth_secret='s'))) @patch('lti_consumer.outcomes.parse_grade_xml_body', Mock(return_value=('', '', 0.5, 'replaceResultRequest'))) def test_invalid_signature(self, mock_verify): """ @@ -422,7 +422,7 @@ def test_invalid_signature(self, mock_verify): self.assertIn('failure', self.outcome_service.handle_request(request)) @patch('lti_consumer.outcomes.verify_oauth_body_signature', Mock(return_value=True)) - @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.lti_provider_key_secret', PropertyMock(return_value=('t', 's'))) + @patch('lti_consumer.lti_xblock.LtiConsumerXBlock._get_lti_consumer', Mock(return_value=Mock(oauth_secret='s'))) @patch('lti_consumer.outcomes.parse_grade_xml_body', Mock(return_value=('', '', 0.5, 'replaceResultRequest'))) def test_user_not_found(self): """ @@ -437,7 +437,7 @@ def test_user_not_found(self): self.assertIn('User not found', response) @patch('lti_consumer.outcomes.verify_oauth_body_signature', Mock(return_value=True)) - @patch('lti_consumer.lti_xblock.LtiConsumerXBlock.lti_provider_key_secret', PropertyMock(return_value=('t', 's'))) + @patch('lti_consumer.lti_xblock.LtiConsumerXBlock._get_lti_consumer', Mock(return_value=Mock(oauth_secret='s'))) @patch('lti_consumer.outcomes.parse_grade_xml_body', Mock(return_value=('', '', 0.5, 'unsupportedRequest'))) def test_unsupported_action(self): """