mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-01-17 13:38:32 +01:00
Code Issues Projects Releases Wiki Activity

Merge branch 'MDL-67499' of git://github.com/paulholden/moodle

Browse Source
This commit is contained in:
Eloy Lafuente (stronk7) 2020-04-15 12:49:14 +02:00
commit d9776bc02a
4 changed files with 318 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
backup/util/dbops/restore_dbops.class.php
View File

@ -1394,7 +1394,9 @@ abstract class restore_dbops {
// Note: for DB deleted users md5(username) is stored *sometimes* in the email field,
// hence we are looking there for usernames if not empty. See delete_user()
// If match by id and mnethost and user is deleted in DB and
// match by username LIKE 'backup_email.%' or by non empty email = md5(username) => ok, return target user
// match by username LIKE 'substring(backup_email).%' where the substr length matches the retained data in the
// username field (100 - (timestamp + 1) characters), or by non empty email = md5(username) => ok, return target user.
$usernamelookup = core_text::substr($user->email, 0, 89) . '.%';
if ($rec = $DB->get_record_sql("SELECT *
FROM {user} u
WHERE id = ?
@ -1407,13 +1409,14 @@ abstract class restore_dbops {
AND email = ?
)
)",
array($user->id, $user->mnethostid, $user->email.'.%', md5($user->username)))) {
array($user->id, $user->mnethostid, $usernamelookup, md5($user->username)))) {
return $rec; // Matching user, deleted in DB found, return it
}
// 1D - Handle users deleted in backup file and "alive" in DB
// If match by id and mnethost and user is deleted in backup file
// and match by email = email_without_time(backup_email) => ok, return target user
// and match by substring(email) = email_without_time(backup_email) where the substr length matches the retained data
// in the username field (100 - (timestamp + 1) characters) => ok, return target user.
if ($user->deleted) {
// Note: for DB deleted users email is stored in username field, hence we
// are looking there for emails. See delete_user()
@ -1423,7 +1426,7 @@ abstract class restore_dbops {
FROM {user} u
WHERE id = ?
AND mnethostid = ?
AND UPPER(email) = UPPER(?)",
AND " . $DB->sql_substr('UPPER(email)', 1, 89) . " = UPPER(?)",
array($user->id, $user->mnethostid, $trimemail))) {
return $rec; // Matching user, deleted in backup file found, return it
}
@ -1470,7 +1473,8 @@ abstract class restore_dbops {
// Note: for DB deleted users md5(username) is stored *sometimes* in the email field,
// hence we are looking there for usernames if not empty. See delete_user()
// 2B1 - If match by mnethost and user is deleted in DB and not empty email = md5(username) and
// (by username LIKE 'backup_email.%' or non-zero firstaccess) => ok, return target user
// (by username LIKE 'substring(backup_email).%' or non-zero firstaccess) => ok, return target user.
$usernamelookup = core_text::substr($user->email, 0, 89) . '.%';
if ($rec = $DB->get_record_sql("SELECT *
FROM {user} u
WHERE mnethostid = ?
@ -1484,14 +1488,15 @@ abstract class restore_dbops {
AND firstaccess = ?
)
)",
array($user->mnethostid, md5($user->username), $user->email.'.%', $user->firstaccess))) {
array($user->mnethostid, md5($user->username), $usernamelookup, $user->firstaccess))) {
return $rec; // Matching user found, return it
}
// 2B2 - If match by mnethost and user is deleted in DB and
// username LIKE 'backup_email.%' and non-zero firstaccess) => ok, return target user
// username LIKE 'substring(backup_email).%' and non-zero firstaccess) => ok, return target user
// (this covers situations where md5(username) wasn't being stored so we require both
// the email & non-zero firstaccess to match)
$usernamelookup = core_text::substr($user->email, 0, 89) . '.%';
if ($rec = $DB->get_record_sql("SELECT *
FROM {user} u
WHERE mnethostid = ?
@ -1499,13 +1504,13 @@ abstract class restore_dbops {
AND UPPER(username) LIKE UPPER(?)
AND firstaccess != 0
AND firstaccess = ?",
array($user->mnethostid, $user->email.'.%', $user->firstaccess))) {
array($user->mnethostid, $usernamelookup, $user->firstaccess))) {
return $rec; // Matching user found, return it
}
// 2C - Handle users deleted in backup file and "alive" in DB
// If match mnethost and user is deleted in backup file
// and match by email = email_without_time(backup_email) and non-zero firstaccess=> ok, return target user
// and match by substring(email) = email_without_time(backup_email) and non-zero firstaccess=> ok, return target user.
if ($user->deleted) {
// Note: for DB deleted users email is stored in username field, hence we
// are looking there for emails. See delete_user()
@ -1514,7 +1519,7 @@ abstract class restore_dbops {
if ($rec = $DB->get_record_sql("SELECT *
FROM {user} u
WHERE mnethostid = ?
AND UPPER(email) = UPPER(?)
AND " . $DB->sql_substr('UPPER(email)', 1, 89) . " = UPPER(?)
AND firstaccess != 0
AND firstaccess = ?",
array($user->mnethostid, $trimemail, $user->firstaccess))) {

242
backup/util/dbops/tests/restore_dbops_test.php
View File

@ -119,4 +119,246 @@ class restore_dbops_testcase extends advanced_testcase {
$this->assertSame('Table "backup_ids_temp" does not exist', $e->getMessage());
}
}
/**
* Data provider for {@link test_precheck_user()}
*/
public function precheck_user_provider() {
$emailmultiplier = [
'shortmail' => 'normalusername@example.com',
'longmail' => str_repeat('a', 100) // It's not validated, hence any string is ok.
];
$providercases = [];
foreach ($emailmultiplier as $emailk => $email) {
// Get the related cases.
$cases = $this->precheck_user_cases($email);
// Rename them (keys).
foreach ($cases as $key => $case) {
$providercases[$key . ' - ' . $emailk] = $case;
}
}
return $providercases;
}
/**
* Get all the cases implemented in {@link restore_dbops::precheck_users()}
*
* @param string $email
*/
private function precheck_user_cases($email) {
global $CFG;
$baseuserarr = [
'username' => 'normalusername',
'email' => $email,
'mnethostid' => $CFG->mnet_localhost_id,
'firstaccess' => 123456789,
'deleted' => 0,
'forceemailcleanup' => false, // Hack to force the DB record to have empty mail.
'forceduplicateadminallowed' => false]; // Hack to enable import_general_duplicate_admin_allowed.
return [
// Cases with samesite = true.
'samesite match existing (1A)' => [
'dbuser' => $baseuserarr,
'backupuser' => $baseuserarr,
'samesite' => true,
'outcome' => 'match'
],
'samesite match existing anon (1B)' => [
'dbuser' => array_merge($baseuserarr, [
'username' => 'anon01']),
'backupuser' => array_merge($baseuserarr, [
'id' => -1, 'username' => 'anon01', 'firstname' => 'anonfirstname01',
'lastname' => 'anonlastname01', 'email' => 'anon01@doesntexist.invalid']),
'samesite' => true,
'outcome' => 'match'
],
'samesite match existing deleted in db, alive in backup, by db username (1C)' => [
'dbuser' => array_merge($baseuserarr, [
'deleted' => 1]),
'backupuser' => array_merge($baseuserarr, [
'username' => 'this_wont_match']),
'samesite' => true,
'outcome' => 'match'
],
'samesite match existing deleted in db, alive in backup, by db email (1C)' => [
'dbuser' => array_merge($baseuserarr, [
'deleted' => 1]),
'backupuser' => array_merge($baseuserarr, [
'email' => 'this_wont_match']),
'samesite' => true,
'outcome' => 'match'
],
'samesite match existing alive in db, deleted in backup (1D)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($baseuserarr, [
'deleted' => 1]),
'samesite' => true,
'outcome' => 'match'
],
'samesite conflict (1E)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($baseuserarr, ['id' => -1]),
'samesite' => true,
'outcome' => false
],
'samesite create user (1F)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($baseuserarr, [
'username' => 'newusername']),
'samesite' => false,
'outcome' => true
],
// Cases with samesite = false.
'no samesite match existing, by db email (2A1)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($baseuserarr, [
'firstaccess' => 0]),
'samesite' => false,
'outcome' => 'match'
],
'no samesite match existing, by db firstaccess (2A1)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($baseuserarr, [
'email' => 'this_wont_match@example.con']),
'samesite' => false,
'outcome' => 'match'
],
'no samesite match existing anon (2A1 too)' => [
'dbuser' => array_merge($baseuserarr, [
'username' => 'anon01']),
'backupuser' => array_merge($baseuserarr, [
'id' => -1, 'username' => 'anon01', 'firstname' => 'anonfirstname01',
'lastname' => 'anonlastname01', 'email' => 'anon01@doesntexist.invalid']),
'samesite' => false,
'outcome' => 'match'
],
'no samesite match dupe admin (2A2)' => [
'dbuser' => array_merge($baseuserarr, [
'username' => 'admin_old_site_id',
'forceduplicateadminallowed' => true]),
'backupuser' => array_merge($baseuserarr, [
'username' => 'admin']),
'samesite' => false,
'outcome' => 'match'
],
'no samesite match existing deleted in db, alive in backup, by db username (2B1)' => [
'dbuser' => array_merge($baseuserarr, [
'deleted' => 1]),
'backupuser' => array_merge($baseuserarr, [
'firstaccess' => 0]),
'samesite' => false,
'outcome' => 'match'
],
'no samesite match existing deleted in db, alive in backup, by db firstaccess (2B1)' => [
'dbuser' => array_merge($baseuserarr, [
'deleted' => 1]),
'backupuser' => array_merge($baseuserarr, [
'mail' => 'this_wont_match']),
'samesite' => false,
'outcome' => 'match'
],
'no samesite match existing deleted in db, alive in backup (2B2)' => [
'dbuser' => array_merge($baseuserarr, [
'deleted' => 1,
'forceemailcleanup' => true]),
'backupuser' => $baseuserarr,
'samesite' => false,
'outcome' => 'match'
],
'no samesite match existing alive in db, deleted in backup (2C)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($baseuserarr, [
'deleted' => 1]),
'samesite' => false,
'outcome' => 'match'
],
'no samesite conflict (2D)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($baseuserarr, [
'email' => 'anotheruser@example.com', 'firstaccess' => 0]),
'samesite' => false,
'outcome' => false
],
'no samesite create user (2E)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($baseuserarr, [
'username' => 'newusername']),
'samesite' => false,
'outcome' => true
],
];
}
/**
* Test restore precheck_user method
*
* @dataProvider precheck_user_provider
* @covers restore_dbops::precheck_user()
*
* @param array $dbuser
* @param array $backupuser
* @param bool $samesite
* @param mixed $outcome
**/
public function test_precheck_user($dbuser, $backupuser, $samesite, $outcome) {
global $DB;
$this->resetAfterTest();
$dbuser = (object)$dbuser;
$backupuser = (object)$backupuser;
$siteid = null;
// If the backup user must be deleted, simulate it (by temp inserting to DB, deleting and fetching it back).
if ($backupuser->deleted) {
$backupuser->id = $DB->insert_record('user', array_merge((array)$backupuser, ['deleted' => 0]));
delete_user($backupuser);
$backupuser = $DB->get_record('user', ['id' => $backupuser->id]);
$DB->delete_records('user', ['id' => $backupuser->id]);
unset($backupuser->id);
}
// Create the db user, normally.
$dbuser->id = $DB->insert_record('user', array_merge((array)$dbuser, ['deleted' => 0]));
$backupuser->id = $backupuser->id ?? $dbuser->id;
// We may want to enable the import_general_duplicate_admin_allowed setting and look for old admin records.
if ($dbuser->forceduplicateadminallowed) {
set_config('import_general_duplicate_admin_allowed', true, 'backup');
$siteid = 'old_site_id';
}
// If the DB user must be deleted, do it and fetch it back.
if ($dbuser->deleted) {
delete_user($dbuser);
// We may want to clean the mail field (old behavior, not containing the current md5(username).
if ($dbuser->forceemailcleanup) {
$DB->set_field('user', 'email', '', ['id' => $dbuser->id]);
}
}
// Get the dbuser record, because we may have changed it above.
$dbuser = $DB->get_record('user', ['id' => $dbuser->id]);
$method = (new ReflectionClass('restore_dbops'))->getMethod('precheck_user');
$method->setAccessible(true);
$result = $method->invoke(null, $backupuser, $samesite, $siteid);
if (is_bool($result)) {
$this->assertSame($outcome, $result);
} else {
$outcome = $dbuser; // Outcome is not bool, matching found, so it must be the dbuser,
// Just check ids, it means the expected match has been found in database.
$this->assertSame($outcome->id, $result->id);
}
}
}

