From 0caedaab7cd5a46331d56654ce9301b0a5a04c56 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Thu, 15 Feb 2024 17:26:38 +0100 Subject: [PATCH] MDL-80959 tool_mobile: Use different user keys for QR and auto login --- admin/tool/mobile/classes/api.php | 4 ++-- admin/tool/mobile/classes/external.php | 4 ++-- admin/tool/mobile/classes/privacy/provider.php | 7 ++++++- admin/tool/mobile/tests/privacy/provider_test.php | 12 +++++++++--- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/admin/tool/mobile/classes/api.php b/admin/tool/mobile/classes/api.php index fbbb3a60607..de270581ae3 100644 --- a/admin/tool/mobile/classes/api.php +++ b/admin/tool/mobile/classes/api.php @@ -432,13 +432,13 @@ class api { public static function get_qrlogin_key(stdClass $mobilesettings) { global $USER; // Delete previous keys. - delete_user_key('tool_mobile', $USER->id); + delete_user_key('tool_mobile/qrlogin', $USER->id); // Create a new key. $iprestriction = !empty($mobilesettings->qrsameipcheck) ? getremoteaddr(null) : null; $qrkeyttl = !empty($mobilesettings->qrkeyttl) ? $mobilesettings->qrkeyttl : self::LOGIN_QR_KEY_TTL; $validuntil = time() + $qrkeyttl; - return create_user_key('tool_mobile', $USER->id, null, $iprestriction, $validuntil); + return create_user_key('tool_mobile/qrlogin', $USER->id, null, $iprestriction, $validuntil); } /** diff --git a/admin/tool/mobile/classes/external.php b/admin/tool/mobile/classes/external.php index ec79cd91220..34eba17682f 100644 --- a/admin/tool/mobile/classes/external.php +++ b/admin/tool/mobile/classes/external.php @@ -648,8 +648,8 @@ class external extends external_api { api::check_autologin_prerequisites($params['userid']); // Checks https, avoid site admins using this... // Validate and delete the key. - $key = validate_user_key($params['qrloginkey'], 'tool_mobile', null); - delete_user_key('tool_mobile', $params['userid']); + $key = validate_user_key($params['qrloginkey'], 'tool_mobile/qrlogin', null); + delete_user_key('tool_mobile/qrlogin', $params['userid']); // Double check key belong to user. if ($key->userid != $params['userid']) { diff --git a/admin/tool/mobile/classes/privacy/provider.php b/admin/tool/mobile/classes/privacy/provider.php index 54c61a1c2a8..2a60cc5699b 100644 --- a/admin/tool/mobile/classes/privacy/provider.php +++ b/admin/tool/mobile/classes/privacy/provider.php @@ -66,7 +66,7 @@ class provider implements FROM {user_private_key} k JOIN {user} u ON k.userid = u.id JOIN {context} ctx ON ctx.instanceid = u.id AND ctx.contextlevel = :contextlevel - WHERE k.userid = :userid AND k.script = 'tool_mobile'"; + WHERE k.userid = :userid AND (k.script = 'tool_mobile' OR k.script = 'tool_mobile/qrlogin')"; $params = ['userid' => $userid, 'contextlevel' => CONTEXT_USER]; $contextlist = new contextlist(); $contextlist->add_from_sql($sql, $params); @@ -88,6 +88,7 @@ class provider implements // Add users based on userkey. \core_userkey\privacy\provider::get_user_contexts_with_script($userlist, $context, 'tool_mobile'); + \core_userkey\privacy\provider::get_user_contexts_with_script($userlist, $context, 'tool_mobile/qrlogin'); } /** @@ -108,6 +109,7 @@ class provider implements } // Export associated userkeys. \core_userkey\privacy\provider::export_userkeys($context, [], 'tool_mobile'); + \core_userkey\privacy\provider::export_userkeys($context, [], 'tool_mobile/qrlogin'); } /** * Export all user preferences for the plugin. @@ -138,6 +140,7 @@ class provider implements $userid = $context->instanceid; // Delete all the userkeys. \core_userkey\privacy\provider::delete_userkeys('tool_mobile', $userid); + \core_userkey\privacy\provider::delete_userkeys('tool_mobile/qrlogin', $userid); } /** * Delete all user data for the specified user, in the specified contexts. @@ -158,6 +161,7 @@ class provider implements $userid = $context->instanceid; // Delete all the userkeys. \core_userkey\privacy\provider::delete_userkeys('tool_mobile', $userid); + \core_userkey\privacy\provider::delete_userkeys('tool_mobile/qrlogin', $userid); } /** @@ -178,5 +182,6 @@ class provider implements // Delete all the userkeys. \core_userkey\privacy\provider::delete_userkeys('tool_mobile', $userid); + \core_userkey\privacy\provider::delete_userkeys('tool_mobile/qrlogin', $userid); } } diff --git a/admin/tool/mobile/tests/privacy/provider_test.php b/admin/tool/mobile/tests/privacy/provider_test.php index 92add10ad65..d85ee43cf91 100644 --- a/admin/tool/mobile/tests/privacy/provider_test.php +++ b/admin/tool/mobile/tests/privacy/provider_test.php @@ -87,7 +87,8 @@ class provider_test extends \core_privacy\tests\provider_testcase { $context1 = \context_user::instance($user1->id); $context2 = \context_user::instance($user2->id); $key1 = get_user_key('tool_mobile', $user1->id); - $key2 = get_user_key('tool_mobile', $user2->id); + $key2 = get_user_key('tool_mobile/qrlogin', $user1->id); + $key3 = get_user_key('tool_mobile', $user2->id); // Ensure only user1 is found in context1. $userlist = new \core_privacy\local\request\userlist($context1, $component); @@ -175,12 +176,15 @@ class provider_test extends \core_privacy\tests\provider_testcase { $context1 = \context_user::instance($user1->id); $context2 = \context_user::instance($user2->id); $keyvalue1 = get_user_key('tool_mobile', $user1->id); - $keyvalue2 = get_user_key('tool_mobile', $user2->id); + $keyvalue2 = get_user_key('tool_mobile/qrlogin', $user1->id); + $keyvalue3 = get_user_key('tool_mobile', $user2->id); $key1 = $DB->get_record('user_private_key', ['value' => $keyvalue1]); - // Before deletion, we should have 2 user_private_keys. + // Before deletion, we should have 2 user_private_keys for tool_mobile and one for tool_mobile/qrlogin. $count = $DB->count_records('user_private_key', ['script' => 'tool_mobile']); $this->assertEquals(2, $count); + $count = $DB->count_records('user_private_key', ['script' => 'tool_mobile/qrlogin']); + $this->assertEquals(1, $count); // Ensure deleting wrong user in the user context does nothing. $approveduserids = [$user2->id]; @@ -198,6 +202,8 @@ class provider_test extends \core_privacy\tests\provider_testcase { // Ensure only user1's data is deleted, user2's remains. $count = $DB->count_records('user_private_key', ['script' => 'tool_mobile']); $this->assertEquals(1, $count); + $count = $DB->count_records('user_private_key', ['script' => 'tool_mobile/qrlogin']); + $this->assertEquals(0, $count); $params = ['script' => $component]; $userid = $DB->get_field_select('user_private_key', 'userid', 'script = :script', $params);