From f0dd2ae1f58457227a7f8d99aa7701885e204576 Mon Sep 17 00:00:00 2001 From: meirzamoodle <meirza.arson@moodle.com> Date: Sat, 27 Jul 2024 04:27:46 +0700 Subject: [PATCH] MDL-80064 authentication: password can be null The Open ID Connect plugin uses null for the password, which makes the internal password update fail to proceed. Allowing null resolved the problem. As a note, there is a potential issue if the authentication method has a false return for the prevent_local_password because it will trigger the hash_internal_user_password() where the $password can not be null. Since this only addresses the oauth2 issue, we should ignore it. --- lib/moodlelib.php | 4 ++-- lib/tests/moodlelib_test.php | 29 ++++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 94e62c601ea..7be631c1781 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -4356,7 +4356,7 @@ function hash_internal_user_password(#[\SensitiveParameter] string $password, $f * It will remove Web Services user tokens too. * * @param stdClass $user User object (password property may be updated). - * @param string $password Plain text password. + * @param string|null $password Plain text password. * @param bool $fasthash If true, use a low cost factor when generating the hash * This is much faster to generate but makes the hash * less secure. It is used when lots of hashes need to @@ -4365,7 +4365,7 @@ function hash_internal_user_password(#[\SensitiveParameter] string $password, $f */ function update_internal_user_password( stdClass $user, - #[\SensitiveParameter] string $password, + #[\SensitiveParameter] ?string $password, bool $fasthash = false ): bool { global $CFG, $DB; diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php index e0022a40573..e4fea44c050 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -2922,20 +2922,43 @@ EOF; /** * Testing that if the password is not cached, that it does not update * the user table and fire event. + * + * @dataProvider update_internal_user_password_no_cache_provider + * @covers ::update_internal_user_password + * + * @param string $authmethod The authentication method to set for the user. + * @param string|null $password The new password to set for the user. */ - public function test_update_internal_user_password_no_cache(): void { + public function test_update_internal_user_password_no_cache( + string $authmethod, + ?string $password, + ): void { global $DB; $this->resetAfterTest(); - $user = $this->getDataGenerator()->create_user(array('auth' => 'cas')); + $user = $this->getDataGenerator()->create_user(['auth' => $authmethod]); $DB->update_record('user', ['id' => $user->id, 'password' => AUTH_PASSWORD_NOT_CACHED]); $user->password = AUTH_PASSWORD_NOT_CACHED; $sink = $this->redirectEvents(); - update_internal_user_password($user, 'wonkawonka'); + update_internal_user_password($user, $password); $this->assertEquals(0, $sink->count(), 'User updated event should not fire'); } + /** + * The data provider will test the {@see test_update_internal_user_password_no_cache} + * for accounts using the authentication method with prevent_local_passwords set to true (no cache). + * + * @return array + */ + public static function update_internal_user_password_no_cache_provider(): array { + return [ + 'Password is not empty' => ['cas', 'wonkawonka'], + 'Password is an empty string' => ['oauth2', ''], + 'Password is null' => ['oauth2', null], + ]; + } + /** * Test if the user has a password hash, but now their auth method * says not to cache it. Then it should update.