From 96d8569faeb4a44bfcf86fbec2ad07bd04eda711 Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Fri, 17 May 2024 17:10:22 +1000 Subject: [PATCH] fix: permission check for uploads to be in context the f2f session pair --- classes/booking_manager.php | 10 ++++++++++ lang/en/facetoface.php | 3 ++- tests/upload_test.php | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/classes/booking_manager.php b/classes/booking_manager.php index ee166aa..37644d0 100644 --- a/classes/booking_manager.php +++ b/classes/booking_manager.php @@ -199,6 +199,16 @@ public function validate(): array { if ($session) { $timenow = time(); + // If the session supplied does not link to the face-to-face module expected, then it's invalid. + if ($session->facetoface != $this->f) { + $errors[] = [ + $row, + new lang_string('error:tryingtoupdatesessionfromanothermodule', 'mod_facetoface', (object) [ + 'session' => $entry->session, + 'f' => $this->f, + ]), + ]; + } // Don't allow user to cancel a session that has already occurred. if ($entry->status === 'cancelled' && facetoface_has_session_started($session, $timenow)) { $errors[] = [$row, new lang_string('error:sessionalreadystarted', 'mod_facetoface', $entry->session)]; diff --git a/lang/en/facetoface.php b/lang/en/facetoface.php index b35488a..284a140 100644 --- a/lang/en/facetoface.php +++ b/lang/en/facetoface.php @@ -142,7 +142,8 @@ $string['email:subject'] = 'Subject'; $string['emptylocation'] = 'Location was empty'; $string['enrolled'] = 'enrolled'; -$string['error:cannotloadfile'] = "Unable to load the file, please check the file and try again."; +$string['error:tryingtoupdatesessionfromanothermodule'] = 'Unable to update session id {$a->session} which belongs to another Face-to-Face activity with id {$a->f}.'; +$string['error:cannotloadfile'] = 'Unable to load the file, please check the file and try again.'; $string['error:invalidstatusspecified'] = "Invalid status specified. Expecting 'booked', 'cancelled', 'no_show', 'partially_attended', or 'fully_attended'. Defaults to 'booked' if empty. Value provided was '{\$a}'"; $string['error:invalidnotificationtypespecified'] = "Invalid notification type specified. Expecting 'ical', 'email', 'both', or '', but actual was '{\$a}'"; $string['error:sessionalreadystarted'] = 'Unable to use session {$a}, as it which has already started.'; diff --git a/tests/upload_test.php b/tests/upload_test.php index f4be435..d4cf0f6 100644 --- a/tests/upload_test.php +++ b/tests/upload_test.php @@ -132,7 +132,10 @@ public function test_user_validation() { $generator = $this->getDataGenerator()->get_plugin_generator('mod_facetoface'); $course = $this->getDataGenerator()->create_course(); + $anothercourse = $this->getDataGenerator()->create_course(); $facetoface = $generator->create_instance(['course' => $course->id]); + $anotherfacetoface = $generator->create_instance(['course' => $anothercourse->id]); + // Generate users. $user = $this->getDataGenerator()->create_user(); $student = $this->getDataGenerator()->create_and_enrol($course, 'student'); @@ -153,6 +156,20 @@ public function test_user_validation() { ], ]); + $remotesession = $generator->create_session([ + 'facetoface' => $anotherfacetoface->id, + 'capacity' => '3', + 'allowoverbook' => '0', + 'details' => 'xyz', + 'duration' => '1.5', // One and half hours. + 'normalcost' => '111', + 'discountcost' => '11', + 'allowcancellations' => '0', + 'sessiondates' => [ + ['timestart' => $now + 3 * DAYSECS, 'timefinish' => $now + 2 * DAYSECS], + ], + ]); + $bm = new booking_manager($facetoface->id); $records = [ @@ -188,6 +205,14 @@ public function test_user_validation() { 'notificationtype' => 'phone', 'discountcode' => '', ], + // Test permissions (e.g. user not able to upload/process for a f2f activity loaded). + (object) [ + 'email' => $student->email, + 'session' => $remotesession->id, + 'status' => '', + 'notificationtype' => '', + 'discountcode' => '', + ], ]; $bm->load_from_array($records); @@ -237,6 +262,18 @@ public function test_user_validation() { ), 'Expecting status error, since the status should be either booked or cancelled.' ); + + $this->assertTrue( + $this->check_row_validation_error_exists( + $errors, + 5, + new lang_string('error:tryingtoupdatesessionfromanothermodule', 'mod_facetoface', (object) [ + 'session' => $remotesession->id, + 'f' => $facetoface->id, + ]) + ), + 'Expecting permission check conflict due to session->facetoface + facetoface id mismatcherror.' + ); } /**