From 00b2219fd4a9829dd38679f1ced0e954ead5d0b0 Mon Sep 17 00:00:00 2001 From: Claude Vervoort Date: Tue, 18 Aug 2020 09:40:18 -0400 Subject: [PATCH] MDL-68384 mod_lti: fix spec violations bo claim name and dl value type --- mod/lti/locallib.php | 28 ++++++++++++------- .../classes/local/service/memberships.php | 2 +- mod/lti/tests/locallib_test.php | 25 ++++++++++------- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/mod/lti/locallib.php b/mod/lti/locallib.php index a5db3ec0585..c6b438d3a46 100644 --- a/mod/lti/locallib.php +++ b/mod/lti/locallib.php @@ -126,7 +126,8 @@ function lti_get_jwt_claim_mapping() { 'suffix' => 'dl', 'group' => 'deep_linking_settings', 'claim' => 'accept_copy_advice', - 'isarray' => false + 'isarray' => false, + 'type' => 'boolean' ], 'accept_media_types' => [ 'suffix' => 'dl', @@ -138,7 +139,8 @@ function lti_get_jwt_claim_mapping() { 'suffix' => 'dl', 'group' => 'deep_linking_settings', 'claim' => 'accept_multiple', - 'isarray' => false + 'isarray' => false, + 'type' => 'boolean' ], 'accept_presentation_document_targets' => [ 'suffix' => 'dl', @@ -156,19 +158,22 @@ function lti_get_jwt_claim_mapping() { 'suffix' => 'dl', 'group' => 'deep_linking_settings', 'claim' => 'accept_unsigned', - 'isarray' => false + 'isarray' => false, + 'type' => 'boolean' ], 'auto_create' => [ 'suffix' => 'dl', 'group' => 'deep_linking_settings', 'claim' => 'auto_create', - 'isarray' => false + 'isarray' => false, + 'type' => 'boolean' ], 'can_confirm' => [ 'suffix' => 'dl', 'group' => 'deep_linking_settings', 'claim' => 'can_confirm', - 'isarray' => false + 'isarray' => false, + 'type' => 'boolean' ], 'content_item_return_url' => [ 'suffix' => 'dl', @@ -389,7 +394,7 @@ function lti_get_jwt_claim_mapping() { 'tool_consumer_info_product_family_code' => [ 'suffix' => '', 'group' => 'tool_platform', - 'claim' => 'family_code', + 'claim' => 'product_family_code', 'isarray' => false ], 'tool_consumer_info_version' => [ @@ -483,14 +488,14 @@ function lti_get_jwt_claim_mapping() { 'isarray' => false ], 'lis_outcome_service_url' => [ - 'suffix' => 'bos', - 'group' => 'basicoutcomesservice', + 'suffix' => 'bo', + 'group' => 'basicoutcome', 'claim' => 'lis_outcome_service_url', 'isarray' => false ], 'lis_result_sourcedid' => [ - 'suffix' => 'bos', - 'group' => 'basicoutcomesservice', + 'suffix' => 'bo', + 'group' => 'basicoutcome', 'claim' => 'lis_result_sourcedid', 'isarray' => false ], @@ -3172,9 +3177,12 @@ function lti_sign_jwt($parms, $endpoint, $oauthconsumerkey, $typeid = 0, $nonce $claim = LTI_JWT_CLAIM_PREFIX; if (array_key_exists($key, $claimmapping)) { $mapping = $claimmapping[$key]; + $type = $mapping["type"] ?? "string"; if ($mapping['isarray']) { $value = explode(',', $value); sort($value); + } else if ($type == 'boolean') { + $value = isset($value) && ($value == 'true'); } if (!empty($mapping['suffix'])) { $claim .= "-{$mapping['suffix']}"; diff --git a/mod/lti/service/memberships/classes/local/service/memberships.php b/mod/lti/service/memberships/classes/local/service/memberships.php index 231e3cf1b26..46d437c538d 100644 --- a/mod/lti/service/memberships/classes/local/service/memberships.php +++ b/mod/lti/service/memberships/classes/local/service/memberships.php @@ -447,7 +447,7 @@ class memberships extends \mod_lti\local\ltiservice\service_base { $serviceurl = lti_ensure_url_is_https($serviceurl); } $basicoutcome->lis_outcome_service_url = $serviceurl; - $message->{'https://purl.imsglobal.org/spec/lti-bos/claim/basicoutcomesservice'} = $basicoutcome; + $message->{'https://purl.imsglobal.org/spec/lti-bo/claim/basicoutcome'} = $basicoutcome; } $member->message = [$message]; } diff --git a/mod/lti/tests/locallib_test.php b/mod/lti/tests/locallib_test.php index 1a9800ae803..089f565a154 100644 --- a/mod/lti/tests/locallib_test.php +++ b/mod/lti/tests/locallib_test.php @@ -627,7 +627,8 @@ class mod_lti_locallib_testcase extends advanced_testcase { 'suffix' => 'dl', 'group' => 'deep_linking_settings', 'claim' => 'accept_copy_advice', - 'isarray' => false + 'isarray' => false, + 'type' => 'boolean' ], 'accept_media_types' => [ 'suffix' => 'dl', @@ -639,7 +640,8 @@ class mod_lti_locallib_testcase extends advanced_testcase { 'suffix' => 'dl', 'group' => 'deep_linking_settings', 'claim' => 'accept_multiple', - 'isarray' => false + 'isarray' => false, + 'type' => 'boolean' ], 'accept_presentation_document_targets' => [ 'suffix' => 'dl', @@ -657,19 +659,22 @@ class mod_lti_locallib_testcase extends advanced_testcase { 'suffix' => 'dl', 'group' => 'deep_linking_settings', 'claim' => 'accept_unsigned', - 'isarray' => false + 'isarray' => false, + 'type' => 'boolean' ], 'auto_create' => [ 'suffix' => 'dl', 'group' => 'deep_linking_settings', 'claim' => 'auto_create', - 'isarray' => false + 'isarray' => false, + 'type' => 'boolean' ], 'can_confirm' => [ 'suffix' => 'dl', 'group' => 'deep_linking_settings', 'claim' => 'can_confirm', - 'isarray' => false + 'isarray' => false, + 'type' => 'boolean' ], 'content_item_return_url' => [ 'suffix' => 'dl', @@ -890,7 +895,7 @@ class mod_lti_locallib_testcase extends advanced_testcase { 'tool_consumer_info_product_family_code' => [ 'suffix' => '', 'group' => 'tool_platform', - 'claim' => 'family_code', + 'claim' => 'product_family_code', 'isarray' => false ], 'tool_consumer_info_version' => [ @@ -984,14 +989,14 @@ class mod_lti_locallib_testcase extends advanced_testcase { 'isarray' => false ], 'lis_outcome_service_url' => [ - 'suffix' => 'bos', - 'group' => 'basicoutcomesservice', + 'suffix' => 'bo', + 'group' => 'basicoutcome', 'claim' => 'lis_outcome_service_url', 'isarray' => false ], 'lis_result_sourcedid' => [ - 'suffix' => 'bos', - 'group' => 'basicoutcomesservice', + 'suffix' => 'bo', + 'group' => 'basicoutcome', 'claim' => 'lis_result_sourcedid', 'isarray' => false ],