From f3c94ce7062ee98d1bfa7633724b4ba04283350f Mon Sep 17 00:00:00 2001 From: Lai Wei Date: Thu, 6 Aug 2026 12:28:45 +0100 Subject: [PATCH] Fix stale Entra ID object ID not being repaired during user sync The N+1-avoidance refactor in usersync main.php replaced a direct per-user local_o365_objects lookup with a cache keyed by objectid, which only ever contains rows whose stored objectid is still active in Entra ID. This meant the object ID repair logic could never fire for users whose linked Entra account was deleted and recreated with a new object ID, leaving them permanently mismatched. The daily enabled-status sync task then kept re-suspending these accounts even after being manually re-enabled, since it also trusts the stale objectid. Fix by selecting the currently stored objectid alongside the existing bulk user query (no extra query) and comparing it in memory, only issuing an UPDATE when it has actually changed. --- local/o365/classes/feature/usersync/main.php | 29 +++++--- local/o365/classes/task/forcefullusersync.php | 71 +++++++++++++++++++ local/o365/db/upgrade.php | 17 +++++ local/o365/version.php | 2 +- 4 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 local/o365/classes/task/forcefullusersync.php diff --git a/local/o365/classes/feature/usersync/main.php b/local/o365/classes/feature/usersync/main.php index 2eee727f5..344f55c29 100644 --- a/local/o365/classes/feature/usersync/main.php +++ b/local/o365/classes/feature/usersync/main.php @@ -1310,7 +1310,8 @@ public function sync_users(array $entraidusers = [], string $bindingusernameclai assign.assigned assigned, assign.photoid photoid, assign.photoupdated photoupdated, - obj.id AS objectid + obj.id AS objectid, + obj.objectid AS o365objectid FROM {user} u LEFT JOIN {auth_oidc_token} tok ON tok.userid = u.id LEFT JOIN {local_o365_connections} conn ON conn.muserid = u.id @@ -1389,7 +1390,8 @@ public function sync_users(array $entraidusers = [], string $bindingusernameclai assign.assigned assigned, assign.photoid photoid, assign.photoupdated photoupdated, - obj.id AS objectid + obj.id AS objectid, + obj.objectid AS o365objectid FROM {user} u LEFT JOIN {auth_oidc_token} tok ON tok.userid = u.id LEFT JOIN {local_o365_connections} conn ON conn.muserid = u.id @@ -1913,13 +1915,22 @@ protected function sync_existing_user( } } - // Use pre-fetched O365 object record to avoid N+1 query problem. - $localo365objectrecord = $this->o365objectsbymoodleid[$existinguser->muserid] ?? null; - if ($localo365objectrecord && $localo365objectrecord->id == $existinguser->objectid) { - if ($localo365objectrecord->objectid != $userobjectid) { - $localo365objectrecord->objectid = $userobjectid; - $DB->update_record('local_o365_objects', $localo365objectrecord); - $this->mtrace('Updated user object ID in local_o365_object record.'); + // The stored Entra ID is usually already available from the existing user query. Fall back to the + // pre-fetched object cache (no extra DB query) if this record was built without it, to detect a + // changed GUID (e.g. the Entra ID account was deleted and recreated). + if (!empty($existinguser->objectid)) { + if (isset($existinguser->o365objectid)) { + $storedobjectid = $existinguser->o365objectid; + } else { + $cachedobject = $this->o365objectsbymoodleid[$existinguser->muserid] ?? null; + $storedobjectid = $cachedobject->objectid ?? null; + } + + if ($storedobjectid != $userobjectid) { + $updated = $DB->set_field('local_o365_objects', 'objectid', $userobjectid, ['id' => $existinguser->objectid]); + if ($updated) { + $this->mtrace('Updated user object ID in local_o365_objects record.'); + } } } diff --git a/local/o365/classes/task/forcefullusersync.php b/local/o365/classes/task/forcefullusersync.php new file mode 100644 index 000000000..474e4fa09 --- /dev/null +++ b/local/o365/classes/task/forcefullusersync.php @@ -0,0 +1,71 @@ +. + +/** + * An adhoc task to force a one-off full Microsoft Entra ID user sync. + * + * @package local_o365 + * @author Lai Wei + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @copyright (C) 2026 onwards Microsoft, Inc. (http://microsoft.com/) + */ + +namespace local_o365\task; + +use core\task\adhoc_task; +use local_o365\feature\usersync\main; + +/** + * Adhoc task to force a one-off full Microsoft Entra ID user sync. + * + * Queued during upgrade to repair Moodle accounts whose linked Microsoft Entra ID object ID + * went stale (e.g. the Entra ID account was deleted and recreated more than 30 days ago) before + * the fix to the object ID repair logic in the user sync task. Delta sync only reports users + * that changed since the last sync, so accounts that were already stale before this fix would + * not otherwise be picked up again on their own. If the site currently uses delta sync, this + * temporarily enables a full sync for a single run, then restores the previous setting. If a + * full sync already runs every time, no override is needed. + */ +class forcefullusersync extends adhoc_task { + /** + * Execute the task. + * + * @return bool + */ + public function execute(): bool { + if (main::sync_option_enabled('nodelta')) { + mtrace('Full sync already runs every time (nodelta enabled). No override needed.'); + + return true; + } + + $originalsetting = (string) get_config('local_o365', 'usersync'); + $originaloptions = array_filter(explode(',', $originalsetting)); + + mtrace('Delta sync is currently in use. Temporarily forcing a full sync for this run...'); + set_config('usersync', implode(',', array_merge($originaloptions, ['nodelta'])), 'local_o365'); + + try { + $task = new usersync(); + $task->execute(); + } finally { + set_config('usersync', implode(',', $originaloptions), 'local_o365'); + mtrace('Restored previous delta sync setting.'); + } + + return true; + } +} diff --git a/local/o365/db/upgrade.php b/local/o365/db/upgrade.php index d52d4ce4c..d9424a526 100644 --- a/local/o365/db/upgrade.php +++ b/local/o365/db/upgrade.php @@ -1641,5 +1641,22 @@ function xmldb_local_o365_upgrade($oldversion) { upgrade_plugin_savepoint(true, 2024100725.03, 'local', 'o365'); } + if ($oldversion < 2024100735.04) { + // Queue adhoc task to force a one-off full user sync, repairing accounts whose linked + // Microsoft Entra ID object ID went stale (e.g. the Entra ID account was deleted and + // recreated) before the object ID repair logic in the user sync task was fixed. Delta + // sync only reports users that changed since the last sync, so already-stale accounts + // would not otherwise be picked up again on their own. + $task = new \local_o365\task\forcefullusersync(); + // Set next run time to ensure it runs in the next cron cycle, not during upgrade. + $task->set_next_run_time(time() + 60); + \core\task\manager::queue_adhoc_task($task); + + mtrace('Queued adhoc task to force a full user sync and repair stale Entra ID object IDs.'); + + // O365 savepoint reached. + upgrade_plugin_savepoint(true, 2024100735.04, 'local', 'o365'); + } + return true; } diff --git a/local/o365/version.php b/local/o365/version.php index 1662c0452..191c18ae5 100644 --- a/local/o365/version.php +++ b/local/o365/version.php @@ -26,7 +26,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2024100735; +$plugin->version = 2024100735.04; $plugin->requires = 2024100700; $plugin->release = '4.5.7'; $plugin->component = 'local_o365';