From 977f196c11f27e82c8f8b413b8a00b716325fbcd Mon Sep 17 00:00:00 2001 From: Ferran Recio Date: Thu, 21 Apr 2022 13:43:34 +0200 Subject: [PATCH] MDL-71623 core_search: fix user document visibility --- search/classes/document.php | 24 +++-- search/tests/document_test.php | 185 +++++++++++++++++++++++++++++---- 2 files changed, 183 insertions(+), 26 deletions(-) diff --git a/search/classes/document.php b/search/classes/document.php index e7dc914326b..d7cf6709d1e 100644 --- a/search/classes/document.php +++ b/search/classes/document.php @@ -24,6 +24,8 @@ namespace core_search; +use context; + defined('MOODLE_INTERNAL') || die(); /** @@ -613,7 +615,10 @@ class document implements \renderable, \templatable { * @return array */ public function export_for_template(\renderer_base $output) { + global $USER; + list($componentname, $areaname) = \core_search\manager::extract_areaid_parts($this->get('areaid')); + $context = context::instance_by_id($this->get('contextid')); $searcharea = \core_search\manager::get_search_area($this->data['areaid']); $title = $this->is_set('title') ? $this->format_text($searcharea->get_document_display_title($this)) : ''; @@ -621,7 +626,7 @@ class document implements \renderable, \templatable { 'componentname' => $componentname, 'areaname' => $areaname, 'courseurl' => course_get_url($this->get('courseid')), - 'coursefullname' => format_string($this->get('coursefullname'), true, array('context' => $this->get('contextid'))), + 'coursefullname' => format_string($this->get('coursefullname'), true, ['context' => $context->id]), 'modified' => userdate($this->get('modified')), 'title' => ($title !== '') ? $title : get_string('notitle', 'search'), 'docurl' => $this->get_doc_url(), @@ -635,21 +640,28 @@ class document implements \renderable, \templatable { $files = $this->get_files(); if (!empty($files)) { if (count($files) > 1) { - $filenames = array(); + $filenames = []; foreach ($files as $file) { - $filenames[] = format_string($file->get_filename(), true, array('context' => $this->get('contextid'))); + $filenames[] = format_string($file->get_filename(), true, ['context' => $context->id]); } $data['multiplefiles'] = true; $data['filenames'] = $filenames; } else { $file = reset($files); - $data['filename'] = format_string($file->get_filename(), true, array('context' => $this->get('contextid'))); + $data['filename'] = format_string($file->get_filename(), true, ['context' => $context->id]); } } if ($this->is_set('userid')) { - $data['userurl'] = new \moodle_url('/user/view.php', array('id' => $this->get('userid'), 'course' => $this->get('courseid'))); - $data['userfullname'] = format_string($this->get('userfullname'), true, array('context' => $this->get('contextid'))); + if ($this->get('userid') == $USER->id || + (has_capability('moodle/user:viewdetails', $context) && + has_capability('moodle/course:viewparticipants', $context))) { + $data['userurl'] = new \moodle_url( + '/user/view.php', + ['id' => $this->get('userid'), 'course' => $this->get('courseid')] + ); + $data['userfullname'] = format_string($this->get('userfullname'), true, ['context' => $context->id]); + } } if ($docicon = $this->get_doc_icon()) { diff --git a/search/tests/document_test.php b/search/tests/document_test.php index c60acba18e4..dcf45e87d4e 100644 --- a/search/tests/document_test.php +++ b/search/tests/document_test.php @@ -14,19 +14,14 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Search document unit tests. - * - * @package core_search - * @category phpunit - * @copyright 2016 Eric Merrill {@link http://www.merrilldigital.com} - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ +namespace core_search; -defined('MOODLE_INTERNAL') || die(); - -require_once(__DIR__ . '/fixtures/testable_core_search.php'); -require_once(__DIR__ . '/fixtures/mock_search_area.php'); +use advanced_testcase; +use context_course; +use core_mocksearch\search\mock_search_area; +use mock_search\engine; +use testable_core_search; +use stdClass; /** * Unit tests for search document. @@ -35,8 +30,18 @@ require_once(__DIR__ . '/fixtures/mock_search_area.php'); * @category phpunit * @copyright 2016 Eric Merrill {@link http://www.merrilldigital.com} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @coversDefaultClass \core_search\document */ -class search_document_testcase extends advanced_testcase { +class document_test extends advanced_testcase { + + /** + * Setup to ensure that fixtures are loaded. + */ + public static function setupBeforeClass(): void { + global $CFG; + require_once($CFG->dirroot . '/search/tests/fixtures/testable_core_search.php'); + require_once($CFG->dirroot . '/search/tests/fixtures/mock_search_area.php'); + } /** * @var Instace of core_search_generator. @@ -57,22 +62,23 @@ class search_document_testcase extends advanced_testcase { /** * Adding this test here as get_areas_user_accesses process is the same, results just depend on the context level. * + * @covers ::export_for_template * @return void */ public function test_search_user_accesses() { - global $DB, $PAGE; + global $PAGE; - $area = new \core_mocksearch\search\mock_search_area(); + $area = new mock_search_area(); $renderer = $PAGE->get_renderer('core_search'); - $engine = new \mock_search\engine(); + $engine = new engine(); - $course = $this->getDataGenerator()->create_course(array('fullname' => 'Course & Title')); - $coursectx = context_course::instance($course->id); - $user = $this->getDataGenerator()->create_user(array('firstname' => 'User', 'lastname' => 'Escape & Name')); + $course = $this->getDataGenerator()->create_course(['fullname' => 'Course & Title']); + $user = $this->getDataGenerator()->create_user(['firstname' => 'User', 'lastname' => 'Escape & Name']); $this->getDataGenerator()->enrol_user($user->id, $course->id, 'teacher'); + $this->setAdminUser(); // Make a record to enter in the search area. - $record = new \stdClass(); + $record = new stdClass(); $record->title = 'Escape & Title'; $record->content = 'Escape & Content'; $record->description1 = 'Escape & Description1'; @@ -105,6 +111,8 @@ class search_document_testcase extends advanced_testcase { /** * Test we can set and get document icon. + * + * @covers ::set_doc_icon */ public function test_get_and_set_doc_icon() { $document = $this->getMockBuilder('\core_search\document') @@ -127,4 +135,141 @@ class search_document_testcase extends advanced_testcase { $this->generator = null; } } + + /** + * Test the document author visibility depending on the user capabilities. + * + * @covers ::export_for_template + * @dataProvider document_author_visibility_provider + * @param string $rolename the role name + * @param array $capexceptions the capabilities exceptions + * @param bool $expected the expected author visibility + * @param bool $owndocument if the resulting document belongs to the current user + */ + public function test_document_author_visibility( + string $rolename = 'editingteacher', + array $capexceptions = [], + bool $expected = true, + bool $owndocument = false + ) { + global $DB, $PAGE; + + $area = new mock_search_area(); + $renderer = $PAGE->get_renderer('core_search'); + $engine = new engine(); + + $course = $this->getDataGenerator()->create_course(['fullname' => 'Course & Title']); + $context = context_course::instance($course->id); + + $roleid = $DB->get_field('role', 'id', ['shortname' => $rolename]); + foreach ($capexceptions as $capability) { + assign_capability($capability, CAP_PROHIBIT, $roleid, $context->id); + } + + $user = $this->getDataGenerator()->create_user(['firstname' => 'Test', 'lastname' => 'User']); + $this->getDataGenerator()->enrol_user($user->id, $course->id, $rolename); + $this->setUser($user); + + if ($owndocument) { + $author = $user; + } else { + $author = $this->getDataGenerator()->create_user(['firstname' => 'User', 'lastname' => 'Escape & Name']); + $this->getDataGenerator()->enrol_user($author->id, $course->id, 'student'); + } + + // Make a record to enter in the search area. + $record = new stdClass(); + $record->title = 'Escape & Title'; + $record->content = 'Escape & Content'; + $record->description1 = 'Escape & Description1'; + $record->description2 = 'Escape & Description2'; + $record->userid = $author->id; + $record->courseid = $course->id; + $record->contextid = $context->id; + $record = $this->generator->create_record($record); + + // Convert to a 'doc data' type format. + $docdata = $area->convert_record_to_doc_array($record); + + // First see that the document has the user information. + $doc = $engine->to_document($area, $docdata); + $this->assertEquals(fullname($author), $doc->get('userfullname')); + + // Export for template, and see if it the user information is exported. + $export = $doc->export_for_template($renderer); + + if ($expected) { + $authorname = htmlentities(fullname($author)); + $this->assertEquals($authorname, $export['userfullname']); + } else { + $this->assertArrayNotHasKey('userfullname', $export); + } + } + + /** + * Data provider for test_document_author_visibility(). + * + * @return array + */ + public function document_author_visibility_provider(): array { + return [ + 'Teacher' => [ + 'rolename' => 'editingteacher', + 'capexceptions' => [], + 'expected' => true, + 'owndocument' => false, + ], + 'Non editing teacher' => [ + 'rolename' => 'teacher', + 'capexceptions' => [], + 'expected' => true, + 'owndocument' => false, + ], + 'Student' => [ + 'rolename' => 'student', + 'capexceptions' => [], + 'expected' => true, + 'owndocument' => false, + ], + // Adding capability exceptions. + 'Student without view profiles' => [ + 'rolename' => 'student', + 'capexceptions' => ['moodle/user:viewdetails'], + 'expected' => false, + 'owndocument' => false, + ], + 'Student without view participants' => [ + 'rolename' => 'student', + 'capexceptions' => ['moodle/course:viewparticipants'], + 'expected' => false, + 'owndocument' => false, + ], + 'Student without view participants or profiles' => [ + 'rolename' => 'student', + 'capexceptions' => ['moodle/user:viewdetails', 'moodle/course:viewparticipants'], + 'expected' => false, + 'owndocument' => false, + ], + // Users should be able to see its own documents. + 'Student author without view profiles' => [ + 'rolename' => 'student', + 'capexceptions' => ['moodle/user:viewdetails'], + 'expected' => true, + 'owndocument' => true, + ], + 'Student author without view participants' => [ + 'rolename' => 'student', + 'capexceptions' => ['moodle/course:viewparticipants'], + 'expected' => true, + 'owndocument' => true, + ], + 'Student author without view participants or profiles' => [ + 'rolename' => 'student', + 'capexceptions' => ['moodle/user:viewdetails', 'moodle/course:viewparticipants'], + 'expected' => true, + 'owndocument' => true, + ], + + ]; + } }