mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-01-17 21:49:15 +01:00
Code Issues Projects Releases Wiki Activity

MDL-69331 core_contentbank: Hide disabled H5P content-types

Browse Source 
If a H5P content-type is disabled:
- The content bank won't display existing contents having it as a
main library.
- The content bank won't allow to create new contents using it.
This commit is contained in:
Sara Arjona 2021-02-25 17:21:32 +01:00
parent dbfd4aebb5
commit 39fa45e299
18 changed files with 565 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
contentbank/classes/form/upload_files.php
View File

@ -151,16 +151,35 @@ class upload_files extends \core_form\dynamic_form {
if (!empty($files)) {
$file = reset($files);
$cb = new \core_contentbank\contentbank();
if ($this->get_data()->id) {
$content = $cb->get_content_from_id($this->get_data()->id);
$contenttype = $content->get_content_type_instance();
$content = $contenttype->replace_content($file, $content);
} else {
$content = $cb->create_content_from_file($this->get_context_for_dynamic_submission(), $USER->id, $file);
try {
if ($this->get_data()->id) {
$content = $cb->get_content_from_id($this->get_data()->id);
$contenttype = $content->get_content_type_instance();
$content = $contenttype->replace_content($file, $content);
} else {
$content = $cb->create_content_from_file($this->get_context_for_dynamic_submission(), $USER->id, $file);
}
$params = ['id' => $content->get_id(), 'contextid' => $this->get_context_for_dynamic_submission()->id];
$url = new \moodle_url('/contentbank/view.php', $params);
} catch (\Exception $e) {
// Redirect to the right page (depending on if content is new or existing) and display an error.
if ($this->get_data()->id) {
$content = $cb->get_content_from_id($this->get_data()->id);
$params = [
'id' => $content->get_id(),
'contextid' => $this->get_context_for_dynamic_submission()->id,
'errormsg' => 'notvalidpackage',
];
$url = new \moodle_url('/contentbank/view.php', $params);
} else {
$url = new \moodle_url('/contentbank/index.php', [
'contextid' => $this->get_context_for_dynamic_submission()->id,
'errormsg' => 'notvalidpackage'],
);
}
}
$params = ['id' => $content->get_id(), 'contextid' => $this->get_context_for_dynamic_submission()->id];
$viewurl = new \moodle_url('/contentbank/view.php', $params);
return ['returnurl' => $viewurl->out(false)];
return ['returnurl' => $url->out(false)];
}
return null;

58
contentbank/contenttype/h5p/classes/content.php
View File

@ -24,9 +24,6 @@
namespace contenttype_h5p;
use stdClass;
use html_writer;
/**
* H5P Content manager class
*
@ -36,4 +33,59 @@ use html_writer;
*/
class content extends \core_contentbank\content {
/**
* Returns user has access permission for the content itself.
* If the H5P content-type library is disabled, the user won't have access to it.
*
* @return bool True if content could be accessed. False otherwise.
*/
public function is_view_allowed(): bool {
// Force H5P content to be deployed.
$fileurl = $this->get_file_url();
// Skip capability check when creating the H5P content (because it has been created by trusted users).
$h5pplayer = new \core_h5p\player($fileurl, new \stdClass(), true, '', true);
// Flush error messages.
$h5pplayer->get_messages();
// Check if the H5P entry has been created and if the main library is enabled.
$file = $this->get_file();
if (!empty($file)) {
$h5p = \core_h5p\api::get_content_from_pathnamehash($file->get_pathnamehash());
if (empty($h5p)) {
// If there is no H5P entry for this content, it won't be displayed unless the user has the manageanycontent
// capability. Reasons for contents without a proper H5P entry in DB:
// - Invalid H5P package (it won't be never deployed).
// - Disabled content-type library (it can't be deployed so there is no way to know the mainlibraryid).
$context = \context::instance_by_id($this->content->contextid);
if (!has_capability('moodle/contentbank:manageanycontent', $context)) {
return false;
}
} else if (!\core_h5p\api::is_library_enabled((object) ['id' => $h5p->mainlibraryid])) {
// If the main library is disabled, it won't be displayed.
return false;
}
}
return parent::is_view_allowed();
}
/**
* Import a file as a valid content.
* Before importing the file, this method will check if the file is a valid H5P package. If it's not valid, it will thrown
* an exception.
*
* @throws \file_exception If file operations fail
* @param \stored_file $file File to store in the content file area.
* @return \stored_file|null the stored content file or null if the file is discarted.
*/
public function import_file(\stored_file $file): ?\stored_file {
// The H5P content can be only deployed if the author of the .h5p file can update libraries or if all the
// content-type libraries exist, to avoid users without the h5p:updatelibraries capability upload malicious content.
$onlyupdatelibs = !\core_h5p\helper::can_update_library($file);
if (!\core_h5p\api::is_valid_package($file, $onlyupdatelibs)) {
throw new \file_exception('invalidpackage');
}
return parent::import_file($file);
}
}

35
contentbank/contenttype/h5p/classes/contenttype.php
View File

@ -148,22 +148,25 @@ class contenttype extends \core_contentbank\contenttype {
$types = [];
$h5pfilestorage = new file_storage();
foreach ($h5pcontenttypes as $h5pcontenttype) {
$library = [
'name' => $h5pcontenttype->machine_name,
'majorVersion' => $h5pcontenttype->major_version,
'minorVersion' => $h5pcontenttype->minor_version,
];
$key = H5PCore::libraryToString($library);
$type = new stdClass();
$type->key = $key;
$type->typename = $h5pcontenttype->title;
$type->typeeditorparams = 'library=' . $key;
$type->typeicon = $h5pfilestorage->get_icon_url(
$h5pcontenttype->id,
$h5pcontenttype->machine_name,
$h5pcontenttype->major_version,
$h5pcontenttype->minor_version);
$types[] = $type;
if ($h5pcontenttype->enabled) {
// Only enabled content-types will be displayed.
$library = [
'name' => $h5pcontenttype->machine_name,
'majorVersion' => $h5pcontenttype->major_version,
'minorVersion' => $h5pcontenttype->minor_version,
];
$key = H5PCore::libraryToString($library);
$type = new stdClass();
$type->key = $key;
$type->typename = $h5pcontenttype->title;
$type->typeeditorparams = 'library=' . $key;
$type->typeicon = $h5pfilestorage->get_icon_url(
$h5pcontenttype->id,
$h5pcontenttype->machine_name,
$h5pcontenttype->major_version,
$h5pcontenttype->minor_version);
$types[] = $type;
}
}
return $types;

101
contentbank/contenttype/h5p/tests/behat/disable_contenttypes.feature Normal file
View File

@ -0,0 +1,101 @@
@core @core_contentbank @core_h5p @contenttype_h5p @_file_upload @javascript
Feature: Disable H5P content-types from the content bank
In order to disable H5P content-types
As an admin
I need to be able to check they are not displayed in the content bank
Background:
Given the following "users" exist:
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
And the following "courses" exist:
| fullname | shortname | category |
| Course 1 | C1 | 0 |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
And the following "contentbank contents" exist:
| contextlevel | reference | contenttype | user | contentname | filepath |
| Course | C1 | contenttype_h5p | admin | filltheblanks | /h5p/tests/fixtures/filltheblanks.h5p |
| Course | C1 | contenttype_h5p | admin | accordion | /h5p/tests/fixtures/ipsums.h5p |
| Course | C1 | contenttype_h5p | admin | invalidh5p | /h5p/tests/fixtures/h5ptest.zip |
And I log in as "admin"
And I am on "Course 1" course homepage with editing mode on
And I add the "Navigation" block if not present
And I log out
Scenario: Teachers cannot view disabled or invalid content-types
Given I log in as "teacher1"
And I am on "Course 1" course homepage
And I expand "Site pages" node
And I click on "Content bank" "link"
And I should see "accordion"
And I should see "filltheblanks"
And I should not see "invalidh5p"
And I log out
And I log in as "admin"
And I navigate to "H5P > Manage H5P content types" in site administration
And I click on "Disable" "link" in the "Accordion" "table_row"
And I log out
When I log in as "teacher1"
And I am on "Course 1" course homepage
And I expand "Site pages" node
And I click on "Content bank" "link"
Then I should not see "accordion"
And I should see "filltheblanks"
And I should not see "invalidh5p"
Scenario: Admins cannot view disabled content-types
Given I log in as "admin"
And I am on "Course 1" course homepage
And I expand "Site pages" node
And I click on "Content bank" "link"
And I should see "accordion"
And I should see "filltheblanks"
And I should see "invalidh5p"
And I navigate to "H5P > Manage H5P content types" in site administration
And I click on "Disable" "link" in the "Accordion" "table_row"
When I am on "Course 1" course homepage
And I expand "Site pages" node
And I click on "Content bank" "link"
Then I should not see "accordion"
And I should see "filltheblanks"
And I should see "invalidh5p"
Scenario: Teachers cannot create disabled content-types
Given I log in as "teacher1"
And I am on "Course 1" course homepage
And I expand "Site pages" node
And I click on "Content bank" "link"
And I click on "[data-action=Add-content]" "css_element"
And I should see "Accordion"
And I should see "Fill in the Blanks"
And I log out
And I log in as "admin"
And I navigate to "H5P > Manage H5P content types" in site administration
And I click on "Disable" "link" in the "Accordion" "table_row"
And I log out
When I log in as "teacher1"
And I am on "Course 1" course homepage
And I expand "Site pages" node
And I click on "Content bank" "link"
And I click on "[data-action=Add-content]" "css_element"
Then I should not see "Accordion"
And I should see "Fill in the Blanks"
Scenario: Admins cannot create disabled content-types
Given I log in as "admin"
And I am on "Course 1" course homepage
And I expand "Site pages" node
And I click on "Content bank" "link"
And I click on "[data-action=Add-content]" "css_element"
And I should see "Accordion"
And I should see "Fill in the Blanks"
And I navigate to "H5P > Manage H5P content types" in site administration
And I click on "Disable" "link" in the "Accordion" "table_row"
When I am on "Course 1" course homepage
And I expand "Site pages" node
And I click on "Content bank" "link"
And I click on "[data-action=Add-content]" "css_element"
Then I should not see "Accordion"
And I should see "Fill in the Blanks"

23
contentbank/contenttype/h5p/tests/behat/teacher_upload_content.feature
View File

@ -72,7 +72,7 @@ Feature: H5P file upload to content bank for non admins
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
Scenario: Teachers can not upload and deploy 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
@ -89,14 +89,14 @@ Feature: H5P file upload to content bank for non admins
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
Then I should see "Sorry, this file is not valid."
And I should not see "filltheblanks.h5p"
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"
And I am on "Course 1" course homepage
And I expand "Site pages" node
And I click on "Content bank" "link"
And I should not see "filltheblanks.h5p"
Scenario: Teachers can not see existing contents when libraries are not installed
Given I log out
@ -138,8 +138,13 @@ Feature: H5P file upload to content bank for non admins
Given I am on "Course 1" course homepage
When I expand "Site pages" node
And I click on "Content bank" "link"
Then I should not see "filltheblanks.h5p"
And I log out
And I log in as "admin"
And I am on "Course 1" course homepage
And 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"
And I should see "missing-required-library"

128
contentbank/contenttype/h5p/tests/content_h5p_test.php
View File

@ -66,4 +66,132 @@ class contenttype_h5p_content_plugin_testcase extends advanced_testcase {
$this->assertInstanceOf(\stored_file::class, $file);
$this->assertEquals($filename, $file->get_filename());
}
/**
* Tests for is view allowed content.
*
* @covers ::is_view_allowed
* @dataProvider is_view_allowed_provider
*
* @param string $role User role to use for create and view contents.
* @param array $disabledlibraries Library names to disable.
* @param array $expected Array with the expected values for the contents in the following order:
* ['H5P.Blanks deployed', 'H5P.Accordion deployed', 'H5P.Accordion undeployed', 'Invalid content'].
*/
public function test_is_view_allowed(string $role, array $disabledlibraries, array $expected): void {
global $CFG, $USER, $DB;
$this->resetAfterTest();
// Create a course.
$course = $this->getDataGenerator()->create_course();
$coursecontext = \context_course::instance($course->id);
// Set user.
if ($role == 'admin') {
$this->setAdminUser();
} else {
// Enrol user to the course.
$user = $this->getDataGenerator()->create_and_enrol($course, $role);
$this->setUser($user);
}
// Add contents to the content bank.
$generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank');
$filepath = $CFG->dirroot . '/h5p/tests/fixtures/filltheblanks.h5p';
$contents = $generator->generate_contentbank_data('contenttype_h5p', 1, $USER->id, $coursecontext, true, $filepath);
$filltheblanks = array_shift($contents);
$filepath = $CFG->dirroot . '/h5p/tests/fixtures/ipsums.h5p';
$contents = $generator->generate_contentbank_data('contenttype_h5p', 2, $USER->id, $coursecontext, true, $filepath);
$accordion1 = array_shift($contents);
$accordion2 = array_shift($contents);
$filepath = $CFG->dirroot . '/h5p/tests/fixtures/invalid.zip';
$contents = $generator->generate_contentbank_data('contenttype_h5p', 1, $USER->id, $coursecontext, true, $filepath);
$invalid = array_shift($contents);
// Load some of these H5P files though the player to create the H5P DB entries.
$h5pplayer = new \core_h5p\player($filltheblanks->get_file_url(), new \stdClass(), true);
$h5pplayer = new \core_h5p\player($accordion1->get_file_url(), new \stdClass(), true);
// Check the expected H5P content has been created.
$this->assertEquals(2, $DB->count_records('h5p'));
$this->assertEquals(4, $DB->count_records('contentbank_content'));
// Disable libraries.
foreach ($disabledlibraries as $libraryname) {
$libraryid = $DB->get_field('h5p_libraries', 'id', ['machinename' => $libraryname]);
\core_h5p\api::set_library_enabled((int) $libraryid, false);
}
$this->assertEquals($expected[0], $filltheblanks->is_view_allowed());
$this->assertEquals($expected[1], $accordion1->is_view_allowed());
$this->assertEquals($expected[2], $accordion2->is_view_allowed());
$this->assertEquals($expected[3], $invalid->is_view_allowed());
// Check that after enabling libraries again, all the content return true (but the invalid package).
foreach ($disabledlibraries as $libraryname) {
$libraryid = $DB->get_field('h5p_libraries', 'id', ['machinename' => $libraryname]);
\core_h5p\api::set_library_enabled((int) $libraryid, true);
}
$this->assertEquals(true, $filltheblanks->is_view_allowed());
$this->assertEquals(true, $accordion1->is_view_allowed());
$this->assertEquals(true, $accordion2->is_view_allowed()); // It will be deployed, so now it will always return true.
$this->assertEquals($expected[3], $invalid->is_view_allowed());
}
/**
* Data provider for test_is_view_allowed.
*
* @return array
*/
public function is_view_allowed_provider(): array {
return [
'Editing teacher with all libraries enabled' => [
'role' => 'editingteacher',
'disabledlibraries' => [],
'expected' => [true, true, true, false],
],
'Manager with all libraries enabled' => [
'role' => 'manager',
'disabledlibraries' => [],
'expected' => [true, true, true, true],
],
'Admin with all libraries enabled' => [
'role' => 'admin',
'disabledlibraries' => [],
'expected' => [true, true, true, true],
],
'Editing teacher with H5P.Accordion disabled' => [
'role' => 'editingteacher',
'disabledlibraries' => ['H5P.Accordion'],
'expected' => [true, false, false, false],
],
'Manager with H5P.Accordion disabled' => [
'role' => 'manager',
'disabledlibraries' => ['H5P.Accordion'],
'expected' => [true, false, true, true],
],
'Admin with H5P.Accordion disabled' => [
'role' => 'admin',
'disabledlibraries' => ['H5P.Accordion'],
'expected' => [true, false, true, true],
],
'Editing teacher with all libraries disabled' => [
'role' => 'editingteacher',
'disabledlibraries' => ['H5P.Accordion', 'H5P.Blanks'],
'expected' => [false, false, false, false],
],
'Manager with all libraries disabled' => [
'role' => 'manager',
'disabledlibraries' => ['H5P.Accordion', 'H5P.Blanks'],
'expected' => [false, false, true, true],
],
'Admin with all libraries disabled' => [
'role' => 'admin',
'disabledlibraries' => ['H5P.Accordion', 'H5P.Blanks'],
'expected' => [false, false, true, true],
],
];
}
}

16
contentbank/tests/contentbank_test.php
View File

@ -46,7 +46,7 @@ require_once($CFG->dirroot . '/contentbank/tests/fixtures/testable_content.php')
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @coversDefaultClass \core_contentbank\contentbank
*/
class core_contentbank_testcase extends advanced_testcase {
class contentbank_test extends advanced_testcase {
/**
* Setup to ensure that fixtures are loaded.
@ -206,9 +206,10 @@ class core_contentbank_testcase extends advanced_testcase {
*/
public function test_search_contents(?string $search, string $where, int $expectedresult, array $contexts = [],
array $contenttypes = null): void {
global $DB;
global $DB, $CFG;
$this->resetAfterTest();
$this->setAdminUser();
// Create users.
$managerroleid = $DB->get_field('role', 'id', ['shortname' => 'manager']);
@ -230,11 +231,12 @@ class core_contentbank_testcase extends advanced_testcase {
}
// Add some content to the content bank.
$filepath = $CFG->dirroot . '/h5p/tests/fixtures/filltheblanks.h5p';
$generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank');
foreach ($contexts as $context) {
$contextinstance = $existingcontexts[$context];
$records = $generator->generate_contentbank_data('contenttype_h5p', 3,
$manager->id, $contextinstance, false);
$manager->id, $contextinstance, false, $filepath);
}
// Search for some content.
@ -357,12 +359,12 @@ class core_contentbank_testcase extends advanced_testcase {
* @covers ::create_content_from_file
*/
public function test_create_content_from_file() {
global $USER;
global $USER, $CFG;
$this->resetAfterTest();
$this->setAdminUser();
$systemcontext = \context_system::instance();
$name = 'dummy_h5p.h5p';
$name = 'greeting-card-887.h5p';
// Create a dummy H5P file.
$dummyh5p = array(
@ -374,8 +376,8 @@ class core_contentbank_testcase extends advanced_testcase {
'filename' => $name,
'userid' => $USER->id
);
$fs = get_file_storage();
$dummyh5pfile = $fs->create_file_from_string($dummyh5p, 'Dummy H5Pcontent');
$path = $CFG->dirroot . '/h5p/tests/fixtures/' . $name;
$dummyh5pfile = \core_h5p\helper::create_fake_stored_file_from_path($path);
$cb = new contentbank();
$content = $cb->create_content_from_file($systemcontext, $USER->id, $dummyh5pfile);

3
contentbank/view.php
View File

@ -58,7 +58,8 @@ $contenttype = $content->get_content_type_instance();
$pageheading = $record->name;
if (!$content->is_view_allowed()) {
print_error('notavailable', 'contentbank');
$cburl = new \moodle_url('/contentbank/index.php', ['contextid' => $context->id, 'errormsg' => 'notavailable']);
redirect($cburl);
}
if ($content->get_visibility() == content::VISIBILITY_UNLISTED) {

70
h5p/classes/api.php
View File

@ -185,16 +185,20 @@ class api {
*
* @param string $url H5P pluginfile URL.
* @param bool $preventredirect Set to true in scripts that can not redirect (CLI, RSS feeds, etc.), throws exceptions
* @param bool $skipcapcheck Whether capabilities should be checked or not to get the pluginfile URL because sometimes they
* might be controlled before calling this method.
*
* @return array of [file, stdClass|false]:
* - file local file for this $url.
* - stdClass is an H5P object or false if there isn't any H5P with this URL.
*/
public static function get_content_from_pluginfile_url(string $url, bool $preventredirect = true): array {
public static function get_content_from_pluginfile_url(string $url, bool $preventredirect = true,
bool $skipcapcheck = false): array {
global $DB;
// Deconstruct the URL and get the pathname associated.
if (self::can_access_pluginfile_hash($url, $preventredirect)) {
if ($skipcapcheck || self::can_access_pluginfile_hash($url, $preventredirect)) {
$pathnamehash = self::get_pluginfile_hash($url);
}
@ -223,17 +227,19 @@ class api {
* @param factory $factory The \core_h5p\factory object
* @param stdClass $messages The error, exception and info messages, raised while preparing and running an H5P content.
* @param bool $preventredirect Set to true in scripts that can not redirect (CLI, RSS feeds, etc.), throws exceptions
* @param bool $skipcapcheck Whether capabilities should be checked or not to get the pluginfile URL because sometimes they
* might be controlled before calling this method.
*
* @return array of [file, h5pid]:
* - file local file for this $url.
* - h5pid is the H5P identifier or false if there isn't any H5P with this URL.
*/
public static function create_content_from_pluginfile_url(string $url, \stdClass $config, factory $factory,
\stdClass &$messages, bool $preventredirect = true): array {
\stdClass &$messages, bool $preventredirect = true, bool $skipcapcheck = false): array {
global $USER;
$core = $factory->get_core();
list($file, $h5p) = self::get_content_from_pluginfile_url($url, $preventredirect);
list($file, $h5p) = self::get_content_from_pluginfile_url($url, $preventredirect, $skipcapcheck);
if (!$file) {
$core->h5pF->setErrorMessage(get_string('h5pfilenotfound', 'core_h5p'));
@ -657,4 +663,60 @@ class api {
return true;
}
/**
* Check whether an H5P package is valid or not.
*
* @param \stored_file $file The file with the H5P content.
* @param bool $onlyupdatelibs Whether new libraries can be installed or only the existing ones can be updated
* @param bool $skipcontent Should the content be skipped (so only the libraries will be saved)?
* @param factory|null $factory The \core_h5p\factory object
* @param bool $deletefiletree Should the temporary files be deleted before returning?
* @return bool True if the H5P file is valid (expected format, valid libraries...); false otherwise.
*/
public static function is_valid_package(\stored_file $file, bool $onlyupdatelibs, bool $skipcontent = false,
?factory $factory = null, bool $deletefiletree = true): bool {
// This may take a long time.
\core_php_time_limit::raise();
$isvalid = false;
if (empty($factory)) {
$factory = new factory();
}
$core = $factory->get_core();
$h5pvalidator = $factory->get_validator();
// Set the H5P file path.
$core->h5pF->set_file($file);
$path = $core->fs->getTmpPath();
$core->h5pF->getUploadedH5pFolderPath($path);
// Add manually the extension to the file to avoid the validation fails.
$path .= '.h5p';
$core->h5pF->getUploadedH5pPath($path);
// Copy the .h5p file to the temporary folder.
$file->copy_content_to($path);
if ($h5pvalidator->isValidPackage($skipcontent, $onlyupdatelibs)) {
if ($skipcontent) {
$isvalid = true;
} else if (!empty($h5pvalidator->h5pC->mainJsonData['mainLibrary'])) {
$mainlibrary = (object) ['machinename' => $h5pvalidator->h5pC->mainJsonData['mainLibrary']];
if (self::is_library_enabled($mainlibrary)) {
$isvalid = true;
} else {
// If the main library of the package is disabled, the H5P content will be considered invalid.
$core->h5pF->setErrorMessage(get_string('mainlibrarydisabled', 'core_h5p'));
}
}
}
if ($deletefiletree) {
// Remove temp content folder.
\H5PCore::deleteFileTree($path);
}
return $isvalid;
}
}

3
h5p/classes/editor_ajax.php
View File

@ -51,7 +51,8 @@ class editor_ajax implements H5PEditorAjaxInterface {
global $DB;
$sql = "SELECT hl2.id, hl2.machinename as machine_name, hl2.title, hl2.majorversion as major_version,
hl2.minorversion AS minor_version, hl2.patchversion as patch_version, '' as has_icon, 0 as restricted
hl2.minorversion AS minor_version, hl2.patchversion as patch_version, '' as has_icon, 0 as restricted,
hl2.enabled
FROM {h5p_libraries} hl2
LEFT JOIN {h5p_libraries} hl1
ON hl1.machinename = hl2.machinename

27
h5p/classes/helper.php
View File

@ -50,30 +50,10 @@ class helper {
*/
public static function save_h5p(factory $factory, \stored_file $file, \stdClass $config, bool $onlyupdatelibs = false,
bool $skipcontent = false) {
// This may take a long time.
\core_php_time_limit::raise();
$core = $factory->get_core();
$core->h5pF->set_file($file);
$path = $core->fs->getTmpPath();
$core->h5pF->getUploadedH5pFolderPath($path);
// Add manually the extension to the file to avoid the validation fails.
$path .= '.h5p';
$core->h5pF->getUploadedH5pPath($path);
// Copy the .h5p file to the temporary folder.
$file->copy_content_to($path);
// Check if the h5p file is valid before saving it.
$h5pvalidator = $factory->get_validator();
if ($h5pvalidator->isValidPackage($skipcontent, $onlyupdatelibs)) {
// If the main library of the package is disabled, the H5P content won't be saved.
$mainlibrary = (object) ['machinename' => $h5pvalidator->h5pC->mainJsonData['mainLibrary']];
if (!api::is_library_enabled($mainlibrary)) {
$core->h5pF->setErrorMessage(get_string('mainlibrarydisabled', 'core_h5p'));
return false;
}
if (api::is_valid_package($file, $onlyupdatelibs, $skipcontent, $factory, false)) {
$core = $factory->get_core();
$h5pvalidator = $factory->get_validator();
$h5pstorage = $factory->get_storage();
$content = [
@ -90,6 +70,7 @@ class helper {
return $h5pstorage->contentId;
}
return false;
}

8
h5p/classes/player.php
View File

@ -105,8 +105,11 @@ class player {
* @param stdClass $config Configuration for H5P buttons.
* @param bool $preventredirect Set to true in scripts that can not redirect (CLI, RSS feeds, etc.), throws exceptions
* @param string $component optional moodle component to sent xAPI tracking
* @param bool $skipcapcheck Whether capabilities should be checked or not to get the pluginfile URL because sometimes they
* might be controlled before calling this method.
*/
public function __construct(string $url, \stdClass $config, bool $preventredirect = true, string $component = '') {
public function __construct(string $url, \stdClass $config, bool $preventredirect = true, string $component = '',
bool $skipcapcheck = false) {
if (empty($url)) {
throw new \moodle_exception('h5pinvalidurl', 'core_h5p');
}
@ -128,7 +131,8 @@ class player {
$config,
$this->factory,
$this->messages,
$this->preventredirect
$this->preventredirect,
$skipcapcheck
);
if ($file) {
$this->context = \context::instance_by_id($file->get_contextid());

99
h5p/tests/api_test.php
View File

@ -37,7 +37,7 @@ defined('MOODLE_INTERNAL') || die();
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @coversDefaultClass \core_h5p\api
*/
class api_testcase extends \advanced_testcase {
class api_test extends \advanced_testcase {
/**
* Test the behaviour of delete_library().
@ -742,4 +742,101 @@ class api_testcase extends \advanced_testcase {
],
];
}
/**
* Test the behaviour of is_valid_package().
* @runInSeparateProcess
*
* @covers ::is_valid_package
* @dataProvider is_valid_package_provider
*
* @param string $filename The H5P content to validate.
* @param bool $expected Expected result after calling the method.
* @param bool $isadmin Whether the user calling the method will be admin or not.
* @param bool $onlyupdatelibs Whether new libraries can be installed or only the existing ones can be updated.
* @param bool $skipcontent Should the content be skipped (so only the libraries will be saved)?
*/
public function test_is_valid_package(string $filename, bool $expected, bool $isadmin = false, bool $onlyupdatelibs = false,
bool $skipcontent = false): void {
global $USER;
$this->resetAfterTest();
if ($isadmin) {
$this->setAdminUser();
$user = $USER;
} else {
// Create a user.
$user = $this->getDataGenerator()->create_user();
$this->setUser($user);
}
// Prepare the file.
$path = __DIR__ . $filename;
$file = helper::create_fake_stored_file_from_path($path, (int)$user->id);
// Check if the H5P content is valid or not.
$result = api::is_valid_package($file, $onlyupdatelibs, $skipcontent);
$this->assertEquals($expected, $result);
}
/**
* Data provider for test_is_valid_package().
*
* @return array
*/
public function is_valid_package_provider(): array {
return [
'Valid H5P file (as admin)' => [
'filename' => '/fixtures/greeting-card-887.h5p',
'expected' => true,
'isadmin' => true,
],
'Valid H5P file (as user) without library update and checking content' => [
'filename' => '/fixtures/greeting-card-887.h5p',
'expected' => false, // Libraries are missing and user hasn't the right permissions to upload them.
'isadmin' => false,
'onlyupdatelibs' => false,
'skipcontent' => false,
],
'Valid H5P file (as user) with library update and checking content' => [
'filename' => '/fixtures/greeting-card-887.h5p',
'expected' => false, // Libraries are missing and user hasn't the right permissions to upload them.
'isadmin' => false,
'onlyupdatelibs' => true,
'skipcontent' => false,
],
'Valid H5P file (as user) without library update and skipping content' => [
'filename' => '/fixtures/greeting-card-887.h5p',
'expected' => true, // Content check is skipped so the package will be considered valid.
'isadmin' => false,
'onlyupdatelibs' => false,
'skipcontent' => true,
],
'Valid H5P file (as user) with library update and skipping content' => [
'filename' => '/fixtures/greeting-card-887.h5p',
'expected' => true, // Content check is skipped so the package will be considered valid.
'isadmin' => false,
'onlyupdatelibs' => true,
'skipcontent' => true,
],
'Invalid H5P file (as admin)' => [
'filename' => '/fixtures/h5ptest.zip',
'expected' => false,
'isadmin' => true,
],
'Invalid H5P file (as user)' => [
'filename' => '/fixtures/h5ptest.zip',
'expected' => false,
'isadmin' => false,
],
'Invalid H5P file (as user) skipping content' => [
'filename' => '/fixtures/h5ptest.zip',
'expected' => true, // Content check is skipped so the package will be considered valid.
'isadmin' => false,
'onlyupdatelibs' => false,
'skipcontent' => true,
],
];
}
}

11
h5p/upgrade.txt
View File

@ -1,6 +1,17 @@
This files describes API changes in core libraries and APIs,
information provided here is intended especially for developers.
=== 3.11 ===
* Added $skipcapcheck parameter to H5P constructor, api::create_content_from_pluginfile_url() and
api::get_content_from_pluginfile_url() to let skip capabilities check to get the pluginfile URL.
* Added new field "enabled" to h5p_libraries to let define if a content type is enabled (1) or not (0).
For now, only runnable content-types can be disabled/enabled. When a content-type is disabled, their
contents are not displayed and no new contents using it can be created/uploaded.
Some extra methods have been added to the api too in order to support this field:
- set_library_enabled
- is_library_enabled
- is_valid_package
=== 3.10 ===
* Added a new cache for h5p_library_files (MDL-69207)

1
lang/en/contentbank.php
View File

@ -61,6 +61,7 @@ $string['nocontenttypes'] = 'No content types available';
$string['notavailable'] = 'Sorry, this content is not available.';
$string['nopermissiontodelete'] = 'You do not have permission to delete content.';
$string['nopermissiontomanage'] = 'You do not have permission to manage content.';
$string['notvalidpackage'] = 'Sorry, this file is not valid.';
$string['privacy:metadata:content:contenttype'] = 'The contenttype plugin of the content in the content bank.';
$string['privacy:metadata:content:name'] = 'Name of the content in the content bank.';
$string['privacy:metadata:content:timecreated'] = 'The time when the content was created.';

8
lib/tests/event/contentbank_content_uploaded_test.php
View File

@ -36,7 +36,7 @@ use core_contentbank\contentbank;
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @coversDefaultClass \core\event\contentbank_content_uploaded
*/
class contentbank_content_uploaded_testcase extends \advanced_testcase {
class contentbank_content_uploaded_test extends \advanced_testcase {
/**
* Setup to ensure that fixtures are loaded.
@ -54,7 +54,7 @@ class contentbank_content_uploaded_testcase extends \advanced_testcase {
* @covers ::create_from_record
*/
public function test_content_created() {
global $USER;
global $USER, $CFG;
$this->resetAfterTest();
$this->setAdminUser();
@ -69,8 +69,8 @@ class contentbank_content_uploaded_testcase extends \advanced_testcase {
'filepath' => '/',
'filename' => 'dummy_h5p.h5p'
);
$fs = get_file_storage();
$dummyh5pfile = $fs->create_file_from_string($dummyh5p, 'Dummy H5Pcontent');
$path = $CFG->dirroot . '/h5p/tests/fixtures/greeting-card-887.h5p';
$dummyh5pfile = \core_h5p\helper::create_fake_stored_file_from_path($path);
// Trigger and capture the event when creating content from a file.
$sink = $this->redirectEvents();

15
repository/contentbank/tests/browser_test.php
View File

@ -43,7 +43,7 @@ class repository_contentbank_browser_testcase extends advanced_testcase {
* the system context.
*/
public function test_get_content_system_context_user_has_capabilities() {
global $DB;
global $DB, $CFG;
$this->resetAfterTest(true);
@ -66,8 +66,9 @@ class repository_contentbank_browser_testcase extends advanced_testcase {
// Add some content to the content bank.
$generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank');
// Add some content bank files in the system context.
$filepath = $CFG->dirroot . '/h5p/tests/fixtures/filltheblanks.h5p';
$contentbankcontents = $generator->generate_contentbank_data('contenttype_h5p', 3, $admin->id,
$systemcontext, true);
$systemcontext, true, $filepath);
// Log in as admin.
$this->setUser($admin);
@ -156,6 +157,8 @@ class repository_contentbank_browser_testcase extends advanced_testcase {
* any category course should be able to access/view the content in the course category context.
*/
public function test_get_content_course_category_context_user_has_capabilities() {
global $CFG;
$this->resetAfterTest(true);
// Create a course category.
@ -175,8 +178,9 @@ class repository_contentbank_browser_testcase extends advanced_testcase {
// Add some content to the content bank.
$generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank');
// Add some content bank files in the course category context.
$filepath = $CFG->dirroot . '/h5p/tests/fixtures/filltheblanks.h5p';
$contentbankcontents = $generator->generate_contentbank_data('contenttype_h5p', 3, $admin->id,
$coursecatcontext, true);
$coursecatcontext, true, $filepath);
$this->setUser($admin);
// Get the content bank nodes displayed to the admin in the course category context.
@ -277,6 +281,8 @@ class repository_contentbank_browser_testcase extends advanced_testcase {
* in the course should be able to access/view the content.
*/
public function test_get_content_course_context_user_has_capabilities() {
global $CFG;
$this->resetAfterTest(true);
// Create course1.
@ -290,8 +296,9 @@ class repository_contentbank_browser_testcase extends advanced_testcase {
// Add some content to the content bank.
$generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank');
// Add some content bank files in the course context.
$filepath = $CFG->dirroot . '/h5p/tests/fixtures/filltheblanks.h5p';
$contentbankcontents = $generator->generate_contentbank_data('contenttype_h5p', 3, $admin->id,
$coursecontext, true);
$coursecontext, true, $filepath);
$this->setUser($admin);
// Get the content bank nodes displayed to the admin in the course context.

9
repository/contentbank/tests/search_test.php
View File

@ -182,6 +182,8 @@ class repository_contentbank_search_testcase extends advanced_testcase {
* and system content. Other authenticated users should be able to access only the system content.
*/
public function test_get_search_contents_user_can_access_certain_content() {
global $CFG;
$this->resetAfterTest(true);
$systemcontext = \context_system::instance();
@ -201,16 +203,17 @@ class repository_contentbank_search_testcase extends advanced_testcase {
// Add some content to the content bank in different contexts.
$generator = $this->getDataGenerator()->get_plugin_generator('core_contentbank');
// Add a content bank file in the system context.
$filepath = $CFG->dirroot . '/h5p/tests/fixtures/ipsums.h5p';
$systemcontents = $generator->generate_contentbank_data('contenttype_h5p', 1, $admin->id,
$systemcontext, true, 'file.h5p', 'systemcontentfile');
$systemcontext, true, $filepath, 'systemcontentfile');
$systemcontent = reset($systemcontents);
// Add a content bank file in the course1 context.
$course1contents = $generator->generate_contentbank_data('contenttype_h5p', 1, $admin->id,
$course1context, true, 'file.h5p', 'coursecontentfile1');
$course1context, true, $filepath, 'coursecontentfile1');
$course1content = reset($course1contents);
// Add a content bank file in the course2 context.
$generator->generate_contentbank_data('contenttype_h5p', 1, $admin->id,
$course2context, true, 'file.h5p', 'coursecontentfile2');
$course2context, true, $filepath, 'coursecontentfile2');
// Log in as an editing teacher.
$this->setUser($editingteacher);