From 6157f5930f6847f9fff0f723fa9285ae9dc15cfc Mon Sep 17 00:00:00 2001 From: Amaia Anabitarte Date: Wed, 3 Jun 2020 22:30:07 +0200 Subject: [PATCH 1/3] MDL-68641 core_h5p: Fixing capability checks in ajax.php --- h5p/ajax.php | 34 +++++++++------------------------- h5p/classes/core.php | 21 --------------------- h5p/classes/editor.php | 3 ++- 3 files changed, 11 insertions(+), 47 deletions(-) diff --git a/h5p/ajax.php b/h5p/ajax.php index 88e8e17d606..2291e2ff90e 100644 --- a/h5p/ajax.php +++ b/h5p/ajax.php @@ -24,24 +24,22 @@ use core_h5p\factory; use core_h5p\framework; +use core_h5p\local\library\autoloader; define('AJAX_SCRIPT', true); require(__DIR__ . '/../config.php'); require_once($CFG->libdir . '/filelib.php'); -require_login(); - -$action = required_param('action', PARAM_ALPHA); -$contextid = required_param('contextId', PARAM_INT); - -$context = context::instance_by_id($contextid); - -if (!has_capability('moodle/h5p:updatelibraries', $context)) { - H5PCore::ajaxError(get_string('nopermissiontoedit', 'h5p')); +if (!confirm_sesskey()) { + autoloader::register(); + H5PCore::ajaxError(get_string('invalidsesskey', 'error')); header('HTTP/1.1 403 Forbidden'); return; } +require_login(); + +$action = required_param('action', PARAM_ALPHA); $factory = new factory(); $editor = $factory->get_editor(); @@ -71,6 +69,8 @@ switch ($action) { break; // Handle file upload through the editor. + // This endpoint needs a token that only users with H5P editor access could get. + // TODO: MDL-68907 to check capabilities. case 'files': $token = required_param('token', PARAM_RAW); $contentid = required_param('contentId', PARAM_INT); @@ -78,22 +78,6 @@ switch ($action) { $editor->ajax->action(H5PEditorEndpoints::FILES, $token, $contentid); break; - // Install libraries from H5P and retrieve content json. - case 'libraryinstall': - $token = required_param('token', PARAM_RAW); - $machinename = required_param('id', PARAM_TEXT); - $editor->ajax->action(H5PEditorEndpoints::LIBRARY_INSTALL, $token, $machinename); - break; - - // Handle file upload through the editor. - case 'libraryupload': - $token = required_param('token', PARAM_RAW); - - $uploadpath = $_FILES['h5p']['tmp_name']; - $contentid = optional_param('contentId', 0, PARAM_INT); - $editor->ajax->action(H5PEditorEndpoints::LIBRARY_UPLOAD, $token, $uploadpath, $contentid); - break; - // Get the $language libraries translations. case 'translations': $language = required_param('language', PARAM_RAW); diff --git a/h5p/classes/core.php b/h5p/classes/core.php index 4fb007b5aea..c3a9a884fec 100644 --- a/h5p/classes/core.php +++ b/h5p/classes/core.php @@ -357,27 +357,6 @@ class core extends \H5PCore { return true; } - /** - * Use sesskey instead of the H5P security token. - * - * @param string $action Not used. - * @return string sesskey - */ - public static function createToken($action) { - return sesskey(); - } - - /** - * Check if the token matches the sesskey. - * - * @param string $action Not used. - * @param string $token Token submitted. - * @return boolean valid token - */ - public static function validToken($action, $token) { - return confirm_sesskey($token); - } - /** * Get the library string from a DB library record. * diff --git a/h5p/classes/editor.php b/h5p/classes/editor.php index eed37740cab..8faa32f21c1 100644 --- a/h5p/classes/editor.php +++ b/h5p/classes/editor.php @@ -391,6 +391,7 @@ class editor { $contentvalidator = $factory->get_content_validator(); $editorajaxtoken = core::createToken(editor_ajax::EDITOR_AJAX_TOKEN); + $sesskey = sesskey(); $settings['editor'] = [ 'filesPath' => $filespathbase . 'editor', 'fileIcon' => [ @@ -398,7 +399,7 @@ class editor { 'width' => 50, 'height' => 50, ], - 'ajaxPath' => $CFG->wwwroot . '/h5p/' . "ajax.php?contextId={$context->id}&token={$editorajaxtoken}&action=", + 'ajaxPath' => $CFG->wwwroot . "/h5p/ajax.php?sesskey={$sesskey}&token={$editorajaxtoken}&action=", 'libraryUrl' => $url, 'copyrightSemantics' => $contentvalidator->getCopyrightSemantics(), 'metadataSemantics' => $contentvalidator->getMetadataSemantics(), From 2721dd3a630a83c32939bdfddb012eaaa637bb1e Mon Sep 17 00:00:00 2001 From: Amaia Anabitarte Date: Wed, 3 Jun 2020 22:29:36 +0200 Subject: [PATCH 2/3] MDL-68641 contenttype_h5p: Showing permission errors --- .../contenttype/h5p/classes/form/editor.php | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/contentbank/contenttype/h5p/classes/form/editor.php b/contentbank/contenttype/h5p/classes/form/editor.php index b7229b9e50e..2db471445f0 100644 --- a/contentbank/contenttype/h5p/classes/form/editor.php +++ b/contentbank/contenttype/h5p/classes/form/editor.php @@ -30,6 +30,7 @@ use core_contentbank\form\edit_content; use core_h5p\api; use core_h5p\editor as h5peditor; use core_h5p\factory; +use core_h5p\helper; use stdClass; /** @@ -53,6 +54,8 @@ class editor extends edit_content { global $DB; $mform = $this->_form; + $errors = []; + $notifications = []; // Id of the content to edit. $id = $this->_customdata['id']; @@ -73,9 +76,22 @@ class editor extends edit_content { $file = $this->content->get_file(); $h5p = api::get_content_from_pathnamehash($file->get_pathnamehash()); - $mform->addElement('hidden', 'h5pid', $h5p->id); - $mform->setType('h5pid', PARAM_INT); - $this->h5peditor->set_content($h5p->id); + if (!$h5p) { + // H5P content has not been deployed yet. Let's check why. + $factory = new \core_h5p\factory(); + $factory->get_framework()->set_file($file); + + $h5pid = helper::save_h5p($factory, $file, new stdClass()); + $errors = $factory->get_framework()->getMessages('error'); + $notifications = $factory->get_framework()->getMessages('info'); + } else { + $h5pid = $h5p->id; + } + if ($h5pid) { + $mform->addElement('hidden', 'h5pid', $h5pid); + $mform->setType('h5pid', PARAM_INT); + $this->h5peditor->set_content($h5pid); + } } else { // The H5P editor needs the H5P content type library name for a new content. $mform->addElement('hidden', 'library', $library); @@ -86,11 +102,20 @@ class editor extends edit_content { $mformid = 'coolh5peditor'; $mform->setAttributes(array('id' => $mformid) + $mform->getAttributes()); - $this->add_action_buttons(); - - $this->h5peditor->add_editor_to_form($mform); - - $this->add_action_buttons(); + if ($errors || $notifications) { + // Show the error messages and a Cancel button. + foreach ($errors as $error) { + $mform->addElement('warning', $error->code, 'notify', $error->message); + } + foreach ($notifications as $key => $notification) { + $mform->addElement('warning', 'notification_'.$key, 'notify', $notification); + } + $mform->addElement('cancel', 'cancel', get_string('back')); + } else { + $this->add_action_buttons(); + $this->h5peditor->add_editor_to_form($mform); + $this->add_action_buttons(); + } } /** From 747f6012e45a7072b337d63a95f6e53e561cb64e Mon Sep 17 00:00:00 2001 From: Amaia Anabitarte Date: Thu, 21 May 2020 00:26:50 +0200 Subject: [PATCH 3/3] MDL-68641 contenttype_h5p: Libraries permissions tests --- .../tests/behat/admin_upload_content.feature | 20 ++++++ .../behat/teacher_upload_content.feature | 72 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/contentbank/contenttype/h5p/tests/behat/admin_upload_content.feature b/contentbank/contenttype/h5p/tests/behat/admin_upload_content.feature index 8639d35e937..5e3bcbe82ae 100644 --- a/contentbank/contenttype/h5p/tests/behat/admin_upload_content.feature +++ b/contentbank/contenttype/h5p/tests/behat/admin_upload_content.feature @@ -70,3 +70,23 @@ Feature: H5P file upload to content bank for admins And I expand "Site pages" node And I click on "Content bank" "link" And I should not see "filltheblanks.h5p" + + Scenario: Admins can upload and deployed content types when libraries are not installed + Given I navigate to "H5P > Manage H5P content types" in site administration + And I should not see "Fill in the Blanks" + And I follow "Dashboard" in the user menu + And I expand "Site pages" node + And I click on "Content bank" "link" + And I should not see "filltheblanks.h5p" + When I click on "Upload" "link" + And I click on "Choose a file..." "button" + And I click on "Private files" "link" in the ".fp-repo-area" "css_element" + And I click on "filltheblanks.h5p" "link" + And I click on "Select this file" "button" + And I click on "Save changes" "button" + And I switch to "h5p-player" class iframe + And I switch to "h5p-iframe" class iframe + Then I should see "Of which countries" + And I switch to the main frame + And I navigate to "H5P > Manage H5P content types" in site administration + And I should see "Fill in the Blanks" diff --git a/contentbank/contenttype/h5p/tests/behat/teacher_upload_content.feature b/contentbank/contenttype/h5p/tests/behat/teacher_upload_content.feature index 9c25ac96dae..48f6e222b2b 100644 --- a/contentbank/contenttype/h5p/tests/behat/teacher_upload_content.feature +++ b/contentbank/contenttype/h5p/tests/behat/teacher_upload_content.feature @@ -71,3 +71,75 @@ Feature: H5P file upload to content bank for non admins And I expand "Site pages" node And I click on "Content bank" "link" Then I should see "filltheblanks.h5p" + + Scenario: Teachers can not upload and deployed content types when libraries are not installed + Given I log out + And I log in as "admin" + And I navigate to "H5P > Manage H5P content types" in site administration + And I should not see "Fill in the Blanks" + And I log out + And I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on + And I add the "Navigation" block if not present + And I expand "Site pages" node + And I click on "Content bank" "link" + When I click on "Upload" "link" + And I click on "Choose a file..." "button" + And I click on "Private files" "link" in the ".fp-repo-area" "css_element" + And I click on "filltheblanks.h5p" "link" + And I click on "Select this file" "button" + And I click on "Save changes" "button" + And I switch to "h5p-player" class iframe + Then I should not see "Of which countries" + And I should see "missing-required-library" + And I switch to the main frame + And I log out + And I log in as "admin" + And I navigate to "H5P > Manage H5P content types" in site administration + And I should not see "Fill in the Blanks" + + Scenario: Teachers can not see existing contents when libraries are not installed + Given I log out + And I log in as "admin" + And I follow "Manage private files..." + And I upload "h5p/tests/fixtures/filltheblanks.h5p" file to "Files" filemanager + And I click on "Save changes" "button" + And I navigate to "H5P > Manage H5P content types" in site administration + And I should not see "Fill in the Blanks" + When I upload "h5p/tests/fixtures/filltheblanks.h5p" file to "H5P content type" filemanager + And I click on "Upload H5P content types" "button" in the "#fitem_id_uploadlibraries" "css_element" + And I wait until the page is ready + And I should see "Fill in the Blanks" + And I log out + And I log in as "teacher1" + Given I am on "Course 1" course homepage with editing mode on + And I add the "Navigation" block if not present + When I expand "Site pages" node + And I click on "Content bank" "link" + And I click on "Upload" "link" + And I click on "Choose a file..." "button" + And I click on "Private files" "link" in the ".fp-repo-area" "css_element" + And I click on "filltheblanks.h5p" "link" + And I click on "Select this file" "button" + And I click on "Save changes" "button" + And I switch to "h5p-player" class iframe + And I switch to "h5p-iframe" class iframe + Then I should see "Of which countries" + Then I should not see "missing-required-library" + And I switch to the main frame + Given I log out + And I log in as "admin" + And I navigate to "H5P > Manage H5P content types" in site administration + When I click on "Delete version" "link" in the "Fill in the Blanks" "table_row" + And I press "Continue" + Then I should not see "Fill in the Blanks" + And I log out + And I log in as "teacher1" + Given I am on "Course 1" course homepage + When I expand "Site pages" node + And I click on "Content bank" "link" + And I should see "filltheblanks.h5p" + And I click on "filltheblanks.h5p" "link" + And I switch to "h5p-player" class iframe + Then I should not see "Of which countries" + Then I should see "missing-required-library"