From 052007b7324ef24aebe36a876ffa4fa97fac4f81 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Fri, 6 Dec 2024 16:38:18 +0000 Subject: [PATCH] MDL-65356 mod_data: update entry editing/deletion to not leak sesskey. --- mod/data/classes/template.php | 2 -- mod/data/edit.php | 5 ----- mod/data/tests/template_test.php | 4 ++-- mod/data/view.php | 15 ++++++++++----- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/mod/data/classes/template.php b/mod/data/classes/template.php index 8bde97eeda8..f53f4e3f884 100644 --- a/mod/data/classes/template.php +++ b/mod/data/classes/template.php @@ -801,7 +801,6 @@ class template { $editurl = new moodle_url('/mod/data/edit.php', $this->baseurl->params()); $editurl->params([ 'rid' => $entry->id, - 'sesskey' => sesskey(), 'backto' => urlencode($backurl->out(false)) ]); @@ -814,7 +813,6 @@ class template { // Delete entry. $deleteurl = new moodle_url($this->baseurl, [ 'delete' => $entry->id, - 'sesskey' => sesskey(), 'mode' => 'single', ]); diff --git a/mod/data/edit.php b/mod/data/edit.php index 9b72fcc71d0..454c1121498 100644 --- a/mod/data/edit.php +++ b/mod/data/edit.php @@ -80,11 +80,6 @@ if ($manager->can_manage_templates()) { } } -if ($rid) { - // When editing an existing record, we require the session key. - require_sesskey(); -} - // Get Group information for permission testing and record creation. $currentgroup = groups_get_activity_group($cm); $groupmode = groups_get_activity_groupmode($cm); diff --git a/mod/data/tests/template_test.php b/mod/data/tests/template_test.php index ac9ecfe1347..9ac4df78b25 100644 --- a/mod/data/tests/template_test.php +++ b/mod/data/tests/template_test.php @@ -407,7 +407,7 @@ final class template_test extends \advanced_testcase { ], 'Teacher actionsmenu tag with default options' => [ 'templatecontent' => 'Some ##actionsmenu## tag', - 'expected' => '|Some .*edit.*{entryid}.*sesskey.*Edit.* .*delete.*{entryid}.*sesskey.*Delete.* tag|', + 'expected' => '|Some .*edit.*{entryid}.*Edit.* .*delete.*{entryid}.*Delete.* tag|', 'rolename' => 'editingteacher', ], 'Teacher actionsmenu tag with default options (check Show more is not there)' => [ @@ -723,7 +723,7 @@ final class template_test extends \advanced_testcase { ], 'Student actionsmenu tag with default options' => [ 'templatecontent' => 'Some ##actionsmenu## tag', - 'expected' => '|Some .*edit.*{entryid}.*sesskey.*Edit.* .*delete.*{entryid}.*sesskey.*Delete.* tag|', + 'expected' => '|Some .*edit.*{entryid}.*Edit.* .*delete.*{entryid}.*Delete.* tag|', 'rolename' => 'student', ], 'Student actionsmenu tag with default options (check Show more is not there)' => [ diff --git a/mod/data/view.php b/mod/data/view.php index 1cff81f5d23..b5624b888d9 100644 --- a/mod/data/view.php +++ b/mod/data/view.php @@ -243,7 +243,7 @@ if (!empty(trim($search))) { $PAGE->set_title(implode(moodle_page::TITLE_SEPARATOR, $titleparts)); $PAGE->set_heading($course->fullname); $PAGE->force_settings_menu(true); -if ($delete && confirm_sesskey() && (data_user_can_manage_entry($delete, $data, $context))) { +if ($delete && data_user_can_manage_entry($delete, $data, $context)) { $PAGE->activityheader->disable(); } @@ -284,8 +284,10 @@ if ($data->intro and empty($page) and empty($record) and $mode != 'single') { /// Delete any requested records -if ($delete && confirm_sesskey() && (data_user_can_manage_entry($delete, $data, $context))) { +if ($delete && data_user_can_manage_entry($delete, $data, $context)) { if ($confirm) { + require_sesskey(); + if (data_delete_record($delete, $data, $course->id, $cm->id)) { echo $OUTPUT->notification(get_string('recorddeleted','data'), 'notifysuccess'); } @@ -324,8 +326,10 @@ if ($serialdelete) { $multidelete = json_decode($serialdelete); } -if ($multidelete && confirm_sesskey() && $canmanageentries) { - if ($confirm = optional_param('confirm', 0, PARAM_INT)) { +if ($multidelete && $canmanageentries) { + if ($confirm) { + require_sesskey(); + foreach ($multidelete as $value) { data_delete_record($value, $data, $course->id, $cm->id); } @@ -373,7 +377,8 @@ if ($showactivity) { // Approve or disapprove any requested records $approvecap = has_capability('mod/data:approve', $context); - if (($approve || $disapprove) && confirm_sesskey() && $approvecap) { + if (($approve || $disapprove) && $approvecap) { + require_sesskey(); $newapproved = $approve ? true : false; $recordid = $newapproved ? $approve : $disapprove; if ($approverecord = $DB->get_record('data_records', array('id' => $recordid))) { // Need to check this is valid