Skip to content

Commit

Permalink
MDL-67499 restore: account for deleted user truncated email/username.
Browse files Browse the repository at this point in the history
paulholden committed Apr 6, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 68ba708 commit 0c68ae8
Showing 2 changed files with 33 additions and 19 deletions.
25 changes: 15 additions & 10 deletions backup/util/dbops/restore_dbops.class.php
Original file line number Diff line number Diff line change
@@ -1394,7 +1394,9 @@ protected static function precheck_user($user, $samesite, $siteid = null) {
// 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 @@ protected static function precheck_user($user, $samesite, $siteid = null) {
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 @@ protected static function precheck_user($user, $samesite, $siteid = null) {
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 @@ protected static function precheck_user($user, $samesite, $siteid = null) {
// 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,28 +1488,29 @@ protected static function precheck_user($user, $samesite, $siteid = null) {
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 = ?
AND deleted = 1
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 @@ protected static function precheck_user($user, $samesite, $siteid = null) {
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))) {
27 changes: 18 additions & 9 deletions backup/util/dbops/tests/restore_dbops_test.php
Original file line number Diff line number Diff line change
@@ -127,7 +127,7 @@ public function precheck_user_provider() {

$emailmultiplier = [
'shortmail' => '[email protected]',
//'longmail' => str_repeat('a', 100) // It's not validated, hence any string is ok.
'longmail' => str_repeat('a', 100) // It's not validated, hence any string is ok.
];

$providercases = [];
@@ -146,6 +146,8 @@ public function precheck_user_provider() {

/**
* Get all the cases implemented in {@link restore_dbops::precheck_users()}
*
* @param string $email
*/
private function precheck_user_cases($email) {
global $CFG;
@@ -154,7 +156,7 @@ private function precheck_user_cases($email) {
'username' => 'normalusername',
'email' => $email,
'mnethostid' => $CFG->mnet_localhost_id,
'firstaccess'=> 123456789,
'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.
@@ -207,7 +209,7 @@ private function precheck_user_cases($email) {
],
'samesite create user (1F)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($baseuserarr,[
'backupuser' => array_merge($baseuserarr, [
'username' => 'newusername']),
'samesite' => false,
'outcome' => true
@@ -216,14 +218,14 @@ private function precheck_user_cases($email) {
// Cases with samesite = false.
'no samesite match existing, by db email (2A1)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($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,[
'backupuser' => array_merge($baseuserarr, [
'email' => '[email protected]']),
'samesite' => false,
'outcome' => 'match'
@@ -279,14 +281,14 @@ private function precheck_user_cases($email) {
],
'no samesite conflict (2D)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($baseuserarr,[
'backupuser' => array_merge($baseuserarr, [
'email' => '[email protected]', 'firstaccess' => 0]),
'samesite' => false,
'outcome' => false
],
'no samesite create user (2E)' => [
'dbuser' => $baseuserarr,
'backupuser' => array_merge($baseuserarr,[
'backupuser' => array_merge($baseuserarr, [
'username' => 'newusername']),
'samesite' => false,
'outcome' => true
@@ -296,9 +298,16 @@ private function precheck_user_cases($email) {
}

/**
* 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;

@@ -331,7 +340,7 @@ public function test_precheck_user($dbuser, $backupuser, $samesite, $outcome) {
// 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)
// 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]);
}

0 comments on commit 0c68ae8

Please sign in to comment.