From 8faaca0ea2d11cb8992e716b6ab78b5953f6bd2e Mon Sep 17 00:00:00 2001 From: Mark Nielsen Date: Wed, 30 Jul 2014 08:41:40 -0700 Subject: [PATCH] MDL-46536: OAuth signature fails with inconsistent new lines When the LTI intro contains a single \n or \r newline character then the OAuth signature fails. All newlines should be \r\n Conflicts: mod/lti/tests/locallib_test.php --- mod/lti/locallib.php | 13 +++++++-- mod/lti/tests/locallib_test.php | 49 +++++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/mod/lti/locallib.php b/mod/lti/locallib.php index 1ec37750a74..74fa34b2491 100644 --- a/mod/lti/locallib.php +++ b/mod/lti/locallib.php @@ -227,7 +227,7 @@ function lti_build_sourcedid($instanceid, $userid, $launchid = null, $servicesal * This function builds the request that must be sent to the tool producer * * @param object $instance Basic LTI instance object - * @param object $typeconfig Basic LTI tool configuration + * @param array $typeconfig Basic LTI tool configuration * @param object $course Course object * * @return array $request Request details @@ -241,10 +241,19 @@ function lti_build_request($instance, $typeconfig, $course) { $role = lti_get_ims_role($USER, $instance->cmid, $instance->course); + $intro = ''; + if (!empty($instance->cmid)) { + $intro = format_module_intro('lti', $instance, $instance->cmid); + $intro = html_to_text($intro, 0, false); + + // This may look weird, but this is required for new lines + // so we generate the same OAuth signature as the tool provider. + $intro = str_replace("\n", "\r\n", $intro); + } $requestparams = array( 'resource_link_id' => $instance->id, 'resource_link_title' => $instance->name, - 'resource_link_description' => $instance->intro, + 'resource_link_description' => $intro, 'user_id' => $USER->id, 'roles' => $role, 'context_id' => $course->id, diff --git a/mod/lti/tests/locallib_test.php b/mod/lti/tests/locallib_test.php index 33a21878bb7..9d3f050454b 100644 --- a/mod/lti/tests/locallib_test.php +++ b/mod/lti/tests/locallib_test.php @@ -53,8 +53,14 @@ global $CFG; require_once($CFG->dirroot . '/mod/lti/locallib.php'); require_once($CFG->dirroot . '/mod/lti/servicelib.php'); - -class mod_lti_locallib_testcase extends basic_testcase { +/** + * Local library tests + * + * @package mod_lti + * @copyright Copyright (c) 2012 Moodlerooms Inc. (http://www.moodlerooms.com) + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class mod_lti_locallib_testcase extends advanced_testcase { public function test_split_custom_parameters() { $this->assertEquals(lti_split_custom_parameters("x=1\ny=2"), @@ -145,4 +151,43 @@ class mod_lti_locallib_testcase extends basic_testcase { $this->assertEquals('https://moodle.org', lti_ensure_url_is_https('moodle.org')); $this->assertEquals('https://moodle.org', lti_ensure_url_is_https('https://moodle.org')); } + + /** + * Test lti_build_request's resource_link_description and ensure + * that the newlines in the description are correct. + */ + public function test_lti_build_request_description() { + $this->resetAfterTest(); + + self::setUser($this->getDataGenerator()->create_user()); + $course = $this->getDataGenerator()->create_course(); + $instance = $this->getDataGenerator()->create_module('lti', array( + 'intro' => "

This

\nhas\r\n

some

\nnew\n\rlines", + 'introformat' => FORMAT_HTML, + 'course' => $course->id, + )); + + $typeconfig = array( + 'acceptgrades' => 1, + 'forcessl' => 0, + 'sendname' => 2, + 'sendemailaddr' => 2, + 'customparameters' => '', + ); + + $params = lti_build_request($instance, $typeconfig, $course, null); + + $ncount = substr_count($params['resource_link_description'], "\n"); + $this->assertGreaterThan(0, $ncount); + + $rcount = substr_count($params['resource_link_description'], "\r"); + $this->assertGreaterThan(0, $rcount); + + $this->assertEquals($ncount, $rcount, 'The number of \n characters should be the same as the number of \r characters'); + + $rncount = substr_count($params['resource_link_description'], "\r\n"); + $this->assertGreaterThan(0, $rncount); + + $this->assertEquals($ncount, $rncount, 'All newline characters should be a combination of \r\n'); + } }