From 8420c3bbc2873e87bed7cf935b27a589a78c8e1e Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Thu, 1 Nov 2018 12:12:49 +0800 Subject: [PATCH] MDL-63703 core_role: Add support for removal of context users This issue is a part of the MDL-62560 Epic. --- admin/roles/classes/privacy/provider.php | 90 +++++++- admin/roles/tests/privacy_test.php | 270 +++++++++++++++++++++++ 2 files changed, 355 insertions(+), 5 deletions(-) diff --git a/admin/roles/classes/privacy/provider.php b/admin/roles/classes/privacy/provider.php index e420f9e80ff..d711db66254 100644 --- a/admin/roles/classes/privacy/provider.php +++ b/admin/roles/classes/privacy/provider.php @@ -13,6 +13,7 @@ // // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . + /** * Privacy Subsystem implementation for core_role. * @@ -20,7 +21,9 @@ * @copyright 2018 Carlos Escobedo * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ + namespace core_role\privacy; + defined('MOODLE_INTERNAL') || die(); use \core_privacy\local\metadata\collection; @@ -28,6 +31,8 @@ use \core_privacy\local\request\contextlist; use \core_privacy\local\request\approved_contextlist; use \core_privacy\local\request\transform; use \core_privacy\local\request\writer; +use \core_privacy\local\request\userlist; +use \core_privacy\local\request\approved_userlist; /** * Privacy provider for core_role. @@ -39,7 +44,8 @@ class provider implements \core_privacy\local\metadata\provider, \core_privacy\local\request\subsystem\provider, \core_privacy\local\request\subsystem\plugin_provider, - \core_privacy\local\request\user_preference_provider { + \core_privacy\local\request\user_preference_provider, + \core_privacy\local\request\core_userlist_provider { /** * Get information about the user data stored by this plugin. @@ -151,6 +157,50 @@ class provider implements return $contextlist; } + + /** + * Get the list of users within a specific context. + * + * @param userlist $userlist The userlist containing the list of users who have data in this context/plugin combination. + */ + public static function get_users_in_context(userlist $userlist) { + + if (empty($userlist)) { + return; + } + + $context = $userlist->get_context(); + + // Include users who created or modified role capabilities. + $sql = "SELECT modifierid as userid + FROM {role_capabilities} + WHERE contextid = :contextid"; + + $params = [ + 'contextid' => $context->id + ]; + + $userlist->add_from_sql('userid', $sql, $params); + + // Include users that have a role assigned to them. + $sql = "SELECT userid + FROM {role_assignments} + WHERE contextid = :contextid"; + + $userlist->add_from_sql('userid', $sql, $params); + + // Include users who created or modified the role assignment. + // Differentiate and exclude special cases where tool_cohortroles adds records through the + // "Assign user roles to cohort" feature into the role_assignments table. + // These records should be separately processed in tool_cohortroles. + $sql = "SELECT modifierid as userid + FROM {role_assignments} + WHERE contextid = :contextid + AND component != 'tool_cohortroles'"; + + $userlist->add_from_sql('userid', $sql, $params); + } + /** * Export all user data for the specified user, in the specified contexts. * @@ -315,6 +365,32 @@ class provider implements // Remove data from role_assignments. $DB->delete_records('role_assignments', ['contextid' => $context->id]); } + + /** + * Delete multiple users within a single context. + * + * @param approved_userlist $userlist The approved context and user information to delete information for. + */ + public static function delete_data_for_users(approved_userlist $userlist) { + global $DB; + + // Don't remove data from role_capabilities. + // Because this data affects the whole Moodle, there are override capabilities. + // Don't belong to the modifier user. + $context = $userlist->get_context(); + $userids = $userlist->get_userids(); + + if (empty($userids)) { + return; + } + list($usersql, $userparams) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED); + $params = ['contextid' => $context->id] + $userparams; + + // Remove data from role_assignments. + $DB->delete_records_select('role_assignments', + "contextid = :contextid AND userid {$usersql}", $params); + } + /** * Delete all user data for this user only. * @@ -332,10 +408,14 @@ class provider implements return; } $userid = $contextlist->get_user()->id; - foreach ($contextlist->get_contexts() as $context) { - // Only delete the roles assignments where the user is assigned in all contexts. - $DB->delete_records('role_assignments', ['userid' => $userid, 'contextid' => $context->id]); - } + $contextids = $contextlist->get_contextids(); + + list($contextsql, $contextparams) = $DB->get_in_or_equal($contextids, SQL_PARAMS_NAMED); + $params = ['userid' => $userid] + $contextparams; + + // Only delete the roles assignments where the user is assigned in all contexts. + $DB->delete_records_select('role_assignments', + "userid = :userid AND contextid {$contextsql}", $params); } /** * Delete user entries in role_assignments related to the feature diff --git a/admin/roles/tests/privacy_test.php b/admin/roles/tests/privacy_test.php index bffc9199b67..258b5a70541 100644 --- a/admin/roles/tests/privacy_test.php +++ b/admin/roles/tests/privacy_test.php @@ -13,6 +13,7 @@ // // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . + /** * Privacy test for core_role * @@ -21,13 +22,16 @@ * @copyright 2018 Carlos Escobedo * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ + defined('MOODLE_INTERNAL') || die(); + use \core_role\privacy\provider; use \core_privacy\local\request\approved_contextlist; use \core_privacy\local\request\writer; use \core_privacy\tests\provider_testcase; use \core_privacy\local\request\transform; use \tool_cohortroles\api; +use \core_privacy\local\request\approved_userlist; /** * Privacy test for core_role @@ -54,6 +58,7 @@ class core_role_privacy_testcase extends provider_testcase { $this->assertEquals(get_string('privacy:metadata:preference:showadvanced', 'core_role'), $prefs->definerole_showadvanced->description); } + /** * Check all contexts are returned if there is any user data for this user. */ @@ -231,6 +236,7 @@ class core_role_privacy_testcase extends provider_testcase { } } } + /** * Test for provider::delete_data_for_all_users_in_context(). */ @@ -316,6 +322,7 @@ class core_role_privacy_testcase extends provider_testcase { $count = $DB->count_records('role_assignments', ['contextid' => $usercontext2->id]); $this->assertEquals(0, $count); } + /** * Test for provider::delete_data_for_user(). */ @@ -373,6 +380,7 @@ class core_role_privacy_testcase extends provider_testcase { $count = $DB->count_records('role_assignments', ['modifierid' => $user->id]); $this->assertEquals(2, $count); } + /** * Export for a user with a key against a script where no instance is specified. */ @@ -426,6 +434,7 @@ class core_role_privacy_testcase extends provider_testcase { $exported = $writer->get_related_data($subcontextteacher, 'cohortroles'); $this->assertEquals($user2->id, reset($exported)->userid); } + /** * Test for provider::delete_user_role_to_cohort(). */ @@ -460,6 +469,267 @@ class core_role_privacy_testcase extends provider_testcase { $count = $DB->count_records('role_assignments', ['userid' => $user->id, 'component' => 'tool_cohortroles']); $this->assertEquals(0, $count); } + + /** + * Test that only users within a course context are fetched. + */ + public function test_get_users_in_context() { + global $DB; + + $this->resetAfterTest(); + + $component = 'core_role'; + + $this->setAdminUser(); + $admin = \core_user::get_user_by_username('admin'); + // Create user1. + $user1 = $this->getDataGenerator()->create_user(); + $usercontext1 = \context_user::instance($user1->id); + // Create user2. + $user2 = $this->getDataGenerator()->create_user(); + $usercontext2 = \context_user::instance($user2->id); + // Create course1. + $course1 = $this->getDataGenerator()->create_course(); + $coursecontext1 = \context_course::instance($course1->id); + // Create course category. + $coursecat = $this->getDataGenerator()->create_category(); + $coursecatcontext = \context_coursecat::instance($coursecat->id); + // Create chat module. + $cm = $this->getDataGenerator()->create_module('chat', ['course' => $course1->id]); + $cmcontext = \context_module::instance($cm->cmid); + + $systemcontext = \context_system::instance(); + // Create a block. + $block = $this->getDataGenerator()->create_block('online_users'); + $blockcontext = \context_block::instance($block->id); + + $studentrole = $DB->get_record('role', array('shortname' => 'student'), '*', MUST_EXIST); + $managerrole = $DB->get_record('role', array('shortname' => 'manager'), '*', MUST_EXIST); + + // Role assignments CONTEXT_COURSE. + role_assign($studentrole->id, $user1->id, $coursecontext1->id); + role_assign($studentrole->id, $user2->id, $coursecontext1->id); + // Role assignments CONTEXT_COURSECAT. + role_assign($studentrole->id, $user2->id, $coursecatcontext->id); + // Role assignments CONTEXT_SYSTEM. + role_assign($studentrole->id, $user1->id, $systemcontext->id); + // Role assignments CONTEXT_MODULE. + role_assign($studentrole->id, $user2->id, $cmcontext->id); + // Role assigments CONTEXT_BLOCK. + role_assign($studentrole->id, $user1->id, $blockcontext->id); + // Role assigments CONTEXT_USER. + role_assign($managerrole->id, $user1->id, $usercontext2->id); + + // Role capabilities. + $this->setUser($user1); + assign_capability('moodle/backup:backupcourse', CAP_ALLOW, $studentrole->id, $cmcontext->id); + + // The user list for usercontext1 should not return any users. + $userlist1 = new \core_privacy\local\request\userlist($usercontext1, $component); + provider::get_users_in_context($userlist1); + $this->assertCount(0, $userlist1); + // The user list for usercontext2 should user1 and admin (role creator). + $userlist2 = new \core_privacy\local\request\userlist($usercontext2, $component); + provider::get_users_in_context($userlist2); + $this->assertCount(2, $userlist2); + $expected = [ + $user1->id, + $admin->id + ]; + $this->assertEquals($expected, $userlist2->get_userids(), '', 0.0, 10, true); + + // The user list for coursecontext1 should user1, user2 and admin (role creator). + $userlist3 = new \core_privacy\local\request\userlist($coursecontext1, $component); + provider::get_users_in_context($userlist3); + $this->assertCount(3, $userlist3); + $expected = [ + $user1->id, + $user2->id, + $admin->id + ]; + $this->assertEquals($expected, $userlist3->get_userids(), '', 0.0, 10, true); + + // The user list for coursecatcontext should user2 and admin (role creator). + $userlist4 = new \core_privacy\local\request\userlist($coursecatcontext, $component); + provider::get_users_in_context($userlist4); + $this->assertCount(2, $userlist4); + $expected = [ + $user2->id, + $admin->id + ]; + $this->assertEquals($expected, $userlist4->get_userids(), '', 0.0, 10, true); + + // The user list for systemcontext should user1 and admin (role creator). + $userlist6 = new \core_privacy\local\request\userlist($systemcontext, $component); + provider::get_users_in_context($userlist6); + $this->assertCount(2, $userlist6); + $expected = [ + $user1->id, + $admin->id + ]; + $this->assertEquals($expected, $userlist6->get_userids(), '', 0.0, 10, true); + + // The user list for cmcontext should user1, user2 and admin (role creator). + $userlist7 = new \core_privacy\local\request\userlist($cmcontext, $component); + provider::get_users_in_context($userlist7); + $this->assertCount(3, $userlist7); + $expected = [ + $user1->id, + $user2->id, + $admin->id + ]; + $this->assertEquals($expected, $userlist7->get_userids(), '', 0.0, 10, true); + + // The user list for blockcontext should user1 and admin (role creator). + $userlist8 = new \core_privacy\local\request\userlist($blockcontext, $component); + provider::get_users_in_context($userlist8); + $this->assertCount(2, $userlist8); + $expected = [ + $user1->id, + $admin->id + ]; + $this->assertEquals($expected, $userlist8->get_userids(), '', 0.0, 10, true); + } + + /** + * Test that data for users in approved userlist is deleted. + */ + public function test_delete_data_for_users() { + global $DB; + + $this->resetAfterTest(); + + $component = 'core_role'; + + $this->setAdminUser(); + $admin = \core_user::get_user_by_username('admin'); + // Create user1. + $user1 = $this->getDataGenerator()->create_user(); + // Create user2. + $user2 = $this->getDataGenerator()->create_user(); + $usercontext2 = \context_user::instance($user2->id); + // Create course1. + $course1 = $this->getDataGenerator()->create_course(); + $coursecontext1 = \context_course::instance($course1->id); + // Create course category. + $coursecat = $this->getDataGenerator()->create_category(); + $coursecatcontext = \context_coursecat::instance($coursecat->id); + // Create chat module. + $cm = $this->getDataGenerator()->create_module('chat', ['course' => $course1->id]); + $cmcontext = \context_module::instance($cm->cmid); + + $systemcontext = \context_system::instance(); + // Create a block. + $block = $this->getDataGenerator()->create_block('online_users'); + $blockcontext = \context_block::instance($block->id); + + $studentrole = $DB->get_record('role', array('shortname' => 'student'), '*', MUST_EXIST); + $managerrole = $DB->get_record('role', array('shortname' => 'manager'), '*', MUST_EXIST); + + // Role assignments CONTEXT_COURSE. + role_assign($studentrole->id, $user1->id, $coursecontext1->id); + role_assign($studentrole->id, $user2->id, $coursecontext1->id); + // Role assignments CONTEXT_COURSECAT. + role_assign($studentrole->id, $user2->id, $coursecatcontext->id); + // Role assignments CONTEXT_SYSTEM. + role_assign($studentrole->id, $user1->id, $systemcontext->id); + // Role assignments CONTEXT_MODULE. + role_assign($studentrole->id, $user2->id, $cmcontext->id); + // Role assigments CONTEXT_BLOCK. + role_assign($studentrole->id, $user1->id, $blockcontext->id); + // Role assigments CONTEXT_USER. + role_assign($managerrole->id, $user1->id, $usercontext2->id); + + // Role capabilities. + $this->setUser($user1); + assign_capability('moodle/backup:backupcourse', CAP_ALLOW, $studentrole->id, $cmcontext->id); + + // The user list for usercontext2 should user1 and admin (role creator). + $userlist1 = new \core_privacy\local\request\userlist($usercontext2, $component); + provider::get_users_in_context($userlist1); + $this->assertCount(2, $userlist1); + // The user list for coursecontext1 should user1, user2 and admin (role creator). + $userlist2 = new \core_privacy\local\request\userlist($coursecontext1, $component); + provider::get_users_in_context($userlist2); + $this->assertCount(3, $userlist2); + // The user list for coursecatcontext should user2 and admin (role creator). + $userlist3 = new \core_privacy\local\request\userlist($coursecatcontext, $component); + provider::get_users_in_context($userlist3); + $this->assertCount(2, $userlist3); + // The user list for systemcontext should user1 and admin (role creator). + $userlist4 = new \core_privacy\local\request\userlist($systemcontext, $component); + provider::get_users_in_context($userlist4); + $this->assertCount(2, $userlist4); + // The user list for cmcontext should user1, user2 and admin (role creator). + $userlist5 = new \core_privacy\local\request\userlist($cmcontext, $component); + provider::get_users_in_context($userlist5); + $this->assertCount(3, $userlist5); + // The user list for blockcontext should user1 and admin (role creator). + $userlist6 = new \core_privacy\local\request\userlist($blockcontext, $component); + provider::get_users_in_context($userlist6); + $this->assertCount(2, $userlist6); + + // Convert $userlist1 into an approved_contextlist. + $approvedlist1 = new approved_userlist($usercontext2, $component, $userlist1->get_userids()); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist1); + // Re-fetch users in usercontext2. + $userlist1 = new \core_privacy\local\request\userlist($usercontext2, $component); + provider::get_users_in_context($userlist1); + $this->assertCount(0, $userlist1); + + // Convert $userlist2 into an approved_contextlist. + $approvedlist2 = new approved_userlist($coursecontext1, $component, $userlist2->get_userids()); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist2); + // Re-fetch users in coursecontext1. + $userlist2 = new \core_privacy\local\request\userlist($coursecontext1, $component); + provider::get_users_in_context($userlist2); + $this->assertCount(0, $userlist2); + + // Convert $userlist3 into an approved_contextlist. + $approvedlist3 = new approved_userlist($coursecatcontext, $component, $userlist3->get_userids()); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist3); + // Re-fetch users in coursecatcontext. + $userlist3 = new \core_privacy\local\request\userlist($coursecatcontext, $component); + provider::get_users_in_context($userlist3); + $this->assertCount(0, $userlist3); + + // Convert $userlist4 into an approved_contextlist. + $approvedlist4 = new approved_userlist($systemcontext, $component, $userlist4->get_userids()); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist4); + // Re-fetch users in systemcontext. + $userlist4 = new \core_privacy\local\request\userlist($systemcontext, $component); + provider::get_users_in_context($userlist4); + // The data from role_capabilities should still be present. The user list should return the admin user. + $this->assertCount(1, $userlist4); + $expected = [$admin->id]; + $this->assertEquals($expected, $userlist4->get_userids()); + + // Convert $userlist5 into an approved_contextlist. + $approvedlist5 = new approved_userlist($cmcontext, $component, $userlist5->get_userids()); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist5); + // Re-fetch users in cmcontext. + $userlist5 = new \core_privacy\local\request\userlist($cmcontext, $component); + provider::get_users_in_context($userlist5); + // The data from role_capabilities should still be present. The user list should return user1. + $this->assertCount(1, $userlist5); + $expected = [$user1->id]; + $this->assertEquals($expected, $userlist5->get_userids()); + + // Convert $userlist6 into an approved_contextlist. + $approvedlist6 = new approved_userlist($blockcontext, $component, $userlist6->get_userids()); + // Delete using delete_data_for_user. + provider::delete_data_for_users($approvedlist6); + // Re-fetch users in blockcontext. + $userlist6 = new \core_privacy\local\request\userlist($blockcontext, $component); + provider::get_users_in_context($userlist6); + $this->assertCount(0, $userlist6); + } + /** * Supoort function to get all the localised roles name * in a simple array for testing.