From 14329d9faa3ac4a22e8e034c61a3af366a013349 Mon Sep 17 00:00:00 2001 From: "Emma C. Hughes" <84008144+emmachughes@users.noreply.github.com> Date: Thu, 13 Feb 2025 14:08:11 +0100 Subject: [PATCH] Normalize email address before saving (#2934) * normalize email before saving * fix copy/paste error * Run php-cs-fixer --- .../hub/app/Http/Requests/Api/UserRequest.php | 14 ++++++++ .../app/Http/Requests/StoreUserRequest.php | 14 ++++++++ .../app/Http/Requests/UpdateUserRequest.php | 14 ++++++++ sourcecode/hub/app/Models/User.php | 4 +-- sourcecode/hub/tests/Browser/UserTest.php | 34 +++++++++++++++++++ sourcecode/hub/tests/Feature/Api/UserTest.php | 22 ++++++++++++ 6 files changed, 100 insertions(+), 2 deletions(-) diff --git a/sourcecode/hub/app/Http/Requests/Api/UserRequest.php b/sourcecode/hub/app/Http/Requests/Api/UserRequest.php index fb0c780ef4..0a8d61a5d6 100644 --- a/sourcecode/hub/app/Http/Requests/Api/UserRequest.php +++ b/sourcecode/hub/app/Http/Requests/Api/UserRequest.php @@ -10,8 +10,22 @@ use Illuminate\Foundation\Http\FormRequest; use Illuminate\Validation\Rule; +use function is_string; +use function strtolower; + class UserRequest extends FormRequest { + protected function prepareForValidation(): void + { + $email = $this->input('email'); + + if (is_string($email)) { + $this->merge([ + 'email' => strtolower($email), + ]); + } + } + /** * @return array */ diff --git a/sourcecode/hub/app/Http/Requests/StoreUserRequest.php b/sourcecode/hub/app/Http/Requests/StoreUserRequest.php index 449b17e0b5..7266057687 100644 --- a/sourcecode/hub/app/Http/Requests/StoreUserRequest.php +++ b/sourcecode/hub/app/Http/Requests/StoreUserRequest.php @@ -9,8 +9,22 @@ use Illuminate\Validation\Rule; use Illuminate\Validation\Rules\Password; +use function is_string; +use function strtolower; + final class StoreUserRequest extends FormRequest { + protected function prepareForValidation(): void + { + $email = $this->input('email'); + + if (is_string($email)) { + $this->merge([ + 'email' => strtolower($email), + ]); + } + } + /** * @return array */ diff --git a/sourcecode/hub/app/Http/Requests/UpdateUserRequest.php b/sourcecode/hub/app/Http/Requests/UpdateUserRequest.php index eb307b5e42..b6d17fa69e 100644 --- a/sourcecode/hub/app/Http/Requests/UpdateUserRequest.php +++ b/sourcecode/hub/app/Http/Requests/UpdateUserRequest.php @@ -7,8 +7,22 @@ use Illuminate\Foundation\Http\FormRequest; use Illuminate\Validation\Rules\Password; +use function is_string; +use function strtolower; + class UpdateUserRequest extends FormRequest { + protected function prepareForValidation(): void + { + $email = $this->input('email'); + + if (is_string($email)) { + $this->merge([ + 'email' => strtolower($email), + ]); + } + } + /** * @return array */ diff --git a/sourcecode/hub/app/Models/User.php b/sourcecode/hub/app/Models/User.php index 04a7d36cd9..cfe4995bde 100644 --- a/sourcecode/hub/app/Models/User.php +++ b/sourcecode/hub/app/Models/User.php @@ -78,9 +78,9 @@ class User extends Model implements AuthenticatableContract 'email_verified' => true, ]; - public function setEmailAttribute(string|null $email): void + public function setEmailAttribute(string $email): void { - $this->attributes['email'] = $email; + $this->attributes['email'] = strtolower($email); if ($this->exists && $email !== $this->getOriginal('email')) { $this->attributes['email_verified'] = false; diff --git a/sourcecode/hub/tests/Browser/UserTest.php b/sourcecode/hub/tests/Browser/UserTest.php index d23280e072..ff8f114233 100644 --- a/sourcecode/hub/tests/Browser/UserTest.php +++ b/sourcecode/hub/tests/Browser/UserTest.php @@ -47,6 +47,22 @@ public function testUserCannotSignUpWithDuplicateEmail(): void }); } + public function testEmailIsNormalizedUponRegistration(): void + { + $this->browse(function (Browser $browser) { + $browser->visit('/register') + ->assertGuest() + ->type('name', 'E. Mel') + ->type('email', 'E.MEL@EDLIB.TEST') + ->type('password', 'my password') + ->type('password_confirmation', 'my password') + ->press('Sign up') + ->assertAuthenticated() + ->visit('/my-account') + ->assertInputValue('email', 'e.mel@edlib.test'); + }); + } + public function testUserCanChangeLanguage(): void { User::factory()->create([ @@ -169,6 +185,24 @@ public function testUserCanChangeEmail(): void }); } + public function testEmailIsNormalizedUponChanging(): void + { + User::factory()->withEmail('e.mel@edlib.test')->create(); + + $this->browse( + fn(Browser $browser) => $browser + ->loginAs('e.mel@edlib.test') + ->assertAuthenticated() + ->visit('/my-account') + ->type('email', 'E.MEL@EDLIB.TEST') + ->press('Save') + // The login should be invalid if the email didn't normalize. + // In that case, we wouldn't be able to see these. + ->assertSee('Account updated successfully') + ->assertInputValue('email', 'e.mel@edlib.test'), + ); + } + public function testUserCanDisconnectFacebookAndGoogleIDWithPassword(): void { User::factory()->create([ diff --git a/sourcecode/hub/tests/Feature/Api/UserTest.php b/sourcecode/hub/tests/Feature/Api/UserTest.php index 6c460e627d..d716d14556 100644 --- a/sourcecode/hub/tests/Feature/Api/UserTest.php +++ b/sourcecode/hub/tests/Feature/Api/UserTest.php @@ -134,4 +134,26 @@ public function testCreatedAtIsOverrideable(): void ), ); } + + public function testEmailIsNormalizedBeforeSaving(): void + { + $user = User::factory()->admin()->create(); + + $this + ->withBasicAuth($user->getApiKey(), $user->getApiSecret()) + ->postJson('/api/users', [ + 'name' => 'E. Mel', + 'email' => 'E.MEL@EDLIB.TEST', + ]) + ->assertCreated() + ->assertJson( + fn(AssertableJson $json) => $json + ->has( + 'data', + fn(AssertableJson $json) => $json + ->where('email', 'e.mel@edlib.test') + ->etc(), + ), + ); + } }