mirror of
https://github.com/moodle/moodle.git
synced 2025-03-14 12:40:01 +01:00
MDL-45641 event: Removed multiple user_updated event trigger
user_updated event was being triggred while setting password, which is not correct. It now trigger user_password_updated event. Few more modifications done: 1. Correct event is being triggred. 2. Event is only triggred when password is chnaged. 3. Password is updated via single api. 4. Updated unit test
This commit is contained in:
parent
2b55cb1b18
commit
a7466eb448
@ -4710,9 +4710,13 @@ function hash_internal_user_password($password, $fasthash = false) {
|
||||
*
|
||||
* @param stdClass $user User object (password property may be updated).
|
||||
* @param string $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
|
||||
* be generated quickly.
|
||||
* @return bool Always returns true.
|
||||
*/
|
||||
function update_internal_user_password($user, $password) {
|
||||
function update_internal_user_password($user, $password, $fasthash = false) {
|
||||
global $CFG, $DB;
|
||||
require_once($CFG->libdir.'/password_compat/lib/password.php');
|
||||
|
||||
@ -4721,25 +4725,26 @@ function update_internal_user_password($user, $password) {
|
||||
if ($authplugin->prevent_local_passwords()) {
|
||||
$hashedpassword = AUTH_PASSWORD_NOT_CACHED;
|
||||
} else {
|
||||
$hashedpassword = hash_internal_user_password($password);
|
||||
$hashedpassword = hash_internal_user_password($password, $fasthash);
|
||||
}
|
||||
|
||||
// If verification fails then it means the password has changed.
|
||||
$passwordchanged = !password_verify($password, $user->password);
|
||||
$algorithmchanged = password_needs_rehash($user->password, PASSWORD_DEFAULT);
|
||||
if (isset($user->password)) {
|
||||
// While creating new user, password in unset in $user object, to avoid
|
||||
// saving it with user_create()
|
||||
$passwordchanged = !password_verify($password, $user->password);
|
||||
$algorithmchanged = password_needs_rehash($user->password, PASSWORD_DEFAULT);
|
||||
} else {
|
||||
$passwordchanged = true;
|
||||
}
|
||||
|
||||
if ($passwordchanged || $algorithmchanged) {
|
||||
$DB->set_field('user', 'password', $hashedpassword, array('id' => $user->id));
|
||||
$user->password = $hashedpassword;
|
||||
|
||||
// Trigger event.
|
||||
$event = \core\event\user_updated::create(array(
|
||||
'objectid' => $user->id,
|
||||
'relateduserid' => $user->id,
|
||||
'context' => context_user::instance($user->id)
|
||||
));
|
||||
$event->add_record_snapshot('user', $user);
|
||||
$event->trigger();
|
||||
$user = $DB->get_record('user', array('id' => $user->id));
|
||||
\core\event\user_password_updated::create_from_user($user)->trigger();
|
||||
}
|
||||
|
||||
return true;
|
||||
@ -5969,18 +5974,7 @@ function setnew_password_and_mail($user, $fasthash = false) {
|
||||
|
||||
$newpassword = generate_password();
|
||||
|
||||
$hashedpassword = hash_internal_user_password($newpassword, $fasthash);
|
||||
$DB->set_field('user', 'password', $hashedpassword, array('id' => $user->id));
|
||||
$user->password = $hashedpassword;
|
||||
|
||||
// Trigger event.
|
||||
$event = \core\event\user_updated::create(array(
|
||||
'objectid' => $user->id,
|
||||
'relateduserid' => $user->id,
|
||||
'context' => context_user::instance($user->id)
|
||||
));
|
||||
$event->add_record_snapshot('user', $user);
|
||||
$event->trigger();
|
||||
update_internal_user_password($user, $newpassword, $fasthash);
|
||||
|
||||
$a = new stdClass();
|
||||
$a->firstname = fullname($user, true);
|
||||
|
@ -2247,11 +2247,21 @@ class core_moodlelib_testcase extends advanced_testcase {
|
||||
// Manually set the user's password to the md5 of the string 'password'.
|
||||
$DB->set_field('user', 'password', '5f4dcc3b5aa765d61d8327deb882cf99', array('id' => $user->id));
|
||||
|
||||
$sink = $this->redirectEvents();
|
||||
// Update the password.
|
||||
update_internal_user_password($user, 'password');
|
||||
$events = $sink->get_events();
|
||||
$sink->close();
|
||||
$event = array_pop($events);
|
||||
|
||||
// Password should have been updated to a bcrypt hash.
|
||||
$this->assertFalse(password_is_legacy_hash($user->password));
|
||||
|
||||
// Verify event information.
|
||||
$this->assertInstanceOf('\core\event\user_password_updated', $event);
|
||||
$this->assertSame($user->id, $event->relateduserid);
|
||||
$this->assertEquals(context_user::instance($user->id), $event->get_context());
|
||||
$this->assertEventContextNotUsed($event);
|
||||
}
|
||||
|
||||
public function test_fullname() {
|
||||
@ -2562,9 +2572,9 @@ class core_moodlelib_testcase extends advanced_testcase {
|
||||
}
|
||||
|
||||
/**
|
||||
* Test user_updated event trigger by various apis.
|
||||
* Test setnew_password_and_mail.
|
||||
*/
|
||||
public function test_user_updated_event() {
|
||||
public function test_setnew_password_and_mail() {
|
||||
global $DB, $CFG;
|
||||
|
||||
$this->resetAfterTest();
|
||||
@ -2578,27 +2588,21 @@ class core_moodlelib_testcase extends advanced_testcase {
|
||||
$sink = $this->redirectEvents();
|
||||
$sink2 = $this->redirectEmails(); // Make sure we are redirecting emails.
|
||||
setnew_password_and_mail($user);
|
||||
update_internal_user_password($user, 'randompass');
|
||||
$events = $sink->get_events();
|
||||
$sink->close();
|
||||
$sink2->close();
|
||||
$event = array_pop($events);
|
||||
|
||||
// Test updated value.
|
||||
$dbuser = $DB->get_record('user', array('id' => $user->id));
|
||||
$this->assertSame($user->firstname, $dbuser->firstname);
|
||||
$this->assertNotSame('M00dLe@T', $dbuser->password);
|
||||
$this->assertNotEmpty($dbuser->password);
|
||||
|
||||
// Test event.
|
||||
foreach ($events as $event) {
|
||||
$this->assertInstanceOf('\core\event\user_updated', $event);
|
||||
$this->assertSame($user->id, $event->objectid);
|
||||
$this->assertSame('user_updated', $event->get_legacy_eventname());
|
||||
$this->assertEventLegacyData($user, $event);
|
||||
$this->assertEquals(context_user::instance($user->id), $event->get_context());
|
||||
$expectedlogdata = array(SITEID, 'user', 'update', 'view.php?id='.$user->id, '');
|
||||
$this->assertEventLegacyLogData($expectedlogdata, $event);
|
||||
$this->assertEventContextNotUsed($event);
|
||||
}
|
||||
$this->assertInstanceOf('\core\event\user_password_updated', $event);
|
||||
$this->assertSame($user->id, $event->relateduserid);
|
||||
$this->assertEquals(context_user::instance($user->id), $event->get_context());
|
||||
$this->assertEventContextNotUsed($event);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -122,9 +122,6 @@ if ($mform->is_cancelled()) {
|
||||
unset_user_preference('auth_forcepasswordchange', $USER);
|
||||
unset_user_preference('create_password', $USER);
|
||||
|
||||
$user = $DB->get_record('user', array('id' => $USER->id), '*', MUST_EXIST);
|
||||
\core\event\user_password_updated::create_from_user($user)->trigger();
|
||||
|
||||
$strpasswordchanged = get_string('passwordchanged');
|
||||
|
||||
$fullname = fullname($USER, true);
|
||||
|
@ -251,9 +251,6 @@ function core_login_process_password_set($token) {
|
||||
}
|
||||
complete_user_login($user); // Triggers the login event.
|
||||
|
||||
$user = $DB->get_record('user', array('id' => $user->id), '*', MUST_EXIST);
|
||||
\core\event\user_password_updated::create_from_user($user, true)->trigger();
|
||||
|
||||
$urltogo = core_login_get_return_url();
|
||||
unset($SESSION->wantsurl);
|
||||
redirect($urltogo, get_string('passwordset'), 1);
|
||||
|
@ -167,7 +167,6 @@ if ($usernew = $userform->get_data()) {
|
||||
|
||||
$usernew->timemodified = time();
|
||||
$createpassword = false;
|
||||
$passwordupdated = false;
|
||||
|
||||
if ($usernew->id == -1) {
|
||||
unset($usernew->id);
|
||||
@ -192,8 +191,6 @@ if ($usernew = $userform->get_data()) {
|
||||
if (!$authplugin->user_update_password($usernew, $usernew->newpassword)) {
|
||||
// Do not stop here, we need to finish user creation.
|
||||
debugging(get_string('cannotupdatepasswordonextauth', '', '', $usernew->auth), DEBUG_NONE);
|
||||
} else {
|
||||
$passwordupdated = true;
|
||||
}
|
||||
}
|
||||
$usercreated = true;
|
||||
@ -211,8 +208,6 @@ if ($usernew = $userform->get_data()) {
|
||||
if ($authplugin->can_change_password()) {
|
||||
if (!$authplugin->user_update_password($usernew, $usernew->newpassword)) {
|
||||
print_error('cannotupdatepasswordonextauth', '', '', $usernew->auth);
|
||||
} else {
|
||||
$passwordupdated = true;
|
||||
}
|
||||
unset_user_preference('create_password', $usernew); // Prevent cron from generating the password.
|
||||
}
|
||||
@ -251,6 +246,12 @@ if ($usernew = $userform->get_data()) {
|
||||
// Reload from db.
|
||||
$usernew = $DB->get_record('user', array('id' => $usernew->id));
|
||||
|
||||
if ($createpassword) {
|
||||
setnew_password_and_mail($usernew);
|
||||
unset_user_preference('create_password', $usernew);
|
||||
set_user_preference('auth_forcepasswordchange', 1, $usernew);
|
||||
}
|
||||
|
||||
// Trigger update/create event, after all fields are stored.
|
||||
if ($usercreated) {
|
||||
\core\event\user_created::create_from_userid($usernew->id)->trigger();
|
||||
@ -258,16 +259,6 @@ if ($usernew = $userform->get_data()) {
|
||||
\core\event\user_updated::create_from_userid($usernew->id)->trigger();
|
||||
}
|
||||
|
||||
if ($passwordupdated) {
|
||||
\core\event\user_password_updated::create_from_user($usernew)->trigger();
|
||||
}
|
||||
|
||||
if ($createpassword) {
|
||||
setnew_password_and_mail($usernew);
|
||||
unset_user_preference('create_password', $usernew);
|
||||
set_user_preference('auth_forcepasswordchange', 1, $usernew);
|
||||
}
|
||||
|
||||
if ($user->id == $USER->id) {
|
||||
// Override old $USER session variable.
|
||||
foreach ((array)$usernew as $variable => $value) {
|
||||
|
@ -1,7 +0,0 @@
|
||||
This files describes API changes in /user/, information provided
|
||||
here is intended especially for developers.
|
||||
|
||||
=== 2.7.1 ===
|
||||
|
||||
* user_update_user() and user_create_user() api's accept optional param
|
||||
$triggerevent to avoid respective events to be triggred from the api's.
|
Loading…
x
Reference in New Issue
Block a user