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.
This commit is contained in:
meirzamoodle 2024-07-27 04:27:46 +07:00
parent d8a1a67fd7
commit f0dd2ae1f5
2 changed files with 28 additions and 5 deletions

View File

@ -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;

View File

@ -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.