Skip to content
This repository has been archived by the owner on Oct 1, 2021. It is now read-only.

Commit

Permalink
refactor: case insensitive login & store lowercase email (#175)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexbarnsley authored Aug 18, 2021
1 parent 4ad7681 commit 2e32335
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 13 deletions.
6 changes: 3 additions & 3 deletions src/Actions/AuthenticateUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,17 @@ private function fetchUser(): ?Authenticatable

$query = Models::user()::query();

$query->where(Fortify::username(), $username);
$query->whereRaw(sprintf('LOWER(%s) = ?', Fortify::username()), [$username]);

if ($usernameAlt = Config::get('fortify.username_alt')) {
$query->orWhere($usernameAlt, $username);
$query->orWhereRaw(sprintf('LOWER(%s) = ?', $usernameAlt), [$username]);
}

return $query->first();
}

private function getUsername(): ?string
{
return $this->request->get($this->username);
return strtolower($this->request->get($this->username));
}
}
4 changes: 2 additions & 2 deletions src/Actions/CreateNewUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ public function getUserData(array $input): array
{
$userData = [
'name' => $input['name'],
Fortify::username() => $input[Fortify::username()],
Fortify::username() => strtolower($input[Fortify::username()]),
'password' => Hash::make($input['password']),
];

if ($usernameAlt = Config::get('fortify.username_alt')) {
$userData[$usernameAlt] = $input[$usernameAlt];
$userData[$usernameAlt] = strtolower($input[$usernameAlt]);
}

return $userData;
Expand Down
8 changes: 5 additions & 3 deletions src/Actions/UpdateUserProfileInformation.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,14 @@ public function update($user, array $input)
'email' => ['required', 'email', 'max:255', Rule::unique('users')->ignore($user->id)],
])->validateWithBag('updateProfileInformation');

if ($input['email'] !== $user->email && $user instanceof MustVerifyEmail) {
$newEmail = strtolower($input['email']);
$oldEmail = strtolower($user->email);
if ($newEmail !== $oldEmail && $user instanceof MustVerifyEmail) {
$this->updateVerifiedUser($user, $input);
} else {
$user->forceFill([
'name' => $input['name'],
'email' => $input['email'],
'email' => $newEmail,
])->save();
}
}
Expand All @@ -59,7 +61,7 @@ protected function updateVerifiedUser($user, array $input)
{
$user->forceFill([
'name' => $input['name'],
'email' => $input['email'],
'email' => strtolower($input['email']),
'email_verified_at' => null,
])->save();

Expand Down
41 changes: 41 additions & 0 deletions tests/Actions/AuthenticateUserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,26 @@
$this->assertFalse($request->filled('remember'));
});

it('login the user with case insensitive email', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);

$user = User::factory()->create();

$request = new Request();

$request->replace([
'email' => strtoupper($user->email),
'password' => 'password',
]);

$authenticator = new AuthenticateUser($request);
$loggedUser = $authenticator->handle();

$this->assertNotNull($loggedUser);
$this->assertTrue($user->is($loggedUser));
$this->assertFalse($request->filled('remember'));
});

it('login the user by the email when alt username is set', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);
Config::set('fortify.username_alt', 'username');
Expand Down Expand Up @@ -69,6 +89,27 @@
$this->assertFalse($request->filled('remember'));
});

it('login the user with case insensitive alt username', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);
Config::set('fortify.username_alt', 'username');

$user = User::factory()->withUsername()->create();

$request = new Request();

$request->replace([
'email' => strtoupper($user->username),
'password' => 'password',
]);

$authenticator = new AuthenticateUser($request);
$loggedUser = $authenticator->handle();

$this->assertNotNull($loggedUser);
$this->assertTrue($user->is($loggedUser));
$this->assertFalse($request->filled('remember'));
});

it('doesnt login the user by the alt username if not set (username)', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);
Config::set('fortify.username_alt', null);
Expand Down
40 changes: 35 additions & 5 deletions tests/Actions/CreateNewUserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,36 @@
$this->assertTrue(Hash::check($this->validPassword, $user->password));
});

it('should create user and force lowercase email', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);

$user = (new CreateNewUser())->create([
'name' => 'John Doe',
'username' => 'alfonsobries',
'email' => '[email protected]',
'password' => $this->validPassword,
'password_confirmation' => $this->validPassword,
'terms' => true,
]);

$this->assertSame('[email protected]', $user->email);
$this->assertSame('John Doe', $user->name);
$this->assertTrue(Hash::check($this->validPassword, $user->password));
});

it('should not create user with uppercase characters', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);

expectValidationError(fn () => (new CreateNewUser())->create([
'name' => 'John Doe',
'username' => 'JOHNDOE',
'email' => '[email protected]',
'password' => 'sec$r2t12345',
'password_confirmation' => 'sec$r2t12345',
'terms' => true,
]), 'username', trans('fortify::validation.messages.username.lowercase_only'));
});

it('should create a valid user with username if the username_alt setting is set', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);
Config::set('fortify.username_alt', 'username');
Expand Down Expand Up @@ -107,7 +137,7 @@
]), 'terms', 'The terms must be accepted.');
});

it('should match the confirmation', function () {
it('password should match the confirmation', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);

expectValidationError(fn () => (new CreateNewUser())->create([
Expand All @@ -120,7 +150,7 @@
]), 'password', 'The password confirmation does not match.');
});

it('should be equal to or longer than 12 characters', function () {
it('password should be equal to or longer than 12 characters', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);

expectValidationError(fn () => (new CreateNewUser())->create([
Expand All @@ -133,7 +163,7 @@
]), 'password', 'The password must be at least 12 characters.');
});

it('should require an uppercase letter', function () {
it('password should require an uppercase letter', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);

expectValidationError(fn () => (new CreateNewUser())->create([
Expand All @@ -146,7 +176,7 @@
]), 'password', 'The password must contain at least one uppercase and one lowercase letter.');
});

it('should require one number', function () {
it('password should require one number', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);

expectValidationError(fn () => (new CreateNewUser())->create([
Expand All @@ -159,7 +189,7 @@
]), 'password', 'The password must contain at least one number.');
});

it('should require one special character', function () {
it('password should require one special character', function () {
Config::set('fortify.models.user', \ARKEcosystem\Fortify\Models\User::class);

expectValidationError(fn () => (new CreateNewUser())->create([
Expand Down
32 changes: 32 additions & 0 deletions tests/Actions/UpdateUserProfileInformationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,38 @@
expect($user->email_verified_at)->toBeNull();
});

it('should update with lowercase email', function () {
$user = createUserModel(UserWithoutVerification::class);

expect($user->name)->toBe('John Doe');
expect($user->email)->toBe('[email protected]');

resolve(UpdateUserProfileInformation::class)->update($user, [
'name' => 'Jane Doe',
'email' => '[email protected]',
]);

expect($user->name)->toBe('Jane Doe');
expect($user->email)->toBe('[email protected]');
});

it('should update with lowercase email for a user that requires verification', function () {
$user = createUserModel(UserWithNotifications::class);

expect($user->name)->toBe('John Doe');
expect($user->email)->toBe('[email protected]');
expect($user->email_verified_at)->not()->toBeNull();

resolve(UpdateUserProfileInformation::class)->update($user, [
'name' => 'Jane Doe',
'email' => '[email protected]',
]);

expect($user->name)->toBe('Jane Doe');
expect($user->email)->toBe('[email protected]');
expect($user->email_verified_at)->toBeNull();
});

it('should throw an exception if the name is missing', function () {
$user = createUserModel();

Expand Down

0 comments on commit 2e32335

Please sign in to comment.