From a000765a13bbfee0ca9e6740bc7bc83b64574e7d Mon Sep 17 00:00:00 2001 From: Michael Hawkins Date: Wed, 7 Nov 2018 13:53:52 +0800 Subject: [PATCH 1/2] MDL-63867 tool_dataprivacy: Fix context expired/unprotected user check It now checks the system context has been defined, since that is required for data privacy to be set up correctly, and the check to be valid. This also fixes an error being thrown when checking pending delete requests in cron. --- admin/tool/dataprivacy/classes/expired_contexts_manager.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/admin/tool/dataprivacy/classes/expired_contexts_manager.php b/admin/tool/dataprivacy/classes/expired_contexts_manager.php index c2dc16949a5..751ee1ad573 100644 --- a/admin/tool/dataprivacy/classes/expired_contexts_manager.php +++ b/admin/tool/dataprivacy/classes/expired_contexts_manager.php @@ -887,6 +887,11 @@ class expired_contexts_manager { * @return bool */ public static function is_context_expired_or_unprotected_for_user(\context $context, \stdClass $user) : bool { + // User/course contexts can't expire if no purpose is set in the system context. + if (!data_registry::defaults_set()) { + return false; + } + $parents = $context->get_parent_contexts(true); foreach ($parents as $parent) { if ($parent instanceof \context_course) { From b2e67dc9139b6842b07ba10bd8f6e59696a60560 Mon Sep 17 00:00:00 2001 From: Michael Hawkins Date: Wed, 7 Nov 2018 22:19:49 +0800 Subject: [PATCH 2/2] MDL-63867 tool_dataprivacy: Add site purpose checks to tasks If a site purpose is not defined, requests cannot be processed. --- .../task/process_data_request_task.php | 7 ++++ .../tests/behat/datadelete.feature | 7 ++++ .../tests/behat/dataexport.feature | 7 ++++ .../tests/expired_data_requests_test.php | 32 +++++++++++++++++++ 4 files changed, 53 insertions(+) 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 c44b661d97f..8b6350a1696 100644 --- a/admin/tool/dataprivacy/classes/task/process_data_request_task.php +++ b/admin/tool/dataprivacy/classes/task/process_data_request_task.php @@ -74,6 +74,13 @@ class process_data_request_task extends adhoc_task { return; } + // If no site purpose is defined, reject requests since they cannot be processed. + if (!\tool_dataprivacy\data_registry::defaults_set()) { + api::update_request_status($requestid, api::DATAREQUEST_STATUS_REJECTED); + mtrace('No site purpose defined. Request ' . $requestid . ' rejected.'); + return; + } + // Get the user details now. We might not be able to retrieve it later if it's a deletion processing. $foruser = core_user::get_user($request->userid); $usercontext = \context_user::instance($foruser->id); diff --git a/admin/tool/dataprivacy/tests/behat/datadelete.feature b/admin/tool/dataprivacy/tests/behat/datadelete.feature index 99daa149851..7161f6a25d1 100644 --- a/admin/tool/dataprivacy/tests/behat/datadelete.feature +++ b/admin/tool/dataprivacy/tests/behat/datadelete.feature @@ -20,6 +20,13 @@ Feature: Data delete from the privacy API | parent | tired | User | victim | And the following config values are set as admin: | contactdataprotectionofficer | 1 | tool_dataprivacy | + And the following data privacy "categories" exist: + | name | + | Site category | + And the following data privacy "purposes" exist: + | name | retentionperiod | + | Site purpose | P10Y | + And I set the site category and purpose to "Site category" and "Site purpose" @javascript Scenario: As admin, delete a user and their data diff --git a/admin/tool/dataprivacy/tests/behat/dataexport.feature b/admin/tool/dataprivacy/tests/behat/dataexport.feature index b2a82355967..e0ab9841f7f 100644 --- a/admin/tool/dataprivacy/tests/behat/dataexport.feature +++ b/admin/tool/dataprivacy/tests/behat/dataexport.feature @@ -21,6 +21,13 @@ Feature: Data export from the privacy API And the following config values are set as admin: | contactdataprotectionofficer | 1 | tool_dataprivacy | | privacyrequestexpiry | 55 | tool_dataprivacy | + And the following data privacy "categories" exist: + | name | + | Site category | + And the following data privacy "purposes" exist: + | name | retentionperiod | + | Site purpose | P10Y | + And I set the site category and purpose to "Site category" and "Site purpose" @javascript Scenario: As admin, export data for a user and download it, unless it has expired diff --git a/admin/tool/dataprivacy/tests/expired_data_requests_test.php b/admin/tool/dataprivacy/tests/expired_data_requests_test.php index 662d9abd51f..8a5c3122a35 100644 --- a/admin/tool/dataprivacy/tests/expired_data_requests_test.php +++ b/admin/tool/dataprivacy/tests/expired_data_requests_test.php @@ -23,7 +23,9 @@ */ use tool_dataprivacy\api; +use tool_dataprivacy\category; use tool_dataprivacy\data_request; +use tool_dataprivacy\purpose; defined('MOODLE_INTERNAL') || die(); global $CFG; @@ -62,6 +64,9 @@ class tool_dataprivacy_expired_data_requests_testcase extends data_privacy_testc $dpouser = $this->getDataGenerator()->create_user(); $this->assign_site_dpo($dpouser); + // Set site purpose. + $this->create_system_purpose(); + // Set request expiry to 5 minutes. set_config('privacyrequestexpiry', 300, 'tool_dataprivacy'); @@ -130,6 +135,9 @@ class tool_dataprivacy_expired_data_requests_testcase extends data_privacy_testc $admin = get_admin(); $this->setAdminUser(); + // Set site purpose. + $this->create_system_purpose(); + // Create export request. $datarequest = api::create_data_request($admin->id, api::DATAREQUEST_TYPE_EXPORT); $requestid = $datarequest->get('id'); @@ -170,4 +178,28 @@ class tool_dataprivacy_expired_data_requests_testcase extends data_privacy_testc $result = data_request::is_expired($request); $this->assertTrue($result); } + + /** + * Create a site (system context) purpose and category. + * + * @return void + */ + protected function create_system_purpose() { + $purpose = new purpose(0, (object) [ + 'name' => 'Test purpose ' . rand(1, 1000), + 'retentionperiod' => 'P1D', + 'lawfulbases' => 'gdpr_art_6_1_a', + ]); + $purpose->create(); + + $cat = new category(0, (object) ['name' => 'Test category']); + $cat->create(); + + $record = (object) [ + 'purposeid' => $purpose->get('id'), + 'categoryid' => $cat->get('id'), + 'contextlevel' => CONTEXT_SYSTEM, + ]; + api::set_contextlevel($record); + } }