Merge pull request #268 from kenjis/fix-username-validation-rules

fix: username validation rules

Author: kenjis

docs/authentication.md

@@ -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());

docs/customization.md

@@ -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]',
 ];

src/Config/AuthSession.php

@@ -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',
+    ];
+}

src/Config/Registrar.php

@@ -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}]',
-            ],
         ];
     }
 

src/Controllers/LoginController.php

@@ -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',
         ];
     }

src/Controllers/MagicLinkController.php

@@ -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,
         ];
     }
 }

src/Controllers/RegisterController.php

@@ -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]',
         ];

src/Models/UserModel.php

@@ -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,
         ]);
     }

tests/Controllers/LoginTest.php

@@ -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([
@@ -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
@@ -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());
     }
 }

tests/Controllers/RegisterTest.php

@@ -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());
 
@@ -96,6 +98,7 @@ public function testRegisterRedirectsIfNotAllowed(): void
 
         $result->assertStatus(302);
         $result->assertRedirect();
+        $result->assertSessionHas('error');
     }
 
     public function testRegisterActionRedirectsIfNotAllowed(): void
@@ -108,6 +111,7 @@ public function testRegisterActionRedirectsIfNotAllowed(): void
 
         $result->assertStatus(302);
         $result->assertRedirect();
+        $result->assertSessionHas('error');
     }
 
     public function testRegisterActionInvalidData(): void