10
lib/moodlelib.php
View File

@ -4302,7 +4302,13 @@ function delete_user(stdClass $user) {
// Generate username from email address, or a fake email.
$delemail = !empty($user->email) ? $user->email : $user->username . '.' . $user->id . '@unknownemail.invalid';
$delname = clean_param($delemail . "." . time(), PARAM_USERNAME);
$deltime = time();
$deltimelength = core_text::strlen((string) $deltime);
// Max username length is 100 chars. Select up to limit - (length of current time + 1 [period character]) from users email.
$delname = clean_param($delemail, PARAM_USERNAME);
$delname = core_text::substr($delname, 0, 100 - ($deltimelength + 1)) . ".{$deltime}";
// Workaround for bulk deletes of users with the same email address.
while ($DB->record_exists('user', array('username' => $delname))) { // No need to use mnethostid here.
@ -4317,7 +4323,7 @@ function delete_user(stdClass $user) {
$updateuser->email = md5($user->username);// Store hash of username, useful importing/restoring users.
$updateuser->idnumber = ''; // Clear this field to free it up.
$updateuser->picture = 0;
$updateuser->timemodified = time();
$updateuser->timemodified = $deltime;
// Don't trigger update event, as user is being deleted.
user_update_user($updateuser, false, false);

53
lib/tests/moodlelib_test.php
View File

@ -2465,6 +2465,59 @@ class core_moodlelib_testcase extends advanced_testcase {
$this->resetDebugging();
}
/**
* Test deletion of user with long username
*/
public function test_delete_user_long_username() {
global $DB;
$this->resetAfterTest();
// For users without an e-mail, one will be created during deletion using {$username}.{$id}@unknownemail.invalid format.
$user = $this->getDataGenerator()->create_user([
'username' => str_repeat('a', 75),
'email' => '',
]);
delete_user($user);
// The username for the deleted user shouldn't exceed 100 characters.
$usernamedeleted = $DB->get_field('user', 'username', ['id' => $user->id]);
$this->assertEquals(100, core_text::strlen($usernamedeleted));
$timestrlength = core_text::strlen((string) time());
// It should start with the user name, and end with the current time.
$this->assertStringStartsWith("{$user->username}.{$user->id}@", $usernamedeleted);
$this->assertRegExp('/\.\d{' . $timestrlength . '}$/', $usernamedeleted);
}
/**
* Test deletion of user with long email address
*/
public function test_delete_user_long_email() {
global $DB;
$this->resetAfterTest();
// Create user with 90 character email address.
$user = $this->getDataGenerator()->create_user([
'email' => str_repeat('a', 78) . '@example.com',
]);
delete_user($user);
// The username for the deleted user shouldn't exceed 100 characters.
$usernamedeleted = $DB->get_field('user', 'username', ['id' => $user->id]);
$this->assertEquals(100, core_text::strlen($usernamedeleted));
$timestrlength = core_text::strlen((string) time());
// Max username length is 100 chars. Select up to limit - (length of current time + 1 [period character]) from users email.
$expectedemail = core_text::substr($user->email, 0, 100 - ($timestrlength + 1));
$this->assertRegExp('/^' . preg_quote($expectedemail) . '\.\d{' . $timestrlength . '}$/', $usernamedeleted);
}
/**
* Test function convert_to_array()
*/