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..7412224 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 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; + + $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); diff --git a/version.php b/version.php index 25c39df..8a41086 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 = 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;