From 63dd2951bba67ca2505e982f8649b6aaa6a91864 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Thu, 9 May 2019 10:59:13 +0800 Subject: [PATCH 1/3] MDL-65536 mod_lti: fix stale platformid when wwwroot is changed If the site is installed, and then the wwwroot changed, the plugin config value for platformid for mod_lti is then stale, resulting in OIDC errors. Given this config value was set using the wwwroot, we should just be able to use the value of $CFG->wwwroot direcly, thus avoiding this problem. --- mod/lti/db/install.php | 5 ----- mod/lti/db/upgrade.php | 3 --- mod/lti/locallib.php | 12 +++++++----- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/mod/lti/db/install.php b/mod/lti/db/install.php index 9adf9db60ba..e70c95c55c2 100644 --- a/mod/lti/db/install.php +++ b/mod/lti/db/install.php @@ -28,11 +28,6 @@ defined('MOODLE_INTERNAL') || die(); * Stub for database installation. */ function xmldb_lti_install() { - global $CFG; - - // Add platform ID configuration setting. - set_config('platformid', $CFG->wwwroot, 'mod_lti'); - // Create the private key. $kid = bin2hex(openssl_random_pseudo_bytes(10)); set_config('kid', $kid, 'mod_lti'); diff --git a/mod/lti/db/upgrade.php b/mod/lti/db/upgrade.php index 135bc2950c8..4908f6cc3e4 100644 --- a/mod/lti/db/upgrade.php +++ b/mod/lti/db/upgrade.php @@ -157,9 +157,6 @@ function xmldb_lti_upgrade($oldversion) { $dbman->add_index($table, $index); } - // Add platform ID configuration setting. - set_config('platformid', $CFG->wwwroot, 'mod_lti'); - // Create the private key. $kid = bin2hex(openssl_random_pseudo_bytes(10)); set_config('kid', $kid, 'mod_lti'); diff --git a/mod/lti/locallib.php b/mod/lti/locallib.php index 3bd9a2a0ca3..f7e665ab288 100644 --- a/mod/lti/locallib.php +++ b/mod/lti/locallib.php @@ -3018,6 +3018,7 @@ function lti_sign_parameters($oldparms, $endpoint, $method, $oauthconsumerkey, $ * @return array|null */ function lti_sign_jwt($parms, $endpoint, $oauthconsumerkey, $typeid = 0, $nonce = '') { + global $CFG; if (empty($typeid)) { $typeid = 0; @@ -3054,7 +3055,7 @@ function lti_sign_jwt($parms, $endpoint, $oauthconsumerkey, $typeid = 0, $nonce 'iat' => $now, 'exp' => $now + 60, ); - $payload['iss'] = get_config('mod_lti', 'platformid'); + $payload['iss'] = $CFG->wwwroot; $payload['aud'] = $oauthconsumerkey; $payload[LTI_JWT_CLAIM_PREFIX . '/claim/deployment_id'] = strval($typeid); $payload[LTI_JWT_CLAIM_PREFIX . '/claim/target_link_uri'] = $endpoint; @@ -3264,7 +3265,7 @@ function lti_post_launch_html($newparms, $endpoint, $debug=false) { */ function lti_initiate_login($courseid, $id, $instance, $config, $messagetype = 'basic-lti-launch-request', $title = '', $text = '') { - global $SESSION, $USER; + global $SESSION, $USER, $CFG; if (!empty($instance)) { $endpoint = !empty($instance->toolurl) ? $instance->toolurl : $config->lti_toolurl; @@ -3284,7 +3285,7 @@ function lti_initiate_login($courseid, $id, $instance, $config, $messagetype = ' } $params = array(); - $params['iss'] = get_config('mod_lti', 'platformid'); + $params['iss'] = $CFG->wwwroot; $params['target_link_uri'] = $endpoint; $params['login_hint'] = $USER->id; $params['lti_message_hint'] = $id; @@ -3839,7 +3840,8 @@ function get_tool_type_state_info(stdClass $type) { * @return array An array with configuration details */ function get_tool_type_config($type) { - $platformid = get_config('mod_lti', 'platformid'); + global $CFG; + $platformid = $CFG->wwwroot; $clientid = $type->clientid; $deploymentid = $type->id; $publickeyseturl = new moodle_url('/mod/lti/certs.php'); @@ -3940,7 +3942,7 @@ function serialise_tool_type(stdClass $type) { 'description' => $description, 'urls' => get_tool_type_urls($type), 'state' => get_tool_type_state_info($type), - 'platformid' => get_config('mod_lti', 'platformid'), + 'platformid' => $CFG->wwwroot, 'clientid' => $type->clientid, 'deploymentid' => $type->id, 'hascapabilitygroups' => !empty($capabilitygroups), From db0ccfa922ac13f49c8de6c5568a33c010099a39 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Fri, 10 May 2019 10:01:56 +0800 Subject: [PATCH 2/3] MDL-65536 mod_lti: OpenSSL configuration LTI 1.3 requires working openssl functions to generate a private key used for the JWT. Some installs (e.g. windows) can have the functions available - but require configuration in php before they actually do anything. --- mod/lti/db/install.php | 18 ++++---- mod/lti/db/upgrade.php | 19 +++----- mod/lti/edit_form.php | 24 +++++++++++ mod/lti/instructor_edit_tool_type.php | 7 ++- mod/lti/lang/en/lti.php | 1 + mod/lti/upgradelib.php | 62 +++++++++++++++++++++++++++ 6 files changed, 108 insertions(+), 23 deletions(-) create mode 100644 mod/lti/upgradelib.php diff --git a/mod/lti/db/install.php b/mod/lti/db/install.php index e70c95c55c2..057d72a49c4 100644 --- a/mod/lti/db/install.php +++ b/mod/lti/db/install.php @@ -28,15 +28,13 @@ defined('MOODLE_INTERNAL') || die(); * Stub for database installation. */ function xmldb_lti_install() { + global $CFG, $OUTPUT; + // Create the private key. - $kid = bin2hex(openssl_random_pseudo_bytes(10)); - set_config('kid', $kid, 'mod_lti'); - $config = array( - "digest_alg" => "sha256", - "private_key_bits" => 2048, - "private_key_type" => OPENSSL_KEYTYPE_RSA, - ); - $res = openssl_pkey_new($config); - openssl_pkey_export($res, $privatekey); - set_config('privatekey', $privatekey, 'mod_lti'); + require_once($CFG->dirroot . '/mod/lti/upgradelib.php'); + + $warning = mod_lti_verify_private_key(); + if (!empty($warning)) { + echo $OUTPUT->notification($warning, 'notifyproblem'); + } } diff --git a/mod/lti/db/upgrade.php b/mod/lti/db/upgrade.php index 4908f6cc3e4..4d313e590d1 100644 --- a/mod/lti/db/upgrade.php +++ b/mod/lti/db/upgrade.php @@ -59,7 +59,7 @@ * @return boolean */ function xmldb_lti_upgrade($oldversion) { - global $CFG, $DB; + global $CFG, $DB, $OUTPUT; $dbman = $DB->get_manager(); @@ -157,17 +157,12 @@ function xmldb_lti_upgrade($oldversion) { $dbman->add_index($table, $index); } - // Create the private key. - $kid = bin2hex(openssl_random_pseudo_bytes(10)); - set_config('kid', $kid, 'mod_lti'); - $config = array( - "digest_alg" => "sha256", - "private_key_bits" => 2048, - "private_key_type" => OPENSSL_KEYTYPE_RSA, - ); - $res = openssl_pkey_new($config); - openssl_pkey_export($res, $privatekey); - set_config('privatekey', $privatekey, 'mod_lti'); + require_once($CFG->dirroot . '/mod/lti/upgradelib.php'); + + $warning = mod_lti_verify_private_key(); + if (!empty($warning)) { + echo $OUTPUT->notification($warning, 'notifyproblem'); + } // Lti savepoint reached. upgrade_mod_savepoint(true, 2019031300, 'lti'); diff --git a/mod/lti/edit_form.php b/mod/lti/edit_form.php index 8a3fe0b4e15..a06170c973e 100644 --- a/mod/lti/edit_form.php +++ b/mod/lti/edit_form.php @@ -339,4 +339,28 @@ class mod_lti_edit_types_form extends moodleform { $service->get_configuration_options($mform); } } + + /** + * Validate the form data before we allow them to save the tool type. + * + * @param array $data + * @param array $files + * @return array Error messages + */ + public function validation($data, $files) { + global $CFG; + + $errors = parent::validation($data, $files); + + if ($data['lti_ltiversion'] == LTI_VERSION_1P3) { + require_once($CFG->dirroot . '/mod/lti/upgradelib.php'); + + $warning = mod_lti_verify_private_key(); + if (!empty($warning)) { + $errors['lti_ltiversion'] = $warning; + return $errors; + } + } + return $errors; + } } diff --git a/mod/lti/instructor_edit_tool_type.php b/mod/lti/instructor_edit_tool_type.php index cca3fb387c3..8925ce10de1 100644 --- a/mod/lti/instructor_edit_tool_type.php +++ b/mod/lti/instructor_edit_tool_type.php @@ -121,8 +121,13 @@ EOF; window.opener.M.mod_lti.editor.addToolType({$json}); EOF; } + echo html_writer::script($script . $closewindow); + } else if ($form->is_cancelled()) { + echo html_writer::script($closewindow); + } else { + echo $OUTPUT->heading(get_string('toolsetup', 'lti')); + $form->display(); } - echo html_writer::script($script . $closewindow); } echo $OUTPUT->footer(); diff --git a/mod/lti/lang/en/lti.php b/mod/lti/lang/en/lti.php index b1e14c8ba98..c3f82737001 100644 --- a/mod/lti/lang/en/lti.php +++ b/mod/lti/lang/en/lti.php @@ -483,6 +483,7 @@ $string['show_in_course_lti2_help'] = 'This tool can be shown in the activity ch $string['show_in_course_no'] = 'Do not show; use only when a matching tool URL is entered'; $string['show_in_course_preconfigured'] = 'Show as preconfigured tool when adding an external tool'; $string['size'] = 'Size parameters'; +$string['opensslconfiginvalid'] = 'LTI 1.3 requires a valid openssl.cnf to be configured and available to your web server. Please contact the site administrator to configure and enable openssl for this site.'; $string['submission'] = 'Submission'; $string['submissions'] = 'Submissions'; $string['submissionsfor'] = 'Submissions for {$a}'; diff --git a/mod/lti/upgradelib.php b/mod/lti/upgradelib.php new file mode 100644 index 00000000000..ee60d08b41b --- /dev/null +++ b/mod/lti/upgradelib.php @@ -0,0 +1,62 @@ +. + +/** + * This file contains functions used by upgrade and install. + * + * Because this is used during install it should not include additional files. + * + * @package mod_lti + * @copyright 2019 Damyon Wiese + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * This function checks if a private key has been generated for this site. + * + * If the key does not exist it generates a new one. If the openssl + * extension is not installed or configured properly it returns a warning message. + * + * @return string A warning message if a private key does not exist and cannot be generated. + */ +function mod_lti_verify_private_key() { + $key = get_config('privatekey', 'mod_lti'); + + // If we already generated a valid key, no need to check. + if (empty($key)) { + + // Create the private key. + $kid = bin2hex(openssl_random_pseudo_bytes(10)); + set_config('kid', $kid, 'mod_lti'); + $config = array( + "digest_alg" => "sha256", + "private_key_bits" => 2048, + "private_key_type" => OPENSSL_KEYTYPE_RSA, + ); + $res = openssl_pkey_new($config); + openssl_pkey_export($res, $privatekey); + + if (!empty($privatekey)) { + set_config('privatekey', $privatekey, 'mod_lti'); + } else { + return get_string('opensslconfiginvalid', 'mod_lti'); + } + } + + return ''; +} From b8b6bd753038de5f50cbf560c15a1aafa274a086 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 10 May 2019 14:10:37 +0800 Subject: [PATCH 3/3] MDL-65536 mod_lti: JWT should use the class loader --- lib/classes/component.php | 1 + mod/lti/locallib.php | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/classes/component.php b/lib/classes/component.php index 6250f3a18fd..6c5d8424860 100644 --- a/lib/classes/component.php +++ b/lib/classes/component.php @@ -87,6 +87,7 @@ class core_component { 'PHPMailer\\PHPMailer' => 'lib/phpmailer/src', 'RedeyeVentures\\GeoPattern' => 'lib/geopattern-php/GeoPattern', 'MongoDB' => 'cache/stores/mongodb/MongoDB', + 'Firebase\\JWT' => 'lib/php-jwt/src', ); /** diff --git a/mod/lti/locallib.php b/mod/lti/locallib.php index f7e665ab288..ec9bd2fb31e 100644 --- a/mod/lti/locallib.php +++ b/mod/lti/locallib.php @@ -59,7 +59,6 @@ require_once($CFG->dirroot.'/mod/lti/OAuth.php'); require_once($CFG->libdir.'/weblib.php'); require_once($CFG->dirroot . '/course/modlib.php'); require_once($CFG->dirroot . '/mod/lti/TrivialStore.php'); -require_once($CFG->libdir . '/php-jwt/src/JWT.php'); define('LTI_URL_DOMAIN_REGEX', '/(?:https?:\/\/)?(?:www\.)?([^\/]+)(?:\/|$)/i');