From b5794b94483ccf5b92596d693be1253aadc28c36 Mon Sep 17 00:00:00 2001 From: Markus Heck Date: Mon, 3 Jun 2024 15:30:38 +0200 Subject: [PATCH] apply attempt id parameter validation improvements to processattempt_page.php --- .../output/pages/processattempt_page.php | 8 ++- classes/local/output/pages/view_page.php | 68 ++++++++++++------- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/classes/local/output/pages/processattempt_page.php b/classes/local/output/pages/processattempt_page.php index c960b79..53873d3 100644 --- a/classes/local/output/pages/processattempt_page.php +++ b/classes/local/output/pages/processattempt_page.php @@ -7,6 +7,7 @@ use dml_exception; use dml_transaction_exception; use local_logging\logger; +use mod_adleradaptivity\local\db\moodle_core_repository; use moodle_exception; use moodle_url; use question_engine; @@ -22,6 +23,7 @@ * This page does not render anything, it only processes the attempt and redirects to the view page. */ class processattempt_page { + private moodle_core_repository $moodle_core_repository; private logger $logger; private int $time_now; @@ -71,20 +73,22 @@ private function process_attempt(question_usage_by_activity $quba, stdClass $cou private function setup_instance_variables(): void { $this->logger = new logger('mod_adleradaptiviy', 'processattempt'); $this->time_now = time(); # Saving time at request start to use the actual submission time + $this->moodle_core_repository = new moodle_core_repository(); } /** * @return array An array containing the course module, course and question usage by activity * @throws coding_exception * @throws dml_exception + * @throws moodle_exception */ private function process_request_parameters(): array { $cmid = optional_param('id', 0, PARAM_INT); - $attempt_id = required_param('attempt', PARAM_INT); + $attempt_id = view_page::get_attempt_id_param(); $cm = get_coursemodule_from_id('adleradaptivity', $cmid, 0, false, MUST_EXIST); $course = get_course($cm->course); - $quba = question_engine::load_questions_usage_by_activity($attempt_id); + $quba = view_page::get_question_usage_by_attempt_id_with_cm_verification($attempt_id, $cm,$this->moodle_core_repository); return array($cm, $course, $quba); } diff --git a/classes/local/output/pages/view_page.php b/classes/local/output/pages/view_page.php index 26826da..3338079 100644 --- a/classes/local/output/pages/view_page.php +++ b/classes/local/output/pages/view_page.php @@ -168,17 +168,7 @@ private function load_or_create_question_usage_by_attempt_id(int|null $attempt_i } else { // Load the attempt $this->logger->trace('Loading existing attempt. Attempt ID: ' . $attempt_id); - try { - $cmid_of_question_usage = $this->moodle_core_repository->get_cmid_by_question_usage_id($attempt_id); - } catch (dml_missing_record_exception $e) { - throw new invalid_parameter_exception('Specified attempt does not exist. Attempt ID: ' . $attempt_id); - } - if ($cm->id == $cmid_of_question_usage) { - // can only happen if attempt id was specified, otherwise only a valid one will be loaded - $quba = question_engine::load_questions_usage_by_activity($attempt_id); - } else { - throw new invalid_parameter_exception('Specified attempt is not for this cm. Attempt ID: ' . $attempt_id); - } + $quba = self::get_question_usage_by_attempt_id_with_cm_verification($attempt_id, $cm, $this->moodle_core_repository); } return $quba; } @@ -195,18 +185,7 @@ private function process_request_parameters(): array { throw new moodle_exception('invalidcoursemodule', 'adleradaptivity'); } - $attempt_id = optional_param('attempt', null, PARAM_RAW); - if ($attempt_id !== null && - !is_int($attempt_id) && - !(is_string($attempt_id) && ctype_digit($attempt_id))) { - throw new moodle_exception('invalidattemptid', 'adleradaptivity'); - } - if ($attempt_id !== null) { - $attempt_id = intval($attempt_id); - if ($attempt_id < 0) { - throw new moodle_exception('invalidattemptid', 'adleradaptivity'); - } - } + $attempt_id = self::get_attempt_id_param(); $cm = moodle_core::get_coursemodule_from_id('adleradaptivity', $cmid, 0, false, MUST_EXIST); $course = moodle_core::get_course($cm->course); @@ -312,4 +291,47 @@ private function check_basic_permissions(stdClass $course, stdClass $cm, context require_login($course, false, $cm); require_capability('mod/adleradaptivity:view', $module_context); } + + /** + * @return int|null + * @throws coding_exception + * @throws moodle_exception + */ + public static function get_attempt_id_param(): int|null { + $attempt_id = optional_param('attempt', null, PARAM_RAW); + if ($attempt_id !== null && + !is_int($attempt_id) && + !(is_string($attempt_id) && ctype_digit($attempt_id))) { + throw new moodle_exception('invalidattemptid', 'adleradaptivity'); + } + if ($attempt_id !== null) { + $attempt_id = intval($attempt_id); + if ($attempt_id < 0) { + throw new moodle_exception('invalidattemptid', 'adleradaptivity'); + } + } + return $attempt_id; + } + + /** + * @param int $attempt_id + * @param stdClass $cm + * @return question_usage_by_activity + * @throws dml_exception + * @throws invalid_parameter_exception + */ + public static function get_question_usage_by_attempt_id_with_cm_verification(int $attempt_id, stdClass $cm, moodle_core_repository $moodle_core_repository): question_usage_by_activity { + try { + $cmid_of_question_usage = $moodle_core_repository->get_cmid_by_question_usage_id($attempt_id); + } catch (dml_missing_record_exception $e) { + throw new invalid_parameter_exception('Specified attempt does not exist. Attempt ID: ' . $attempt_id); + } + if ($cm->id == $cmid_of_question_usage) { + // can only happen if attempt id was specified, otherwise only a valid one will be loaded + $quba = question_engine::load_questions_usage_by_activity($attempt_id); + } else { + throw new invalid_parameter_exception('Specified attempt is not for this cm. Attempt ID: ' . $attempt_id); + } + return $quba; + } }