Skip to content

Commit

Permalink
Merge pull request #268 from kenjis/fix-username-validation-rules
Browse files Browse the repository at this point in the history
fix: username validation rules
  • Loading branch information
kenjis authored Jul 15, 2022
2 parents 52bacb2 + f116e10 commit 794275f
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 18 deletions.
2 changes: 1 addition & 1 deletion docs/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ $credentials = [
'password' => $this->request->getPost('password')
];

$validCreds? = auth()->check($credentials);
$validCreds = auth()->check($credentials);

if (! $validCreds->isOK()) {
return redirect()->back()->with('error', $loginAttempt->reason());
Expand Down
22 changes: 17 additions & 5 deletions docs/customization.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,22 +95,34 @@ Shield has the following rules for registration:

```php
[
'username' => 'required|alpha_numeric_space|min_length[3]|is_unique[users.username]',
'email' => 'required|valid_email|is_unique[auth_identities.secret]',
'username' => [
'required',
'max_length[30]',
'min_length[3]',
'regex_match[/\A[a-zA-Z0-9\.]+\z/]',
'is_unique[users.username]',
],
'email' => 'required|max_length[254]|valid_email|is_unique[auth_identities.secret]',
'password' => 'required|strong_password',
'password_confirm' => 'required|matches[password]',
];
```

If you need a different set of rules for registration, you can specify them in your `Validation` configuration (**app\Config\Validation.php**) like:
If you need a different set of rules for registration, you can specify them in your `Validation` configuration (**app/Config/Validation.php**) like:

```php
//--------------------------------------------------------------------
// Rules
//--------------------------------------------------------------------
public $registration = [
'username' => 'required|alpha_numeric_space|min_length[3]|is_unique[users.username]',
'email' => 'required|valid_email|is_unique[auth_identities.secret]',
'username' => [
'required',
'max_length[30]',
'min_length[3]',
'regex_match[/\A[a-zA-Z0-9\.]+\z/]',
'is_unique[users.username]',
],
'email' => 'required|max_length[254]|valid_email|is_unique[auth_identities.secret]',
'password' => 'required|strong_password',
'password_confirm' => 'required|matches[password]',
];
Expand Down
34 changes: 34 additions & 0 deletions src/Config/AuthSession.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace CodeIgniter\Shield\Config;

use CodeIgniter\Config\BaseConfig;

/**
* Config for Session Authenticator
*/
class AuthSession extends BaseConfig
{
/**
* The validation rules for username
*
* @var string[]
*/
public array $usernameValidationRules = [
'required',
'max_length[30]',
'min_length[3]',
'regex_match[/\A[a-zA-Z0-9\.]+\z/]',
];

/**
* The validation rules for email
*
* @var string[]
*/
public array $emailValidationRules = [
'required',
'max_length[254]',
'valid_email',
];
}
4 changes: 0 additions & 4 deletions src/Config/Registrar.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ public static function Validation(): array
'ruleSets' => [
PasswordRules::class,
],
'users' => [
'email' => 'required|valid_email|unique_email[{id}]',
'username' => 'required|string|is_unique[users.username,id,{id}]',
],
];
}

Expand Down
4 changes: 2 additions & 2 deletions src/Controllers/LoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ public function loginAction(): RedirectResponse
protected function getValidationRules(): array
{
return setting('Validation.login') ?? [
//'username' => 'required|max_length[30]|alpha_numeric_space|min_length[3]',
'email' => 'required|max_length[254]|valid_email',
//'username' => config('AuthSession')->usernameValidationRules,
'email' => config('AuthSession')->emailValidationRules,
'password' => 'required',
];
}
Expand Down
4 changes: 2 additions & 2 deletions src/Controllers/MagicLinkController.php
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,12 @@ private function recordLoginAttempt(
/**
* Returns the rules that should be used for validation.
*
* @return array<string, string>
* @return array<string, array<string, string>>
*/
protected function getValidationRules(): array
{
return [
'email' => 'required|max_length[254]|valid_email',
'email' => config('AuthSession')->emailValidationRules,
];
}
}
13 changes: 11 additions & 2 deletions src/Controllers/RegisterController.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,18 @@ protected function getUserEntity(): User
*/
protected function getValidationRules(): array
{
$registrationUsernameRules = array_merge(
config('AuthSession')->usernameValidationRules,
['is_unique[users.username]']
);
$registrationEmailRules = array_merge(
config('AuthSession')->emailValidationRules,
['is_unique[auth_identities.secret]']
);

return setting('Validation.registration') ?? [
'username' => 'required|alpha_numeric_space|min_length[3]|is_unique[users.username]',
'email' => 'required|valid_email|is_unique[auth_identities.secret]',
'username' => $registrationUsernameRules,
'email' => $registrationEmailRules,
'password' => 'required|strong_password',
'password_confirm' => 'required|matches[password]',
];
Expand Down
2 changes: 1 addition & 1 deletion src/Models/UserModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public function addToDefaultGroup(User $user): void
public function fake(Generator &$faker): User
{
return new User([
'username' => str_replace('.', ' ', $faker->userName),
'username' => $faker->userName,
'active' => true,
]);
}
Expand Down
6 changes: 5 additions & 1 deletion tests/Controllers/LoginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,10 @@ public function testLoginActionUsernameSuccess(): void
// Change the validation rules
$config = new class () extends Validation {
public $login = [
'username' => 'required|max_length[30]|alpha_numeric_space|min_length[3]',
'password' => 'required',
];
};
$config->login['username'] = config('AuthSession')->usernameValidationRules;
Factories::injectMock('config', 'Validation', $config);

$this->user->createEmailIdentity([
Expand All @@ -139,6 +139,8 @@ public function testLoginActionUsernameSuccess(): void
$result->assertSessionHas('user', ['id' => 1]);
$result->assertStatus(302);
$result->assertRedirect();
$result->assertSessionMissing('error');
$result->assertSessionMissing('errors');
$this->assertSame(site_url(), $result->getRedirectUrl());

// Login should have been recorded successfully
Expand Down Expand Up @@ -191,6 +193,8 @@ public function testLoginRedirectsToActionIfDefined(): void
// Should have been redirected to the action's page.
$result->assertStatus(302);
$result->assertRedirect();
$result->assertSessionMissing('error');
$result->assertSessionMissing('errors');
$this->assertSame(site_url('auth/a/show'), $result->getRedirectUrl());
}
}
4 changes: 4 additions & 0 deletions tests/Controllers/RegisterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ public function testRegisterActionSuccess(): void

$result->assertStatus(302);
$result->assertRedirect();
$result->assertSessionMissing('error');
$result->assertSessionMissing('errors');

$this->assertSame(site_url(), $result->getRedirectUrl());

Expand Down Expand Up @@ -96,6 +98,7 @@ public function testRegisterRedirectsIfNotAllowed(): void

$result->assertStatus(302);
$result->assertRedirect();
$result->assertSessionHas('error');
}

public function testRegisterActionRedirectsIfNotAllowed(): void
Expand All @@ -108,6 +111,7 @@ public function testRegisterActionRedirectsIfNotAllowed(): void

$result->assertStatus(302);
$result->assertRedirect();
$result->assertSessionHas('error');
}

public function testRegisterActionInvalidData(): void
Expand Down

0 comments on commit 794275f

Please sign in to comment.