From 7bd269e82d29490ce00859e6652f54b087f92f71 Mon Sep 17 00:00:00 2001 From: Andrew Nicols <andrew@nicols.co.uk> Date: Fri, 1 Jun 2018 11:35:03 +0800 Subject: [PATCH 1/7] MDL-62660 tool_dataprivacy: Add method for unit tests to run adhoc tasks --- lib/phpunit/classes/advanced_testcase.php | 58 +++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/lib/phpunit/classes/advanced_testcase.php b/lib/phpunit/classes/advanced_testcase.php index a0ec58912e1..546d11bee51 100644 --- a/lib/phpunit/classes/advanced_testcase.php +++ b/lib/phpunit/classes/advanced_testcase.php @@ -663,4 +663,62 @@ abstract class advanced_testcase extends base_testcase { usleep(50000); } } + + /** + * Run adhoc tasks, optionally matching the specified classname. + * + * @param string $matchclass The name of the class to match on. + * @param int $matchuserid The userid to match. + */ + protected function runAdhocTasks($matchclass = '', $matchuserid = null) { + global $CFG, $DB; + require_once($CFG->libdir.'/cronlib.php'); + + $params = []; + if (!empty($matchclass)) { + if (strpos($matchclass, '\\') !== 0) { + $matchclass = '\\' . $matchclass; + } + $params['classname'] = $matchclass; + } + + if (!empty($matchuserid)) { + $params['userid'] = $matchuserid; + } + + $lock = $this->createMock(\core\lock\lock::class); + $cronlock = $this->createMock(\core\lock\lock::class); + + $tasks = $DB->get_recordset('task_adhoc', $params); + foreach ($tasks as $record) { + // Note: This is for cron only. + // We do not lock the tasks. + $task = \core\task\manager::adhoc_task_from_record($record); + + $user = null; + if ($userid = $task->get_userid()) { + // This task has a userid specified. + $user = \core_user::get_user($userid); + + // User found. Check that they are suitable. + \core_user::require_active_user($user, true, true); + } + + $task->set_lock($lock); + if (!$task->is_blocking()) { + $cronlock->release(); + } else { + $task->set_cron_lock($cronlock); + } + + cron_prepare_core_renderer(); + $this->setUser($user); + + $task->execute(); + \core\task\manager::adhoc_task_complete($task); + + unset($task); + } + $tasks->close(); + } } From 693f690c18ef1578670670f443f2d0feb3e9e576 Mon Sep 17 00:00:00 2001 From: Michael Hawkins <michaelh@moodle.com> Date: Fri, 8 Jun 2018 16:29:53 +0800 Subject: [PATCH 2/7] MDL-62660 tool_dataprivacy: Add ability to expire data requests Also replaced Completed status with situation specific statuses. Also improved UX on request pages in line with expiries and the aadditional statuses. --- admin/tool/dataprivacy/classes/api.php | 29 ++++- .../tool/dataprivacy/classes/data_request.php | 100 ++++++++++++++++++ .../external/data_request_exporter.php | 7 +- .../tool/dataprivacy/classes/local/helper.php | 4 + .../classes/output/data_requests_table.php | 30 +++--- .../classes/output/my_data_requests_page.php | 20 +++- .../task/process_data_request_task.php | 4 +- admin/tool/dataprivacy/db/upgrade.php | 27 +++++ .../dataprivacy/lang/en/tool_dataprivacy.php | 6 ++ admin/tool/dataprivacy/lib.php | 5 + admin/tool/dataprivacy/settings.php | 6 ++ .../templates/my_data_requests.mustache | 17 ++- admin/tool/dataprivacy/version.php | 2 +- privacy/classes/local/request/writer.php | 18 ++++ 14 files changed, 255 insertions(+), 20 deletions(-) diff --git a/admin/tool/dataprivacy/classes/api.php b/admin/tool/dataprivacy/classes/api.php index bbf57daf39a..6ee970733d7 100644 --- a/admin/tool/dataprivacy/classes/api.php +++ b/admin/tool/dataprivacy/classes/api.php @@ -76,7 +76,7 @@ class api { /** The request is now being processed. */ const DATAREQUEST_STATUS_PROCESSING = 4; - /** Data request completed. */ + /** Information/other request completed. */ const DATAREQUEST_STATUS_COMPLETE = 5; /** Data request cancelled by the user. */ @@ -85,6 +85,15 @@ class api { /** Data request rejected by the DPO. */ const DATAREQUEST_STATUS_REJECTED = 7; + /** Data request download ready. */ + const DATAREQUEST_STATUS_DOWNLOAD_READY = 8; + + /** Data request expired. */ + const DATAREQUEST_STATUS_EXPIRED = 9; + + /** Data delete request completed, account is removed. */ + const DATAREQUEST_STATUS_DELETED = 10; + /** * Determines whether the user can contact the site's Data Protection Officer via Moodle. * @@ -319,6 +328,18 @@ class api { } } + // If any are due to expire, expire them and re-fetch updated data. + if (empty($statuses) + || in_array(self::DATAREQUEST_STATUS_DOWNLOAD_READY, $statuses) + || in_array(self::DATAREQUEST_STATUS_EXPIRED, $statuses)) { + $expiredrequests = data_request::get_expired_requests($userid); + + if (!empty($expiredrequests)) { + data_request::expire($expiredrequests); + $results = self::get_data_requests($userid, $statuses, $types, $sort, $offset, $limit); + } + } + return $results; } @@ -400,6 +421,9 @@ class api { self::DATAREQUEST_STATUS_COMPLETE, self::DATAREQUEST_STATUS_CANCELLED, self::DATAREQUEST_STATUS_REJECTED, + self::DATAREQUEST_STATUS_DOWNLOAD_READY, + self::DATAREQUEST_STATUS_EXPIRED, + self::DATAREQUEST_STATUS_DELETED, ]; list($insql, $inparams) = $DB->get_in_or_equal($nonpendingstatuses, SQL_PARAMS_NAMED); $select = 'type = :type AND userid = :userid AND status NOT ' . $insql; @@ -423,6 +447,9 @@ class api { self::DATAREQUEST_STATUS_COMPLETE, self::DATAREQUEST_STATUS_CANCELLED, self::DATAREQUEST_STATUS_REJECTED, + self::DATAREQUEST_STATUS_DOWNLOAD_READY, + self::DATAREQUEST_STATUS_EXPIRED, + self::DATAREQUEST_STATUS_DELETED, ]; return !in_array($status, $finalstatuses); diff --git a/admin/tool/dataprivacy/classes/data_request.php b/admin/tool/dataprivacy/classes/data_request.php index d5ab2187636..c41db11e839 100644 --- a/admin/tool/dataprivacy/classes/data_request.php +++ b/admin/tool/dataprivacy/classes/data_request.php @@ -85,6 +85,9 @@ class data_request extends persistent { api::DATAREQUEST_STATUS_COMPLETE, api::DATAREQUEST_STATUS_CANCELLED, api::DATAREQUEST_STATUS_REJECTED, + api::DATAREQUEST_STATUS_DOWNLOAD_READY, + api::DATAREQUEST_STATUS_EXPIRED, + api::DATAREQUEST_STATUS_DELETED, ], 'type' => PARAM_INT ], @@ -110,4 +113,101 @@ class data_request extends persistent { ], ]; } + + /** + * Determines whether a completed data export request has expired. + * The response will be valid regardless of the expiry scheduled task having run. + * + * @param data_request $request the data request object whose expiry will be checked. + * @return bool true if the request has expired. + */ + public static function is_expired(data_request $request) { + $result = false; + + // Only export requests expire. + if ($request->get('type') == api::DATAREQUEST_TYPE_EXPORT) { + switch ($request->get('status')) { + // Expired requests are obviously expired. + case api::DATAREQUEST_STATUS_EXPIRED: + $result = true; + break; + // Complete requests are expired if the expiry time has elapsed. + case api::DATAREQUEST_STATUS_DOWNLOAD_READY: + $expiryseconds = get_config('tool_dataprivacy', 'privacyrequestexpiry'); + if ($expiryseconds > 0 && time() >= ($request->get('timemodified') + $expiryseconds)) { + $result = true; + } + break; + } + } + + return $result; + } + + + + /** + * Fetch completed data requests which are due to expire. + * + * @param int $userid Optional user ID to filter by. + * + * @return array Details of completed requests which are due to expire. + */ + public static function get_expired_requests($userid = 0) { + global $DB; + + $expiryseconds = get_config('tool_dataprivacy', 'privacyrequestexpiry'); + $expirytime = strtotime("-{$expiryseconds} second"); + $table = data_request::TABLE; + $sqlwhere = 'type = :export_type AND status = :completestatus AND timemodified <= :expirytime'; + $params = array( + 'export_type' => api::DATAREQUEST_TYPE_EXPORT, + 'completestatus' => api::DATAREQUEST_STATUS_DOWNLOAD_READY, + 'expirytime' => $expirytime, + ); + $sort = 'id'; + $fields = 'id, userid'; + + // Filter by user ID if specified. + if ($userid > 0) { + $sqlwhere .= ' AND (userid = :userid OR requestedby = :requestedby)'; + $params['userid'] = $userid; + $params['requestedby'] = $userid; + } + + return $DB->get_records_select_menu($table, $sqlwhere, $params, $sort, $fields, 0, 2000); + } + + /** + * Expire a given set of data requests. + * Update request status and delete the files. + * + * @param array $expiredrequests [requestid => userid] + * + * @return void + */ + public static function expire($expiredrequests) { + global $DB; + + $ids = array_keys($expiredrequests); + + if (count($ids) > 0) { + list($insql, $inparams) = $DB->get_in_or_equal($ids); + $initialparams = array(api::DATAREQUEST_STATUS_EXPIRED, time()); + $params = array_merge($initialparams, $inparams); + + $update = "UPDATE {" . data_request::TABLE . "} + SET status = ?, timemodified = ? + WHERE id $insql"; + + if ($DB->execute($update, $params)) { + $fs = get_file_storage(); + + foreach ($expiredrequests as $id => $userid) { + $usercontext = \context_user::instance($userid); + $fs->delete_area_files($usercontext->id, 'tool_dataprivacy', 'export', $id); + } + } + } + } } diff --git a/admin/tool/dataprivacy/classes/external/data_request_exporter.php b/admin/tool/dataprivacy/classes/external/data_request_exporter.php index 93b33e3af47..b7d483ca64d 100644 --- a/admin/tool/dataprivacy/classes/external/data_request_exporter.php +++ b/admin/tool/dataprivacy/classes/external/data_request_exporter.php @@ -160,7 +160,7 @@ class data_request_exporter extends persistent_exporter { switch ($this->persistent->get('status')) { case api::DATAREQUEST_STATUS_PENDING: - $values['statuslabelclass'] = 'label-default'; + $values['statuslabelclass'] = 'label-info'; // Request can be manually completed for general enquiry requests. $values['canmarkcomplete'] = $requesttype == api::DATAREQUEST_TYPE_OTHERS; break; @@ -181,6 +181,8 @@ class data_request_exporter extends persistent_exporter { $values['statuslabelclass'] = 'label-info'; break; case api::DATAREQUEST_STATUS_COMPLETE: + case api::DATAREQUEST_STATUS_DOWNLOAD_READY: + case api::DATAREQUEST_STATUS_DELETED: $values['statuslabelclass'] = 'label-success'; break; case api::DATAREQUEST_STATUS_CANCELLED: @@ -189,6 +191,9 @@ class data_request_exporter extends persistent_exporter { case api::DATAREQUEST_STATUS_REJECTED: $values['statuslabelclass'] = 'label-important'; break; + case api::DATAREQUEST_STATUS_EXPIRED: + $values['statuslabelclass'] = 'label-default'; + break; } return $values; diff --git a/admin/tool/dataprivacy/classes/local/helper.php b/admin/tool/dataprivacy/classes/local/helper.php index f98362da954..36dd93aac2b 100644 --- a/admin/tool/dataprivacy/classes/local/helper.php +++ b/admin/tool/dataprivacy/classes/local/helper.php @@ -117,6 +117,7 @@ class helper { if (!isset($statuses[$status])) { throw new moodle_exception('errorinvalidrequeststatus', 'tool_dataprivacy'); } + return $statuses[$status]; } @@ -133,8 +134,11 @@ class helper { api::DATAREQUEST_STATUS_APPROVED => get_string('statusapproved', 'tool_dataprivacy'), api::DATAREQUEST_STATUS_PROCESSING => get_string('statusprocessing', 'tool_dataprivacy'), api::DATAREQUEST_STATUS_COMPLETE => get_string('statuscomplete', 'tool_dataprivacy'), + api::DATAREQUEST_STATUS_DOWNLOAD_READY => get_string('statusready', 'tool_dataprivacy'), + api::DATAREQUEST_STATUS_EXPIRED => get_string('statusexpired', 'tool_dataprivacy'), api::DATAREQUEST_STATUS_CANCELLED => get_string('statuscancelled', 'tool_dataprivacy'), api::DATAREQUEST_STATUS_REJECTED => get_string('statusrejected', 'tool_dataprivacy'), + api::DATAREQUEST_STATUS_DELETED => get_string('statusdeleted', 'tool_dataprivacy'), ]; } diff --git a/admin/tool/dataprivacy/classes/output/data_requests_table.php b/admin/tool/dataprivacy/classes/output/data_requests_table.php index ab40140bb9b..477e5039420 100644 --- a/admin/tool/dataprivacy/classes/output/data_requests_table.php +++ b/admin/tool/dataprivacy/classes/output/data_requests_table.php @@ -59,7 +59,7 @@ class data_requests_table extends table_sql { /** @var bool Whether this table is being rendered for managing data requests. */ protected $manage = false; - /** @var stdClass[] Array of data request persistents. */ + /** @var \tool_dataprivacy\data_request[] Array of data request persistents. */ protected $datarequests = []; /** @@ -206,14 +206,14 @@ class data_requests_table extends table_sql { $actiontext = get_string('denyrequest', 'tool_dataprivacy'); $actions[] = new action_menu_link_secondary($actionurl, null, $actiontext, $actiondata); break; - } - - if ($status == api::DATAREQUEST_STATUS_COMPLETE) { - $userid = $data->foruser->id; - $usercontext = \context_user::instance($userid, IGNORE_MISSING); - if ($usercontext && api::can_download_data_request_for_user($userid, $data->requestedbyuser->id)) { - $actions[] = api::get_download_link($usercontext, $requestid); - } + case api::DATAREQUEST_STATUS_DOWNLOAD_READY: + $userid = $data->foruser->id; + $usercontext = \context_user::instance($userid, IGNORE_MISSING); + // If user has permission to view download link, show relevant action item. + if ($usercontext && api::can_download_data_request_for_user($userid, $data->requestedbyuser->id)) { + $actions[] = api::get_download_link($usercontext, $requestid); + } + break; } $actionsmenu = new action_menu($actions); @@ -236,19 +236,25 @@ class data_requests_table extends table_sql { public function query_db($pagesize, $useinitialsbar = true) { global $PAGE; - // Count data requests from the given conditions. - $total = api::get_data_requests_count($this->userid, $this->statuses, $this->types); - $this->pagesize($pagesize, $total); + // Set dummy page total until we fetch full result set. + $this->pagesize($pagesize, $pagesize + 1); $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()); + + // Count data requests from the given conditions. + $total = api::get_data_requests_count($this->userid, $this->statuses, $this->types); + $this->pagesize($pagesize, $total); + $this->rawdata = []; $context = \context_system::instance(); $renderer = $PAGE->get_renderer('tool_dataprivacy'); + foreach ($datarequests as $persistent) { + $this->datarequests[$persistent->get('id')] = $persistent; $exporter = new data_request_exporter($persistent, ['context' => $context]); $this->rawdata[] = $exporter->export($renderer); } diff --git a/admin/tool/dataprivacy/classes/output/my_data_requests_page.php b/admin/tool/dataprivacy/classes/output/my_data_requests_page.php index d82968c31a0..729a7fe1710 100644 --- a/admin/tool/dataprivacy/classes/output/my_data_requests_page.php +++ b/admin/tool/dataprivacy/classes/output/my_data_requests_page.php @@ -109,13 +109,29 @@ class my_data_requests_page implements renderable, templatable { $item->statuslabelclass = 'label-success'; $item->statuslabel = get_string('statuscomplete', 'tool_dataprivacy'); $cancancel = false; - // Show download links only for export-type data requests. - $candownload = $type == api::DATAREQUEST_TYPE_EXPORT; + break; + case api::DATAREQUEST_STATUS_DOWNLOAD_READY: + $item->statuslabelclass = 'label-success'; + $item->statuslabel = get_string('statusready', 'tool_dataprivacy'); + $cancancel = false; + $candownload = true; + if ($usercontext) { $candownload = api::can_download_data_request_for_user( $request->get('userid'), $request->get('requestedby')); } break; + case api::DATAREQUEST_STATUS_DELETED: + $item->statuslabelclass = 'label-success'; + $item->statuslabel = get_string('statusdeleted', 'tool_dataprivacy'); + $cancancel = false; + break; + case api::DATAREQUEST_STATUS_EXPIRED: + $item->statuslabelclass = 'label-default'; + $item->statuslabel = get_string('statusexpired', 'tool_dataprivacy'); + $item->statuslabeltitle = get_string('downloadexpireduser', 'tool_dataprivacy'); + $cancancel = false; + break; case api::DATAREQUEST_STATUS_CANCELLED: case api::DATAREQUEST_STATUS_REJECTED: $cancancel = false; diff --git a/admin/tool/dataprivacy/classes/task/process_data_request_task.php b/admin/tool/dataprivacy/classes/task/process_data_request_task.php index 6db9252df7f..be0c0608edd 100644 --- a/admin/tool/dataprivacy/classes/task/process_data_request_task.php +++ b/admin/tool/dataprivacy/classes/task/process_data_request_task.php @@ -81,6 +81,7 @@ class process_data_request_task extends adhoc_task { // Update the status of this request as pre-processing. mtrace('Processing request...'); api::update_request_status($requestid, api::DATAREQUEST_STATUS_PROCESSING); + $completestatus = api::DATAREQUEST_STATUS_COMPLETE; if ($request->type == api::DATAREQUEST_TYPE_EXPORT) { // Get the collection of approved_contextlist objects needed for core_privacy data export. @@ -115,10 +116,11 @@ class process_data_request_task extends adhoc_task { $manager->set_observer(new \tool_dataprivacy\manager_observer()); $manager->delete_data_for_user($approvedclcollection); + $completestatus = api::DATAREQUEST_STATUS_DELETED; } // When the preparation of the metadata finishes, update the request status to awaiting approval. - api::update_request_status($requestid, api::DATAREQUEST_STATUS_COMPLETE); + api::update_request_status($requestid, $completestatus); mtrace('The processing of the user data request has been completed...'); // Create message to notify the user regarding the processing results. diff --git a/admin/tool/dataprivacy/db/upgrade.php b/admin/tool/dataprivacy/db/upgrade.php index 8c0b4fc09e3..9885e626346 100644 --- a/admin/tool/dataprivacy/db/upgrade.php +++ b/admin/tool/dataprivacy/db/upgrade.php @@ -23,6 +23,7 @@ */ defined('MOODLE_INTERNAL') || die(); +use tool_dataprivacy\api; /** * Function to upgrade tool_dataprivacy. @@ -145,5 +146,31 @@ function xmldb_tool_dataprivacy_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2018051405, 'tool', 'dataprivacy'); } + if ($oldversion < 2018051406) { + // Update completed delete requests to new delete status. + $query = "UPDATE {tool_dataprivacy_request} + SET status = :setstatus + WHERE type = :type + AND status = :wherestatus"; + $params = array( + 'setstatus' => 10, // Request deleted. + 'type' => 2, // Delete type. + 'wherestatus' => 5, // Request completed. + ); + + $DB->execute($query, $params); + + // Update completed data export requests to new download ready status. + $params = array( + 'setstatus' => 8, // Request download ready. + 'type' => 1, // export type. + 'wherestatus' => 5, // Request completed. + ); + + $DB->execute($query, $params); + + upgrade_plugin_savepoint(true, 2018051406, 'tool', 'dataprivacy'); + } + return true; } diff --git a/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php b/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php index 3c1c31cfa83..e5810c488ed 100644 --- a/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php +++ b/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php @@ -86,6 +86,7 @@ $string['defaultssaved'] = 'Defaults saved'; $string['deny'] = 'Deny'; $string['denyrequest'] = 'Deny request'; $string['download'] = 'Download'; +$string['downloadexpireduser'] = 'Download has expired. Submit a new request if you wish to export your personal data.'; $string['dporolemapping'] = 'Privacy officer role mapping'; $string['dporolemapping_desc'] = 'The privacy officer can manage data requests. The capability tool/dataprivacy:managedatarequests must be allowed for a role to be listed as a privacy officer role mapping option.'; $string['editcategories'] = 'Edit categories'; @@ -192,6 +193,8 @@ $string['privacy:metadata:request:userid'] = 'The ID of the user to whom the req $string['privacy:metadata:request:requestedby'] = 'The ID of the user making the request, if made on behalf of another user.'; $string['privacy:metadata:request:dpocomment'] = 'Any comments made by the site\'s privacy officer regarding the request.'; $string['privacy:metadata:request:timecreated'] = 'The timestamp indicating when the request was made by the user.'; +$string['privacyrequestexpiry'] = 'Data request expiry'; +$string['privacyrequestexpiry_desc'] = 'The amount of time that approved data requests will be available for download before expiring. 0 means no time limit.'; $string['protected'] = 'Protected'; $string['protectedlabel'] = 'The retention of this data has a higher legal precedent over a user\'s request to be forgotten. This data will only be deleted after the retention period has expired.'; $string['purpose'] = 'Purpose'; @@ -241,7 +244,10 @@ $string['statusapproved'] = 'Approved'; $string['statusawaitingapproval'] = 'Awaiting approval'; $string['statuscancelled'] = 'Cancelled'; $string['statuscomplete'] = 'Complete'; +$string['statusready'] = 'Download ready'; +$string['statusdeleted'] = 'Deleted'; $string['statusdetail'] = 'Status:'; +$string['statusexpired'] = 'Expired'; $string['statuspreprocessing'] = 'Pre-processing'; $string['statusprocessing'] = 'Processing'; $string['statuspending'] = 'Pending'; diff --git a/admin/tool/dataprivacy/lib.php b/admin/tool/dataprivacy/lib.php index 73ffc1452b0..fbeb61d7d96 100644 --- a/admin/tool/dataprivacy/lib.php +++ b/admin/tool/dataprivacy/lib.php @@ -199,6 +199,11 @@ function tool_dataprivacy_pluginfile($course, $cm, $context, $filearea, $args, $ return false; } + // Make the file unavailable if it has expired. + if (\tool_dataprivacy\data_request::is_expired($datarequest)) { + send_file_not_found(); + } + // All good. Serve the exported data. $fs = get_file_storage(); $relativepath = implode('/', $args); diff --git a/admin/tool/dataprivacy/settings.php b/admin/tool/dataprivacy/settings.php index 9210f75ed4b..b902d528aeb 100644 --- a/admin/tool/dataprivacy/settings.php +++ b/admin/tool/dataprivacy/settings.php @@ -34,6 +34,12 @@ if ($hassiteconfig) { new lang_string('contactdataprotectionofficer_desc', 'tool_dataprivacy'), 0) ); + // 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'), + new lang_string('privacyrequestexpiry_desc', 'tool_dataprivacy'), + WEEKSECS, 1)); + // Fetch roles that are assignable. $assignableroles = get_assignable_roles(context_system::instance()); diff --git a/admin/tool/dataprivacy/templates/my_data_requests.mustache b/admin/tool/dataprivacy/templates/my_data_requests.mustache index 2b654b29467..6f00965f69f 100644 --- a/admin/tool/dataprivacy/templates/my_data_requests.mustache +++ b/admin/tool/dataprivacy/templates/my_data_requests.mustache @@ -60,7 +60,7 @@ "typename" : "Data deletion", "comments": "Please delete all of my son's personal data.", "statuslabelclass": "label-success", - "statuslabel": "Complete", + "statuslabel": "Deleted", "timecreated" : 1517902087, "requestedbyuser" : { "fullname": "Martha Smith", @@ -90,6 +90,19 @@ "fullname": "Martha Smith", "profileurl": "#" } + }, + { + "id": 6, + "typename" : "Data export", + "comments": "Please let me download my data", + "statuslabelclass": "label", + "statuslabel": "Expired", + "statuslabeltitle": "Download has expired. Submit a new request if you wish to export your personal data.", + "timecreated" : 1517902087, + "requestedbyuser" : { + "fullname": "Martha Smith", + "profileurl": "#" + } } ] } @@ -127,7 +140,7 @@ <td>{{#userdate}} {{timecreated}}, {{#str}} strftimedatetime {{/str}} {{/userdate}}</td> <td><a href="{{requestedbyuser.profileurl}}" title="{{#str}}viewprofile{{/str}}">{{requestedbyuser.fullname}}</a></td> <td> - <span class="label {{statuslabelclass}}">{{statuslabel}}</span> + <span class="label {{statuslabelclass}}" title="{{statuslabeltitle}}">{{statuslabel}}</span> </td> <td>{{comments}}</td> <td> diff --git a/admin/tool/dataprivacy/version.php b/admin/tool/dataprivacy/version.php index f5c7977bfbe..68590327f35 100644 --- a/admin/tool/dataprivacy/version.php +++ b/admin/tool/dataprivacy/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die; -$plugin->version = 2018051405; +$plugin->version = 2018051406; $plugin->requires = 2018050800; // Moodle 3.5dev (Build 2018031600) and upwards. $plugin->component = 'tool_dataprivacy'; diff --git a/privacy/classes/local/request/writer.php b/privacy/classes/local/request/writer.php index 79a6c7ca676..ff04f3d495d 100644 --- a/privacy/classes/local/request/writer.php +++ b/privacy/classes/local/request/writer.php @@ -67,6 +67,24 @@ class writer { return $this->realwriter; } + /** + * Create a real content_writer for use by PHPUnit tests, + * where a mock writer will not suffice. + * + * @return content_writer + */ + public static function setup_real_writer_instance() { + if (!PHPUNIT_TEST) { + throw new coding_exception('setup_real_writer_instance() is only for use with PHPUnit tests.'); + } + + $instance = static::instance(); + + if (null === $instance->realwriter) { + $instance->realwriter = new moodle_content_writer(static::instance()); + } + } + /** * Return an instance of */ From 5f9a31ee796e3c41ae13afc6d5f1757014c07ef7 Mon Sep 17 00:00:00 2001 From: Michael Hawkins <michaelh@moodle.com> Date: Tue, 12 Jun 2018 17:13:48 +0800 Subject: [PATCH 3/7] MDL-62660 tool_dataprivacy: Add scheduled task to expire data requests --- .../classes/task/delete_expired_requests.php | 67 +++++++++++++++++++ .../task/process_data_request_task.php | 2 +- admin/tool/dataprivacy/db/tasks.php | 8 +++ .../dataprivacy/lang/en/tool_dataprivacy.php | 1 + 4 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 admin/tool/dataprivacy/classes/task/delete_expired_requests.php diff --git a/admin/tool/dataprivacy/classes/task/delete_expired_requests.php b/admin/tool/dataprivacy/classes/task/delete_expired_requests.php new file mode 100644 index 00000000000..1ed3ac8d172 --- /dev/null +++ b/admin/tool/dataprivacy/classes/task/delete_expired_requests.php @@ -0,0 +1,67 @@ +<?php +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see <http://www.gnu.org/licenses/>. + +/** + * Scheduled task to delete files and update statuses of expired data requests. + * + * @package tool_dataprivacy + * @copyright 2018 Michael Hawkins + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace tool_dataprivacy\task; + +use coding_exception; +use core\task\scheduled_task; +use tool_dataprivacy\api; + +defined('MOODLE_INTERNAL') || die(); + +require_once($CFG->dirroot . '/' . $CFG->admin . '/tool/dataprivacy/lib.php'); + +/** + * Scheduled task to delete files and update request statuses once they expire. + * + * @package tool_dataprivacy + * @copyright 2018 Michael Hawkins + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class delete_expired_requests extends scheduled_task { + + /** + * Returns the task name. + * + * @return string + */ + public function get_name() { + return get_string('deleteexpireddatarequeststask', 'tool_dataprivacy'); + } + + /** + * Run the task to delete expired data request files and update request statuses. + * + */ + public function execute() { + $expiredrequests = \tool_dataprivacy\data_request::get_expired_requests(); + $deletecount = count($expiredrequests); + + if ($deletecount > 0) { + \tool_dataprivacy\data_request::expire($expiredrequests); + + mtrace($deletecount . ' expired completed data requests have been deleted'); + } + } +} diff --git a/admin/tool/dataprivacy/classes/task/process_data_request_task.php b/admin/tool/dataprivacy/classes/task/process_data_request_task.php index be0c0608edd..c44b661d97f 100644 --- a/admin/tool/dataprivacy/classes/task/process_data_request_task.php +++ b/admin/tool/dataprivacy/classes/task/process_data_request_task.php @@ -106,7 +106,7 @@ class process_data_request_task extends adhoc_task { $filerecord->author = fullname($foruser); // Save somewhere. $thing = $fs->create_file_from_pathname($filerecord, $exportedcontent); - + $completestatus = api::DATAREQUEST_STATUS_DOWNLOAD_READY; } else if ($request->type == api::DATAREQUEST_TYPE_DELETE) { // Get the collection of approved_contextlist objects needed for core_privacy data deletion. $approvedclcollection = api::get_approved_contextlist_collection_for_request($requestpersistent); diff --git a/admin/tool/dataprivacy/db/tasks.php b/admin/tool/dataprivacy/db/tasks.php index 3a4915b5419..5ee3a19aa17 100644 --- a/admin/tool/dataprivacy/db/tasks.php +++ b/admin/tool/dataprivacy/db/tasks.php @@ -42,5 +42,13 @@ $tasks = array( 'day' => '*', 'dayofweek' => '*', 'month' => '*' + ), array( + 'classname' => 'tool_dataprivacy\task\delete_expired_requests', + 'blocking' => 0, + 'minute' => 'R', + 'hour' => 'R', + 'day' => '*', + 'dayofweek' => '*', + 'month' => '*' ), ); diff --git a/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php b/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php index e5810c488ed..a50a2710d41 100644 --- a/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php +++ b/admin/tool/dataprivacy/lang/en/tool_dataprivacy.php @@ -80,6 +80,7 @@ $string['defaultsinfo'] = 'Default categories and purposes are applied to all ne $string['deletecategory'] = 'Delete "{$a}" category'; $string['deletecategorytext'] = 'Are you sure you want to delete "{$a}" category?'; $string['deleteexpiredcontextstask'] = 'Delete expired contexts'; +$string['deleteexpireddatarequeststask'] = 'Delete files from completed data requests that have expired'; $string['deletepurpose'] = 'Delete "{$a}" purpose'; $string['deletepurposetext'] = 'Are you sure you want to delete "{$a}" purpose?'; $string['defaultssaved'] = 'Defaults saved'; From 95a844ebdc729e0db9f76637c5be2cdfa2068337 Mon Sep 17 00:00:00 2001 From: Michael Hawkins <michaelh@moodle.com> Date: Fri, 10 Aug 2018 15:13:42 +0800 Subject: [PATCH 4/7] MDL-62660 tool_dataprivacy: Add and update PHPUnit tests Updated tests with latest statuses. Added testing for request download expiry functionality and method. Added assign_site_dpo within a parent class for unit tests. --- admin/tool/dataprivacy/tests/api_test.php | 11 +- .../tests/data_privacy_testcase.php | 64 +++++++ .../tests/expired_data_requests_test.php | 173 ++++++++++++++++++ .../tests/manager_observer_test.php | 38 +--- 4 files changed, 251 insertions(+), 35 deletions(-) create mode 100644 admin/tool/dataprivacy/tests/data_privacy_testcase.php create mode 100644 admin/tool/dataprivacy/tests/expired_data_requests_test.php diff --git a/admin/tool/dataprivacy/tests/api_test.php b/admin/tool/dataprivacy/tests/api_test.php index c341a6309e7..9feebcf055b 100644 --- a/admin/tool/dataprivacy/tests/api_test.php +++ b/admin/tool/dataprivacy/tests/api_test.php @@ -66,12 +66,12 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $requestid = $datarequest->get('id'); // Update with a valid status. - $result = api::update_request_status($requestid, api::DATAREQUEST_STATUS_COMPLETE); + $result = api::update_request_status($requestid, api::DATAREQUEST_STATUS_DOWNLOAD_READY); $this->assertTrue($result); // Fetch the request record again. $datarequest = new data_request($requestid); - $this->assertEquals(api::DATAREQUEST_STATUS_COMPLETE, $datarequest->get('status')); + $this->assertEquals(api::DATAREQUEST_STATUS_DOWNLOAD_READY, $datarequest->get('status')); // Update with an invalid status. $this->expectException(invalid_persistent_exception::class); @@ -468,8 +468,8 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * @return array */ public function get_data_requests_provider() { - $completeonly = [api::DATAREQUEST_STATUS_COMPLETE]; - $completeandcancelled = [api::DATAREQUEST_STATUS_COMPLETE, api::DATAREQUEST_STATUS_CANCELLED]; + $completeonly = [api::DATAREQUEST_STATUS_COMPLETE, api::DATAREQUEST_STATUS_DOWNLOAD_READY, api::DATAREQUEST_STATUS_DELETED]; + $completeandcancelled = array_merge($completeonly, [api::DATAREQUEST_STATUS_CANCELLED]); return [ // Own data requests. @@ -612,6 +612,9 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { [api::DATAREQUEST_STATUS_COMPLETE, false], [api::DATAREQUEST_STATUS_CANCELLED, false], [api::DATAREQUEST_STATUS_REJECTED, false], + [api::DATAREQUEST_STATUS_DOWNLOAD_READY, false], + [api::DATAREQUEST_STATUS_EXPIRED, false], + [api::DATAREQUEST_STATUS_DELETED, false], ]; } diff --git a/admin/tool/dataprivacy/tests/data_privacy_testcase.php b/admin/tool/dataprivacy/tests/data_privacy_testcase.php new file mode 100644 index 00000000000..16b48de4c83 --- /dev/null +++ b/admin/tool/dataprivacy/tests/data_privacy_testcase.php @@ -0,0 +1,64 @@ +<?php +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see <http://www.gnu.org/licenses/>. + +/** + * Parent class for tests which need data privacy functionality. + * + * @package tool_dataprivacy + * @copyright 2018 Michael Hawkins + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * Parent class for tests which need data privacy functionality. + * + * @package tool_dataprivacy + * @copyright 2018 Michael Hawkins + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +abstract class data_privacy_testcase extends advanced_testcase { + + /** + * Assign one or more user IDs as site DPO + * + * @param stdClass|array $users User ID or array of user IDs to be assigned as site DPO + * @return void + */ + protected function assign_site_dpo($users) { + global $DB; + $this->resetAfterTest(); + + if (!is_array($users)) { + $users = array($users); + } + + $context = context_system::instance(); + + // Give the manager role with the capability to manage data requests. + $managerroleid = $DB->get_field('role', 'id', array('shortname' => 'manager')); + assign_capability('tool/dataprivacy:managedatarequests', CAP_ALLOW, $managerroleid, $context->id, true); + + // Assign user(s) as manager. + foreach ($users as $user) { + role_assign($managerroleid, $user->id, $context->id); + } + + // Only map the manager role to the DPO role. + set_config('dporoles', $managerroleid, 'tool_dataprivacy'); + } +} diff --git a/admin/tool/dataprivacy/tests/expired_data_requests_test.php b/admin/tool/dataprivacy/tests/expired_data_requests_test.php new file mode 100644 index 00000000000..00c6259cc92 --- /dev/null +++ b/admin/tool/dataprivacy/tests/expired_data_requests_test.php @@ -0,0 +1,173 @@ +<?php +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see <http://www.gnu.org/licenses/>. + +/** + * Expired data requests tests. + * + * @package tool_dataprivacy + * @copyright 2018 Michael Hawkins + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +use tool_dataprivacy\api; +use tool_dataprivacy\data_request; + +defined('MOODLE_INTERNAL') || die(); +global $CFG; + +require_once('data_privacy_testcase.php'); + +/** + * Expired data requests tests. + * + * @package tool_dataprivacy + * @copyright 2018 Michael Hawkins + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class tool_dataprivacy_expired_data_requests_testcase extends data_privacy_testcase { + + /** + * Test tearDown. + */ + public function tearDown() { + \core_privacy\local\request\writer::reset(); + } + + /** + * Test finding and deleting expired data requests + */ + public function test_data_request_expiry() { + global $DB; + $this->resetAfterTest(); + \core_privacy\local\request\writer::setup_real_writer_instance(); + + // Set up test users. + $this->setAdminUser(); + $studentuser = $this->getDataGenerator()->create_user(); + $studentusercontext = context_user::instance($studentuser->id); + + $dpouser = $this->getDataGenerator()->create_user(); + $this->assign_site_dpo($dpouser); + + // Set request expiry to 5 minutes. + set_config('privacyrequestexpiry', 300, 'tool_dataprivacy'); + + // Create and approve data request. + $this->setUser($studentuser->id); + $datarequest = api::create_data_request($studentuser->id, api::DATAREQUEST_TYPE_EXPORT); + $this->setAdminUser(); + ob_start(); + $this->runAdhocTasks('\tool_dataprivacy\task\initiate_data_request_task'); + $requestid = $datarequest->get('id'); + $this->setUser($dpouser->id); + api::approve_data_request($requestid); + $this->setAdminUser(); + $this->runAdhocTasks('\tool_dataprivacy\task\process_data_request_task'); + ob_end_clean(); + + // Confirm approved and exported. + $request = new data_request($requestid); + $this->assertEquals(api::DATAREQUEST_STATUS_DOWNLOAD_READY, $request->get('status')); + $fileconditions = array( + 'userid' => $studentuser->id, + 'component' => 'tool_dataprivacy', + 'filearea' => 'export', + 'itemid' => $requestid, + 'contextid' => $studentusercontext->id, + ); + $this->assertEquals(2, $DB->count_records('files', $fileconditions)); + + // Run expiry deletion - should not affect test export. + $expiredrequests = data_request::get_expired_requests(); + $this->assertEquals(0, count($expiredrequests)); + data_request::expire($expiredrequests); + + // Confirm test export was not deleted. + $request = new data_request($requestid); + $this->assertEquals(api::DATAREQUEST_STATUS_DOWNLOAD_READY, $request->get('status')); + $this->assertEquals(2, $DB->count_records('files', $fileconditions)); + + // Change request expiry to 1 second and allow it to elapse. + set_config('privacyrequestexpiry', 1, 'tool_dataprivacy'); + $this->waitForSecond(); + + // Re-run expiry deletion, confirm the request expires and export is deleted. + $expiredrequests = data_request::get_expired_requests(); + $this->assertEquals(1, count($expiredrequests)); + data_request::expire($expiredrequests); + + $request = new data_request($requestid); + $this->assertEquals(api::DATAREQUEST_STATUS_EXPIRED, $request->get('status')); + $this->assertEquals(0, $DB->count_records('files', $fileconditions)); + } + + + /** + * Test for \tool_dataprivacy\data_request::is_expired() + * Tests for the expected request status to protect from false positive/negative, + * then tests is_expired() is returning the expected response. + */ + public function test_is_expired() { + $this->resetAfterTest(); + \core_privacy\local\request\writer::setup_real_writer_instance(); + + // Set request expiry beyond this test. + set_config('privacyrequestexpiry', 20, 'tool_dataprivacy'); + + $admin = get_admin(); + $this->setAdminUser(); + + // Create export request. + $datarequest = api::create_data_request($admin->id, api::DATAREQUEST_TYPE_EXPORT); + $requestid = $datarequest->get('id'); + + // Approve the request. + ob_start(); + $this->runAdhocTasks('\tool_dataprivacy\task\initiate_data_request_task'); + $this->setAdminUser(); + api::approve_data_request($requestid); + $this->runAdhocTasks('\tool_dataprivacy\task\process_data_request_task'); + ob_end_clean(); + + // Test Download ready (not expired) response. + $request = new data_request($requestid); + $this->assertEquals(api::DATAREQUEST_STATUS_DOWNLOAD_READY, $request->get('status')); + $result = data_request::is_expired($request); + $this->assertFalse($result); + + // Let request expiry time lapse. + set_config('privacyrequestexpiry', 1, 'tool_dataprivacy'); + $this->waitForSecond(); + + // Test Download ready (time expired) response. + $request = new data_request($requestid); + $this->assertEquals(api::DATAREQUEST_STATUS_DOWNLOAD_READY, $request->get('status')); + $result = data_request::is_expired($request); + $this->assertTrue($result); + + // Run the expiry task to properly expire the request. + ob_start(); + $task = \core\task\manager::get_scheduled_task('\tool_dataprivacy\task\delete_expired_requests'); + $task->execute(); + ob_end_clean(); + + // Test Expired response status response. + $request = new data_request($requestid); + $this->assertEquals(api::DATAREQUEST_STATUS_EXPIRED, $request->get('status')); + $result = data_request::is_expired($request); + $this->assertTrue($result); + } +} diff --git a/admin/tool/dataprivacy/tests/manager_observer_test.php b/admin/tool/dataprivacy/tests/manager_observer_test.php index 6c710277501..1c471536fd3 100644 --- a/admin/tool/dataprivacy/tests/manager_observer_test.php +++ b/admin/tool/dataprivacy/tests/manager_observer_test.php @@ -23,6 +23,7 @@ */ defined('MOODLE_INTERNAL') || die(); +require_once('data_privacy_testcase.php'); /** * API tests. @@ -31,35 +32,7 @@ defined('MOODLE_INTERNAL') || die(); * @copyright 2018 Andrew Nicols <andrew@nicols.co.uk> * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class tool_dataprivacy_manager_observer_testcase extends advanced_testcase { - - /** - * Helper to set andn return two users who are DPOs. - */ - protected function setup_site_dpos() { - global $DB; - $this->resetAfterTest(); - - $generator = new testing_data_generator(); - $u1 = $this->getDataGenerator()->create_user(); - $u2 = $this->getDataGenerator()->create_user(); - - $context = context_system::instance(); - - // Give the manager role with the capability to manage data requests. - $managerroleid = $DB->get_field('role', 'id', array('shortname' => 'manager')); - assign_capability('tool/dataprivacy:managedatarequests', CAP_ALLOW, $managerroleid, $context->id, true); - - // Assign both users as manager. - role_assign($managerroleid, $u1->id, $context->id); - role_assign($managerroleid, $u2->id, $context->id); - - // Only map the manager role to the DPO role. - set_config('dporoles', $managerroleid, 'tool_dataprivacy'); - - return \tool_dataprivacy\api::get_site_dpos(); - } - +class tool_dataprivacy_manager_observer_testcase extends data_privacy_testcase { /** * Ensure that when users are configured as DPO, they are sent an message upon failure. */ @@ -69,8 +42,11 @@ class tool_dataprivacy_manager_observer_testcase extends advanced_testcase { // Create another user who is not a DPO. $this->getDataGenerator()->create_user(); - // Create the DPOs. - $dpos = $this->setup_site_dpos(); + // Create two DPOs. + $dpo1 = $this->getDataGenerator()->create_user(); + $dpo2 = $this->getDataGenerator()->create_user(); + $this->assign_site_dpo(array($dpo1, $dpo2)); + $dpos = \tool_dataprivacy\api::get_site_dpos(); $observer = new \tool_dataprivacy\manager_observer(); From a1e1cf63fd81f22644adb2564f38345aee3a1d97 Mon Sep 17 00:00:00 2001 From: Michael Hawkins <michaelh@moodle.com> Date: Thu, 9 Aug 2018 16:29:07 +0800 Subject: [PATCH 5/7] MDL-62660 tool_dataprivacy: Behat additions for deletion and data expiry --- .../tests/behat/datadelete.feature | 118 ++++++++++++++++++ .../tests/behat/dataexport.feature | 36 +++++- 2 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 admin/tool/dataprivacy/tests/behat/datadelete.feature diff --git a/admin/tool/dataprivacy/tests/behat/datadelete.feature b/admin/tool/dataprivacy/tests/behat/datadelete.feature new file mode 100644 index 00000000000..a7d94d6e7a2 --- /dev/null +++ b/admin/tool/dataprivacy/tests/behat/datadelete.feature @@ -0,0 +1,118 @@ +@tool @tool_dataprivacy +Feature: Data delete from the privacy API + In order to delete data for users and meet legal requirements + As an admin, user, or parent + I need to be able to request a user and their data data be deleted + + Background: + Given the following "users" exist: + | username | firstname | lastname | + | victim | Victim User | 1 | + | parent | Long-suffering | Parent | + And the following "roles" exist: + | shortname | name | archetype | + | tired | Tired | | + And the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | tool/dataprivacy:makedatarequestsforchildren | Allow | tired | System | | + And the following "role assigns" exist: + | user | role | contextlevel | reference | + | parent | tired | User | victim | + And the following config values are set as admin: + | contactdataprotectionofficer | 1 | tool_dataprivacy | + + @javascript + Scenario: As admin, delete a user and their data + Given I log in as "victim" + And I should see "Victim User 1" + And I log out + + And I log in as "admin" + And I navigate to "Users > Privacy and policies > Data requests" in site administration + And I follow "New request" + And I set the field "Requesting for" to "Victim User 1" + And I set the field "Type" to "Delete all of my personal data" + And I press "Save changes" + Then I should see "Victim User 1" + And I should see "Pending" in the "Victim User 1" "table_row" + And I run all adhoc tasks + And I reload the page + And I should see "Awaiting approval" in the "Victim User 1" "table_row" + And I follow "Actions" + And I follow "Approve request" + And I press "Approve request" + And I should see "Approved" in the "Victim User 1" "table_row" + And I run all adhoc tasks + And I reload the page + And I should see "Deleted" in the "Victim User 1" "table_row" + + And I log out + And I log in as "victim" + And I should see "Invalid login" + + @javascript + Scenario: As a student, request deletion of account and data + Given I log in as "victim" + And I follow "Profile" in the user menu + And I follow "Data requests" + And I follow "New request" + And I set the field "Type" to "Delete all of my personal data" + And I press "Save changes" + Then I should see "Delete all of my personal data" + And I should see "Pending" in the "Delete all of my personal data" "table_row" + And I run all adhoc tasks + And I reload the page + And I should see "Awaiting approval" in the "Delete all of my personal data" "table_row" + + And I log out + And I log in as "admin" + And I navigate to "Users > Privacy and policies > Data requests" in site administration + And I follow "Actions" + And I follow "Approve request" + And I press "Approve request" + + And I log out + And I log in as "victim" + And I follow "Profile" in the user menu + And I follow "Data requests" + And I should see "Approved" in the "Delete all of my personal data" "table_row" + And I run all adhoc tasks + And I reload the page + And I should see "Your session has timed out" + And I log in as "victim" + And I should see "Invalid login" + + And I log in as "admin" + And I navigate to "Users > Privacy and policies > Data requests" in site administration + And I should see "Deleted" + + @javascript + Scenario: As a parent, request account and data deletion for my child + Given I log in as "parent" + And I follow "Profile" in the user menu + And I follow "Data requests" + And I follow "New request" + And I set the field "Requesting for" to "Victim User 1" + And I set the field "Type" to "Delete all of my personal data" + And I press "Save changes" + Then I should see "Victim User 1" + And I should see "Pending" in the "Victim User 1" "table_row" + And I run all adhoc tasks + And I reload the page + And I should see "Awaiting approval" in the "Victim User 1" "table_row" + + And I log out + And I log in as "admin" + And I navigate to "Users > Privacy and policies > Data requests" in site administration + And I follow "Actions" + And I follow "Approve request" + And I press "Approve request" + + And I log out + And I log in as "parent" + And I follow "Profile" in the user menu + And I follow "Data requests" + And I should see "Approved" in the "Victim User 1" "table_row" + And I run all adhoc tasks + And I reload the page + And I should see "You don't have any personal data requests" diff --git a/admin/tool/dataprivacy/tests/behat/dataexport.feature b/admin/tool/dataprivacy/tests/behat/dataexport.feature index 3ab0467f409..50c58bc2373 100644 --- a/admin/tool/dataprivacy/tests/behat/dataexport.feature +++ b/admin/tool/dataprivacy/tests/behat/dataexport.feature @@ -19,10 +19,11 @@ Feature: Data export from the privacy API | user | role | contextlevel | reference | | parent | tired | User | victim | And the following config values are set as admin: - | contactdataprotectionofficer | 1 | tool_dataprivacy | + | contactdataprotectionofficer | 1 | tool_dataprivacy | + | privacyrequestexpiry | 55 | tool_dataprivacy | @javascript - Scenario: As admin, export data for a user and download it + Scenario: As admin, export data for a user and download it, unless it has expired Given I log in as "admin" And I navigate to "Users > Privacy and policies > Data requests" in site administration And I follow "New request" @@ -39,12 +40,19 @@ Feature: Data export from the privacy API And I should see "Approved" in the "Victim User 1" "table_row" And I run all adhoc tasks And I reload the page - And I should see "Complete" in the "Victim User 1" "table_row" + And I should see "Download ready" in the "Victim User 1" "table_row" And I follow "Actions" And following "Download" should download between "1" and "100000" bytes + And the following config values are set as admin: + | privacyrequestexpiry | 1 | tool_dataprivacy | + And I wait "1" seconds + And I navigate to "Users > Privacy and policies > Data requests" in site administration + And I should see "Expired" in the "Victim User 1" "table_row" + And I follow "Actions" + And I should not see "Download" @javascript - Scenario: As a student, request data export and then download it when approved + Scenario: As a student, request data export and then download it when approved, unless it has expired Given I log in as "victim" And I follow "Profile" in the user menu And I follow "Data requests" @@ -70,10 +78,18 @@ Feature: Data export from the privacy API And I should see "Approved" in the "Export all of my personal data" "table_row" And I run all adhoc tasks And I reload the page - And I should see "Complete" in the "Export all of my personal data" "table_row" + And I should see "Download ready" in the "Export all of my personal data" "table_row" And I follow "Actions" And following "Download" should download between "1" and "100000" bytes + And the following config values are set as admin: + | privacyrequestexpiry | 1 | tool_dataprivacy | + And I wait "1" seconds + And I reload the page + + And I should see "Expired" in the "Export all of my personal data" "table_row" + And I should not see "Actions" + @javascript Scenario: As a parent, request data export for my child because I don't trust the little blighter Given I log in as "parent" @@ -102,6 +118,14 @@ Feature: Data export from the privacy API And I should see "Approved" in the "Victim User 1" "table_row" And I run all adhoc tasks And I reload the page - And I should see "Complete" in the "Victim User 1" "table_row" + And I should see "Download ready" in the "Victim User 1" "table_row" And I follow "Actions" And following "Download" should download between "1" and "100000" bytes + + And the following config values are set as admin: + | privacyrequestexpiry | 1 | tool_dataprivacy | + And I wait "1" seconds + And I reload the page + + And I should see "Expired" in the "Victim User 1" "table_row" + And I should not see "Actions" From 237c85d303a1f001bb02cb5ef5f421a2c69d5767 Mon Sep 17 00:00:00 2001 From: Jun Pataleta <jun@moodle.com> Date: Tue, 21 Aug 2018 09:18:00 +0800 Subject: [PATCH 6/7] MDL-62660 tool_dataprivacy: CI fixes --- admin/tool/dataprivacy/classes/data_request.php | 4 ++-- admin/tool/dataprivacy/db/upgrade.php | 1 - .../dataprivacy/tests/expired_data_requests_test.php | 10 +++++----- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/admin/tool/dataprivacy/classes/data_request.php b/admin/tool/dataprivacy/classes/data_request.php index c41db11e839..92c8c1ff243 100644 --- a/admin/tool/dataprivacy/classes/data_request.php +++ b/admin/tool/dataprivacy/classes/data_request.php @@ -158,7 +158,7 @@ class data_request extends persistent { $expiryseconds = get_config('tool_dataprivacy', 'privacyrequestexpiry'); $expirytime = strtotime("-{$expiryseconds} second"); - $table = data_request::TABLE; + $table = self::TABLE; $sqlwhere = 'type = :export_type AND status = :completestatus AND timemodified <= :expirytime'; $params = array( 'export_type' => api::DATAREQUEST_TYPE_EXPORT, @@ -196,7 +196,7 @@ class data_request extends persistent { $initialparams = array(api::DATAREQUEST_STATUS_EXPIRED, time()); $params = array_merge($initialparams, $inparams); - $update = "UPDATE {" . data_request::TABLE . "} + $update = "UPDATE {" . self::TABLE . "} SET status = ?, timemodified = ? WHERE id $insql"; diff --git a/admin/tool/dataprivacy/db/upgrade.php b/admin/tool/dataprivacy/db/upgrade.php index 9885e626346..6ad876c0e20 100644 --- a/admin/tool/dataprivacy/db/upgrade.php +++ b/admin/tool/dataprivacy/db/upgrade.php @@ -23,7 +23,6 @@ */ defined('MOODLE_INTERNAL') || die(); -use tool_dataprivacy\api; /** * Function to upgrade tool_dataprivacy. diff --git a/admin/tool/dataprivacy/tests/expired_data_requests_test.php b/admin/tool/dataprivacy/tests/expired_data_requests_test.php index 00c6259cc92..662d9abd51f 100644 --- a/admin/tool/dataprivacy/tests/expired_data_requests_test.php +++ b/admin/tool/dataprivacy/tests/expired_data_requests_test.php @@ -115,11 +115,11 @@ class tool_dataprivacy_expired_data_requests_testcase extends data_privacy_testc } - /** - * Test for \tool_dataprivacy\data_request::is_expired() - * Tests for the expected request status to protect from false positive/negative, - * then tests is_expired() is returning the expected response. - */ + /** + * Test for \tool_dataprivacy\data_request::is_expired() + * Tests for the expected request status to protect from false positive/negative, + * then tests is_expired() is returning the expected response. + */ public function test_is_expired() { $this->resetAfterTest(); \core_privacy\local\request\writer::setup_real_writer_instance(); From 4ecb1c622106bfb0ecf5af28daea0a105351eb90 Mon Sep 17 00:00:00 2001 From: Jun Pataleta <jun@moodle.com> Date: Tue, 21 Aug 2018 10:27:10 +0800 Subject: [PATCH 7/7] MDL-62660 tool_dataprivacy: Behat fix for data deletion --- admin/tool/dataprivacy/tests/behat/datadelete.feature | 1 + 1 file changed, 1 insertion(+) diff --git a/admin/tool/dataprivacy/tests/behat/datadelete.feature b/admin/tool/dataprivacy/tests/behat/datadelete.feature index a7d94d6e7a2..c6454bd5504 100644 --- a/admin/tool/dataprivacy/tests/behat/datadelete.feature +++ b/admin/tool/dataprivacy/tests/behat/datadelete.feature @@ -83,6 +83,7 @@ Feature: Data delete from the privacy API And I should see "Invalid login" And I log in as "admin" + And I am on site homepage And I navigate to "Users > Privacy and policies > Data requests" in site administration And I should see "Deleted"