From 607ec454683afccc3e28cb36f8e99ec35c189bf5 Mon Sep 17 00:00:00 2001 From: Martin Rader Date: Wed, 29 Jul 2026 09:23:31 +0200 Subject: [PATCH 1/4] Fix login when a different user is already logged in require_logout() closes the PHP session, so completing the login in the same request made complete_user_login()'s session_regenerate_id() run against a closed session. Log the old user out and redirect back to the login endpoint so the key login runs on a clean session; the key is not consumed until after that branch, so it is still valid on the second pass. Fixes #102 --- auth.php | 16 +++++++++++++- tests/auth_plugin_test.php | 43 +++++++++++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/auth.php b/auth.php index df80d54..50475a1 100644 --- a/auth.php +++ b/auth.php @@ -159,8 +159,22 @@ public function user_login_userkey() { if (isloggedin()) { if ($USER->id != $key->userid) { - // Logout the current user if it's different to one that associated to the valid key. + // A different user is currently logged in. require_logout() closes the PHP + // session; continuing the login in the same request would then make + // complete_user_login()'s session_regenerate_id() run against a closed + // session (the "Session ID cannot be regenerated when there is no active + // session" warning plus the "mutated the session after it was closed" + // notice). Instead, log the old user out and bounce back to this same + // endpoint so the key login runs on a clean, logged-out session. The key + // is not consumed until below, so it is still valid on the second pass. require_logout(); + + $params = ['key' => $keyvalue]; + if (!empty($wantsurl)) { + $params['wantsurl'] = $wantsurl; + } + $loginurl = new moodle_url('/auth/userkey/login.php', $params); + $this->redirect($loginurl->out(false)); } else { // Don't process further if the user is already logged in. $this->userkeymanager->delete_keys($key->userid); diff --git a/tests/auth_plugin_test.php b/tests/auth_plugin_test.php index 672a057..b017150 100644 --- a/tests/auth_plugin_test.php +++ b/tests/auth_plugin_test.php @@ -1024,7 +1024,37 @@ public function test_pre_loginpage_hook_does_not_redirect_if_skipsso_set_and_sso } /** - * Test that if one user logged, he will be logged out before a new one is authorised. + * Test that if one user logged, he will be logged out and redirected back to the login endpoint. + */ + public function test_that_different_authorised_user_is_logged_out_and_redirected_back(): void { + global $USER; + + $user = $this->getDataGenerator()->create_user(); + $this->setUser($user); + $this->assertEquals($USER->id, $user->id); + + $this->create_user_private_key(); + + $_POST['key'] = 'TestKey'; + $_POST['wantsurl'] = self::REDIRECTION_PATH; + + try { + // Using @ is the only way to test this. Thanks moodle! + @$this->auth->user_login_userkey(); + $this->fail('A redirect back to the login endpoint was expected.'); + } catch (moodle_exception $e) { + // The other user is logged out, and the login is not completed in this request: + // require_logout() has closed the session, so complete_user_login() could not + // regenerate the session id here. The key is untouched and used on the next request. + $this->assertFalse(isloggedin()); + $this->assertStringContainsString('/auth/userkey/login.php', $e->getMessage()); + $this->assertStringContainsString('key=TestKey', $e->getMessage()); + $this->assertStringContainsString('wantsurl=' . rawurlencode(self::REDIRECTION_PATH), $e->getMessage()); + } + } + + /** + * Test that the key still logs the new user in on the request following the logout redirect. */ public function test_that_different_authorised_user_is_logged_out_and_new_one_logged_in(): void { global $USER, $SESSION; @@ -1037,9 +1067,20 @@ public function test_that_different_authorised_user_is_logged_out_and_new_one_lo $_POST['key'] = 'TestKey'; + // First request: logs the other user out and redirects back to the login endpoint. + try { + // Using @ is the only way to test this. Thanks moodle! + @$this->auth->user_login_userkey(); + $this->fail('A redirect back to the login endpoint was expected.'); + } catch (moodle_exception $e) { + $this->assertFalse(isloggedin()); + } + + // Second request: the key was not consumed above, so it now logs the key's user in. try { // Using @ is the only way to test this. Thanks moodle! @$this->auth->user_login_userkey(); + $this->fail('A redirect to the target url was expected.'); } catch (moodle_exception $e) { $this->assertEquals($this->user->id, $USER->id); $this->assertSame(sesskey(), $USER->sesskey); From 7fbbf622e2ab623ea833767c341fe5b37bd5f1d9 Mon Sep 17 00:00:00 2001 From: Umberto Date: Tue, 1 Sep 2026 17:58:17 +0200 Subject: [PATCH 2/4] Improve test description for user logout and redirection scenario --- tests/auth_plugin_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/auth_plugin_test.php b/tests/auth_plugin_test.php index b017150..7412224 100644 --- a/tests/auth_plugin_test.php +++ b/tests/auth_plugin_test.php @@ -1024,7 +1024,7 @@ public function test_pre_loginpage_hook_does_not_redirect_if_skipsso_set_and_sso } /** - * Test that if one user logged, he will be logged out and redirected back to the login endpoint. + * Test that if one user is logged in, he will be logged out and redirected back to the login endpoint. */ public function test_that_different_authorised_user_is_logged_out_and_redirected_back(): void { global $USER; From 3c53ffd37def137b885d15b7a34fc593662e4620 Mon Sep 17 00:00:00 2001 From: Umberto Date: Tue, 1 Sep 2026 17:59:08 +0200 Subject: [PATCH 3/4] Bump plugin version and release to 2026090100 - fix CI requirement --- version.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.php b/version.php index 25c39df..f66e638 100644 --- a/version.php +++ b/version.php @@ -24,8 +24,8 @@ defined('MOODLE_INTERNAL') || die; -$plugin->version = 2025120300; // The current plugin version (Date: YYYYMMDDXX). -$plugin->release = 2025120300; // Match release exactly to version. +$plugin->version = 2026090100; // The current plugin version (Date: YYYYMMDDXX). +$plugin->release = 2026090100; // Match release exactly to version. $plugin->requires = 2017051500; // Requires Moodle 3.3 version. $plugin->component = 'auth_userkey'; // Full name of the plugin (used for diagnostics). $plugin->maturity = MATURITY_STABLE; From bb39c9974d7b7cacfe159596dadbcd29fe75455d Mon Sep 17 00:00:00 2001 From: Umberto Date: Wed, 2 Sep 2026 09:08:54 +0200 Subject: [PATCH 4/4] fix CI version - Bump plugin version and release from 2026090100 to 2025120301 --- version.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.php b/version.php index f66e638..8a41086 100644 --- a/version.php +++ b/version.php @@ -24,8 +24,8 @@ defined('MOODLE_INTERNAL') || die; -$plugin->version = 2026090100; // The current plugin version (Date: YYYYMMDDXX). -$plugin->release = 2026090100; // Match release exactly to version. +$plugin->version = 2025120301; // The current plugin version (Date: YYYYMMDDXX). +$plugin->release = 2025120301; // Match release exactly to version. $plugin->requires = 2017051500; // Requires Moodle 3.3 version. $plugin->component = 'auth_userkey'; // Full name of the plugin (used for diagnostics). $plugin->maturity = MATURITY_STABLE;