mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-03-14 12:40:01 +01:00
Code Issues Projects Releases Wiki Activity

MDL-77665 core_h5p: Allow deployment if user has been deleted

Browse Source 
* If user has been deleted we consider that the content can be deployed
by a user who can deploy other packages
* Add behat test to cover for this use case
* Fix the content bank generator as it was failing to upload the content
as the given user
This commit is contained in:
Laurent David 2024-04-11 09:07:02 +02:00
parent fbed8034eb
commit 941c866216
4 changed files with 104 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
h5p/classes/helper.php
View File

@ -26,6 +26,7 @@ namespace core_h5p;
use context_system;
use core_h5p\local\library\autoloader;
use core_user;
defined('MOODLE_INTERNAL') || die();
@ -176,46 +177,50 @@ class helper {
/**
* Checks if the author of the .h5p file is "trustable". If the file hasn't been uploaded by a user with the
* required capability, the content won't be deployed.
* required capability, the content won't be deployed, unless the user has been deleted, in this
* case we check the capability against current user.
*
* @param stored_file $file The .h5p file to be deployed
* @return bool Returns true if the file can be deployed, false otherwise.
*/
public static function can_deploy_package(\stored_file $file): bool {
if (null === $file->get_userid()) {
$userid = $file->get_userid();
if (null === $userid) {
// If there is no userid, it is owned by the system.
return true;
}
$context = \context::instance_by_id($file->get_contextid());
if (has_capability('moodle/h5p:deploy', $context, $file->get_userid())) {
return true;
$fileuser = core_user::get_user($userid);
if (empty($fileuser) || $fileuser->deleted) {
$userid = null;
}
return false;
return has_capability('moodle/h5p:deploy', $context, $userid);
}
/**
* Checks if the content-type libraries can be upgraded.
* The H5P content-type libraries can only be upgraded if the author of the .h5p file can manage content-types or if all the
* content-types exist, to avoid users without the required capability to upload malicious content.
* content-types exist, to avoid users without the required capability to upload malicious content. If user has been deleted
* we check against current user.
*
* @param stored_file $file The .h5p file to be deployed
* @return bool Returns true if the content-type libraries can be created/updated, false otherwise.
*/
public static function can_update_library(\stored_file $file): bool {
if (null === $file->get_userid()) {
$userid = $file->get_userid();
if (null === $userid) {
// If there is no userid, it is owned by the system.
return true;
}
// Check if the owner of the .h5p file has the capability to manage content-types.
$context = \context::instance_by_id($file->get_contextid());
if (has_capability('moodle/h5p:updatelibraries', $context, $file->get_userid())) {
return true;
$fileuser = core_user::get_user($userid);
if (empty($fileuser) || $fileuser->deleted) {
$userid = null;
}
return false;
return has_capability('moodle/h5p:updatelibraries', $context, $userid);
}
/**

64
h5p/tests/behat/h5p_deployment.feature Normal file
View File

@ -0,0 +1,64 @@
@core_h5p @_file_upload @_switch_iframe @editor_tiny
Feature: Undeployed H5P content should be only available to users that can deploy packages.
Background:
Given the following "users" exist:
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
| teacher2 | Teacher | 2 | teacher2@example.com |
| student1 | Student | 1 | student1@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 |
| teacher2 | C1 | editingteacher |
| student1 | C1 | student |
# Make sure that the teacher2 can update libraries so it show the right info when.
And the following "permission overrides" exist:
| capability | permission | role | contextlevel | reference |
| moodle/h5p:updatelibraries | Allow | editingteacher | System | |
And the following "activities" exist:
| activity | name | intro | introformat | course | content | contentformat | idnumber |
| page | H5PPage | PageDesc1 | 1 | C1 | H5Ptest | 1 | 1 |
And I am on the H5PPage "page activity editing" page logged in as teacher1
And the following "contentbank content" exist:
| contextlevel | reference | contenttype | user | contentname | filepath |
| Course | C1 | contenttype_h5p | teacher1 | filltheblanks.h5p | /h5p/tests/fixtures/filltheblanks.h5p |
And I click on the "Configure H5P content" button for the "Page content" TinyMCE editor
And I click on "Browse repositories..." "button" in the "Insert H5P content" "dialogue"
And I click on "Content bank" "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 "Insert H5P content" "button" in the "Insert H5P content" "dialogue"
# This is important here not to do Save and display as if not this will be deployed and the student will see it in the first step.
And I click on "Save and return to course" "button"
And I log out
And I log in as "admin"
And I navigate to "Users > Accounts > Browse list of users" in site administration
And I press "Delete" action in the "Teacher 1" report row
And I click on "Delete" "button" in the "Delete user" "dialogue"
And I should see "Deleted user Teacher 1"
@javascript
Scenario: A student I should not be able to see a package that has been deployed by a deleted user. Then if another user deploys the package, I can see it.
Given I am on the "H5PPage" "page activity" page logged in as student1
And I switch to "h5p-iframe" class iframe
And I should see "This file can't be displayed"
And I switch to the main frame
And I log out
# Then teacher2 will be allowed to deploy the package.
When I am on the "H5PPage" "page activity" page logged in as teacher2
# Note the double switch to iframe is needed because the first iframe is the one that contains the H5P package and
# the second iframe is the one that contains the H5P content.
And I switch to "h5p-iframe" class iframe
And I switch to "h5p-iframe" class iframe
Then I should see "Of which countries are Berlin"
And I switch to the main frame
And I log out
# Now student1 should be able to see the package.
And I am on the "H5PPage" "page activity" page logged in as student1
And I switch to "h5p-iframe" class iframe
And I switch to "h5p-iframe" class iframe
And I should see "Of which countries are Berlin"

20
h5p/tests/helper_test.php
View File

@ -311,6 +311,16 @@ class helper_test extends \advanced_testcase {
$factory->get_framework()->set_file($file);
$candeploy = helper::can_deploy_package($file);
$this->assertTrue($candeploy);
$usertobedeleted = $this->getDataGenerator()->create_user();
$this->setUser($usertobedeleted);
$file = helper::create_fake_stored_file_from_path($path, (int)$usertobedeleted->id);
$factory->get_framework()->set_file($file);
// Then we delete this user.
$this->setAdminUser();
delete_user($usertobedeleted);
$candeploy = helper::can_deploy_package($file);
$this->assertTrue($candeploy); // We can update as admin.
}
/**
@ -339,6 +349,16 @@ class helper_test extends \advanced_testcase {
$factory->get_framework()->set_file($file);
$candeploy = helper::can_update_library($file);
$this->assertTrue($candeploy);
$usertobedeleted = $this->getDataGenerator()->create_user();
$this->setUser($usertobedeleted);
$file = helper::create_fake_stored_file_from_path($path, (int)$usertobedeleted->id);
$factory->get_framework()->set_file($file);
// Then we delete this user.
$this->setAdminUser();
delete_user($usertobedeleted);
$canupdate = helper::can_update_library($file);
$this->assertTrue($canupdate); // We can update as admin.
}
/**

6
lib/behat/classes/behat_core_generator.php
View File

@ -1052,15 +1052,15 @@ class behat_core_generator extends behat_generator_base {
if (!empty($data['filepath'])) {
$filename = basename($data['filepath']);
$fs = get_file_storage();
$filerecord = array(
$filerecord = [
'component' => 'contentbank',
'filearea' => 'public',
'contextid' => $context->id,
'userid' => $data['userid'],
'itemid' => $content->get_id(),
'filename' => $filename,
'filepath' => '/'
);
'filepath' => '/',
];
$fs->create_file_from_pathname($filerecord, $CFG->dirroot . $data['filepath']);
}
} else {