From cbae8dcd57967e820209544fb8ceae79ce964542 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 20 Sep 2018 11:03:35 +0800 Subject: [PATCH] MDL-63401 tool_dataprivacy: Move cap checks to endpoints from API --- admin/tool/dataprivacy/classes/api.php | 63 +-- admin/tool/dataprivacy/classes/external.php | 43 +- admin/tool/dataprivacy/createdatarequest.php | 9 +- admin/tool/dataprivacy/tests/api_test.php | 421 ++++++++------ .../tool/dataprivacy/tests/external_test.php | 530 ++++++++++++++---- 5 files changed, 724 insertions(+), 342 deletions(-) diff --git a/admin/tool/dataprivacy/classes/api.php b/admin/tool/dataprivacy/classes/api.php index 7e5f35057ac..3f7fb5e825e 100644 --- a/admin/tool/dataprivacy/classes/api.php +++ b/admin/tool/dataprivacy/classes/api.php @@ -244,14 +244,6 @@ class api { if (self::is_site_dpo($requestinguser)) { // The user making the request is a DPO. Should be fine. $datarequest->set('dpo', $requestinguser); - } else { - // If not a DPO, only users with the capability to make data requests for the user should be allowed. - // (e.g. users with the Parent role, etc). - if (!self::can_create_data_request_for_user($foruser)) { - $forusercontext = \context_user::instance($foruser); - throw new required_capability_exception($forusercontext, - 'tool/dataprivacy:makedatarequestsforchildren', 'nopermissions', ''); - } } } // The user making the request. @@ -667,16 +659,31 @@ class api { /** * Checks whether a non-DPO user can make a data request for another user. * - * @param int $user The user ID of the target user. - * @param int $requester The user ID of the user making the request. - * @return bool - * @throws coding_exception + * @param int $user The user ID of the target user. + * @param int $requester The user ID of the user making the request. + * @return bool */ public static function can_create_data_request_for_user($user, $requester = null) { $usercontext = \context_user::instance($user); + return has_capability('tool/dataprivacy:makedatarequestsforchildren', $usercontext, $requester); } + /** + * Require that the current user can make a data request for the specified other user. + * + * @param int $user The user ID of the target user. + * @param int $requester The user ID of the user making the request. + * @return bool + */ + public static function require_can_create_data_request_for_user($user, $requester = null) { + $usercontext = \context_user::instance($user); + + require_capability('tool/dataprivacy:makedatarequestsforchildren', $usercontext, $requester); + + return true; + } + /** * Checks whether a user can download a data request. * @@ -732,8 +739,6 @@ class api { * @return \tool_dataprivacy\purpose. */ public static function create_purpose(stdClass $record) { - self::check_can_manage_data_registry(); - $purpose = new purpose(0, $record); $purpose->create(); @@ -747,8 +752,6 @@ class api { * @return \tool_dataprivacy\purpose. */ public static function update_purpose(stdClass $record) { - self::check_can_manage_data_registry(); - if (!isset($record->sensitivedatareasons)) { $record->sensitivedatareasons = ''; } @@ -768,8 +771,6 @@ class api { * @return bool */ public static function delete_purpose($id) { - self::check_can_manage_data_registry(); - $purpose = new purpose($id); if ($purpose->is_used()) { throw new \moodle_exception('Purpose with id ' . $id . ' can not be deleted because it is used.'); @@ -783,8 +784,6 @@ class api { * @return \tool_dataprivacy\purpose[] */ public static function get_purposes() { - self::check_can_manage_data_registry(); - return purpose::get_records([], 'name', 'ASC'); } @@ -795,8 +794,6 @@ class api { * @return \tool_dataprivacy\category. */ public static function create_category(stdClass $record) { - self::check_can_manage_data_registry(); - $category = new category(0, $record); $category->create(); @@ -810,8 +807,6 @@ class api { * @return \tool_dataprivacy\category. */ public static function update_category(stdClass $record) { - self::check_can_manage_data_registry(); - $category = new category($record->id); $category->from_record($record); @@ -827,8 +822,6 @@ class api { * @return bool */ public static function delete_category($id) { - self::check_can_manage_data_registry(); - $category = new category($id); if ($category->is_used()) { throw new \moodle_exception('Category with id ' . $id . ' can not be deleted because it is used.'); @@ -842,8 +835,6 @@ class api { * @return \tool_dataprivacy\category[] */ public static function get_categories() { - self::check_can_manage_data_registry(); - return category::get_records([], 'name', 'ASC'); } @@ -854,8 +845,6 @@ class api { * @return \tool_dataprivacy\context_instance */ public static function set_context_instance($record) { - self::check_can_manage_data_registry($record->contextid); - if ($instance = context_instance::get_record_by_contextid($record->contextid, false)) { // Update. $instance->from_record($record); @@ -882,7 +871,6 @@ class api { * @return null */ public static function unset_context_instance(context_instance $instance) { - self::check_can_manage_data_registry($instance->get('contextid')); $instance->delete(); } @@ -896,9 +884,6 @@ class api { public static function set_contextlevel($record) { global $DB; - // Only manager at system level can set this. - self::check_can_manage_data_registry(); - if ($record->contextlevel != CONTEXT_SYSTEM && $record->contextlevel != CONTEXT_USER) { throw new \coding_exception('Only context system and context user can set a contextlevel ' . 'purpose and retention'); @@ -930,7 +915,6 @@ class api { * @return category|false */ public static function get_effective_context_category(\context $context, $forcedvalue=false) { - self::check_can_manage_data_registry($context->id); if (!data_registry::defaults_set()) { return false; } @@ -946,7 +930,6 @@ class api { * @return purpose|false */ public static function get_effective_context_purpose(\context $context, $forcedvalue=false) { - self::check_can_manage_data_registry($context->id); if (!data_registry::defaults_set()) { return false; } @@ -962,7 +945,6 @@ class api { * @return category|false */ public static function get_effective_contextlevel_category($contextlevel, $forcedvalue=false) { - self::check_can_manage_data_registry(\context_system::instance()->id); if (!data_registry::defaults_set()) { return false; } @@ -978,7 +960,6 @@ class api { * @return purpose|false */ public static function get_effective_contextlevel_purpose($contextlevel, $forcedvalue=false) { - self::check_can_manage_data_registry(\context_system::instance()->id); if (!data_registry::defaults_set()) { return false; } @@ -993,8 +974,6 @@ class api { * @return \tool_dataprivacy\expired_context */ public static function create_expired_context($contextid) { - self::check_can_manage_data_registry(); - $record = (object)[ 'contextid' => $contextid, 'status' => expired_context::STATUS_EXPIRED, @@ -1012,8 +991,6 @@ class api { * @return bool True on success. */ public static function delete_expired_context($id) { - self::check_can_manage_data_registry(); - $expiredcontext = new expired_context($id); return $expiredcontext->delete(); } @@ -1026,8 +1003,6 @@ class api { * @return null */ public static function set_expired_context_status(expired_context $expiredctx, $status) { - self::check_can_manage_data_registry(); - $expiredctx->set('status', $status); $expiredctx->save(); } diff --git a/admin/tool/dataprivacy/classes/external.php b/admin/tool/dataprivacy/classes/external.php index c3f9f65ce71..f99ed346396 100644 --- a/admin/tool/dataprivacy/classes/external.php +++ b/admin/tool/dataprivacy/classes/external.php @@ -92,17 +92,30 @@ class external extends external_api { ]); $requestid = $params['requestid']; - // Validate context. + // Validate context and access to manage the registry. $context = context_user::instance($USER->id); self::validate_context($context); // Ensure the request exists. $select = 'id = :id AND (userid = :userid OR requestedby = :requestedby)'; $params = ['id' => $requestid, 'userid' => $USER->id, 'requestedby' => $USER->id]; - $requestexists = data_request::record_exists_select($select, $params); + $requests = data_request::get_records_select($select, $params); + $requestexists = count($requests) === 1; $result = false; if ($requestexists) { + $request = reset($requests); + $datasubject = $request->get('userid'); + + if ($datasubject !== $USER->id) { + // The user is not the subject. Check that they can cancel this request. + if (!api::can_create_data_request_for_user($datasubject)) { + $forusercontext = \context_user::instance($datasubject); + throw new required_capability_exception($forusercontext, + 'tool/dataprivacy:makedatarequestsforchildren', 'nopermissions', ''); + } + } + // TODO: Do we want a request to be non-cancellable past a certain point? E.g. When it's already approved/processing. $result = api::update_request_status($requestid, api::DATAREQUEST_STATUS_CANCELLED); } else { @@ -257,9 +270,10 @@ class external extends external_api { ]); $requestid = $params['requestid']; - // Validate context. + // Validate context and access to manage the registry. $context = context_system::instance(); self::validate_context($context); + api::check_can_manage_data_registry(); $message = get_string('markedcomplete', 'tool_dataprivacy'); // Update the data request record. @@ -748,7 +762,9 @@ class external extends external_api { 'jsonformdata' => $jsonformdata ]); + // Validate context and access to manage the registry. self::validate_context(\context_system::instance()); + api::check_can_manage_data_registry(); $serialiseddata = json_decode($params['jsonformdata']); $data = array(); @@ -816,6 +832,10 @@ class external extends external_api { 'id' => $id ]); + // Validate context and access to manage the registry. + self::validate_context(\context_system::instance()); + api::check_can_manage_data_registry(); + $result = api::delete_purpose($params['id']); return [ @@ -865,7 +885,9 @@ class external extends external_api { 'jsonformdata' => $jsonformdata ]); + // Validate context and access to manage the registry. self::validate_context(\context_system::instance()); + api::check_can_manage_data_registry(); $serialiseddata = json_decode($params['jsonformdata']); $data = array(); @@ -933,6 +955,10 @@ class external extends external_api { 'id' => $id ]); + // Validate context and access to manage the registry. + self::validate_context(\context_system::instance()); + api::check_can_manage_data_registry(); + $result = api::delete_category($params['id']); return [ @@ -982,8 +1008,9 @@ class external extends external_api { 'jsonformdata' => $jsonformdata ]); - // Extra permission checkings are delegated to api::set_contextlevel. + // Validate context and access to manage the registry. self::validate_context(\context_system::instance()); + api::check_can_manage_data_registry(); $serialiseddata = json_decode($params['jsonformdata']); $data = array(); @@ -1063,6 +1090,7 @@ class external extends external_api { $customdata = \tool_dataprivacy\form\context_instance::get_context_instance_customdata($context); $mform = new \tool_dataprivacy\form\context_instance(null, $customdata, 'post', '', null, true, $data); if ($validateddata = $mform->get_data()) { + api::check_can_manage_data_registry($validateddata->contextid); $context = api::set_context_instance($validateddata); } else if ($errors = $mform->is_validated()) { $warnings[] = json_encode($errors); @@ -1192,9 +1220,9 @@ class external extends external_api { ]); $ids = $params['ids']; - // Validate context. - $context = context_system::instance(); - self::validate_context($context); + // Validate context and access to manage the registry. + self::validate_context(\context_system::instance()); + api::check_can_manage_data_registry(); $result = true; if (!empty($ids)) { @@ -1366,6 +1394,7 @@ class external extends external_api { $context = context_system::instance(); self::validate_context($context); + api::check_can_manage_data_registry(); $categories = api::get_categories(); $options = data_registry_page::category_options($categories, $includenotset, $includeinherit); diff --git a/admin/tool/dataprivacy/createdatarequest.php b/admin/tool/dataprivacy/createdatarequest.php index 2a2fb810904..c0ea7acc3e4 100644 --- a/admin/tool/dataprivacy/createdatarequest.php +++ b/admin/tool/dataprivacy/createdatarequest.php @@ -24,7 +24,6 @@ require_once('../../../config.php'); require_once('lib.php'); -require_once('classes/api.php'); require_once('createdatarequest_form.php'); $manage = optional_param('manage', 0, PARAM_INT); @@ -67,6 +66,14 @@ if ($mform->is_cancelled()) { // Data request submitted. if ($data = $mform->get_data()) { + if ($data->userid != $USER->id) { + if (!\tool_dataprivacy\api::can_manage_data_requests($USER->id)) { + // If not a DPO, only users with the capability to make data requests for the user should be allowed. + // (e.g. users with the Parent role, etc). + \tool_dataprivacy\api::require_can_create_data_request_for_user($data->userid); + } + } + \tool_dataprivacy\api::create_data_request($data->userid, $data->type, $data->comments); if ($manage) { diff --git a/admin/tool/dataprivacy/tests/api_test.php b/admin/tool/dataprivacy/tests/api_test.php index 820611c70b3..1864a03b260 100644 --- a/admin/tool/dataprivacy/tests/api_test.php +++ b/admin/tool/dataprivacy/tests/api_test.php @@ -46,16 +46,65 @@ global $CFG; class tool_dataprivacy_api_testcase extends advanced_testcase { /** - * setUp. + * Ensure that the check_can_manage_data_registry function fails cap testing when a user without capabilities is + * tested with the default context. */ - public function setUp() { + public function test_check_can_manage_data_registry_admin() { $this->resetAfterTest(); + + $this->setAdminUser(); + // Technically this actually returns void, but assertNull will suffice to avoid a pointless test. + $this->assertNull(api::check_can_manage_data_registry()); + } + + /** + * Ensure that the check_can_manage_data_registry function fails cap testing when a user without capabilities is + * tested with the default context. + */ + public function test_check_can_manage_data_registry_without_cap_default() { + $this->resetAfterTest(); + + $user = $this->getDataGenerator()->create_user(); + $this->setUser($user); + + $this->expectException(required_capability_exception::class); + api::check_can_manage_data_registry(); + } + + /** + * Ensure that the check_can_manage_data_registry function fails cap testing when a user without capabilities is + * tested with the default context. + */ + public function test_check_can_manage_data_registry_without_cap_system() { + $this->resetAfterTest(); + + $user = $this->getDataGenerator()->create_user(); + $this->setUser($user); + + $this->expectException(required_capability_exception::class); + api::check_can_manage_data_registry(\context_system::instance()->id); + } + + /** + * Ensure that the check_can_manage_data_registry function fails cap testing when a user without capabilities is + * tested with the default context. + */ + public function test_check_can_manage_data_registry_without_cap_own_user() { + $this->resetAfterTest(); + + $user = $this->getDataGenerator()->create_user(); + $this->setUser($user); + + $this->expectException(required_capability_exception::class); + api::check_can_manage_data_registry(\context_user::instance($user->id)->id); } /** * Test for api::update_request_status(). */ public function test_update_request_status() { + $this->resetAfterTest(); + $generator = new testing_data_generator(); $s1 = $generator->create_user(); $this->setUser($s1); @@ -65,6 +114,26 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $requestid = $datarequest->get('id'); + // Update with a comment. + $comment = 'This is an example of a comment'; + $result = api::update_request_status($requestid, api::DATAREQUEST_STATUS_AWAITING_APPROVAL, 0, $comment); + $this->assertTrue($result); + $datarequest = new data_request($requestid); + $this->assertStringEndsWith($comment, $datarequest->get('dpocomment')); + + // Update with a comment which will be trimmed. + $result = api::update_request_status($requestid, api::DATAREQUEST_STATUS_AWAITING_APPROVAL, 0, ' '); + $this->assertTrue($result); + $datarequest = new data_request($requestid); + $this->assertStringEndsWith($comment, $datarequest->get('dpocomment')); + + // Update with a comment. + $secondcomment = ' - More comments - '; + $result = api::update_request_status($requestid, api::DATAREQUEST_STATUS_AWAITING_APPROVAL, 0, $secondcomment); + $this->assertTrue($result); + $datarequest = new data_request($requestid); + $this->assertRegExp("/.*{$comment}.*{$secondcomment}/s", $datarequest->get('dpocomment')); + // Update with a valid status. $result = api::update_request_status($requestid, api::DATAREQUEST_STATUS_DOWNLOAD_READY); $this->assertTrue($result); @@ -82,6 +151,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * Test for api::get_site_dpos() when there are no users with the DPO role. */ public function test_get_site_dpos_no_dpos() { + $this->resetAfterTest(); + $admin = get_admin(); $dpos = api::get_site_dpos(); @@ -95,6 +166,9 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { */ public function test_get_site_dpos() { global $DB; + + $this->resetAfterTest(); + $generator = new testing_data_generator(); $u1 = $generator->create_user(); $u2 = $generator->create_user(); @@ -130,6 +204,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { public function test_get_assigned_privacy_officer_roles() { global $DB; + $this->resetAfterTest(); + // Erroneously set the manager roles as the PO, even if it doesn't have the managedatarequests capability yet. $managerroleid = $DB->get_field('role', 'id', array('shortname' => 'manager')); set_config('dporoles', $managerroleid, 'tool_dataprivacy'); @@ -171,6 +247,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { public function test_approve_data_request() { global $DB; + $this->resetAfterTest(); + $generator = new testing_data_generator(); $s1 = $generator->create_user(); $u1 = $generator->create_user(); @@ -213,6 +291,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { public function test_approve_data_request_not_yet_ready() { global $DB; + $this->resetAfterTest(); + $generator = new testing_data_generator(); $s1 = $generator->create_user(); $u1 = $generator->create_user(); @@ -243,6 +323,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * Test for api::approve_data_request() when called by a user who doesn't have the DPO role. */ public function test_approve_data_request_non_dpo_user() { + $this->resetAfterTest(); + $generator = new testing_data_generator(); $student = $generator->create_user(); $teacher = $generator->create_user(); @@ -252,17 +334,14 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $datarequest = api::create_data_request($student->id, api::DATAREQUEST_TYPE_EXPORT); $requestid = $datarequest->get('id'); - - // Login as a user without DPO role. - $this->setUser($teacher); - $this->expectException(required_capability_exception::class); - api::approve_data_request($requestid); } /** * Test for api::can_contact_dpo() */ public function test_can_contact_dpo() { + $this->resetAfterTest(); + // Default ('contactdataprotectionofficer' is disabled by default). $this->assertFalse(api::can_contact_dpo()); @@ -281,6 +360,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { public function test_can_manage_data_requests() { global $DB; + $this->resetAfterTest(); + // No configured site DPOs yet. $admin = get_admin(); $this->assertTrue(api::can_manage_data_requests($admin->id)); @@ -317,10 +398,120 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->assertFalse(api::can_manage_data_requests($nondpoincapable->id)); } + /** + * Test that a user who has no capability to make any data requests for children cannot create data requests for any + * other user. + */ + public function test_can_create_data_request_for_user_no() { + $this->resetAfterTest(); + + $parent = $this->getDataGenerator()->create_user(); + $otheruser = $this->getDataGenerator()->create_user(); + + $this->setUser($parent); + $this->assertFalse(api::can_create_data_request_for_user($otheruser->id)); + } + + /** + * Test that a user who has the capability to make any data requests for one other user cannot create data requests + * for any other user. + */ + public function test_can_create_data_request_for_user_some() { + $this->resetAfterTest(); + + $parent = $this->getDataGenerator()->create_user(); + $child = $this->getDataGenerator()->create_user(); + $otheruser = $this->getDataGenerator()->create_user(); + + $systemcontext = \context_system::instance(); + $parentrole = $this->getDataGenerator()->create_role(); + assign_capability('tool/dataprivacy:makedatarequestsforchildren', CAP_ALLOW, $parentrole, $systemcontext); + role_assign($parentrole, $parent->id, \context_user::instance($child->id)); + + $this->setUser($parent); + $this->assertFalse(api::can_create_data_request_for_user($otheruser->id)); + } + + /** + * Test that a user who has the capability to make any data requests for one other user cannot create data requests + * for any other user. + */ + public function test_can_create_data_request_for_user_own_child() { + $this->resetAfterTest(); + + $parent = $this->getDataGenerator()->create_user(); + $child = $this->getDataGenerator()->create_user(); + + $systemcontext = \context_system::instance(); + $parentrole = $this->getDataGenerator()->create_role(); + assign_capability('tool/dataprivacy:makedatarequestsforchildren', CAP_ALLOW, $parentrole, $systemcontext); + role_assign($parentrole, $parent->id, \context_user::instance($child->id)); + + $this->setUser($parent); + $this->assertTrue(api::can_create_data_request_for_user($child->id)); + } + + /** + * Test that a user who has no capability to make any data requests for children cannot create data requests for any + * other user. + */ + public function test_require_can_create_data_request_for_user_no() { + $this->resetAfterTest(); + + $parent = $this->getDataGenerator()->create_user(); + $otheruser = $this->getDataGenerator()->create_user(); + + $this->setUser($parent); + $this->expectException('required_capability_exception'); + api::require_can_create_data_request_for_user($otheruser->id); + } + + /** + * Test that a user who has the capability to make any data requests for one other user cannot create data requests + * for any other user. + */ + public function test_require_can_create_data_request_for_user_some() { + $this->resetAfterTest(); + + $parent = $this->getDataGenerator()->create_user(); + $child = $this->getDataGenerator()->create_user(); + $otheruser = $this->getDataGenerator()->create_user(); + + $systemcontext = \context_system::instance(); + $parentrole = $this->getDataGenerator()->create_role(); + assign_capability('tool/dataprivacy:makedatarequestsforchildren', CAP_ALLOW, $parentrole, $systemcontext); + role_assign($parentrole, $parent->id, \context_user::instance($child->id)); + + $this->setUser($parent); + $this->expectException('required_capability_exception'); + api::require_can_create_data_request_for_user($otheruser->id); + } + + /** + * Test that a user who has the capability to make any data requests for one other user cannot create data requests + * for any other user. + */ + public function test_require_can_create_data_request_for_user_own_child() { + $this->resetAfterTest(); + + $parent = $this->getDataGenerator()->create_user(); + $child = $this->getDataGenerator()->create_user(); + + $systemcontext = \context_system::instance(); + $parentrole = $this->getDataGenerator()->create_role(); + assign_capability('tool/dataprivacy:makedatarequestsforchildren', CAP_ALLOW, $parentrole, $systemcontext); + role_assign($parentrole, $parent->id, \context_user::instance($child->id)); + + $this->setUser($parent); + $this->assertTrue(api::require_can_create_data_request_for_user($child->id)); + } + /** * Test for api::can_download_data_request_for_user() */ public function test_can_download_data_request_for_user() { + $this->resetAfterTest(); + $generator = $this->getDataGenerator(); // Three victims. @@ -372,6 +563,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * Test for api::create_data_request() */ public function test_create_data_request() { + $this->resetAfterTest(); + $generator = new testing_data_generator(); $user = $generator->create_user(); $comment = 'sample comment'; @@ -399,6 +592,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { public function test_create_data_request_by_dpo() { global $USER; + $this->resetAfterTest(); + $generator = new testing_data_generator(); $user = $generator->create_user(); $comment = 'sample comment'; @@ -426,6 +621,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { public function test_create_data_request_by_parent() { global $DB; + $this->resetAfterTest(); + $generator = new testing_data_generator(); $user = $generator->create_user(); $parent = $generator->create_user(); @@ -461,6 +658,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * Test for api::deny_data_request() */ public function test_deny_data_request() { + $this->resetAfterTest(); + $generator = new testing_data_generator(); $user = $generator->create_user(); $comment = 'sample comment'; @@ -482,27 +681,6 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->assertTrue($result); } - /** - * Test for api::deny_data_request() - */ - public function test_deny_data_request_without_permissions() { - $generator = new testing_data_generator(); - $user = $generator->create_user(); - $comment = 'sample comment'; - - // Login as user. - $this->setUser($user->id); - - // Test data request creation. - $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT, $comment); - - // Login as a non-DPO user and try to call deny_data_request. - $user2 = $generator->create_user(); - $this->setUser($user2); - $this->expectException(required_capability_exception::class); - api::deny_data_request($datarequest->get('id')); - } - /** * Data provider for \tool_dataprivacy_api_testcase::test_get_data_requests(). * @@ -535,6 +713,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * @param int[] $statuses Status filters. */ public function test_get_data_requests($usertype, $fetchall, $statuses) { + $this->resetAfterTest(); + $generator = new testing_data_generator(); $user1 = $generator->create_user(); $user2 = $generator->create_user(); @@ -667,6 +847,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * @param bool $expected The expected result. */ public function test_has_ongoing_request($status, $expected) { + $this->resetAfterTest(); + $generator = new testing_data_generator(); $user1 = $generator->create_user(); @@ -700,6 +882,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { public function test_is_site_dpo() { global $DB; + $this->resetAfterTest(); + // No configured site DPOs yet. $admin = get_admin(); $this->assertTrue(api::is_site_dpo($admin->id)); @@ -750,6 +934,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * @param string $comments The requestor's message to the DPO. */ public function test_notify_dpo($byadmin, $type, $typestringid, $comments) { + $this->resetAfterTest(); + $generator = new testing_data_generator(); $user1 = $generator->create_user(); // Let's just use admin as DPO (It's the default if not set). @@ -784,86 +970,13 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->assertContains(fullname($user1), $message->fullmessage); } - /** - * Test of creating purpose as a user without privileges. - */ - public function test_create_purpose_non_dpo_user() { - $pleb = $this->getDataGenerator()->create_user(); - - $this->setUser($pleb); - $this->expectException(required_capability_exception::class); - api::create_purpose((object)[ - 'name' => 'aaa', - 'description' => 'yeah', - 'descriptionformat' => 1, - 'retentionperiod' => 'PT1M' - ]); - } - - /** - * Test fetching of purposes as a user without privileges. - */ - public function test_get_purposes_non_dpo_user() { - $pleb = $this->getDataGenerator()->create_user(); - $this->setAdminUser(); - api::create_purpose((object)[ - 'name' => 'bbb', - 'description' => 'yeah', - 'descriptionformat' => 1, - 'retentionperiod' => 'PT1M', - 'lawfulbases' => 'gdpr_art_6_1_a' - ]); - - $this->setUser($pleb); - $this->expectException(required_capability_exception::class); - api::get_purposes(); - } - - /** - * Test updating of purpose as a user without privileges. - */ - public function test_update_purposes_non_dpo_user() { - $pleb = $this->getDataGenerator()->create_user(); - $this->setAdminUser(); - $purpose = api::create_purpose((object)[ - 'name' => 'bbb', - 'description' => 'yeah', - 'descriptionformat' => 1, - 'retentionperiod' => 'PT1M', - 'lawfulbases' => 'gdpr_art_6_1_a' - ]); - - $this->setUser($pleb); - $this->expectException(required_capability_exception::class); - $purpose->set('retentionperiod', 'PT2M'); - api::update_purpose($purpose->to_record()); - } - - /** - * Test purpose deletion as a user without privileges. - */ - public function test_delete_purpose_non_dpo_user() { - $pleb = $this->getDataGenerator()->create_user(); - $this->setAdminUser(); - $purpose = api::create_purpose((object)[ - 'name' => 'bbb', - 'description' => 'yeah', - 'descriptionformat' => 1, - 'retentionperiod' => 'PT1M', - 'lawfulbases' => 'gdpr_art_6_1_a' - ]); - - $this->setUser($pleb); - $this->expectException(required_capability_exception::class); - api::delete_purpose($purpose->get('id')); - } - /** * Test data purposes CRUD actions. * * @return null */ public function test_purpose_crud() { + $this->resetAfterTest(); $this->setAdminUser(); @@ -899,86 +1012,13 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->assertCount(0, api::get_purposes()); } - /** - * Test creation of data categories as a user without privileges. - */ - public function test_create_category_non_dpo_user() { - $pleb = $this->getDataGenerator()->create_user(); - - $this->setUser($pleb); - $this->expectException(required_capability_exception::class); - api::create_category((object)[ - 'name' => 'bbb', - 'description' => 'yeah', - 'descriptionformat' => 1 - ]); - } - - /** - * Test fetching of data categories as a user without privileges. - */ - public function test_get_categories_non_dpo_user() { - $pleb = $this->getDataGenerator()->create_user(); - - $this->setAdminUser(); - api::create_category((object)[ - 'name' => 'bbb', - 'description' => 'yeah', - 'descriptionformat' => 1 - ]); - - // Back to a regular user. - $this->setUser($pleb); - $this->expectException(required_capability_exception::class); - api::get_categories(); - } - - /** - * Test updating of data category as a user without privileges. - */ - public function test_update_category_non_dpo_user() { - $pleb = $this->getDataGenerator()->create_user(); - - $this->setAdminUser(); - $category = api::create_category((object)[ - 'name' => 'bbb', - 'description' => 'yeah', - 'descriptionformat' => 1 - ]); - - // Back to a regular user. - $this->setUser($pleb); - $this->expectException(required_capability_exception::class); - $category->set('name', 'yeah'); - api::update_category($category->to_record()); - } - - /** - * Test deletion of data category as a user without privileges. - */ - public function test_delete_category_non_dpo_user() { - $pleb = $this->getDataGenerator()->create_user(); - - $this->setAdminUser(); - $category = api::create_category((object)[ - 'name' => 'bbb', - 'description' => 'yeah', - 'descriptionformat' => 1 - ]); - - // Back to a regular user. - $this->setUser($pleb); - $this->expectException(required_capability_exception::class); - api::delete_category($category->get('id')); - $this->fail('Users shouldn\'t be allowed to manage categories by default'); - } - /** * Test data categories CRUD actions. * * @return null */ public function test_category_crud() { + $this->resetAfterTest(); $this->setAdminUser(); @@ -1018,6 +1058,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { public function test_context_instances() { global $DB; + $this->resetAfterTest(); + $this->setAdminUser(); list($purposes, $categories, $courses, $modules) = $this->add_purposes_and_categories(); @@ -1052,6 +1094,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { public function test_contextlevel() { global $DB; + $this->resetAfterTest(); + $this->setAdminUser(); list($purposes, $categories, $courses, $modules) = $this->add_purposes_and_categories(); @@ -1086,6 +1130,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { public function test_effective_contextlevel_defaults() { $this->setAdminUser(); + $this->resetAfterTest(); + list($purposes, $categories, $courses, $modules) = $this->add_purposes_and_categories(); list($purposeid, $categoryid) = data_registry::get_effective_default_contextlevel_purpose_and_category(CONTEXT_SYSTEM); @@ -1129,14 +1175,22 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $this->assertEquals($categories[0]->get('id'), $categoryid); } + public function test_get_effective_contextlevel_category() { + // Before setup, get_effective_contextlevel_purpose will return false. + $this->assertFalse(api::get_effective_contextlevel_category(CONTEXT_SYSTEM)); + } + /** * Test effective contextlevel return. - * - * @return null */ public function test_effective_contextlevel() { $this->setAdminUser(); + $this->resetAfterTest(); + + // Before setup, get_effective_contextlevel_purpose will return false. + $this->assertFalse(api::get_effective_contextlevel_purpose(CONTEXT_SYSTEM)); + list($purposes, $categories, $courses, $modules) = $this->add_purposes_and_categories(); // Set the system context level to purpose 1. @@ -1184,6 +1238,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * @return null */ public function test_effective_context() { + $this->resetAfterTest(); + $this->setAdminUser(); list($purposes, $categories, $courses, $modules) = $this->add_purposes_and_categories(); @@ -1317,6 +1373,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * @return null */ protected function add_purposes_and_categories() { + $this->resetAfterTest(); $purpose1 = api::create_purpose((object)['name' => 'p1', 'retentionperiod' => 'PT1H', 'lawfulbases' => 'gdpr_art_6_1_a']); $purpose2 = api::create_purpose((object)['name' => 'p2', 'retentionperiod' => 'PT2H', 'lawfulbases' => 'gdpr_art_6_1_b']); @@ -1344,6 +1401,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * Test that delete requests filter out protected purpose contexts. */ public function test_add_request_contexts_with_status_delete() { + $this->resetAfterTest(); + $data = $this->setup_test_add_request_contexts_with_status(api::DATAREQUEST_TYPE_DELETE); $contextids = $data->list->get_contextids(); @@ -1355,6 +1414,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * Test that export requests don't filter out protected purpose contexts. */ public function test_add_request_contexts_with_status_export() { + $this->resetAfterTest(); + $data = $this->setup_test_add_request_contexts_with_status(api::DATAREQUEST_TYPE_EXPORT); $contextids = $data->list->get_contextids(); @@ -1406,6 +1467,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { public function test_set_context_defaults($contextlevel, $inheritcategory, $inheritpurpose, $foractivity, $override) { $this->setAdminUser(); + $this->resetAfterTest(); + $generator = $this->getDataGenerator(); // Generate course cat, course, block, assignment, forum instances. @@ -1527,6 +1590,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * @return \stdClass */ protected function setup_test_add_request_contexts_with_status($type) { + $this->resetAfterTest(); + $this->setAdminUser(); // User under test. diff --git a/admin/tool/dataprivacy/tests/external_test.php b/admin/tool/dataprivacy/tests/external_test.php index b76878610bc..0db55d2bf31 100644 --- a/admin/tool/dataprivacy/tests/external_test.php +++ b/admin/tool/dataprivacy/tests/external_test.php @@ -40,71 +40,85 @@ use tool_dataprivacy\external; */ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { - /** @var stdClass The user making the request. */ - protected $requester; - - /** @var int The data request ID. */ - protected $requestid; - - /** - * Setup function- we will create a course and add an assign instance to it. - */ - protected function setUp() { - $this->resetAfterTest(); - - $generator = new testing_data_generator(); - $requester = $generator->create_user(); - - $comment = 'sample comment'; - - // Login as user. - $this->setUser($requester->id); - - // Test data request creation. - $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); - $this->requestid = $datarequest->get('id'); - $this->requester = $requester; - - // Log out the user and set force login to true. - $this->setUser(); - } - /** * Test for external::approve_data_request() with the user not logged in. */ public function test_approve_data_request_not_logged_in() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $comment = 'sample comment'; + + // Test data request creation. + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + + // Log out the user and set force login to true. + $this->setUser(); + $this->expectException(require_login_exception::class); - external::approve_data_request($this->requestid); + external::approve_data_request($datarequest->get('id')); } /** * Test for external::approve_data_request() with the user not having a DPO role. */ public function test_approve_data_request_not_dpo() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $comment = 'sample comment'; + + // Test data request creation. + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + // Login as the requester. - $this->setUser($this->requester->id); + $this->setUser($requester); $this->expectException(required_capability_exception::class); - external::approve_data_request($this->requestid); + external::approve_data_request($datarequest->get('id')); } /** * Test for external::approve_data_request() for request that's not ready for approval */ public function test_approve_data_request_not_waiting_for_approval() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $comment = 'sample comment'; + + // Test data request creation. + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + // Admin as DPO. (The default when no one's assigned as a DPO in the site). $this->setAdminUser(); $this->expectException(moodle_exception::class); - external::approve_data_request($this->requestid); + external::approve_data_request($datarequest->get('id')); } /** * Test for external::approve_data_request() */ public function test_approve_data_request() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $comment = 'sample comment'; + + // Test data request creation. + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + // Admin as DPO. (The default when no one's assigned as a DPO in the site). $this->setAdminUser(); - api::update_request_status($this->requestid, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); - $result = external::approve_data_request($this->requestid); + api::update_request_status($datarequest->get('id'), api::DATAREQUEST_STATUS_AWAITING_APPROVAL); + $result = external::approve_data_request($datarequest->get('id')); $return = (object) external_api::clean_returnvalue(external::approve_data_request_returns(), $result); $this->assertTrue($return->result); $this->assertEmpty($return->warnings); @@ -114,10 +128,13 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * Test for external::approve_data_request() for a non-existent request ID. */ public function test_approve_data_request_non_existent() { + $this->resetAfterTest(); + // Admin as DPO. (The default when no one's assigned as a DPO in the site). $this->setAdminUser(); - api::update_request_status($this->requestid, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); - $result = external::approve_data_request($this->requestid + 1); + + $result = external::approve_data_request(1); + $return = (object) external_api::clean_returnvalue(external::approve_data_request_returns(), $result); $this->assertFalse($return->result); $this->assertCount(1, $return->warnings); @@ -129,13 +146,21 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * Test for external::cancel_data_request() of another user. */ public function test_cancel_data_request_other_user() { - $generator = $this->getDataGenerator(); - $otheruser = $generator->create_user(); + $this->resetAfterTest(); - // Login as another user. + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $otheruser = $generator->create_user(); + $comment = 'sample comment'; + + // Test data request creation. + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + + // Login as other user. $this->setUser($otheruser); - $result = external::cancel_data_request($this->requestid); + $result = external::cancel_data_request($datarequest->get('id')); $return = (object) external_api::clean_returnvalue(external::approve_data_request_returns(), $result); $this->assertFalse($return->result); $this->assertCount(1, $return->warnings); @@ -143,14 +168,107 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { $this->assertEquals('errorrequestnotfound', $warning['warningcode']); } + /** + * Test cancellation of a request where you are the requester of another user's data. + */ + public function test_cancel_data_request_other_user_as_requester() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $otheruser = $generator->create_user(); + $comment = 'sample comment'; + + // Assign requester as otheruser'sparent. + $systemcontext = \context_system::instance(); + $parentrole = $generator->create_role(); + assign_capability('tool/dataprivacy:makedatarequestsforchildren', CAP_ALLOW, $parentrole, $systemcontext); + role_assign($parentrole, $requester->id, \context_user::instance($otheruser->id)); + + // Test data request creation. + $this->setUser($requester); + $datarequest = api::create_data_request($otheruser->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + + $result = external::cancel_data_request($datarequest->get('id')); + $return = (object) external_api::clean_returnvalue(external::approve_data_request_returns(), $result); + $this->assertTrue($return->result); + $this->assertEmpty($return->warnings); + } + + /** + * Test cancellation of a request where you are the requester of another user's data. + */ + public function test_cancel_data_request_requester_lost_permissions() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $otheruser = $generator->create_user(); + $comment = 'sample comment'; + + // Assign requester as otheruser'sparent. + $systemcontext = \context_system::instance(); + $parentrole = $generator->create_role(); + assign_capability('tool/dataprivacy:makedatarequestsforchildren', CAP_ALLOW, $parentrole, $systemcontext); + role_assign($parentrole, $requester->id, \context_user::instance($otheruser->id)); + + $this->setUser($requester); + $datarequest = api::create_data_request($otheruser->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + + // Unassign the role. + role_unassign($parentrole, $requester->id, \context_user::instance($otheruser->id)->id); + + // This user can no longer make the request. + $this->expectException(required_capability_exception::class); + + $result = external::cancel_data_request($datarequest->get('id')); + } + + /** + * Test cancellation of a request where you are the requester of another user's data. + */ + public function test_cancel_data_request_other_user_as_child() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $otheruser = $generator->create_user(); + $comment = 'sample comment'; + + // Assign requester as otheruser'sparent. + $systemcontext = \context_system::instance(); + $parentrole = $generator->create_role(); + assign_capability('tool/dataprivacy:makedatarequestsforchildren', CAP_ALLOW, $parentrole, $systemcontext); + role_assign($parentrole, $requester->id, \context_user::instance($otheruser->id)); + + // Test data request creation. + $this->setUser($otheruser); + $datarequest = api::create_data_request($otheruser->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + + $result = external::cancel_data_request($datarequest->get('id')); + $return = (object) external_api::clean_returnvalue(external::approve_data_request_returns(), $result); + $this->assertTrue($return->result); + $this->assertEmpty($return->warnings); + } + /** * Test for external::cancel_data_request() */ public function test_cancel_data_request() { - // Login as the requester. - $this->setUser($this->requester); + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $comment = 'sample comment'; + + // Test data request creation. + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + + // Test cancellation. + $this->setUser($requester); + $result = external::cancel_data_request($datarequest->get('id')); - $result = external::cancel_data_request($this->requestid); $return = (object) external_api::clean_returnvalue(external::approve_data_request_returns(), $result); $this->assertTrue($return->result); $this->assertEmpty($return->warnings); @@ -160,10 +278,12 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * Test contact DPO. */ public function test_contact_dpo() { - $generator = $this->getDataGenerator(); - $user = $generator->create_user(); - $this->setUser($user); + $this->resetAfterTest(); + $generator = new testing_data_generator(); + $user = $generator->create_user(); + + $this->setUser($user); $message = 'Hello world!'; $result = external::contact_dpo($message); $return = (object) external_api::clean_returnvalue(external::contact_dpo_returns(), $result); @@ -175,10 +295,12 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * Test contact DPO with message containing invalid input. */ public function test_contact_dpo_with_nasty_input() { - $generator = $this->getDataGenerator(); - $user = $generator->create_user(); - $this->setUser($user); + $this->resetAfterTest(); + $generator = new testing_data_generator(); + $user = $generator->create_user(); + + $this->setUser($user); $this->expectException('invalid_parameter_exception'); external::contact_dpo('de<>\\..scription'); } @@ -187,38 +309,77 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * Test for external::deny_data_request() with the user not logged in. */ public function test_deny_data_request_not_logged_in() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $comment = 'sample comment'; + + // Test data request creation. + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + + // Log out. + $this->setUser(); $this->expectException(require_login_exception::class); - external::deny_data_request($this->requestid); + external::deny_data_request($datarequest->get('id')); } /** * Test for external::deny_data_request() with the user not having a DPO role. */ public function test_deny_data_request_not_dpo() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $comment = 'sample comment'; + + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + // Login as the requester. - $this->setUser($this->requester->id); + $this->setUser($requester); $this->expectException(required_capability_exception::class); - external::deny_data_request($this->requestid); + external::deny_data_request($datarequest->get('id')); } /** * Test for external::deny_data_request() for request that's not ready for approval */ public function test_deny_data_request_not_waiting_for_approval() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $comment = 'sample comment'; + + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + // Admin as DPO. (The default when no one's assigned as a DPO in the site). $this->setAdminUser(); $this->expectException(moodle_exception::class); - external::deny_data_request($this->requestid); + external::deny_data_request($datarequest->get('id')); } /** * Test for external::deny_data_request() */ public function test_deny_data_request() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $comment = 'sample comment'; + + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + // Admin as DPO. (The default when no one's assigned as a DPO in the site). $this->setAdminUser(); - api::update_request_status($this->requestid, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); - $result = external::approve_data_request($this->requestid); + api::update_request_status($datarequest->get('id'), api::DATAREQUEST_STATUS_AWAITING_APPROVAL); + $result = external::approve_data_request($datarequest->get('id')); $return = (object) external_api::clean_returnvalue(external::deny_data_request_returns(), $result); $this->assertTrue($return->result); $this->assertEmpty($return->warnings); @@ -228,10 +389,12 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * Test for external::deny_data_request() for a non-existent request ID. */ public function test_deny_data_request_non_existent() { + $this->resetAfterTest(); + // Admin as DPO. (The default when no one's assigned as a DPO in the site). $this->setAdminUser(); - api::update_request_status($this->requestid, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); - $result = external::deny_data_request($this->requestid + 1); + $result = external::deny_data_request(1); + $return = (object) external_api::clean_returnvalue(external::deny_data_request_returns(), $result); $this->assertFalse($return->result); $this->assertCount(1, $return->warnings); @@ -243,34 +406,62 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * Test for external::get_data_request() with the user not logged in. */ public function test_get_data_request_not_logged_in() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $comment = 'sample comment'; + + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + + $this->setUser(); $this->expectException(require_login_exception::class); - external::get_data_request($this->requestid); + external::get_data_request($datarequest->get('id')); } /** * Test for external::get_data_request() with the user not having a DPO role. */ public function test_get_data_request_not_dpo() { - $generator = $this->getDataGenerator(); + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); $otheruser = $generator->create_user(); - // Login as the requester. + $comment = 'sample comment'; + + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + + // Login as the otheruser. $this->setUser($otheruser); $this->expectException(required_capability_exception::class); - external::get_data_request($this->requestid); + external::get_data_request($datarequest->get('id')); } /** * Test for external::get_data_request() */ public function test_get_data_request() { + $this->resetAfterTest(); + + $generator = new testing_data_generator(); + $requester = $generator->create_user(); + $comment = 'sample comment'; + + $this->setUser($requester); + $datarequest = api::create_data_request($requester->id, api::DATAREQUEST_TYPE_EXPORT, $comment); + // Admin as DPO. (The default when no one's assigned as a DPO in the site). $this->setAdminUser(); - $result = external::get_data_request($this->requestid); + $result = external::get_data_request($datarequest->get('id')); + $return = (object) external_api::clean_returnvalue(external::get_data_request_returns(), $result); $this->assertEquals(api::DATAREQUEST_TYPE_EXPORT, $return->result['type']); $this->assertEquals('sample comment', $return->result['comments']); - $this->assertEquals($this->requester->id, $return->result['userid']); - $this->assertEquals($this->requester->id, $return->result['requestedby']); + $this->assertEquals($requester->id, $return->result['userid']); + $this->assertEquals($requester->id, $return->result['requestedby']); $this->assertEmpty($return->warnings); } @@ -278,10 +469,12 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * Test for external::get_data_request() for a non-existent request ID. */ public function test_get_data_request_non_existent() { + $this->resetAfterTest(); + // Admin as DPO. (The default when no one's assigned as a DPO in the site). $this->setAdminUser(); $this->expectException(dml_missing_record_exception::class); - external::get_data_request($this->requestid + 1); + external::get_data_request(1); } /** @@ -289,6 +482,8 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * when called by a user that doesn't have the manage registry capability. */ public function test_set_context_defaults_no_capability() { + $this->resetAfterTest(); + $generator = $this->getDataGenerator(); $user = $generator->create_user(); $this->setUser($user); @@ -307,6 +502,8 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * @param bool $override Whether to override instances. */ public function test_set_context_defaults($modulelevel, $override) { + $this->resetAfterTest(); + $this->setAdminUser(); $generator = $this->getDataGenerator(); @@ -364,9 +561,11 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * when called by a user that doesn't have the manage registry capability. */ public function test_get_category_options_no_capability() { - $generator = $this->getDataGenerator(); - $user = $generator->create_user(); + $this->resetAfterTest(); + + $user = $this->getDataGenerator()->create_user(); $this->setUser($user); + $this->expectException(required_capability_exception::class); external::get_category_options(true, true); } @@ -391,6 +590,7 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * @param bool $includenotset Whether "Not set" would be included to the options. */ public function test_get_category_options($includeinherit, $includenotset) { + $this->resetAfterTest(); $this->setAdminUser(); // Prepare our expected options. @@ -436,6 +636,7 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * when called by a user that doesn't have the manage registry capability. */ public function test_get_purpose_options_no_capability() { + $this->resetAfterTest(); $generator = $this->getDataGenerator(); $user = $generator->create_user(); $this->setUser($user); @@ -451,6 +652,7 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * @param bool $includenotset Whether "Not set" would be included to the options. */ public function test_get_purpose_options($includeinherit, $includenotset) { + $this->resetAfterTest(); $this->setAdminUser(); // Prepare our expected options. @@ -518,6 +720,7 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * @param bool $nodefaults Whether to fetch only activities that don't have defaults. */ public function test_get_activity_options($inheritcategory, $inheritpurpose, $nodefaults) { + $this->resetAfterTest(); $this->setAdminUser(); $category = api::create_category((object)['name' => 'Test category']); @@ -565,21 +768,25 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * Test for external::bulk_approve_data_requests(). */ public function test_bulk_approve_data_requests() { - $generator = new testing_data_generator(); - $requester1 = $generator->create_user(); - $comment1 = 'sample comment'; - // Login as requester2. + $this->resetAfterTest(); + + // Create delete data requests. + $requester1 = $this->getDataGenerator()->create_user(); $this->setUser($requester1->id); - // Create delete data request. - $datarequest1 = api::create_data_request($requester1->id, api::DATAREQUEST_TYPE_DELETE, $comment1); - + $datarequest1 = api::create_data_request($requester1->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); $requestid1 = $datarequest1->get('id'); - $requestid2 = $this->requestid; + $requester2 = $this->getDataGenerator()->create_user(); + $this->setUser($requester2->id); + $datarequest2 = api::create_data_request($requester2->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); + $requestid2 = $datarequest2->get('id'); + + // Approve the requests. $this->setAdminUser(); api::update_request_status($requestid1, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); api::update_request_status($requestid2, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); $result = external::bulk_approve_data_requests([$requestid1, $requestid2]); + $return = (object) external_api::clean_returnvalue(external::bulk_approve_data_requests_returns(), $result); $this->assertTrue($return->result); $this->assertEmpty($return->warnings); @@ -589,48 +796,100 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * Test for external::bulk_approve_data_requests() for a non-existent request ID. */ public function test_bulk_approve_data_requests_non_existent() { - $generator = new testing_data_generator(); - $requester1 = $generator->create_user(); - $comment1 = 'sample comment'; - // Login as requester2. - $this->setUser($requester1->id); - // Create delete data request. - $datarequest1 = api::create_data_request($requester1->id, api::DATAREQUEST_TYPE_DELETE, $comment1); - - $requestid1 = $datarequest1->get('id'); - $requestid2 = $this->requestid; + $this->resetAfterTest(); $this->setAdminUser(); - api::update_request_status($requestid1, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); - api::update_request_status($requestid2, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); - $result = external::bulk_approve_data_requests([$requestid1 + 1, $requestid2]); + + $result = external::bulk_approve_data_requests([42]); + $return = (object) external_api::clean_returnvalue(external::bulk_approve_data_requests_returns(), $result); $this->assertFalse($return->result); $this->assertCount(1, $return->warnings); $warning = reset($return->warnings); $this->assertEquals('errorrequestnotfound', $warning['warningcode']); - $this->assertEquals($requestid1 + 1, $warning['item']); + $this->assertEquals(42, $warning['item']); + } + + /** + * Test for external::bulk_deny_data_requests() for a user without permission to deny requests. + */ + public function test_bulk_approve_data_requests_no_permission() { + $this->resetAfterTest(); + + // Create delete data requests. + $requester1 = $this->getDataGenerator()->create_user(); + $this->setUser($requester1->id); + $datarequest1 = api::create_data_request($requester1->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); + $requestid1 = $datarequest1->get('id'); + + $requester2 = $this->getDataGenerator()->create_user(); + $this->setUser($requester2->id); + $datarequest2 = api::create_data_request($requester2->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); + $requestid2 = $datarequest2->get('id'); + + $this->setAdminUser(); + api::update_request_status($requestid1, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); + api::update_request_status($requestid2, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); + + // Approve the requests. + $uut = $this->getDataGenerator()->create_user(); + $this->setUser($uut); + + $this->expectException(required_capability_exception::class); + $result = external::bulk_approve_data_requests([$requestid1, $requestid2]); + } + + /** + * Test for external::bulk_deny_data_requests() for a user without permission to deny requests. + */ + public function test_bulk_approve_data_requests_own_request() { + $this->resetAfterTest(); + + // Create delete data requests. + $requester1 = $this->getDataGenerator()->create_user(); + $this->setUser($requester1->id); + $datarequest1 = api::create_data_request($requester1->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); + $requestid1 = $datarequest1->get('id'); + + $requester2 = $this->getDataGenerator()->create_user(); + $this->setUser($requester2->id); + $datarequest2 = api::create_data_request($requester2->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); + $requestid2 = $datarequest2->get('id'); + + $this->setAdminUser(); + api::update_request_status($requestid1, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); + api::update_request_status($requestid2, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); + + // Deny the requests. + $this->setUser($requester1); + + $this->expectException(required_capability_exception::class); + $result = external::bulk_approve_data_requests([$requestid1]); } /** * Test for external::bulk_deny_data_requests(). */ public function test_bulk_deny_data_requests() { - $generator = new testing_data_generator(); - $requester1 = $generator->create_user(); - $comment1 = 'sample comment'; - // Login as requester2. + $this->resetAfterTest(); + + // Create delete data requests. + $requester1 = $this->getDataGenerator()->create_user(); $this->setUser($requester1->id); - // Create delete data request. - $datarequest1 = api::create_data_request($requester1->id, api::DATAREQUEST_TYPE_DELETE, $comment1); - + $datarequest1 = api::create_data_request($requester1->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); $requestid1 = $datarequest1->get('id'); - $requestid2 = $this->requestid; + $requester2 = $this->getDataGenerator()->create_user(); + $this->setUser($requester2->id); + $datarequest2 = api::create_data_request($requester2->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); + $requestid2 = $datarequest2->get('id'); + + // Deny the requests. $this->setAdminUser(); api::update_request_status($requestid1, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); api::update_request_status($requestid2, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); $result = external::bulk_deny_data_requests([$requestid1, $requestid2]); + $return = (object) external_api::clean_returnvalue(external::bulk_approve_data_requests_returns(), $result); $this->assertTrue($return->result); $this->assertEmpty($return->warnings); @@ -640,26 +899,73 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { * Test for external::bulk_deny_data_requests() for a non-existent request ID. */ public function test_bulk_deny_data_requests_non_existent() { - $generator = new testing_data_generator(); - $requester1 = $generator->create_user(); - $comment1 = 'sample comment'; - // Login as requester2. - $this->setUser($requester1->id); - // Create delete data request. - $datarequest1 = api::create_data_request($requester1->id, api::DATAREQUEST_TYPE_DELETE, $comment1); - - $requestid1 = $datarequest1->get('id'); - $requestid2 = $this->requestid; + $this->resetAfterTest(); $this->setAdminUser(); - api::update_request_status($requestid1, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); - api::update_request_status($requestid2, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); - $result = external::bulk_deny_data_requests([$requestid1 + 1, $requestid2]); + $result = external::bulk_deny_data_requests([42]); $return = (object) external_api::clean_returnvalue(external::bulk_approve_data_requests_returns(), $result); + $this->assertFalse($return->result); $this->assertCount(1, $return->warnings); $warning = reset($return->warnings); $this->assertEquals('errorrequestnotfound', $warning['warningcode']); - $this->assertEquals($requestid1 + 1, $warning['item']); + $this->assertEquals(42, $warning['item']); + } + + /** + * Test for external::bulk_deny_data_requests() for a user without permission to deny requests. + */ + public function test_bulk_deny_data_requests_no_permission() { + $this->resetAfterTest(); + + // Create delete data requests. + $requester1 = $this->getDataGenerator()->create_user(); + $this->setUser($requester1->id); + $datarequest1 = api::create_data_request($requester1->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); + $requestid1 = $datarequest1->get('id'); + + $requester2 = $this->getDataGenerator()->create_user(); + $this->setUser($requester2->id); + $datarequest2 = api::create_data_request($requester2->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); + $requestid2 = $datarequest2->get('id'); + + $this->setAdminUser(); + api::update_request_status($requestid1, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); + api::update_request_status($requestid2, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); + + // Deny the requests. + $uut = $this->getDataGenerator()->create_user(); + $this->setUser($uut); + + $this->expectException(required_capability_exception::class); + $result = external::bulk_deny_data_requests([$requestid1, $requestid2]); + } + + /** + * Test for external::bulk_deny_data_requests() for a user cannot approve their own request. + */ + public function test_bulk_deny_data_requests_own_request() { + $this->resetAfterTest(); + + // Create delete data requests. + $requester1 = $this->getDataGenerator()->create_user(); + $this->setUser($requester1->id); + $datarequest1 = api::create_data_request($requester1->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); + $requestid1 = $datarequest1->get('id'); + + $requester2 = $this->getDataGenerator()->create_user(); + $this->setUser($requester2->id); + $datarequest2 = api::create_data_request($requester2->id, api::DATAREQUEST_TYPE_DELETE, 'Example comment'); + $requestid2 = $datarequest2->get('id'); + + $this->setAdminUser(); + api::update_request_status($requestid1, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); + api::update_request_status($requestid2, api::DATAREQUEST_STATUS_AWAITING_APPROVAL); + + // Deny the requests. + $this->setUser($requester1); + + $this->expectException(required_capability_exception::class); + $result = external::bulk_deny_data_requests([$requestid1]); } }