Skip to content

Commit 16a9f5f

Browse files
authored
Merge pull request #118 from MagnusSamuelsson/fix/validations-in-update_user
Only validate changed user fields when provided
2 parents 84fcbb8 + e4f2872 commit 16a9f5f

File tree

2 files changed

+47
-3
lines changed

2 files changed

+47
-3
lines changed

auth.php

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -340,18 +340,27 @@ protected function update_user(\stdClass $user, array $data) {
340340
}
341341

342342
if (
343+
isset($userdata['username'])
344+
&&
343345
$user->username != $userdata['username']
344346
&&
345347
$DB->record_exists('user', ['username' => $userdata['username'], 'mnethostid' => $CFG->mnet_localhost_id])
346348
) {
347349
throw new invalid_parameter_exception('Username already exists: ' . $userdata['username']);
348350
}
349-
if (!validate_email($userdata['email'])) {
351+
352+
$emailchangerequested = isset($userdata['email']) && $user->email != $userdata['email'];
353+
354+
if (
355+
$emailchangerequested
356+
&&
357+
!validate_email($userdata['email'])
358+
) {
350359
throw new invalid_parameter_exception('Email address is invalid: ' . $userdata['email']);
351360
} else if (
352-
empty($CFG->allowaccountssameemail)
361+
$emailchangerequested
353362
&&
354-
$user->email != $userdata['email']
363+
empty($CFG->allowaccountssameemail)
355364
&&
356365
$DB->record_exists('user', ['email' => $userdata['email'], 'mnethostid' => $CFG->mnet_localhost_id])
357366
) {

tests/auth_plugin_test.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,41 @@ public function test_update_refuse_duplicate_username(): void {
547547
$this->auth->get_login_url($originaluser);
548548
}
549549

550+
/**
551+
* Test that a user can be updated without providing any other fields than the mappingfield.
552+
* (Only auth field should be updated).
553+
*/
554+
public function test_update_allow_unset_fields(): void {
555+
global $DB;
556+
set_config('updateuser', true, 'auth_userkey');
557+
set_config('mappingfield', 'id', 'auth_userkey');
558+
$this->auth = new auth_plugin_userkey();
559+
560+
$userkeymanager = new fake_userkey_manager();
561+
$this->auth->set_userkey_manager($userkeymanager);
562+
563+
$originaluser = new stdClass();
564+
$originaluser->username = 'username';
565+
$originaluser->email = '[email protected]';
566+
$originaluser->firstname = 'user';
567+
$originaluser->lastname = 'name';
568+
569+
$user = self::getDataGenerator()->create_user($originaluser);
570+
571+
$loginuser = new stdClass();
572+
$loginuser->id = $user->id;
573+
574+
$key = $this->auth->get_login_url($loginuser);
575+
576+
$userrecord = $DB->get_record('user', ['id' => $user->id]);
577+
$this->assertNotEmpty($key);
578+
$this->assertEquals('userkey', $userrecord->auth);
579+
$this->assertEquals('username', $userrecord->username);
580+
$this->assertEquals('[email protected]', $userrecord->email);
581+
$this->assertEquals('user', $userrecord->firstname);
582+
$this->assertEquals('name', $userrecord->lastname);
583+
}
584+
550585
/**
551586
* Test that we can get login url if we do not use fake keymanager.
552587
*/

0 commit comments

Comments
 (0)