From bb33fb9ee8b93d047d497a39f9da1365267d88fc Mon Sep 17 00:00:00 2001 From: James McQuillan Date: Fri, 12 Apr 2019 02:42:46 -0400 Subject: [PATCH] MSFTMPP-748: Update privacy providers --- classes/privacy/provider.php | 129 +++++++++++--- lang/en/auth_oidc.php | 5 +- tests/privacy_provider_test.php | 300 ++++++++++++++++++++++++++++++++ 3 files changed, 411 insertions(+), 23 deletions(-) create mode 100644 tests/privacy_provider_test.php diff --git a/classes/privacy/provider.php b/classes/privacy/provider.php index 041328c..10b2dd2 100644 --- a/classes/privacy/provider.php +++ b/classes/privacy/provider.php @@ -28,9 +28,16 @@ use \core_privacy\local\request\approved_contextlist; use \core_privacy\local\request\writer; +if (interface_exists('\core_privacy\local\request\core_userlist_provider')) { + interface auth_oidc_userlist extends \core_privacy\local\request\core_userlist_provider {} +} else { + interface auth_oidc_userlist {}; +} + class provider implements \core_privacy\local\request\plugin\provider, - \core_privacy\local\metadata\provider { + \core_privacy\local\metadata\provider, + auth_oidc_userlist { /** * Returns meta data about this system. @@ -84,10 +91,58 @@ public static function get_metadata(collection $collection): collection { */ public static function get_contexts_for_userid(int $userid) : contextlist { $contextlist = new \core_privacy\local\request\contextlist(); - $contextlist->add_system_context(); + + $sql = "SELECT ctx.id + FROM {auth_oidc_token} tk + JOIN {context} ctx ON ctx.instanceid = tk.userid AND ctx.contextlevel = :contextlevel + WHERE tk.userid = :userid"; + $params = ['userid' => $userid, 'contextlevel' => CONTEXT_USER]; + $contextlist->add_from_sql($sql, $params); + + $sql = "SELECT ctx.id + FROM {auth_oidc_prevlogin} pv + JOIN {context} ctx ON ctx.instanceid = pv.userid AND ctx.contextlevel = :contextlevel + WHERE pv.userid = :userid"; + $params = ['userid' => $userid, 'contextlevel' => CONTEXT_USER]; + $contextlist->add_from_sql($sql, $params); + return $contextlist; } + /** + * Get the list of users who have data within a 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(\core_privacy\local\request\userlist $userlist) { + $context = $userlist->get_context(); + + if (!$context instanceof \context_user) { + return; + } + + $params = [ + 'contextuser' => CONTEXT_USER, + 'contextid' => $context->id + ]; + + $sql = "SELECT ctx.instanceid as userid + FROM {auth_oidc_prevlogin} pl + JOIN {context} ctx + ON ctx.instanceid = pl.userid + AND ctx.contextlevel = :contextuser + WHERE ctx.id = :contextid"; + $userlist->add_from_sql('userid', $sql, $params); + + $sql = "SELECT ctx.instanceid as userid + FROM {auth_oidc_token} tk + JOIN {context} ctx + ON ctx.instanceid = tk.userid + AND ctx.contextlevel = :contextuser + WHERE ctx.id = :contextid"; + $userlist->add_from_sql('userid', $sql, $params); + } + /** * Export all user data for the specified user, in the specified contexts. * @@ -96,50 +151,82 @@ public static function get_contexts_for_userid(int $userid) : contextlist { public static function export_user_data(approved_contextlist $contextlist) { global $DB; $user = $contextlist->get_user(); - $context = \context_system::instance(); + $context = \context_user::instance($contextlist->get_user()->id); $tables = static::get_table_user_map($user); foreach ($tables as $table => $filterparams) { $records = $DB->get_recordset($table, $filterparams); foreach ($records as $record) { - writer::with_context($context)->export_data([], $record); + writer::with_context($context)->export_data([ + get_string('privacy:metadata:auth_oidc', 'auth_oidc'), + get_string('privacy:metadata:'.$table, 'auth_oidc') + ], $record); } } } + /** + * Get a map of database tables that contain user data, and the filters to get records for a user. + * + * @param \stdClass $user The user to get the map for. + * @return array The table user map. + */ + protected static function get_table_user_map(\stdClass $user): array { + $tables = [ + 'auth_oidc_prevlogin' => ['userid' => $user->id], + 'auth_oidc_token' => ['userid' => $user->id], + ]; + return $tables; + } + /** * Delete all data for all users in the specified context. * - * @param context $context The specific context to delete data for. + * @param context $context The specific context to delete data for. */ public static function delete_data_for_all_users_in_context(\context $context) { - // We only have data at the system context. + if ($context->contextlevel == CONTEXT_USER) { + self::delete_user_data($context->instanceid); + } } /** * Delete all user data for the specified user, in the specified contexts. * - * @param approved_contextlist $contextlist The approved contexts and user information to delete information for. + * @param approved_contextlist $contextlist The approved contexts and user information to delete information for. */ public static function delete_data_for_user(approved_contextlist $contextlist) { - global $DB; - $user = $contextlist->get_user(); - $tables = static::get_table_user_map($user); - foreach ($tables as $table => $filterparams) { - $DB->delete_records($table, $filterparams); + if (empty($contextlist->count())) { + return; + } + foreach ($contextlist->get_contexts() as $context) { + if ($context->contextlevel == CONTEXT_USER) { + self::delete_user_data($context->instanceid); + } } } /** - * Get a map of database tables that contain user data, and the filters to get records for a user. + * This does the deletion of user data given a userid. * - * @param \stdClass $user The user to get the map for. - * @return array The table user map. + * @param int $userid The user ID */ - protected static function get_table_user_map(\stdClass $user): array { - $tables = [ - 'auth_oidc_prevlogin' => ['userid' => $user->id], - 'auth_oidc_token' => ['userid' => $user->id], - ]; - return $tables; + private static function delete_user_data(int $userid) { + global $DB; + $DB->delete_records('auth_oidc_prevlogin', ['userid' => $userid]); + $DB->delete_records('auth_oidc_token', ['userid' => $userid]); + } + + /** + * Delete multiple users within a single context. + * + * @param \core_privacy\local\request\approved_userlist $userlist The approved context and user information to delete + * information for. + */ + public static function delete_data_for_users(\core_privacy\local\request\approved_userlist $userlist) { + $context = $userlist->get_context(); + // Because we only use user contexts the instance ID is the user ID. + if ($context instanceof \context_user) { + self::delete_user_data($context->instanceid); + } } } diff --git a/lang/en/auth_oidc.php b/lang/en/auth_oidc.php index 830cc55..acbfb55 100644 --- a/lang/en/auth_oidc.php +++ b/lang/en/auth_oidc.php @@ -125,11 +125,12 @@ $string['oidc:manageconnectionconnect'] = 'Allow OpenID Connection'; $string['oidc:manageconnectiondisconnect'] = 'Allow OpenID Disconnection'; -$string['privacy:metadata:auth_oidc_prevlogin'] = 'Information about users previous login methods to undo Office 365 connections'; +$string['privacy:metadata:auth_oidc'] = 'OpenID Connect Authentication'; +$string['privacy:metadata:auth_oidc_prevlogin'] = 'Previous login methods to undo Office 365 connections'; $string['privacy:metadata:auth_oidc_prevlogin:userid'] = 'The ID of the Moodle user'; $string['privacy:metadata:auth_oidc_prevlogin:method'] = 'The previous login method'; $string['privacy:metadata:auth_oidc_prevlogin:password'] = 'The previous (encrypted) user password field.'; -$string['privacy:metadata:auth_oidc_token'] = 'Information about OpenID Connect tokens for users'; +$string['privacy:metadata:auth_oidc_token'] = 'OpenID Connect tokens'; $string['privacy:metadata:auth_oidc_token:oidcuniqid'] = 'The OIDC unique user identifier.'; $string['privacy:metadata:auth_oidc_token:username'] = 'The username of the Moodle user'; $string['privacy:metadata:auth_oidc_token:userid'] = 'The user ID of the Moodle user'; diff --git a/tests/privacy_provider_test.php b/tests/privacy_provider_test.php new file mode 100644 index 0000000..069924d --- /dev/null +++ b/tests/privacy_provider_test.php @@ -0,0 +1,300 @@ +. + +/** + * Privacy test for auth_oidc + * + * @package auth_oidc + * @author Remote-Learner.net Inc + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @copyright (C) 2019 Remote Learner.net Inc http://www.remote-learner.net + */ + +defined('MOODLE_INTERNAL') || die(); + +use \auth_oidc\privacy\provider; + +/** + * Privacy test for auth_oidc + * + * @group auth_oidc + * @group auth_oidc_privacy + * @group office365 + * @group office365_privacy + */ +class auth_oidc_privacy_testcase extends \core_privacy\tests\provider_testcase { + /** + * Tests set up. + */ + public function setUp() { + global $CFG; + $this->resetAfterTest(); + $this->setAdminUser(); + } + + /** + * Check that a user context is returned if there is any user data for this user. + */ + public function test_get_contexts_for_userid() { + $user = $this->getDataGenerator()->create_user(); + $this->assertEmpty(provider::get_contexts_for_userid($user->id)); + + // Create user records. + self::create_token($user->id); + self::create_prevlogin($user->id); + + $contextlist = provider::get_contexts_for_userid($user->id); + // Check that we only get back one context. + $this->assertCount(1, $contextlist); + + // Check that a context is returned and is the expected context. + $usercontext = \context_user::instance($user->id); + $this->assertEquals($usercontext->id, $contextlist->get_contextids()[0]); + } + + /** + * Test that only users with a user context are fetched. + */ + public function test_get_users_in_context() { + $this->resetAfterTest(); + + $component = 'auth_oidc'; + // Create a user. + $user = $this->getDataGenerator()->create_user(); + $usercontext = context_user::instance($user->id); + + // The list of users should not return anything yet (related data still haven't been created). + $userlist = new \core_privacy\local\request\userlist($usercontext, $component); + provider::get_users_in_context($userlist); + $this->assertCount(0, $userlist); + + // Create user records. + self::create_token($user->id); + self::create_prevlogin($user->id); + + // The list of users for user context should return the user. + provider::get_users_in_context($userlist); + $this->assertCount(1, $userlist); + $expected = [$user->id]; + $actual = $userlist->get_userids(); + $this->assertEquals($expected, $actual); + + // The list of users for system context should not return any users. + $userlist = new \core_privacy\local\request\userlist(context_system::instance(), $component); + provider::get_users_in_context($userlist); + $this->assertCount(0, $userlist); + } + + /** + * Test that user data is exported correctly. + */ + public function test_export_user_data() { + // Create a user record. + $user = $this->getDataGenerator()->create_user(); + $tokenrecord = self::create_token($user->id); + $prevloginrecord = self::create_prevlogin($user->id); + + $usercontext = \context_user::instance($user->id); + + $writer = \core_privacy\local\request\writer::with_context($usercontext); + $this->assertFalse($writer->has_any_data()); + $approvedlist = new core_privacy\local\request\approved_contextlist($user, 'auth_oidc', [$usercontext->id]); + provider::export_user_data($approvedlist); + // Token. + $data = $writer->get_data([ + get_string('privacy:metadata:auth_oidc', 'auth_oidc'), + get_string('privacy:metadata:auth_oidc_token', 'auth_oidc') + ]); + $this->assertEquals($tokenrecord->userid, $data->userid); + $this->assertEquals($tokenrecord->token, $data->token); + // Previous login. + $data = $writer->get_data([ + get_string('privacy:metadata:auth_oidc', 'auth_oidc'), + get_string('privacy:metadata:auth_oidc_prevlogin', 'auth_oidc') + ]); + $this->assertEquals($prevloginrecord->userid, $data->userid); + $this->assertEquals($prevloginrecord->method, $data->method); + $this->assertEquals($prevloginrecord->password, $data->password); + } + + /** + * Test deleting all user data for a specific context. + */ + public function test_delete_data_for_all_users_in_context() { + global $DB; + + // Create a user record. + $user1 = $this->getDataGenerator()->create_user(); + self::create_token($user1->id); + self::create_prevlogin($user1->id); + $user1context = \context_user::instance($user1->id); + + $user2 = $this->getDataGenerator()->create_user(); + self::create_token($user2->id); + self::create_prevlogin($user2->id); + + // Get all accounts. There should be two. + $this->assertCount(2, $DB->get_records('auth_oidc_token', [])); + $this->assertCount(2, $DB->get_records('auth_oidc_prevlogin', [])); + + // Delete everything for the first user context. + provider::delete_data_for_all_users_in_context($user1context); + + $this->assertCount(0, $DB->get_records('auth_oidc_token', ['userid' => $user1->id])); + $this->assertCount(0, $DB->get_records('auth_oidc_prevlogin', ['userid' => $user1->id])); + + // Get all accounts. There should be one. + $this->assertCount(1, $DB->get_records('auth_oidc_token', [])); + $this->assertCount(1, $DB->get_records('auth_oidc_prevlogin', [])); + } + + /** + * This should work identical to the above test. + */ + public function test_delete_data_for_user() { + global $DB; + + // Create a user record. + $user1 = $this->getDataGenerator()->create_user(); + self::create_token($user1->id); + self::create_prevlogin($user1->id); + $user1context = \context_user::instance($user1->id); + + $user2 = $this->getDataGenerator()->create_user(); + self::create_token($user2->id); + self::create_prevlogin($user2->id); + + // Get all accounts. There should be two. + $this->assertCount(2, $DB->get_records('auth_oidc_token', [])); + $this->assertCount(2, $DB->get_records('auth_oidc_prevlogin', [])); + + // Delete everything for the first user. + $approvedlist = new \core_privacy\local\request\approved_contextlist($user1, 'auth_oidc', [$user1context->id]); + provider::delete_data_for_user($approvedlist); + + $this->assertCount(0, $DB->get_records('auth_oidc_token', ['userid' => $user1->id])); + $this->assertCount(0, $DB->get_records('auth_oidc_prevlogin', ['userid' => $user1->id])); + + // Get all accounts. There should be one. + $this->assertCount(1, $DB->get_records('auth_oidc_token', [])); + $this->assertCount(1, $DB->get_records('auth_oidc_prevlogin', [])); + } + + /** + * Test that data for users in approved userlist is deleted. + */ + public function test_delete_data_for_users() { + $this->resetAfterTest(); + + $component = 'auth_oidc'; + // Create user1. + $user1 = $this->getDataGenerator()->create_user(); + $usercontext1 = context_user::instance($user1->id); + self::create_token($user1->id); + self::create_prevlogin($user1->id); + + // Create user2. + $user2 = $this->getDataGenerator()->create_user(); + $usercontext2 = context_user::instance($user2->id); + self::create_token($user2->id); + self::create_prevlogin($user2->id); + + // The list of users for usercontext1 should return user1. + $userlist1 = new \core_privacy\local\request\userlist($usercontext1, $component); + provider::get_users_in_context($userlist1); + $this->assertCount(1, $userlist1); + $expected = [$user1->id]; + $actual = $userlist1->get_userids(); + $this->assertEquals($expected, $actual); + + // The list of users for usercontext2 should return user2. + $userlist2 = new \core_privacy\local\request\userlist($usercontext2, $component); + provider::get_users_in_context($userlist2); + $this->assertCount(1, $userlist2); + $expected = [$user2->id]; + $actual = $userlist2->get_userids(); + $this->assertEquals($expected, $actual); + + // Add userlist1 to the approved user list. + $approvedlist = new \core_privacy\local\request\approved_userlist($usercontext1, $component, $userlist1->get_userids()); + + // Delete user data using delete_data_for_user for usercontext1. + provider::delete_data_for_users($approvedlist); + + // Re-fetch users in usercontext1 - The user list should now be empty. + $userlist1 = new \core_privacy\local\request\userlist($usercontext1, $component); + provider::get_users_in_context($userlist1); + $this->assertCount(0, $userlist1); + // Re-fetch users in usercontext2 - The user list should not be empty (user2). + $userlist2 = new \core_privacy\local\request\userlist($usercontext2, $component); + provider::get_users_in_context($userlist2); + $this->assertCount(1, $userlist2); + + // User data should be only removed in the user context. + $systemcontext = context_system::instance(); + // Add userlist2 to the approved user list in the system context. + $approvedlist = new \core_privacy\local\request\approved_userlist($systemcontext, $component, $userlist2->get_userids()); + // Delete user1 data using delete_data_for_user. + provider::delete_data_for_users($approvedlist); + // Re-fetch users in usercontext2 - The user list should not be empty (user2). + $userlist2 = new \core_privacy\local\request\userlist($usercontext2, $component); + provider::get_users_in_context($userlist2); + $this->assertCount(1, $userlist2); + } + + /** + * Create a token record for the specified userid. + * + * @param int $userid + * @return stdClass + * @throws dml_exception + */ + static private function create_token(int $userid): \stdClass { + global $DB; + $record = new stdClass(); + $record->oidcuniqid = "user@example.com"; + $record->username = "user@example.com"; + $record->userid = $userid; + $record->oidcusername = "user@example.com"; + $record->scope = "All"; + $record->resource = "https://graph.windows.net"; + $record->authcode = "authcode123"; + $record->token = "token123"; + $record->expiry = 12345; + $record->refreshtoken = "refresh123"; + $record->idtoken = "idtoken123"; + $record->id = $DB->insert_record('auth_oidc_token', $record); + return $record; + } + + /** + * Create a previous login record for the specified userid. + * + * @param int $userid + * @return stdClass + * @throws dml_exception + */ + static private function create_prevlogin(int $userid): \stdClass { + global $DB; + $record = new stdClass(); + $record->userid = $userid; + $record->method = "manual"; + $record->password = "abc123"; + $record->id = $DB->insert_record('auth_oidc_prevlogin', $record); + return $record; + } + +}