mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-05-12 19:25:35 +02:00
Code Issues Projects Releases Wiki Activity

MDL-77230 mod_feedback: Validate feedback access

Browse Source 
The get_items() and get_page_items() external methods should return
items only when the user has access. Otherwise, empty array for items
will be returned, with the exact error in the warnings parameter.
This commit is contained in:
Sara Arjona 2023-02-15 18:38:37 +01:00
parent 3312a6814a
commit cc4b43a57b
3 changed files with 279 additions and 20 deletions
mod/feedback
classes
external.php
tests/external
external_test.php
upgrade.txt

104
mod/feedback/classes/external.php

@ -452,19 +452,48 @@ class mod_feedback_external extends external_api {
$params = array('feedbackid' => $feedbackid, 'courseid' => $courseid);
$params = self::validate_parameters(self::get_items_parameters(), $params);
$warnings = array();
$returneditems = array();
list($feedback, $course, $cm, $context, $completioncourse) = self::validate_feedback($params['feedbackid'],
$params['courseid']);
$feedbackstructure = new mod_feedback_structure($feedback, $cm, $completioncourse->id);
$returneditems = array();
if ($items = $feedbackstructure->get_items()) {
foreach ($items as $item) {
$itemnumber = empty($item->itemnr) ? null : $item->itemnr;
unset($item->itemnr); // Added by the function, not part of the record.
$exporter = new feedback_item_exporter($item, array('context' => $context, 'itemnumber' => $itemnumber));
$returneditems[] = $exporter->export($PAGE->get_renderer('core'));
$userhasaccess = true;
try {
// Check the user has access to the feedback.
self::validate_feedback_access($feedback, $completioncourse, $cm, $context, true);
} catch (moodle_exception $e) {
$userhasaccess = false;
$warnings[] = [
'item' => $feedback->id,
'warningcode' => clean_param($e->errorcode, PARAM_ALPHANUM),
'message' => $e->getMessage(),
];
}
// For consistency with the web behaviour, the items should be returned only when the user can edit or view reports (to
// include non-editing teachers too).
$capabilities = [
'mod/feedback:edititems',
'mod/feedback:viewreports',
];
if ($userhasaccess || has_any_capability($capabilities, $context)) {
// Remove previous warnings because, although the user might not have access, they have the proper capability.
$warnings = [];
$feedbackstructure = new mod_feedback_structure($feedback, $cm, $completioncourse->id);
if ($items = $feedbackstructure->get_items()) {
foreach ($items as $item) {
$itemnumber = empty($item->itemnr) ? null : $item->itemnr;
unset($item->itemnr); // Added by the function, not part of the record.
$exporter = new feedback_item_exporter($item, array('context' => $context, 'itemnumber' => $itemnumber));
$returneditems[] = $exporter->export($PAGE->get_renderer('core'));
}
}
} else if ($userhasaccess) {
$warnings[] = [
'item' => $feedback->id,
'warningcode' => 'nopermission',
'message' => 'nopermission',
];
}
$result = array(
@ -586,24 +615,59 @@ class mod_feedback_external extends external_api {
$params = array('feedbackid' => $feedbackid, 'page' => $page, 'courseid' => $courseid);
$params = self::validate_parameters(self::get_page_items_parameters(), $params);
$warnings = array();
$returneditems = array();
$hasprevpage = false;
$hasnextpage = false;
list($feedback, $course, $cm, $context, $completioncourse) = self::validate_feedback($params['feedbackid'],
$params['courseid']);
$feedbackcompletion = new mod_feedback_completion($feedback, $cm, $completioncourse->id);
$userhasaccess = true;
$feedbackcompletion = null;
try {
// Check the user has access to the feedback.
$feedbackcompletion = self::validate_feedback_access($feedback, $completioncourse, $cm, $context, true);
} catch (moodle_exception $e) {
$userhasaccess = false;
$warnings[] = [
'item' => $feedback->id,
'warningcode' => str_replace('_', '', $e->errorcode),
'message' => $e->getMessage(),
];
}
$page = $params['page'];
$pages = $feedbackcompletion->get_pages();
$pageitems = $pages[$page];
$hasnextpage = $page < count($pages) - 1; // Until we complete this page we can not trust get_next_page().
$hasprevpage = $page && ($feedbackcompletion->get_previous_page($page, false) !== null);
// For consistency with the web behaviour, the items should be returned only when the user can edit or view reports (to
// include non-editing teachers too).
$capabilities = [
'mod/feedback:edititems',
'mod/feedback:viewreports',
];
if ($userhasaccess || has_any_capability($capabilities, $context)) {
// Remove previous warnings because, although the user might not have access, they have the proper capability.
$warnings = [];
$returneditems = array();
foreach ($pageitems as $item) {
$itemnumber = empty($item->itemnr) ? null : $item->itemnr;
unset($item->itemnr); // Added by the function, not part of the record.
$exporter = new feedback_item_exporter($item, array('context' => $context, 'itemnumber' => $itemnumber));
$returneditems[] = $exporter->export($PAGE->get_renderer('core'));
if ($feedbackcompletion == null) {
$feedbackcompletion = new mod_feedback_completion($feedback, $cm, $completioncourse->id);
}
$page = $params['page'];
$pages = $feedbackcompletion->get_pages();
$pageitems = $pages[$page];
$hasnextpage = $page < count($pages) - 1; // Until we complete this page we can not trust get_next_page().
$hasprevpage = $page && ($feedbackcompletion->get_previous_page($page, false) !== null);
foreach ($pageitems as $item) {
$itemnumber = empty($item->itemnr) ? null : $item->itemnr;
unset($item->itemnr); // Added by the function, not part of the record.
$exporter = new feedback_item_exporter($item, array('context' => $context, 'itemnumber' => $itemnumber));
$returneditems[] = $exporter->export($PAGE->get_renderer('core'));
}
} else if ($userhasaccess) {
$warnings[] = [
'item' => $feedback->id,
'warningcode' => 'nopermission',
'message' => get_string('nopermission', 'mod_feedback'),
];
}
$result = array(

191
mod/feedback/tests/external/external_test.php vendored

@ -29,6 +29,8 @@ namespace mod_feedback\external;
use externallib_advanced_testcase;
use feedback_item_multichoice;
use mod_feedback_external;
use moodle_exception;
use required_capability_exception;
defined('MOODLE_INTERNAL') || die();
@ -45,6 +47,7 @@ require_once($CFG->dirroot . '/mod/feedback/lib.php');
* @copyright 2017 Juan Leyva <juan@moodle.com>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @since Moodle 3.3
* @covers \mod_feedback_external
*/
class external_test extends externallib_advanced_testcase {
@ -374,6 +377,130 @@ class external_test extends externallib_advanced_testcase {
}
}
/**
* Test get_items, to confirm validation is done too.
*
* @dataProvider items_provider
* @param string $role Whether the current user should be a student or a teacher.
* @param array $info Settings to create the feedback.
* @param string|null $warning The warning message to display or null if warnings result is empty.
*/
public function test_get_items_validation(string $role, array $info, ?string $warning): void {
global $DB;
// Test user with full capabilities.
if ($role === 'teacher') {
$this->setUser($this->teacher);
} else {
$this->setUser($this->student);
}
// Create the feedback.
$data = ['course' => $this->course->id];
if (array_key_exists('closed', $info) && $info['closed']) {
$data['timeopen'] = time() + DAYSECS;
}
$feedback = $this->getDataGenerator()->create_module('feedback', $data);
$empty = true;
if (!array_key_exists('empty', $info) || !$info['empty']) {
$empty = false;
/** @var \mod_feedback_generator $feedbackgenerator */
$feedbackgenerator = $this->getDataGenerator()->get_plugin_generator('mod_feedback');
// Add,at least, one item to the feedback.
$feedbackgenerator->create_item_label($feedback);
}
if (array_key_exists('complete', $info) && !$info['complete']) {
$studentrole = $DB->get_record('role', array('shortname' => 'student'));
$coursecontext = \context_course::instance($this->course->id);
assign_capability('mod/feedback:complete', CAP_PROHIBIT, $studentrole->id, $coursecontext->id);
// Empty all the caches that may be affected by this change.
accesslib_clear_all_caches_for_unit_testing();
\course_modinfo::clear_instance_cache();
}
$result = mod_feedback_external::get_items($feedback->id);
$result = \external_api::clean_returnvalue(mod_feedback_external::get_items_returns(), $result);
if ($warning) {
$this->assertEmpty($result['items']);
$this->assertCount(1, $result['warnings']);
$resultwarning = reset($result['warnings']);
if ($warning == 'required_capability_exception') {
$this->assertStringContainsString('error/nopermission', $resultwarning['message']);
} else {
$this->assertEquals($warning, $resultwarning['message']);
}
} else {
if ($empty) {
$this->assertEmpty($result['items']);
} else {
$this->assertCount(1, $result['items']);
}
$this->assertEmpty($result['warnings']);
}
}
/**
* Data provider for test_get_items_validation() and test_get_page_items_validation().
*
* @return array
*/
public function items_provider(): array {
return [
'Valid feedback (as student)' => [
'role' => 'student',
'info' => [],
'warning' => null,
],
'Closed feedback (as student)' => [
'role' => 'student',
'info' => ['closed' => true],
'warning' => get_string('feedback_is_not_open', 'feedback'),
],
'Empty feedback (as student)' => [
'role' => 'student',
'info' => ['empty' => true],
'warning' => get_string('no_items_available_yet', 'feedback'),
],
'Closed feedback (as student)' => [
'role' => 'student',
'info' => ['closed' => true],
'warning' => get_string('feedback_is_not_open', 'feedback'),
],
'Cannot complete feedback (as student)' => [
'role' => 'student',
'info' => ['complete' => false],
'warning' => 'required_capability_exception',
],
'Valid feedback (as teacher)' => [
'role' => 'teacher',
'info' => [],
'warning' => null,
],
'Closed feedback (as teacher)' => [
'role' => 'teacher',
'info' => ['closed' => true],
'warning' => null,
],
'Empty feedback (as teacher)' => [
'role' => 'teacher',
'info' => ['empty' => true],
'warning' => null,
],
'Closed feedback (as teacher)' => [
'role' => 'teacher',
'info' => ['closed' => true],
'warning' => null,
],
'Cannot complete feedback (as teacher)' => [
'role' => 'teacher',
'info' => ['complete' => false],
'warning' => null,
],
];
}
/**
* Test launch_feedback.
*/
@ -454,6 +581,70 @@ class external_test extends externallib_advanced_testcase {
$this->assertTrue($result['hasprevpage']);
}
/**
* Test get_page_items, to confirm validation is done too.
*
* @dataProvider items_provider
* @param string $role Whether the current user should be a student or a teacher.
* @param array $info Settings to create the feedback.
* @param string|null $warning The warning message to display or null if warnings result is empty.
*/
public function test_get_page_items_validation(string $role, array $info, ?string $warning): void {
global $DB;
// Test user with full capabilities.
if ($role === 'teacher') {
$this->setUser($this->teacher);
} else {
$this->setUser($this->student);
}
// Create the feedback.
$data = ['course' => $this->course->id];
if (array_key_exists('closed', $info) && $info['closed']) {
$data['timeopen'] = time() + DAYSECS;
}
$feedback = $this->getDataGenerator()->create_module('feedback', $data);
$empty = true;
if (!array_key_exists('empty', $info) || !$info['empty']) {
$empty = false;
/** @var \mod_feedback_generator $feedbackgenerator */
$feedbackgenerator = $this->getDataGenerator()->get_plugin_generator('mod_feedback');
// Add,at least, one item to the feedback.
$feedbackgenerator->create_item_label($feedback);
}
if (array_key_exists('complete', $info) && !$info['complete']) {
$studentrole = $DB->get_record('role', array('shortname' => 'student'));
$coursecontext = \context_course::instance($this->course->id);
assign_capability('mod/feedback:complete', CAP_PROHIBIT, $studentrole->id, $coursecontext->id);
// Empty all the caches that may be affected by this change.
accesslib_clear_all_caches_for_unit_testing();
\course_modinfo::clear_instance_cache();
}
$result = mod_feedback_external::get_page_items($feedback->id, 0);
$result = \external_api::clean_returnvalue(mod_feedback_external::get_items_returns(), $result);
if ($warning) {
$this->assertEmpty($result['items']);
$this->assertCount(1, $result['warnings']);
$resultwarning = reset($result['warnings']);
if ($warning == 'required_capability_exception') {
$this->assertStringContainsString('error/nopermission', $resultwarning['message']);
} else {
$this->assertEquals($warning, $resultwarning['message']);
}
} else {
if ($empty) {
$this->assertEmpty($result['items']);
} else {
$this->assertCount(1, $result['items']);
}
$this->assertEmpty($result['warnings']);
}
}
/**
* Test process_page.
*/

4
mod/feedback/upgrade.txt

@ -1,3 +1,7 @@
=== 4.1.2 ===
* The external methods get_items() and get_page_items() now only return the items when the user can access the feedback; if the
user can't see them, a warning message will be returned, with the reasons.
=== 4.0 ===
* The following files and classes within them have been deprecated in favour of dynamic forms:
* edit_form.php