diff --git a/auth/oidc/classes/loginflow/authcode.php b/auth/oidc/classes/loginflow/authcode.php index b12867dca..e79da2f63 100644 --- a/auth/oidc/classes/loginflow/authcode.php +++ b/auth/oidc/classes/loginflow/authcode.php @@ -574,7 +574,11 @@ protected function check_for_matched($entraidupn) { global $DB; if (auth_oidc_is_local_365_installed()) { - $match = $DB->get_record('local_o365_connections', ['entraidupn' => $entraidupn]); + $entraidupn = trim($entraidupn); + $sql = 'SELECT * + FROM {local_o365_connections} + WHERE ' . $DB->sql_equal('entraidupn', ':entraidupn', false); + $match = $DB->get_record_sql($sql, ['entraidupn' => $entraidupn]); if (!empty($match) && \local_o365\utils::is_o365_connected($match->muserid) !== true) { return $DB->get_record('user', ['id' => $match->muserid]); } @@ -850,6 +854,10 @@ protected function handlelogin(string $oidcuniqid, array $authparams, array $tok $matchedwith->entraidupn = $username; throw new moodle_exception('errorusermatched', 'auth_oidc', null, $matchedwith); } + // The matched Moodle user is already set to auth 'oidc': bind the login to that user's own + // username rather than the Microsoft-derived one, which may not match it (e.g. a manual match + // keyed on the full UPN while the Moodle username is only the UPN prefix). + $username = $matchedwith->username; } $username = trim(core_text::strtolower($username)); $tokenrec = $this->createtoken($oidcuniqid, $username, $authparams, $tokenparams, $idtoken, 0, $originalupn); diff --git a/auth/oidc/tests/loginflow/authcode_test.php b/auth/oidc/tests/loginflow/authcode_test.php new file mode 100644 index 000000000..d0caae076 --- /dev/null +++ b/auth/oidc/tests/loginflow/authcode_test.php @@ -0,0 +1,146 @@ +. + +namespace auth_oidc\loginflow; + +use advanced_testcase; +use auth_oidc\jwt; +use core\plugininfo\auth as auth_plugininfo; +use phpunit_util; + +/** + * Unit tests for the class \auth_oidc\loginflow\authcode. + * + * @package auth_oidc + * @copyright 2026 Enovation Solutions + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @group auth_oidc + * @group office365 + * @coversDefaultClass \auth_oidc\loginflow\authcode + */ +final class authcode_test extends advanced_testcase { + /** + * Set up test environment. + * + * @return void + */ + protected function setUp(): void { + parent::setUp(); + $this->resetAfterTest(true); + + auth_plugininfo::enable_plugin('oidc', 1); + set_config('bindingusernameclaim', 'upn', 'auth_oidc'); + } + + /** + * A manually matched user (auth already 'oidc', local_o365_connections keyed on the full Entra UPN, + * Moodle username only the UPN prefix) must complete on the very first OIDC login: the token must be + * bound to the matched Moodle user, not left dangling because the Moodle username differs from the UPN. + * + * Regression test for https://github.com/microsoft/o365-moodle/issues/2875. + * + * @return void + * @covers ::handlelogin + * @covers ::check_for_matched + */ + public function test_handlelogin_completes_manually_matched_user_with_differing_username(): void { + if (!auth_oidc_is_local_365_installed()) { + $this->markTestSkipped('This test requires local_o365 to be installed (local_o365_connections table).'); + } + + $this->assert_login_completes_for_matched_user('asmith@sc.school.edu.au', 'asmith@sc.school.edu.au'); + } + + /** + * A local_o365_connections row stored with mixed-case entraidupn (e.g. saved before UPNs were + * normalized to lower case, or entered that way by an admin) must still match a differently-cased + * UPN claim from the ID token, including on case-sensitive database collations such as PostgreSQL. + * + * @return void + * @covers ::check_for_matched + */ + public function test_handlelogin_completes_manually_matched_user_with_legacy_mixed_case_upn(): void { + if (!auth_oidc_is_local_365_installed()) { + $this->markTestSkipped('This test requires local_o365 to be installed (local_o365_connections table).'); + } + + $this->assert_login_completes_for_matched_user('ASmith@SC.School.edu.AU', 'asmith@sc.school.edu.au'); + } + + /** + * Creates a Moodle user manually matched to the given (as-stored) Entra UPN, drives handlelogin() with + * an ID token carrying the given (as-received) UPN claim, and asserts the login completed and bound + * the auth_oidc_token to the matched user. + * + * @param string $storedentraidupn The UPN as stored in local_o365_connections.entraidupn. + * @param string $tokenupn The UPN as received in the ID token's upn/preferred_username claims. + * @return void + */ + private function assert_login_completes_for_matched_user(string $storedentraidupn, string $tokenupn): void { + global $DB; + + $user = $this->getDataGenerator()->create_user(['username' => 'asmith', 'auth' => 'oidc']); + + // Simulate an admin manually matching the Moodle user to their Entra UPN via + // Manage User Connections, then flipping the account to auth 'oidc'. + $DB->insert_record('local_o365_connections', (object) [ + 'muserid' => $user->id, + 'entraidupn' => $storedentraidupn, + 'uselogin' => 0, + ]); + + $idtoken = new jwt(); + $idtoken->set_claims([ + 'sub' => 'sub-' . $user->id, + 'upn' => $tokenupn, + 'preferred_username' => $tokenupn, + ]); + + $oidcuniqid = 'oidcuniqid-' . $user->id; + $authparams = ['code' => 'authcode-' . $user->id]; + $tokenparams = [ + 'access_token' => 'access-token', + 'id_token' => 'id-token', + 'expires_in' => 3600, + 'resource' => 'resource', + 'scope' => 'scope', + ]; + + // The user_login() method validates the auth code against the current request, so it has to be + // available via optional_param() the same way it would be on a real OIDC callback request. + $_GET['code'] = $authparams['code']; + + try { + $loginflow = new authcode(); + phpunit_util::call_internal_method( + $loginflow, + 'handlelogin', + [$oidcuniqid, $authparams, $tokenparams, $idtoken], + authcode::class + ); + } finally { + unset($_GET['code']); + } + + $tokenrec = $DB->get_record('auth_oidc_token', ['oidcuniqid' => $oidcuniqid]); + $this->assertNotEmpty($tokenrec); + $this->assertEquals($user->id, $tokenrec->userid); + $this->assertEquals('asmith', $tokenrec->username); + + global $USER; + $this->assertEquals($user->id, $USER->id); + } +} diff --git a/local/o365/classes/page/acp.php b/local/o365/classes/page/acp.php index 089650bdc..7aa147f35 100644 --- a/local/o365/classes/page/acp.php +++ b/local/o365/classes/page/acp.php @@ -34,6 +34,7 @@ use core_course_category; use core_php_time_limit; use core_plugin_manager; +use core_text; use core_user; use finfo; use html_table; @@ -2077,11 +2078,15 @@ public function mode_userconnections_manualmatch() { $customdata = ['userid' => $userid]; $mform = new manualusermatch($redirect, $customdata); if ($fromform = $mform->get_data()) { - $o365username = trim($fromform->o365username); - - // Check existing matches for Microsoft user. - $existingmatchforo365user = $DB->get_record('local_o365_connections', ['entraidupn' => $o365username]); - if (!empty($existingmatchforo365user)) { + $o365username = core_text::strtolower(trim($fromform->o365username)); + + // Check existing matches for Microsoft user. Compared case-insensitively so legacy rows stored + // with different casing (e.g. before entraidupn values were normalized to lower case) are found + // too, rather than allowing a duplicate match to be created for the same Microsoft 365 user. + $sql = 'SELECT 1 + FROM {local_o365_connections} + WHERE ' . $DB->sql_equal('entraidupn', ':entraidupn', false); + if ($DB->record_exists_sql($sql, ['entraidupn' => $o365username])) { throw new moodle_exception('acp_userconnections_manualmatch_error_o365usermatched', 'local_o365'); }