From 4bd598cbc9541cae872983d9bbe283fedebf08b8 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Tue, 6 Nov 2018 11:10:20 +0800 Subject: [PATCH 1/3] MDL-62564 privacy: Add unit tests --- .../tests/user_deleted_observer_test.php | 203 ++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 admin/tool/dataprivacy/tests/user_deleted_observer_test.php diff --git a/admin/tool/dataprivacy/tests/user_deleted_observer_test.php b/admin/tool/dataprivacy/tests/user_deleted_observer_test.php new file mode 100644 index 00000000000..64a49cb9e90 --- /dev/null +++ b/admin/tool/dataprivacy/tests/user_deleted_observer_test.php @@ -0,0 +1,203 @@ +. + +/** + * Tests for the event observer. + * + * @package tool_dataprivacy + * @copyright 2018 Mihail Geshoski + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +use \tool_dataprivacy\event\user_deleted_observer; +use \tool_dataprivacy\api; + +/** + * Event observer test. + * + * @package tool_dataprivacy + * @copyright 2018 Mihail Geshoski + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class tool_dataprivacy_user_deleted_observer_testcase extends advanced_testcase { + + /** + * Ensure that a delete data request is created upon user deletion. + */ + public function test_create_delete_data_request() { + $this->resetAfterTest(); + + // Enable automatic creation of delete data requests. + set_config('automaticdeletionrequests', 1, 'tool_dataprivacy'); + + // Create another user who is not a DPO. + $user = $this->getDataGenerator()->create_user(); + + $event = $this->trigger_delete_user_event($user); + + user_deleted_observer::create_delete_data_request($event); + // Validate that delete data request has been created. + $this->assertTrue(api::has_ongoing_request($user->id, api::DATAREQUEST_TYPE_DELETE)); + } + + /** + * Ensure that a delete data request is not created upon user deletion if automatic creation of + * delete data requests is disabled. + */ + public function test_create_delete_data_request_automatic_creation_disabled() { + $this->resetAfterTest(); + + // Disable automatic creation of delete data requests. + set_config('automaticdeletionrequests', 0, 'tool_dataprivacy'); + + // Create another user who is not a DPO. + $user = $this->getDataGenerator()->create_user(); + + $event = $this->trigger_delete_user_event($user); + + user_deleted_observer::create_delete_data_request($event); + // Validate that delete data request has been created. + $this->assertFalse(api::has_ongoing_request($user->id, api::DATAREQUEST_TYPE_DELETE)); + } + + /** + * Ensure that a delete data request is being created upon user deletion + * if an ongoing export data request (or any other except delete data request) for that user already exists. + */ + public function test_create_delete_data_request_export_data_request_preexists() { + $this->resetAfterTest(); + + // Enable automatic creation of delete data requests. + set_config('automaticdeletionrequests', 1, 'tool_dataprivacy'); + + // Create another user who is not a DPO. + $user = $this->getDataGenerator()->create_user(); + // Create a delete data request for $user. + api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT); + // Validate that delete data request has been created. + $this->assertTrue(api::has_ongoing_request($user->id, api::DATAREQUEST_TYPE_EXPORT)); + $this->assertEquals(0, api::get_data_requests_count($user->id, [], [api::DATAREQUEST_TYPE_DELETE])); + + $event = $this->trigger_delete_user_event($user); + + user_deleted_observer::create_delete_data_request($event); + // Validate that delete data request has been created. + $this->assertEquals(1, api::get_data_requests_count($user->id, [], [api::DATAREQUEST_TYPE_DELETE])); + } + + /** + * Ensure that a delete data request is not being created upon user deletion + * if an ongoing delete data request for that user already exists. + */ + public function test_create_delete_data_request_ongoing_delete_data_request_preexists() { + $this->resetAfterTest(); + + // Enable automatic creation of delete data requests. + set_config('automaticdeletionrequests', 1, 'tool_dataprivacy'); + + // Create another user who is not a DPO. + $user = $this->getDataGenerator()->create_user(); + // Create a delete data request for $user. + api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE); + // Validate that delete data request has been created. + $this->assertTrue(api::has_ongoing_request($user->id, api::DATAREQUEST_TYPE_DELETE)); + + $event = $this->trigger_delete_user_event($user); + + user_deleted_observer::create_delete_data_request($event); + // Validate that additional delete data request has not been created. + $this->assertEquals(1, api::get_data_requests_count($user->id, [], [api::DATAREQUEST_TYPE_DELETE])); + } + + /** + * Ensure that a delete data request is being created upon user deletion + * if a finished delete data request (excluding complete) for that user already exists. + */ + public function test_create_delete_data_request_canceled_delete_data_request_preexists() { + $this->resetAfterTest(); + + // Enable automatic creation of delete data requests. + set_config('automaticdeletionrequests', 1, 'tool_dataprivacy'); + + // Create another user who is not a DPO. + $user = $this->getDataGenerator()->create_user(); + // Create a delete data request for $user. + $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE); + $requestid = $datarequest->get('id'); + api::update_request_status($requestid, api::DATAREQUEST_STATUS_CANCELLED); + + // Validate that delete data request has been created and the status has been updated to 'Canceled'. + $this->assertEquals(1, api::get_data_requests_count($user->id, [], [api::DATAREQUEST_TYPE_DELETE])); + $this->assertFalse(api::has_ongoing_request($user->id, api::DATAREQUEST_TYPE_DELETE)); + + $event = $this->trigger_delete_user_event($user); + + user_deleted_observer::create_delete_data_request($event); + // Validate that additional delete data request has been created. + $this->assertEquals(2, api::get_data_requests_count($user->id, [], [api::DATAREQUEST_TYPE_DELETE])); + $this->assertTrue(api::has_ongoing_request($user->id, api::DATAREQUEST_TYPE_DELETE)); + } + + /** + * Ensure that a delete data request is being created upon user deletion + * if a completed delete data request for that user already exists. + */ + public function test_create_delete_data_request_completed_delete_data_request_preexists() { + $this->resetAfterTest(); + + // Enable automatic creation of delete data requests. + set_config('automaticdeletionrequests', 1, 'tool_dataprivacy'); + + // Create another user who is not a DPO. + $user = $this->getDataGenerator()->create_user(); + // Create a delete data request for $user. + $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE); + $requestid = $datarequest->get('id'); + api::update_request_status($requestid, api::DATAREQUEST_STATUS_COMPLETE); + + // Validate that delete data request has been created and the status has been updated to 'Completed'. + $this->assertEquals(1, api::get_data_requests_count($user->id, [], [api::DATAREQUEST_TYPE_DELETE])); + $this->assertFalse(api::has_ongoing_request($user->id, api::DATAREQUEST_TYPE_DELETE)); + + $event = $this->trigger_delete_user_event($user); + + user_deleted_observer::create_delete_data_request($event); + // Validate that additional delete data request has not been created. + $this->assertEquals(1, api::get_data_requests_count($user->id, [], [api::DATAREQUEST_TYPE_DELETE])); + $this->assertFalse(api::has_ongoing_request($user->id, api::DATAREQUEST_TYPE_DELETE)); + } + + /** + * Helper to trigger and capture the delete user event. + * + * @param object $user The user object. + * @return \core\event\user_deleted $event The returned event. + */ + private function trigger_delete_user_event($user) { + + $sink = $this->redirectEvents(); + delete_user($user); + $events = $sink->get_events(); + $sink->close(); + $event = reset($events); + // Validate event data. + $this->assertInstanceOf('\core\event\user_deleted', $event); + + return $event; + } +} From b4ecfa38c2da5dd1f83a2541df521c7b6b915135 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Thu, 8 Nov 2018 09:28:36 +0800 Subject: [PATCH 2/3] MDL-62564 privacy: Improve bulk deletion --- admin/tool/dataprivacy/classes/api.php | 52 +++++- .../tool/dataprivacy/classes/data_request.php | 2 + .../classes/event/user_deleted_observer.php | 63 ++++++++ .../classes/expired_user_contexts.php | 149 ++++++++++++++++++ .../tool/dataprivacy/classes/local/helper.php | 36 +++++ .../classes/output/data_requests_table.php | 11 +- .../dataprivacy/classes/privacy/provider.php | 6 + admin/tool/dataprivacy/datarequests.php | 6 +- admin/tool/dataprivacy/db/events.php | 32 ++++ admin/tool/dataprivacy/db/upgrade.php | 2 + .../dataprivacy/lang/en/tool_dataprivacy.php | 7 + admin/tool/dataprivacy/mydatarequests.php | 2 +- admin/tool/dataprivacy/settings.php | 8 + 13 files changed, 367 insertions(+), 9 deletions(-) create mode 100644 admin/tool/dataprivacy/classes/event/user_deleted_observer.php create mode 100644 admin/tool/dataprivacy/classes/expired_user_contexts.php create mode 100644 admin/tool/dataprivacy/db/events.php diff --git a/admin/tool/dataprivacy/classes/api.php b/admin/tool/dataprivacy/classes/api.php index 10889a915ec..06f20b58c7a 100644 --- a/admin/tool/dataprivacy/classes/api.php +++ b/admin/tool/dataprivacy/classes/api.php @@ -41,6 +41,7 @@ use tool_dataprivacy\external\data_request_exporter; use tool_dataprivacy\local\helper; use tool_dataprivacy\task\initiate_data_request_task; use tool_dataprivacy\task\process_data_request_task; +use tool_dataprivacy\data_request; defined('MOODLE_INTERNAL') || die(); @@ -253,6 +254,8 @@ class api { $datarequest->set('type', $type); // Set request comments. $datarequest->set('comments', $comments); + // Set the creation method. + $datarequest->set('creationmethod', $creationmethod); // Store subject access request. $datarequest->create(); @@ -275,6 +278,7 @@ class api { * @param int $userid The User ID. * @param int[] $statuses The status filters. * @param int[] $types The request type filters. + * @param int[] $creationmethods The request creation method filters. * @param string $sort The order by clause. * @param int $offset Amount of records to skip. * @param int $limit Amount of records to fetch. @@ -282,7 +286,8 @@ class api { * @throws coding_exception * @throws dml_exception */ - public static function get_data_requests($userid = 0, $statuses = [], $types = [], $sort = '', $offset = 0, $limit = 0) { + public static function get_data_requests($userid = 0, $statuses = [], $types = [], $creationmethods = [], + $sort = '', $offset = 0, $limit = 0) { global $DB, $USER; $results = []; $sqlparams = []; @@ -306,6 +311,13 @@ class api { $sqlparams = array_merge($sqlparams, $typeparams); } + // Set request creation method filter. + if (!empty($creationmethods)) { + list($typeinsql, $typeparams) = $DB->get_in_or_equal($creationmethods, SQL_PARAMS_NAMED); + $sqlconditions[] = "creationmethod $typeinsql"; + $sqlparams = array_merge($sqlparams, $typeparams); + } + if ($userid) { // Get the data requests for the user or data requests made by the user. $sqlconditions[] = "(userid = :userid OR requestedby = :requestedby)"; @@ -348,7 +360,7 @@ class api { if (!empty($expiredrequests)) { data_request::expire($expiredrequests); - $results = self::get_data_requests($userid, $statuses, $types, $sort, $offset, $limit); + $results = self::get_data_requests($userid, $statuses, $types, $creationmethods, $sort, $offset, $limit); } } @@ -361,11 +373,12 @@ class api { * @param int $userid The User ID. * @param int[] $statuses The status filters. * @param int[] $types The request type filters. + * @param int[] $creationmethods The request creation method filters. * @return int * @throws coding_exception * @throws dml_exception */ - public static function get_data_requests_count($userid = 0, $statuses = [], $types = []) { + public static function get_data_requests_count($userid = 0, $statuses = [], $types = [], $creationmethods = []) { global $DB, $USER; $count = 0; $sqlparams = []; @@ -379,6 +392,11 @@ class api { $sqlconditions[] = "type $typeinsql"; $sqlparams = array_merge($sqlparams, $typeparams); } + if (!empty($creationmethods)) { + list($typeinsql, $typeparams) = $DB->get_in_or_equal($creationmethods, SQL_PARAMS_NAMED); + $sqlconditions[] = "creationmethod $typeinsql"; + $sqlparams = array_merge($sqlparams, $typeparams); + } if ($userid) { // Get the data requests for the user or data requests made by the user. $sqlconditions[] = "(userid = :userid OR requestedby = :requestedby)"; @@ -965,6 +983,34 @@ class api { return data_registry::get_effective_contextlevel_value($contextlevel, 'purpose', $forcedvalue); } + /** + * Creates an expired context record for the provided context id. + * + * @param int $contextid + * @return \tool_dataprivacy\expired_context + */ + public static function create_expired_context($contextid) { + $record = (object)[ + 'contextid' => $contextid, + 'status' => expired_context::STATUS_EXPIRED, + ]; + $expiredctx = new expired_context(0, $record); + $expiredctx->save(); + + return $expiredctx; + } + + /** + * Deletes an expired context record. + * + * @param int $id The tool_dataprivacy_ctxexpire id. + * @return bool True on success. + */ + public static function delete_expired_context($id) { + $expiredcontext = new expired_context($id); + return $expiredcontext->delete(); + } + /** * Updates the status of an expired context. * diff --git a/admin/tool/dataprivacy/classes/data_request.php b/admin/tool/dataprivacy/classes/data_request.php index 997642261fa..091921e8b08 100644 --- a/admin/tool/dataprivacy/classes/data_request.php +++ b/admin/tool/dataprivacy/classes/data_request.php @@ -21,7 +21,9 @@ * @copyright 2018 Jun Pataleta * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ + namespace tool_dataprivacy; + defined('MOODLE_INTERNAL') || die(); use core\persistent; diff --git a/admin/tool/dataprivacy/classes/event/user_deleted_observer.php b/admin/tool/dataprivacy/classes/event/user_deleted_observer.php new file mode 100644 index 00000000000..380236bd023 --- /dev/null +++ b/admin/tool/dataprivacy/classes/event/user_deleted_observer.php @@ -0,0 +1,63 @@ +. + +/** + * Event observers supported by this module. + * + * @package tool_dataprivacy + * @copyright 2018 Mihail Geshoski + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace tool_dataprivacy\event; + +use \tool_dataprivacy\api; +use \tool_dataprivacy\data_request; + +defined('MOODLE_INTERNAL') || die(); + +/** + * Event observers supported by this module. + * + * @package tool_dataprivacy + * @copyright 2018 Mihail Geshoski + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class user_deleted_observer { + + /** + * Create user data deletion request when the user is deleted. + * + * @param \core\event\user_deleted $event + */ + public static function create_delete_data_request(\core\event\user_deleted $event) { + // Automatic creation of deletion requests must be enabled. + if (get_config('tool_dataprivacy', 'automaticdeletionrequests')) { + $requesttypes = [api::DATAREQUEST_TYPE_DELETE]; + $requeststatuses = [api::DATAREQUEST_STATUS_COMPLETE, api::DATAREQUEST_STATUS_DELETED]; + + $hasongoingdeleterequests = api::has_ongoing_request($event->objectid, $requesttypes[0]); + $hascompleteddeleterequest = (api::get_data_requests_count($event->objectid, $requeststatuses, + $requesttypes) > 0) ? true : false; + + if (!$hasongoingdeleterequests && !$hascompleteddeleterequest) { + api::create_data_request($event->objectid, $requesttypes[0], + get_string('datarequestcreatedupondelete', 'tool_dataprivacy'), + data_request::DATAREQUEST_CREATION_AUTO); + } + } + } +} diff --git a/admin/tool/dataprivacy/classes/expired_user_contexts.php b/admin/tool/dataprivacy/classes/expired_user_contexts.php new file mode 100644 index 00000000000..009e71d047a --- /dev/null +++ b/admin/tool/dataprivacy/classes/expired_user_contexts.php @@ -0,0 +1,149 @@ +. + +/** + * Expired contexts manager for CONTEXT_USER. + * + * @package tool_dataprivacy + * @copyright 2018 David Monllao + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace tool_dataprivacy; + +use core_privacy\manager; + +defined('MOODLE_INTERNAL') || die(); + +/** + * Expired contexts manager for CONTEXT_USER. + * + * @copyright 2018 David Monllao + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class expired_user_contexts extends \tool_dataprivacy\expired_contexts_manager { + + /** + * Only user level. + * + * @return int[] + */ + protected function get_context_levels() { + return [CONTEXT_USER]; + } + + /** + * Returns the user context instances that are expired. + * + * @return \stdClass[] + */ + protected function get_expired_contexts() { + global $DB; + + // Including context info + last login timestamp. + $fields = 'ctx.id AS id, ' . \context_helper::get_preload_record_columns_sql('ctx'); + + $purpose = api::get_effective_contextlevel_purpose(CONTEXT_USER); + + // Calculate what is considered expired according to the context level effective purpose (= now + retention period). + $expiredtime = new \DateTime(); + $retention = new \DateInterval($purpose->get('retentionperiod')); + $expiredtime->sub($retention); + + $sql = "SELECT $fields FROM {context} ctx + JOIN {user} u ON ctx.contextlevel = ? AND ctx.instanceid = u.id + LEFT JOIN {tool_dataprivacy_ctxexpired} expiredctx ON ctx.id = expiredctx.contextid + WHERE u.lastaccess <= ? AND u.lastaccess > 0 AND expiredctx.id IS NULL + ORDER BY ctx.path, ctx.contextlevel ASC"; + $possiblyexpired = $DB->get_recordset_sql($sql, [CONTEXT_USER, $expiredtime->getTimestamp()]); + + $expiredcontexts = []; + foreach ($possiblyexpired as $record) { + + \context_helper::preload_from_record($record); + + // No strict checking as the context may already be deleted (e.g. we just deleted a course, + // module contexts below it will not exist). + $context = \context::instance_by_id($record->id, false); + if (!$context) { + continue; + } + + if (is_siteadmin($context->instanceid)) { + continue; + } + + $courses = enrol_get_users_courses($context->instanceid, false, ['enddate']); + foreach ($courses as $course) { + if (!$course->enddate) { + // We can not know it what is going on here, so we prefer to be conservative. + continue 2; + } + + if ($course->enddate >= time()) { + // Future or ongoing course. + continue 2; + } + } + + $expiredcontexts[$context->id] = $context; + } + + return $expiredcontexts; + } + + /** + * Deletes user data from the provided context. + * + * Overwritten to delete the user. + * + * @param manager $privacymanager + * @param expired_context $expiredctx + * @return \context|false + */ + protected function delete_expired_context(manager $privacymanager, expired_context $expiredctx) { + $context = \context::instance_by_id($expiredctx->get('contextid'), IGNORE_MISSING); + if (!$context) { + return false; + } + + if (!PHPUNIT_TEST) { + mtrace('Deleting context ' . $context->id . ' - ' . + shorten_text($context->get_context_name(true, true))); + } + + // To ensure that all user data is deleted, instead of deleting by context, we run through and collect any stray + // contexts for the user that may still exist and call delete_data_for_user(). + $user = \core_user::get_user($context->instanceid, '*', MUST_EXIST); + $approvedlistcollection = new \core_privacy\local\request\contextlist_collection($user->id); + $contextlistcollection = $privacymanager->get_contexts_for_userid($user->id); + + foreach ($contextlistcollection as $contextlist) { + $approvedlistcollection->add_contextlist(new \core_privacy\local\request\approved_contextlist( + $user, + $contextlist->get_component(), + $contextlist->get_contextids() + )); + } + + $privacymanager->delete_data_for_user($approvedlistcollection); + api::set_expired_context_status($expiredctx, expired_context::STATUS_CLEANED); + + // Delete the user. + delete_user($user); + + return $context; + } +} diff --git a/admin/tool/dataprivacy/classes/local/helper.php b/admin/tool/dataprivacy/classes/local/helper.php index 2609ef192d9..25f5bf819cf 100644 --- a/admin/tool/dataprivacy/classes/local/helper.php +++ b/admin/tool/dataprivacy/classes/local/helper.php @@ -27,6 +27,7 @@ defined('MOODLE_INTERNAL') || die(); use coding_exception; use moodle_exception; use tool_dataprivacy\api; +use tool_dataprivacy\data_request; /** * Class containing helper functions for the data privacy tool. @@ -44,6 +45,9 @@ class helper { /** Filter constant associated with the request status filter. */ const FILTER_STATUS = 2; + /** Filter constant associated with the request creation filter. */ + const FILTER_CREATION = 3; + /** The request filters preference key. */ const PREF_REQUEST_FILTERS = 'tool_dataprivacy_request-filters'; @@ -145,6 +149,34 @@ class helper { ]; } + /** + * Retrieves the human-readable value of a data request creation method. + * + * @param int $creation The request creation method. + * @return string + * @throws moodle_exception + */ + public static function get_request_creation_method_string($creation) { + $creationmethods = self::get_request_creation_methods(); + if (!isset($creationmethods[$creation])) { + throw new moodle_exception('errorinvalidrequestcreationmethod', 'tool_dataprivacy'); + } + + return $creationmethods[$creation]; + } + + /** + * Returns the key value-pairs of request creation method code and string value. + * + * @return array + */ + public static function get_request_creation_methods() { + return [ + data_request::DATAREQUEST_CREATION_MANUAL => get_string('creationmanual', 'tool_dataprivacy'), + data_request::DATAREQUEST_CREATION_AUTO => get_string('creationauto', 'tool_dataprivacy'), + ]; + } + /** * Get the users that a user can make data request for. * @@ -199,6 +231,10 @@ class helper { 'name' => get_string('requeststatus', 'tool_dataprivacy'), 'options' => self::get_request_statuses() ], + self::FILTER_CREATION => (object)[ + 'name' => get_string('requestcreation', 'tool_dataprivacy'), + 'options' => self::get_request_creation_methods() + ], ]; $options = []; foreach ($filters as $category => $filtercategory) { diff --git a/admin/tool/dataprivacy/classes/output/data_requests_table.php b/admin/tool/dataprivacy/classes/output/data_requests_table.php index b6fa957b2bb..01a4d4588a7 100644 --- a/admin/tool/dataprivacy/classes/output/data_requests_table.php +++ b/admin/tool/dataprivacy/classes/output/data_requests_table.php @@ -74,15 +74,17 @@ class data_requests_table extends table_sql { * @param int $userid The user ID * @param int[] $statuses * @param int[] $types + * @param int[] $creationmethods * @param bool $manage * @throws coding_exception */ - public function __construct($userid = 0, $statuses = [], $types = [], $manage = false) { + public function __construct($userid = 0, $statuses = [], $types = [], $creationmethods = [], $manage = false) { parent::__construct('data-requests-table'); $this->userid = $userid; $this->statuses = $statuses; $this->types = $types; + $this->creationmethods = $creationmethods; $this->manage = $manage; $checkboxattrs = [ @@ -273,11 +275,12 @@ class data_requests_table extends table_sql { $sort = $this->get_sql_sort(); // Get data requests from the given conditions. - $datarequests = api::get_data_requests($this->userid, $this->statuses, $this->types, $sort, - $this->get_page_start(), $this->get_page_size()); + $datarequests = api::get_data_requests($this->userid, $this->statuses, $this->types, + $this->creationmethods, $sort, $this->get_page_start(), $this->get_page_size()); // Count data requests from the given conditions. - $total = api::get_data_requests_count($this->userid, $this->statuses, $this->types); + $total = api::get_data_requests_count($this->userid, $this->statuses, $this->types, + $this->creationmethods); $this->pagesize($pagesize, $total); $this->rawdata = []; diff --git a/admin/tool/dataprivacy/classes/privacy/provider.php b/admin/tool/dataprivacy/classes/privacy/provider.php index 193bd70538d..d415002fed5 100644 --- a/admin/tool/dataprivacy/classes/privacy/provider.php +++ b/admin/tool/dataprivacy/classes/privacy/provider.php @@ -167,6 +167,8 @@ class provider implements $data->type = tool_helper::get_shortened_request_type_string($record->type); // Status. $data->status = tool_helper::get_request_status_string($record->status); + // Creation method. + $data->creationmethod = tool_helper::get_request_creation_method_string($record->creationmethod); // Comments. $data->comments = $record->comments; // The DPO's comment about this request. @@ -234,6 +236,10 @@ class provider implements $option->category = get_string('requeststatus', 'tool_dataprivacy'); $option->name = tool_helper::get_request_status_string($value); break; + case tool_helper::FILTER_CREATION: + $option->category = get_string('requestcreation', 'tool_dataprivacy'); + $option->name = tool_helper::get_request_creation_method_string($value); + break; } $descriptions[] = get_string('filteroption', 'tool_dataprivacy', $option); } diff --git a/admin/tool/dataprivacy/datarequests.php b/admin/tool/dataprivacy/datarequests.php index 8b2b16f090a..ec6b9c2b1c5 100644 --- a/admin/tool/dataprivacy/datarequests.php +++ b/admin/tool/dataprivacy/datarequests.php @@ -55,6 +55,7 @@ if (\tool_dataprivacy\api::is_site_dpo($USER->id)) { $types = []; $statuses = []; + $creationmethods = []; foreach ($filtersapplied as $filter) { list($category, $value) = explode(':', $filter); switch($category) { @@ -64,10 +65,13 @@ if (\tool_dataprivacy\api::is_site_dpo($USER->id)) { case \tool_dataprivacy\local\helper::FILTER_STATUS: $statuses[] = $value; break; + case \tool_dataprivacy\local\helper::FILTER_CREATION: + $creationmethods[] = $value; + break; } } - $table = new \tool_dataprivacy\output\data_requests_table(0, $statuses, $types, true); + $table = new \tool_dataprivacy\output\data_requests_table(0, $statuses, $types, $creationmethods, true); if (!empty($perpage)) { set_user_preference(\tool_dataprivacy\local\helper::PREF_REQUEST_PERPAGE, $perpage); } else { diff --git a/admin/tool/dataprivacy/db/events.php b/admin/tool/dataprivacy/db/events.php new file mode 100644 index 00000000000..2278b66ba1c --- /dev/null +++ b/admin/tool/dataprivacy/db/events.php @@ -0,0 +1,32 @@ +. + +/** + * This file defines observers needed by the plugin. + * + * @package tool_dataprivacy + * @copyright 2018 Mihail Geshoski + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$observers = [ + [ + 'eventname' => '\core\event\user_deleted', + 'callback' => '\tool_dataprivacy\event\user_deleted_observer::create_delete_data_request', + ], +]; diff --git a/admin/tool/dataprivacy/db/upgrade.php b/admin/tool/dataprivacy/db/upgrade.php index 5449638ae4d..306337d346c 100644 --- a/admin/tool/dataprivacy/db/upgrade.php +++ b/admin/tool/dataprivacy/db/upgrade.php @@ -252,10 +252,12 @@ function xmldb_tool_dataprivacy_upgrade($oldversion) { // Define field sensitivedatareasons to be added to tool_dataprivacy_purpose. $table = new xmldb_table('tool_dataprivacy_request'); $field = new xmldb_field('creationmethod', XMLDB_TYPE_INTEGER, 10, null, XMLDB_NOTNULL, null, 0, 'timemodified'); + // Conditionally launch add field sensitivedatareasons. if (!$dbman->field_exists($table, $field)) { $dbman->add_field($table, $field); } + // Dataprivacy savepoint reached. upgrade_plugin_savepoint(true, 2018100406, 'tool', 'dataprivacy'); } diff --git a/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php b/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php index 84ca013c60a..72c20c4184e 100644 --- a/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php +++ b/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php @@ -32,6 +32,8 @@ $string['addnewdefaults'] = 'Add a new module default'; $string['addpurpose'] = 'Add purpose'; $string['approve'] = 'Approve'; $string['approverequest'] = 'Approve request'; +$string['automaticdeletionrequests'] = 'Create automatic data deletion requests'; +$string['automaticdeletionrequests_desc'] = 'If enabled, automatic delete data request will be created upon user deletion or for each existing deleted user which data was not fully deleted.'; $string['bulkapproverequests'] = 'Approve requests'; $string['bulkdenyrequests'] = 'Deny requests'; $string['cachedef_purpose'] = 'Data purposes'; @@ -68,6 +70,8 @@ $string['contactdpoviaprivacypolicy'] = 'Please contact the privacy officer as d $string['createcategory'] = 'Create data category'; $string['createnewdatarequest'] = 'Create a new data request'; $string['createpurpose'] = 'Create data purpose'; +$string['creationauto'] = 'Automatically'; +$string['creationmanual'] = 'Manually'; $string['datadeletion'] = 'Data deletion'; $string['datadeletionpagehelp'] = 'Data for which the retention period has expired are listed here. Please review and confirm data deletion, which will then be executed by the "Delete expired contexts" scheduled task.'; $string['dataprivacy:makedatarequestsforchildren'] = 'Make data requests for minors'; @@ -82,6 +86,7 @@ $string['dataretentionsummary'] = 'Data retention summary'; $string['datarequestcreatedforuser'] = 'Data request created for {$a}'; $string['datarequestcreatedfromscheduledtask'] = 'Automatically created from a scheduled task (pre-existing deleted user).'; $string['datarequestemailsubject'] = 'Data request: {$a}'; +$string['datarequestcreatedupondelete'] = 'Automatically created upon user deletion.'; $string['datarequests'] = 'Data requests'; $string['datecomment'] = '[{$a->date}]: ' . PHP_EOL . ' {$a->comment}'; $string['daterequested'] = 'Date requested'; @@ -117,6 +122,7 @@ $string['editpurposes'] = 'Edit purposes'; $string['effectiveretentionperiodcourse'] = '{$a} (after the course end date)'; $string['effectiveretentionperioduser'] = '{$a} (since the last time the user accessed the site)'; $string['emailsalutation'] = 'Dear {$a},'; +$string['errorinvalidrequestcreationmethod'] = 'Invalid request creation method!'; $string['errorinvalidrequeststatus'] = 'Invalid request status!'; $string['errorinvalidrequesttype'] = 'Invalid request type!'; $string['errornocapabilitytorequestforothers'] = 'User {$a->requestedby} doesn\'t have the capability to make a data request on behalf of user {$a->userid}'; @@ -235,6 +241,7 @@ $string['requestby'] = 'Requested by'; $string['requestbydetail'] = 'Requested by:'; $string['requestcomments'] = 'Comments'; $string['requestcomments_help'] = 'This box enables you to enter any further details about your data request.'; +$string['requestcreation'] = 'Creation'; $string['requestdenied'] = 'The request has been denied'; $string['requestemailintro'] = 'You have received a data request:'; $string['requestfor'] = 'User'; diff --git a/admin/tool/dataprivacy/mydatarequests.php b/admin/tool/dataprivacy/mydatarequests.php index f435d0018b3..0a4070a0104 100644 --- a/admin/tool/dataprivacy/mydatarequests.php +++ b/admin/tool/dataprivacy/mydatarequests.php @@ -55,7 +55,7 @@ $PAGE->set_title($title); echo $OUTPUT->header(); echo $OUTPUT->heading($title); -$requests = tool_dataprivacy\api::get_data_requests($USER->id, [], [], 'timecreated DESC'); +$requests = tool_dataprivacy\api::get_data_requests($USER->id, [], [], [], 'timecreated DESC'); $requestlist = new tool_dataprivacy\output\my_data_requests_page($requests); $requestlistoutput = $PAGE->get_renderer('tool_dataprivacy'); echo $requestlistoutput->render($requestlist); diff --git a/admin/tool/dataprivacy/settings.php b/admin/tool/dataprivacy/settings.php index a434384dc39..de3d7151ecb 100644 --- a/admin/tool/dataprivacy/settings.php +++ b/admin/tool/dataprivacy/settings.php @@ -34,6 +34,14 @@ if ($hassiteconfig) { new lang_string('contactdataprotectionofficer_desc', 'tool_dataprivacy'), 0) ); + // Automatically create delete data request for users upon user deletion. + // Automatically create delete data request for pre-existing deleted users. + // Enabled by default. + $privacysettings->add(new admin_setting_configcheckbox('tool_dataprivacy/automaticdeletionrequests', + new lang_string('automaticdeletionrequests', 'tool_dataprivacy'), + new lang_string('automaticdeletionrequests_desc', 'tool_dataprivacy'), 1) + ); + // Set days approved data requests will be accessible. 1 week default. $privacysettings->add(new admin_setting_configduration('tool_dataprivacy/privacyrequestexpiry', new lang_string('privacyrequestexpiry', 'tool_dataprivacy'), From 673d2c58fb7799cf3cc839a2dc3bac2bf170da8e Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Tue, 6 Nov 2018 13:11:34 +0800 Subject: [PATCH 3/3] MDL-62564 privacy: Create request for deleted users when setting enabled --- .../task/delete_existing_deleted_users.php | 31 ++++----- admin/tool/dataprivacy/tests/task_test.php | 63 +++++++++++++++++-- 2 files changed, 74 insertions(+), 20 deletions(-) diff --git a/admin/tool/dataprivacy/classes/task/delete_existing_deleted_users.php b/admin/tool/dataprivacy/classes/task/delete_existing_deleted_users.php index a0787477ea1..83827efeb9c 100644 --- a/admin/tool/dataprivacy/classes/task/delete_existing_deleted_users.php +++ b/admin/tool/dataprivacy/classes/task/delete_existing_deleted_users.php @@ -57,8 +57,10 @@ class delete_existing_deleted_users extends scheduled_task { public function execute() { global $DB; - // Select all deleted users that do not have any delete data requests created for them. - $sql = "SELECT DISTINCT(u.id) + // Automatic creation of deletion requests must be enabled. + if (get_config('tool_dataprivacy', 'automaticdeletionrequests')) { + // Select all deleted users that do not have any delete data requests created for them. + $sql = "SELECT DISTINCT(u.id) FROM {user} u LEFT JOIN {tool_dataprivacy_request} r ON u.id = r.userid @@ -66,23 +68,24 @@ class delete_existing_deleted_users extends scheduled_task { AND (r.id IS NULL OR r.type != ?)"; - $params = [ - 1, - api::DATAREQUEST_TYPE_DELETE - ]; + $params = [ + 1, + api::DATAREQUEST_TYPE_DELETE + ]; - $deletedusers = $DB->get_records_sql($sql, $params); - $createdrequests = 0; + $deletedusers = $DB->get_records_sql($sql, $params); + $createdrequests = 0; - foreach ($deletedusers as $user) { - api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE, + foreach ($deletedusers as $user) { + api::create_data_request($user->id, api::DATAREQUEST_TYPE_DELETE, get_string('datarequestcreatedfromscheduledtask', 'tool_dataprivacy'), data_request::DATAREQUEST_CREATION_AUTO); - $createdrequests++; - } + $createdrequests++; + } - if ($createdrequests > 0) { - mtrace($createdrequests . ' delete data request(s) created for existing deleted users'); + if ($createdrequests > 0) { + mtrace($createdrequests . ' delete data request(s) created for existing deleted users'); + } } } } diff --git a/admin/tool/dataprivacy/tests/task_test.php b/admin/tool/dataprivacy/tests/task_test.php index e6be1b850a5..1006364b580 100644 --- a/admin/tool/dataprivacy/tests/task_test.php +++ b/admin/tool/dataprivacy/tests/task_test.php @@ -53,6 +53,10 @@ class tool_dataprivacy_task_testcase extends data_privacy_testcase { $this->resetAfterTest(); $this->setAdminUser(); + + // Enable automatic creation of delete data requests. + set_config('automaticdeletionrequests', 1, 'tool_dataprivacy'); + // Create a user. $user = $this->getDataGenerator()->create_user(); // Mark the user as deleted. @@ -69,6 +73,35 @@ class tool_dataprivacy_task_testcase extends data_privacy_testcase { [api::DATAREQUEST_TYPE_DELETE])); } + /** + * Ensure that a delete data request for pre-existing deleted users + * is not being created when automatic creation of delete data requests is disabled. + */ + public function test_delete_existing_deleted_users_task_automatic_creation_disabled() { + global $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + // Disable automatic creation of delete data requests. + set_config('automaticdeletionrequests', 0, 'tool_dataprivacy'); + + // Create a user. + $user = $this->getDataGenerator()->create_user(); + // Mark the user as deleted. + $user->deleted = 1; + $DB->update_record('user', $user); + + // The user should not have a delete data request. + $this->assertCount(0, api::get_data_requests($user->id, [], + [api::DATAREQUEST_TYPE_DELETE])); + + $this->execute_task('tool_dataprivacy\task\delete_existing_deleted_users'); + // After running the scheduled task, the deleted user should still not have a delete data request. + $this->assertCount(0, api::get_data_requests($user->id, [], + [api::DATAREQUEST_TYPE_DELETE])); + } + /** * Ensure that a delete data request for pre-existing deleted users * is created when there are existing non-delete data requests @@ -79,6 +112,10 @@ class tool_dataprivacy_task_testcase extends data_privacy_testcase { $this->resetAfterTest(); $this->setAdminUser(); + + // Enable automatic creation of delete data requests. + set_config('automaticdeletionrequests', 1, 'tool_dataprivacy'); + // Create a user. $user = $this->getDataGenerator()->create_user(); // Create export data request for the user. @@ -106,8 +143,14 @@ class tool_dataprivacy_task_testcase extends data_privacy_testcase { * for that particular user. */ public function test_delete_existing_deleted_users_task_existing_ongoing_delete_data_requests() { + global $DB; + $this->resetAfterTest(); $this->setAdminUser(); + + // Enable automatic creation of delete data requests. + set_config('automaticdeletionrequests', 1, 'tool_dataprivacy'); + // Create a user. $user = $this->getDataGenerator()->create_user(); $this->setUser($user); @@ -120,9 +163,10 @@ class tool_dataprivacy_task_testcase extends data_privacy_testcase { $this->assertCount(1, api::get_data_requests($user->id, [api::DATAREQUEST_STATUS_AWAITING_APPROVAL], [api::DATAREQUEST_TYPE_DELETE])); - $this->setAdminUser(); - // Delete the user. - delete_user($user); + // Mark the user as deleted. + $user->deleted = 1; + $DB->update_record('user', $user); + // The user should still have the existing ongoing delete data request. $this->assertCount(1, \tool_dataprivacy\api::get_data_requests($user->id, [api::DATAREQUEST_STATUS_AWAITING_APPROVAL], [api::DATAREQUEST_TYPE_DELETE])); @@ -142,8 +186,14 @@ class tool_dataprivacy_task_testcase extends data_privacy_testcase { * for that particular user. */ public function test_delete_existing_deleted_users_task_existing_finished_delete_data_requests() { + global $DB; + $this->resetAfterTest(); $this->setAdminUser(); + + // Enable automatic creation of delete data requests. + set_config('automaticdeletionrequests', 1, 'tool_dataprivacy'); + // Create a user. $user = $this->getDataGenerator()->create_user(); $this->setUser($user); @@ -158,9 +208,10 @@ class tool_dataprivacy_task_testcase extends data_privacy_testcase { // The user should not have an ongoing data requests. $this->assertFalse(api::has_ongoing_request($user->id, api::DATAREQUEST_TYPE_DELETE)); - $this->setAdminUser(); - // Delete the user. - delete_user($user); + // Mark the user as deleted. + $user->deleted = 1; + $DB->update_record('user', $user); + // The user should still have the existing finished delete data request. $this->assertCount(1, \tool_dataprivacy\api::get_data_requests($user->id, [api::DATAREQUEST_STATUS_CANCELLED], [api::DATAREQUEST_TYPE_DELETE]));