mirror of
https://github.com/moodle/moodle.git
synced 2025-04-07 09:23:31 +02:00
MDL-62409 tool_dataprivacy: Properly validate data request creation
Creating data requests * Add capability check when creating data requests for another user. Ad-hoc task that processes pending data requests * Check if the requesting user has the capability to create the data request for another user. Reject otherwise. Ad-hoc task that processes approved data requests * Validate that the requester can receive the notification about the data request processing results. * Do not send the confirmation link to DPOs/admins
This commit is contained in:
parent
0f7fb98747
commit
7bdb9d877d
@ -187,8 +187,25 @@ class api {
|
||||
$datarequest = new data_request();
|
||||
// The user the request is being made for.
|
||||
$datarequest->set('userid', $foruser);
|
||||
|
||||
$requestinguser = $USER->id;
|
||||
// Check when the user is making a request on behalf of another.
|
||||
if ($requestinguser != $foruser) {
|
||||
if (self::is_site_dpo($requestinguser)) {
|
||||
// The user making the request is a DPO. Should be fine.
|
||||
$datarequest->set('dpo', $requestinguser);
|
||||
} else {
|
||||
// If not a DPO, only users with the capability to make data requests for the user should be allowed.
|
||||
// (e.g. users with the Parent role, etc).
|
||||
if (!api::can_create_data_request_for_user($foruser)) {
|
||||
$forusercontext = \context_user::instance($foruser);
|
||||
throw new required_capability_exception($forusercontext,
|
||||
'tool/dataprivacy:makedatarequestsforchildren', 'nopermissions', '');
|
||||
}
|
||||
}
|
||||
}
|
||||
// The user making the request.
|
||||
$datarequest->set('requestedby', $USER->id);
|
||||
$datarequest->set('requestedby', $requestinguser);
|
||||
// Set status.
|
||||
$datarequest->set('status', self::DATAREQUEST_STATUS_PENDING);
|
||||
// Set request type.
|
||||
@ -304,17 +321,19 @@ class api {
|
||||
* @param int $requestid The request identifier.
|
||||
* @param int $status The request status.
|
||||
* @param int $dpoid The user ID of the Data Protection Officer
|
||||
* @param string $comment The comment about the status update.
|
||||
* @return bool
|
||||
* @throws invalid_persistent_exception
|
||||
* @throws coding_exception
|
||||
*/
|
||||
public static function update_request_status($requestid, $status, $dpoid = 0) {
|
||||
public static function update_request_status($requestid, $status, $dpoid = 0, $comment = '') {
|
||||
// Update the request.
|
||||
$datarequest = new data_request($requestid);
|
||||
$datarequest->set('status', $status);
|
||||
if ($dpoid) {
|
||||
$datarequest->set('dpo', $dpoid);
|
||||
}
|
||||
$datarequest->set('dpocomment', $comment);
|
||||
return $datarequest->update();
|
||||
}
|
||||
|
||||
@ -468,6 +487,19 @@ class api {
|
||||
return message_send($message);
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether a non-DPO user can make a data request for another user.
|
||||
*
|
||||
* @param int $user The user ID of the target user.
|
||||
* @param int $requester The user ID of the user making the request.
|
||||
* @return bool
|
||||
* @throws coding_exception
|
||||
*/
|
||||
public static function can_create_data_request_for_user($user, $requester = null) {
|
||||
$usercontext = \context_user::instance($user);
|
||||
return has_capability('tool/dataprivacy:makedatarequestsforchildren', $usercontext, $requester);
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new data purpose.
|
||||
*
|
||||
|
@ -70,6 +70,27 @@ class initiate_data_request_task extends adhoc_task {
|
||||
return;
|
||||
}
|
||||
|
||||
$requestedby = $datarequest->get('requestedby');
|
||||
$valid = true;
|
||||
$comment = '';
|
||||
$foruser = $datarequest->get('userid');
|
||||
if ($foruser != $requestedby) {
|
||||
if (!$valid = api::can_create_data_request_for_user($foruser, $requestedby)) {
|
||||
$params = (object)[
|
||||
'requestedby' => $requestedby,
|
||||
'userid' => $foruser
|
||||
];
|
||||
$comment = get_string('errornocapabilitytorequestforothers', 'tool_dataprivacy', $params);
|
||||
mtrace($comment);
|
||||
}
|
||||
}
|
||||
// Reject the request outright if it's invalid.
|
||||
if (!$valid) {
|
||||
$dpo = $datarequest->get('dpo');
|
||||
api::update_request_status($requestid, api::DATAREQUEST_STATUS_REJECTED, $dpo, $comment);
|
||||
return;
|
||||
}
|
||||
|
||||
// Update the status of this request as pre-processing.
|
||||
mtrace('Generating the contexts containing personal data for the user...');
|
||||
api::update_request_status($requestid, api::DATAREQUEST_STATUS_PREPROCESSING);
|
||||
|
@ -182,23 +182,27 @@ class process_data_request_task extends adhoc_task {
|
||||
}
|
||||
mtrace('Message sent to user: ' . $messagetextdata['username']);
|
||||
|
||||
// Send to the requester as well. requestedby is 0 if the request was made on behalf of the user by a DPO.
|
||||
if (!empty($request->requestedby) && $foruser->id != $request->requestedby) {
|
||||
$requestedby = core_user::get_user($request->requestedby);
|
||||
$message->userto = $requestedby;
|
||||
$messagetextdata['username'] = fullname($requestedby);
|
||||
// Render message email body.
|
||||
$messagehtml = $output->render_from_template('tool_dataprivacy/data_request_results_email', $messagetextdata);
|
||||
$message->fullmessage = html_to_text($messagehtml);
|
||||
$message->fullmessagehtml = $messagehtml;
|
||||
// Send to requester as well if this request was made on behalf of another user who's not a DPO,
|
||||
// and has the capability to make data requests for the user (e.g. Parent).
|
||||
if (!api::is_site_dpo($request->requestedby) && $foruser->id != $request->requestedby) {
|
||||
// Ensure the requester has the capability to make data requests for this user.
|
||||
if (api::can_create_data_request_for_user($request->userid, $request->requestedby)) {
|
||||
$requestedby = core_user::get_user($request->requestedby);
|
||||
$message->userto = $requestedby;
|
||||
$messagetextdata['username'] = fullname($requestedby);
|
||||
// Render message email body.
|
||||
$messagehtml = $output->render_from_template('tool_dataprivacy/data_request_results_email', $messagetextdata);
|
||||
$message->fullmessage = html_to_text($messagehtml);
|
||||
$message->fullmessagehtml = $messagehtml;
|
||||
|
||||
// Send message.
|
||||
if ($emailonly) {
|
||||
email_to_user($requestedby, $dpo, $subject, $message->fullmessage, $messagehtml);
|
||||
} else {
|
||||
message_send($message);
|
||||
// Send message.
|
||||
if ($emailonly) {
|
||||
email_to_user($requestedby, $dpo, $subject, $message->fullmessage, $messagehtml);
|
||||
} else {
|
||||
message_send($message);
|
||||
}
|
||||
mtrace('Message sent to requester: ' . $messagetextdata['username']);
|
||||
}
|
||||
mtrace('Message sent to requester: ' . $messagetextdata['username']);
|
||||
}
|
||||
|
||||
if ($request->type == api::DATAREQUEST_TYPE_DELETE) {
|
||||
|
@ -89,6 +89,7 @@ $string['effectiveretentionperioduser'] = '{$a} (since the last time the user ac
|
||||
$string['emailsalutation'] = 'Dear {$a},';
|
||||
$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}';
|
||||
$string['errornoexpiredcontexts'] = 'There are no expired contexts to process';
|
||||
$string['errorcontexthasunexpiredchildren'] = 'The context "{$a}" still has sub-contexts that have not yet expired. No contexts have been flagged for deletion.';
|
||||
$string['errorrequestalreadyexists'] = 'You already have an ongoing request.';
|
||||
|
@ -57,6 +57,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
|
||||
public function test_update_request_status() {
|
||||
$generator = new testing_data_generator();
|
||||
$s1 = $generator->create_user();
|
||||
$this->setUser($s1);
|
||||
|
||||
// Create the sample data request.
|
||||
$datarequest = api::create_data_request($s1->id, api::DATAREQUEST_TYPE_EXPORT);
|
||||
@ -145,6 +146,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
|
||||
set_config('dporoles', $managerroleid, 'tool_dataprivacy');
|
||||
|
||||
// Create the sample data request.
|
||||
$this->setUser($s1);
|
||||
$datarequest = api::create_data_request($s1->id, api::DATAREQUEST_TYPE_EXPORT);
|
||||
$requestid = $datarequest->get('id');
|
||||
|
||||
@ -186,6 +188,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
|
||||
set_config('dporoles', $managerroleid, 'tool_dataprivacy');
|
||||
|
||||
// Create the sample data request.
|
||||
$this->setUser($s1);
|
||||
$datarequest = api::create_data_request($s1->id, api::DATAREQUEST_TYPE_EXPORT);
|
||||
$requestid = $datarequest->get('id');
|
||||
|
||||
@ -203,6 +206,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
|
||||
$teacher = $generator->create_user();
|
||||
|
||||
// Create the sample data request.
|
||||
$this->setUser($student);
|
||||
$datarequest = api::create_data_request($student->id, api::DATAREQUEST_TYPE_EXPORT);
|
||||
|
||||
$requestid = $datarequest->get('id');
|
||||
@ -286,6 +290,71 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
|
||||
$datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
|
||||
$this->assertEquals($user->id, $datarequest->get('userid'));
|
||||
$this->assertEquals($user->id, $datarequest->get('requestedby'));
|
||||
$this->assertEquals(0, $datarequest->get('dpo'));
|
||||
$this->assertEquals(api::DATAREQUEST_TYPE_EXPORT, $datarequest->get('type'));
|
||||
$this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $datarequest->get('status'));
|
||||
$this->assertEquals($comment, $datarequest->get('comments'));
|
||||
|
||||
// Test adhoc task creation.
|
||||
$adhoctasks = manager::get_adhoc_tasks(initiate_data_request_task::class);
|
||||
$this->assertCount(1, $adhoctasks);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for api::create_data_request() made by DPO.
|
||||
*/
|
||||
public function test_create_data_request_by_dpo() {
|
||||
global $USER;
|
||||
|
||||
$generator = new testing_data_generator();
|
||||
$user = $generator->create_user();
|
||||
$comment = 'sample comment';
|
||||
|
||||
// Login as DPO (Admin is DPO by default).
|
||||
$this->setAdminUser();
|
||||
|
||||
// Test data request creation.
|
||||
$datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
|
||||
$this->assertEquals($user->id, $datarequest->get('userid'));
|
||||
$this->assertEquals($USER->id, $datarequest->get('requestedby'));
|
||||
$this->assertEquals($USER->id, $datarequest->get('dpo'));
|
||||
$this->assertEquals(api::DATAREQUEST_TYPE_EXPORT, $datarequest->get('type'));
|
||||
$this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $datarequest->get('status'));
|
||||
$this->assertEquals($comment, $datarequest->get('comments'));
|
||||
|
||||
// Test adhoc task creation.
|
||||
$adhoctasks = manager::get_adhoc_tasks(initiate_data_request_task::class);
|
||||
$this->assertCount(1, $adhoctasks);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for api::create_data_request() made by a parent.
|
||||
*/
|
||||
public function test_create_data_request_by_parent() {
|
||||
global $DB;
|
||||
|
||||
$generator = new testing_data_generator();
|
||||
$user = $generator->create_user();
|
||||
$parent = $generator->create_user();
|
||||
$comment = 'sample comment';
|
||||
|
||||
// Get the teacher role pretend it's the parent roles ;).
|
||||
$systemcontext = context_system::instance();
|
||||
$usercontext = context_user::instance($user->id);
|
||||
$parentroleid = $DB->get_field('role', 'id', array('shortname' => 'teacher'));
|
||||
// Give the manager role with the capability to manage data requests.
|
||||
assign_capability('tool/dataprivacy:makedatarequestsforchildren', CAP_ALLOW, $parentroleid, $systemcontext->id, true);
|
||||
// Assign the parent to user.
|
||||
role_assign($parentroleid, $parent->id, $usercontext->id);
|
||||
|
||||
// Login as the user's parent.
|
||||
$this->setUser($parent);
|
||||
|
||||
// Test data request creation.
|
||||
$datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
|
||||
$this->assertEquals($user->id, $datarequest->get('userid'));
|
||||
$this->assertEquals($parent->id, $datarequest->get('requestedby'));
|
||||
$this->assertEquals(0, $datarequest->get('dpo'));
|
||||
$this->assertEquals(api::DATAREQUEST_TYPE_EXPORT, $datarequest->get('type'));
|
||||
$this->assertEquals(api::DATAREQUEST_STATUS_PENDING, $datarequest->get('status'));
|
||||
$this->assertEquals($comment, $datarequest->get('comments'));
|
||||
@ -351,8 +420,10 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
|
||||
$comment = 'sample comment';
|
||||
|
||||
// Make a data request as user 1.
|
||||
$this->setUser($user1);
|
||||
$d1 = api::create_data_request($user1->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
|
||||
// Make a data request as user 2.
|
||||
$this->setUser($user2);
|
||||
$d2 = api::create_data_request($user2->id, api::DATAREQUEST_TYPE_EXPORT, $comment);
|
||||
|
||||
// Fetching data requests of specific users.
|
||||
@ -406,6 +477,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase {
|
||||
$user1 = $generator->create_user();
|
||||
|
||||
// Make a data request as user 1.
|
||||
$this->setUser($user1);
|
||||
$request = api::create_data_request($user1->id, api::DATAREQUEST_TYPE_EXPORT);
|
||||
// Set the status.
|
||||
api::update_request_status($request->get('id'), $status);
|
||||
|
Loading…
x
Reference in New Issue
Block a user