Make the User Name unique

app/Extensions/RemoteUserProvider.php

@@ -29,7 +29,7 @@ class RemoteUserProvider implements UserProvider
     public function retrieveById($identifier)
     {
         // We don't know the id length so we trim it to prevent to long strings in DB
-        $name  = substr($identifier['id'], 0, 180);
+        $name  = substr($identifier['id'], 0, 191);
         $email = null;
 
         $user = User::where('name', $name)->first();
@@ -44,16 +44,16 @@ class RemoteUserProvider implements UserProvider
                 ]);
 
                 if (User::where('id', '<>', $user->id ?? 0)->where('email', $identifier['email'])->count() == 0) {
-                    $email = $identifier['email'];
+                    $email = strtolower($identifier['email']);
                 }
             } catch (ValidationException $e) {
                 // do nothing
             }
         }
 
-        $email = $email ?? $this->remoteEmail((string) $identifier['id']);
-
         if (is_null($user)) {
+            $email = $email ?? $this->fakeRemoteEmail($identifier['id']);
+
             $user = User::create([
                 'name'     => $name,
                 'email'    => strtolower($email),
@@ -68,7 +68,7 @@ class RemoteUserProvider implements UserProvider
             }
         } else {
             // Here we keep the account's email sync-ed
-            if ($user->email != $email) {
+            if ($email && $user->email != $email) {
                 $user->email = $email;
                 $user->save();
             }
@@ -80,10 +80,10 @@ class RemoteUserProvider implements UserProvider
     /**
      * Set a fake email address
      *
-     * @param    $id string
+     * @param    $id mixed
      * @return string
      */
-    protected function remoteEmail(string $id)
+    protected function fakeRemoteEmail(mixed $id)
     {
         return substr($id, 0, 184) . '@remote';
     }

app/Http/Requests/UserStoreRequest.php

@@ -24,7 +24,7 @@ class UserStoreRequest extends FormRequest
     public function rules()
     {
         return [
-            'name'     => 'required|string|max:255',
+            'name'     => 'unique:App\Models\User,name|required|string|max:255',
             'email'    => 'unique:App\Models\User,email|required|string|email|max:255',
             'password' => 'required|string|min:8|confirmed',
         ];

app/Http/Requests/UserUpdateRequest.php

@@ -25,7 +25,7 @@ class UserUpdateRequest extends FormRequest
     public function rules()
     {
         return [
-            'name'     => 'required|string|max:255',
+            'name'     => 'unique:App\Models\User,name|required|string|max:255',
             'email'    => 'unique:App\Models\User,email|required|string|email|max:255',
             'password' => 'required',
         ];

database/migrations/2023_03_13_114928_set_username_unique.php

@@ -0,0 +1,32 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+return new class extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::table('users', function (Blueprint $table) {
+            $table->unique('name');
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::table('users', function (Blueprint $table) {
+            $table->dropUnique('users_name_unique');
+        });
+    }
+};

tests/Feature/Extensions/RemoteUserProviderTest.php

@@ -9,6 +9,7 @@ use Tests\FeatureTestCase;
 
 /**
  * @covers \App\Extensions\RemoteUserProvider
+ * @covers \App\Services\Auth\ReverseProxyGuard
  */
 class RemoteUserProviderTest extends FeatureTestCase
 {
@@ -36,47 +37,57 @@ class RemoteUserProviderTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_user_is_set_from_reverse_proxy_info_with_provided_email()
+    public function test_user_is_set_from_proxy_headers()
     {
         Config::set('auth.auth_proxy_headers.user', 'HTTP_REMOTE_USER');
-        Config::set('auth.auth_proxy_headers.email', 'HTTP_REMOTE_EMAIL');
 
         $this->app['auth']->shouldUse('reverse-proxy-guard');
 
         $this->json('GET', '/api/v1/groups', [], [
-            'HTTP_REMOTE_USER'  => self::USER_NAME,
-            'HTTP_REMOTE_EMAIL' => self::USER_EMAIL,
+            'HTTP_REMOTE_USER' => self::USER_NAME,
         ]);
         $this->assertAuthenticated('reverse-proxy-guard');
 
         $user = $this->app->make('auth')->guard('reverse-proxy-guard')->user();
+
         $this->assertEquals(self::USER_NAME, $user->name);
-        $this->assertEquals(self::USER_EMAIL, $user->email);
+        $this->assertEquals(strtolower(self::USER_NAME) . '@remote', $user->email);
+        $this->assertDatabaseHas('users', [
+            'name'  => self::USER_NAME,
+            'email' => strtolower(self::USER_NAME) . '@remote',
+        ]);
     }
 
     /**
      * @test
      */
-    public function test_user_is_set_from_reverse_proxy_without_email()
+    public function test_user_is_set_from_proxy_headers_with_an_email()
     {
         Config::set('auth.auth_proxy_headers.user', 'HTTP_REMOTE_USER');
+        Config::set('auth.auth_proxy_headers.email', 'HTTP_REMOTE_EMAIL');
 
         $this->app['auth']->shouldUse('reverse-proxy-guard');
 
         $this->json('GET', '/api/v1/groups', [], [
-            'HTTP_REMOTE_USER' => self::USER_NAME,
+            'HTTP_REMOTE_USER'  => self::USER_NAME,
+            'HTTP_REMOTE_EMAIL' => self::USER_EMAIL,
         ]);
         $this->assertAuthenticated('reverse-proxy-guard');
 
         $user = $this->app->make('auth')->guard('reverse-proxy-guard')->user();
+
         $this->assertEquals(self::USER_NAME, $user->name);
-        $this->assertEquals(strtolower(self::USER_NAME) . '@remote', $user->email);
+        $this->assertEquals(self::USER_EMAIL, $user->email);
+        $this->assertDatabaseHas('users', [
+            'name'  => self::USER_NAME,
+            'email' => self::USER_EMAIL,
+        ]);
     }
 
     /**
      * @test
      */
-    public function test_user_is_set_from_reverse_proxy_even_if_identifier_is_long()
+    public function test_user_is_set_from_proxy_headers_even_if_name_is_long()
     {
         Config::set('auth.auth_proxy_headers.user', 'HTTP_REMOTE_USER');
         Config::set('auth.auth_proxy_headers.email', 'HTTP_REMOTE_EMAIL');
@@ -84,6 +95,8 @@ class RemoteUserProviderTest extends FeatureTestCase
         $this->app['auth']->shouldUse('reverse-proxy-guard');
 
         $name = str_pad('john', 300, '_');
+        $inDbName = substr($name, 0, 191);
+        $inDbEmail = substr($name, 0, 184) . '@remote';
 
         $this->json('GET', '/api/v1/groups', [], [
             'HTTP_REMOTE_USER' => $name,
@@ -91,14 +104,40 @@ class RemoteUserProviderTest extends FeatureTestCase
         $this->assertAuthenticated('reverse-proxy-guard');
 
         $user = $this->app->make('auth')->guard('reverse-proxy-guard')->user();
-        $this->assertLessThan(192, strlen($user->name));
-        $this->assertLessThan(192, strlen($user->email));
+
+        $this->assertEquals($inDbName, $user->name);
+        $this->assertEquals($inDbEmail, $user->email);
+        $this->assertDatabaseHas('users', [
+            'name'  => $inDbName,
+            'email' => $inDbEmail,
+        ]);
     }
 
     /**
      * @test
      */
-    public function test_user_email_is_sync_with_email_proxy_header()
+    public function test_user_is_not_set_from_proxy_headers_when_name_is_missing()
+    {
+        Config::set('auth.auth_proxy_headers.user', 'HTTP_REMOTE_USER');
+        Config::set('auth.auth_proxy_headers.email', 'HTTP_REMOTE_EMAIL');
+
+        $this->app['auth']->shouldUse('reverse-proxy-guard');
+
+        $this->json('GET', '/api/v1/groups', [], [
+            'HTTP_REMOTE_USER' => '',
+        ]);
+        $this->assertGuest('reverse-proxy-guard');
+
+        $user = $this->app->make('auth')->guard('reverse-proxy-guard')->user();
+
+        $this->assertNull($user);
+        $this->assertDatabaseCount('users', 0);
+    }
+
+    /**
+     * @test
+     */
+    public function test_user_email_is_synced_with_email_from_proxy_headers()
     {
         Config::set('auth.auth_proxy_headers.user', 'HTTP_REMOTE_USER');
         Config::set('auth.auth_proxy_headers.email', 'HTTP_REMOTE_EMAIL');
@@ -116,7 +155,6 @@ class RemoteUserProviderTest extends FeatureTestCase
         ]);
 
         $this->assertAuthenticated('reverse-proxy-guard');
-
         $this->assertDatabaseHas('users', [
             'id'    => $dbUser->id,
             'email' => self::USER_EMAIL,
@@ -126,7 +164,7 @@ class RemoteUserProviderTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_user_email_is_not_sync_with_invalid_email_proxy_header()
+    public function test_user_email_is_not_synced_when_email_from_proxy_headers_is_missing()
     {
         Config::set('auth.auth_proxy_headers.user', 'HTTP_REMOTE_USER');
         Config::set('auth.auth_proxy_headers.email', 'HTTP_REMOTE_EMAIL');
@@ -140,11 +178,37 @@ class RemoteUserProviderTest extends FeatureTestCase
 
         $this->json('GET', '/api/v1/groups', [], [
             'HTTP_REMOTE_USER'  => self::USER_NAME,
-            'HTTP_REMOTE_EMAIL' => 'bad[at]email',
+            'HTTP_REMOTE_EMAIL' => '',
         ]);
 
         $this->assertAuthenticated('reverse-proxy-guard');
+        $this->assertDatabaseHas('users', [
+            'id'    => $dbUser->id,
+            'email' => strtolower(self::USER_NAME) . '@remote',
+        ]);
+    }
 
+    /**
+     * @test
+     */
+    public function test_user_email_is_not_synced_when_email_from_proxy_headers_is_invalid()
+    {
+        Config::set('auth.auth_proxy_headers.user', 'HTTP_REMOTE_USER');
+        Config::set('auth.auth_proxy_headers.email', 'HTTP_REMOTE_EMAIL');
+
+        $dbUser = User::factory()->create([
+            'name'  => self::USER_NAME,
+            'email' => strtolower(self::USER_NAME) . '@remote',
+        ]);
+
+        $this->app['auth']->shouldUse('reverse-proxy-guard');
+
+        $this->json('GET', '/api/v1/groups', [], [
+            'HTTP_REMOTE_USER'  => self::USER_NAME,
+            'HTTP_REMOTE_EMAIL' => 'bad[at]email',
+        ]);
+
+        $this->assertAuthenticated('reverse-proxy-guard');
         $this->assertDatabaseHas('users', [
             'id'    => $dbUser->id,
             'email' => $dbUser->email,
@@ -154,14 +218,14 @@ class RemoteUserProviderTest extends FeatureTestCase
     /**
      * @test
      */
-    public function test_user_email_is_not_sync_with_already_used_email_proxy_header()
+    public function test_user_email_is_not_sync_when_email_from_proxy_headers_is_already_in_use()
     {
         Config::set('auth.auth_proxy_headers.user', 'HTTP_REMOTE_USER');
         Config::set('auth.auth_proxy_headers.email', 'HTTP_REMOTE_EMAIL');
 
         $dbUser = User::factory()->create([
             'name'  => self::USER_NAME,
-            'email' => strtolower(self::USER_NAME) . '@remote',
+            'email' => self::USER_EMAIL,
         ]);
 
         $otherUser = User::factory()->create([
@@ -176,7 +240,6 @@ class RemoteUserProviderTest extends FeatureTestCase
         ]);
 
         $this->assertAuthenticated('reverse-proxy-guard');
-
         $this->assertDatabaseHas('users', [
             'id'    => $dbUser->id,
             'email' => $dbUser->email,