From 5047a91e4aad9e36c886e41014b9e887bbcc4963 Mon Sep 17 00:00:00 2001 From: cescobedo Date: Wed, 30 Oct 2019 13:50:14 +0100 Subject: [PATCH 1/2] MDL-67082 core_h5p: Check component_callback called for right context --- h5p/classes/player.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/h5p/classes/player.php b/h5p/classes/player.php index 5605a972481..d6913ec1eb5 100644 --- a/h5p/classes/player.php +++ b/h5p/classes/player.php @@ -328,12 +328,15 @@ class player { // Some components, such as mod_page or mod_resource, add the revision to the URL to prevent caching problems. // So the URL contains this revision number as itemid but a 0 is always stored in the files table. // In order to get the proper hash, a callback should be done (looking for those exceptions). - $pathdata = component_callback($component, 'get_path_from_pluginfile', [$filearea, $parts], null); + $pathdata = null; + if ($this->context->contextlevel == CONTEXT_MODULE || $this->context->contextlevel == CONTEXT_BLOCK) { + $pathdata = component_callback($component, 'get_path_from_pluginfile', [$filearea, $parts], null); + } if (null === $pathdata) { // Look for the components and fileareas which have empty itemid defined in xxx_pluginfile. $hasnullitemid = false; $hasnullitemid = $hasnullitemid || ($component === 'user' && ($filearea === 'private' || $filearea === 'profile')); - $hasnullitemid = $hasnullitemid || ($component === 'mod' && $filearea === 'intro'); + $hasnullitemid = $hasnullitemid || (substr($component, 0, 4) === 'mod_' && $filearea === 'intro'); $hasnullitemid = $hasnullitemid || ($component === 'course' && ($filearea === 'summary' || $filearea === 'overviewfiles')); $hasnullitemid = $hasnullitemid || ($component === 'coursecat' && $filearea === 'description'); -- 2.20.1 (Apple Git-117) From b449165d7dfdb4a34ee3abed7dc07ba6d3602941 Mon Sep 17 00:00:00 2001 From: cescobedo Date: Thu, 31 Oct 2019 19:27:32 +0100 Subject: [PATCH 2/2] MDL-67082 core_h5p: Add new contexts to check h5p file permissions --- h5p/classes/player.php | 53 +++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/h5p/classes/player.php b/h5p/classes/player.php index d6913ec1eb5..3381c2ff343 100644 --- a/h5p/classes/player.php +++ b/h5p/classes/player.php @@ -273,7 +273,7 @@ class player { * @return string|false pathnamehash for the file in the internal URL. */ private function get_pluginfile_hash(string $url) { - global $USER; + global $USER, $CFG; // Decode the URL before start processing it. $url = new \moodle_url(urldecode($url)); @@ -310,17 +310,54 @@ class player { throw new \moodle_exception('h5pprivatefile', 'core_h5p'); } - // For CONTEXT_MODULE, check if the user is enrolled in the course and has permissions view this .h5p file. - if ($this->context->contextlevel == CONTEXT_MODULE) { + // For CONTEXT_COURSECAT No login necessary - unless login forced everywhere. + if ($this->context->contextlevel == CONTEXT_COURSECAT) { + if ($CFG->forcelogin) { + require_login(null, true, null, false, true); + } + } + + // For CONTEXT_BLOCK. + if ($this->context->contextlevel == CONTEXT_BLOCK) { + if ($this->context->get_course_context(false)) { + // If block is in course context, then check if user has capability to access course. + require_course_login($course, true, null, false, true); + } else if ($CFG->forcelogin) { + // No login necessary - unless login forced everywhere. + require_login(null, true, null, false, true); + } else { + // Get parent context and see if user have proper permission. + $parentcontext = $this->context->get_parent_context(); + if ($parentcontext->contextlevel === CONTEXT_COURSECAT) { + // Check if category is visible and user can view this category. + if (!core_course_category::get($parentcontext->instanceid, IGNORE_MISSING)) { + send_file_not_found(); + } + } else if ($parentcontext->contextlevel === CONTEXT_USER && $parentcontext->instanceid != $USER->id) { + // The block is in the context of a user, it is only visible to the user who it belongs to. + send_file_not_found(); + } + if ($filearea !== 'content') { + send_file_not_found(); + } + } + } + + // For CONTEXT_MODULE and CONTEXT_COURSE check if the user is enrolled in the course. + // And for CONTEXT_MODULE has permissions view this .h5p file. + if ($this->context->contextlevel == CONTEXT_MODULE || + $this->context->contextlevel == CONTEXT_COURSE) { // Require login to the course first (without login to the module). require_course_login($course, true, null, false, true); // Now check if module is available OR it is restricted but the intro is shown on the course page. - $cminfo = \cm_info::create($cm); - if (!$cminfo->uservisible) { - if (!$cm->showdescription || !$cminfo->is_visible_on_course_page()) { - // Module intro is not visible on the course page and module is not available, show access error. - require_course_login($course, true, $cminfo, false, true); + if ($this->context->contextlevel == CONTEXT_MODULE) { + $cminfo = \cm_info::create($cm); + if (!$cminfo->uservisible) { + if (!$cm->showdescription || !$cminfo->is_visible_on_course_page()) { + // Module intro is not visible on the course page and module is not available, show access error. + require_course_login($course, true, $cminfo, false, true); + } } } } -- 2.20.1 (Apple Git-117)