Make the RemoteUserProvider use a db persisted user & Update tests

app/Api/v1/Controllers/UserController.php

@@ -5,7 +5,6 @@ namespace App\Api\v1\Controllers;
 use App\Api\v1\Requests\SettingUpdateRequest;
 use App\Api\v1\Resources\UserResource;
 use App\Http\Controllers\Controller;
-use App\Models\User;
 use Illuminate\Http\Request;
 use Illuminate\Support\Arr;
 

app/Extensions/RemoteUserProvider.php

@@ -9,20 +9,13 @@ use App\Models\User;
 use Exception;
 use Illuminate\Contracts\Auth\Authenticatable;
 use Illuminate\Contracts\Auth\UserProvider;
-use Illuminate\Support\Arr;
+use Illuminate\Support\Facades\Log;
+use Illuminate\Support\Facades\Validator;
+use Illuminate\Support\Str;
+use Illuminate\Validation\ValidationException;
 
 class RemoteUserProvider implements UserProvider
 {
-    // 2FAuth is single user by design and domain data are not coupled to the user model.
-    // So the RemoteUserProvider provides a non-persisted user, dynamically instanciated using data
-    // from the auth proxy.
-    //
-    // This way no matter the user data set at proxy level, 2FAuth will always
-    // authenticate a request from the proxy and will return domain data without restriction.
-    //
-    // The downside of this approach is that we have to be sure that no change that needs
-    // to be persisted will be made to the user instance afterward (i.e through middlewares).
-
     /**
      * The currently authenticated user.
      *
@@ -30,39 +23,71 @@ class RemoteUserProvider implements UserProvider
      */
     protected $user;
 
-    /**
-     * Get the In-memory user
-     *
-     * @return \App\Models\User
-     */
-    protected function getInMemoryUser()
-    {
-        if (is_null($this->user)) {
-            $this->user        = new User;
-            $this->user->name  = 'Remote User';
-            $this->user->email = 'fake.email@do.not.use';
-        }
-
-        return $this->user;
-    }
-
     /**
      * {@inheritDoc}
      */
     public function retrieveById($identifier)
     {
-        $user = $this->getInMemoryUser();
+        // We don't know the id length so we trim it to prevent to long strings in DB
+        $name  = substr($identifier['id'], 0, 180);
+        $email = null;
 
-        if (Arr::has($identifier, 'user')) {
+        $user = User::where('name', $name)->first();
-            $user->name = $identifier['user'];
+
+        // We use the passed email only if it is valid and no account is using it.
+        if ($identifier['email']) {
+            try {
+                $validated = Validator::validate([
+                    'email' => $identifier['email'],
+                ], [
+                    'email' => 'email',
+                ]);
+
+                if (User::where('id', '<>', $user->id ?? 0)->where('email', $identifier['email'])->count() == 0) {
+                    $email = $identifier['email'];
+                }
+            } catch (ValidationException $e) {
+                // do nothing
+            }
         }
-        if (Arr::has($identifier, 'email')) {
+
-            $user->email = $identifier['email'];
+        $email = $email ?? $this->remoteEmail((string) $identifier['id']);
+
+        if (is_null($user)) {
+            $user = User::create([
+                'name'     => $name,
+                'email'    => strtolower($email),
+                'password' => bcrypt(Str::random(64)),
+            ]);
+
+            Log::info(sprintf('Remote user %s created with email address %s', var_export($user->name, true), var_export($user->email, true)));
+
+            if (User::count() === 1) {
+                $user->is_admin = true;
+                $user->save();
+            }
+        } else {
+            // Here we keep the account's email sync-ed
+            if ($user->email != $email) {
+                $user->email = $email;
+                $user->save();
+            }
         }
 
         return $user;
     }
 
+    /**
+     * Set a fake email address
+     *
+     * @param    $id string
+     * @return string
+     */
+    protected function remoteEmail(string $id)
+    {
+        return substr($id, 0, 184) . '@remote';
+    }
+
     /**
      * {@inheritDoc}
      *
@@ -70,7 +95,7 @@ class RemoteUserProvider implements UserProvider
      */
     public function retrieveByToken($identifier, $token)
     {
-        return $this->retrieveById($identifier);
+        throw new Exception(sprintf('No implementation for %s', __METHOD__));
     }
 
     /**
@@ -90,7 +115,7 @@ class RemoteUserProvider implements UserProvider
      */
     public function retrieveByCredentials(array $credentials)
     {
-        return $this->getInMemoryUser();
+        throw new Exception(sprintf('No implementation for %s', __METHOD__));
     }
 
     /**
@@ -100,6 +125,6 @@ class RemoteUserProvider implements UserProvider
      */
     public function validateCredentials(Authenticatable $user, array $credentials)
     {
-        return true;
+        throw new Exception(sprintf('No implementation for %s', __METHOD__));
     }
 }

app/Services/Auth/ReverseProxyGuard.php

@@ -50,19 +50,20 @@ class ReverseProxyGuard implements Guard
         $identifier       = [];
 
         try {
-            $identifier['user'] = request()->server($remoteUserHeader) ?? apache_request_headers()[$remoteUserHeader] ?? null;
+            $identifier['id'] = request()->server($remoteUserHeader) ?? apache_request_headers()[$remoteUserHeader] ?? null;
         } catch (\Throwable $e) {
-            $identifier['user'] = null;
+            $identifier['id'] = null;
         }
 
-        if (! $identifier['user'] || is_array($identifier['user'])) {
+        if (! $identifier['id'] || is_array($identifier['id'])) {
-            Log::error(sprintf('Proxy remote-user header "%s" is empty or missing.', $remoteUserHeader));
+            Log::error(sprintf('Proxy remote-user header %s is empty or missing.', var_export($remoteUserHeader, true)));
 
             return $this->user = null;
         }
 
         // Get the email identifier from $_SERVER
-        $remoteEmailHeader = config('auth.auth_proxy_headers.email');
+        $remoteEmailHeader   = config('auth.auth_proxy_headers.email');
+        $identifier['email'] = null;
 
         if ($remoteEmailHeader) {
             try {

resources/js/views/settings/Account.vue

@@ -68,7 +68,7 @@
                 formDelete: new Form({
                     password : '',
                 }),
-                isRemoteUser: false,
+                isRemoteUser: this.$root.appConfig.proxyAuth,
                 isAdmin: false,
             }
         },
@@ -77,7 +77,6 @@
             const { data } = await this.formProfile.get('/api/v1/user')
 
             if( data.is_admin === true ) this.isAdmin = true
-            if( data.id === null ) this.isRemoteUser = true
 
             this.formProfile.fill(data)
         },

tests/Feature/Extensions/RemoteUserProviderTest.php

@@ -0,0 +1,185 @@
+<?php
+
+namespace Tests\Unit\Extensions;
+
+use App\Extensions\RemoteUserProvider;
+use App\Models\User;
+use Illuminate\Support\Facades\Config;
+use Tests\FeatureTestCase;
+
+/**
+ * @covers \App\Extensions\RemoteUserProvider
+ */
+class RemoteUserProviderTest extends FeatureTestCase
+{
+    private const USER_NAME = 'John';
+
+    private const USER_EMAIL = 'john@example.com';
+
+    public function test_user_is_retreived_from_db()
+    {
+        $dbUser = User::factory()->create([
+            'name'  => self::USER_NAME,
+            'email' => self::USER_EMAIL,
+        ]);
+
+        $provider = new RemoteUserProvider;
+
+        $user = $provider->retrieveById([
+            'id'    => self::USER_NAME,
+            'email' => null,
+        ]);
+
+        $this->assertEquals($dbUser->id, $user->id);
+    }
+
+    /**
+     * @test
+     */
+    public function test_user_is_set_from_reverse_proxy_info_with_provided_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_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(self::USER_EMAIL, $user->email);
+    }
+
+    /**
+     * @test
+     */
+    public function test_user_is_set_from_reverse_proxy_without_email()
+    {
+        Config::set('auth.auth_proxy_headers.user', 'HTTP_REMOTE_USER');
+
+        $this->app['auth']->shouldUse('reverse-proxy-guard');
+
+        $this->json('GET', '/api/v1/groups', [], [
+            '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(strtolower(self::USER_NAME) . '@remote', $user->email);
+    }
+
+    /**
+     * @test
+     */
+    public function test_user_is_set_from_reverse_proxy_even_if_identifier_is_long()
+    {
+        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');
+
+        $name = str_pad('john', 300, '_');
+
+        $this->json('GET', '/api/v1/groups', [], [
+            'HTTP_REMOTE_USER' => $name,
+        ]);
+        $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));
+    }
+
+    /**
+     * @test
+     */
+    public function test_user_email_is_sync_with_email_proxy_header()
+    {
+        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' => self::USER_EMAIL,
+        ]);
+
+        $this->assertAuthenticated('reverse-proxy-guard');
+
+        $this->assertDatabaseHas('users', [
+            'id'    => $dbUser->id,
+            'email' => self::USER_EMAIL,
+        ]);
+    }
+
+    /**
+     * @test
+     */
+    public function test_user_email_is_not_sync_with_invalid_email_proxy_header()
+    {
+        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,
+        ]);
+    }
+
+    /**
+     * @test
+     */
+    public function test_user_email_is_not_sync_with_already_used_email_proxy_header()
+    {
+        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',
+        ]);
+
+        $otherUser = User::factory()->create([
+            'email' => 'other@example.com',
+        ]);
+
+        $this->app['auth']->shouldUse('reverse-proxy-guard');
+
+        $this->json('GET', '/api/v1/groups', [], [
+            'HTTP_REMOTE_USER'  => self::USER_NAME,
+            'HTTP_REMOTE_EMAIL' => $otherUser->email,
+        ]);
+
+        $this->assertAuthenticated('reverse-proxy-guard');
+
+        $this->assertDatabaseHas('users', [
+            'id'    => $dbUser->id,
+            'email' => $dbUser->email,
+        ]);
+    }
+}

tests/Feature/Http/Middlewares/AuthenticateMiddlewareTest.php

@@ -24,45 +24,6 @@ class AuthenticateMiddlewareTest extends FeatureTestCase
         $this->assertAuthenticated('reverse-proxy-guard');
     }
 
-    /**
-     * @test
-     */
-    public function test_user_is_set_from_reverse_proxy_info()
-    {
-        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,
-        ]);
-        $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);
-    }
-
-    /**
-     * @test
-     */
-    public function test_user_is_set_from_reverse_proxy_without_email()
-    {
-        Config::set('auth.auth_proxy_headers.user', 'HTTP_REMOTE_USER');
-
-        $this->app['auth']->shouldUse('reverse-proxy-guard');
-
-        $this->json('GET', '/api/v1/groups', [], [
-            'HTTP_REMOTE_USER' => self::USER_NAME,
-        ]);
-        $this->assertAuthenticated('reverse-proxy-guard');
-
-        $user = $this->app->make('auth')->guard('reverse-proxy-guard')->user();
-        $this->assertEquals('fake.email@do.not.use', $user->email);
-    }
-
     /**
      * @test
      */

tests/Unit/Extensions/RemoteUserProviderTest.php

@@ -1,25 +0,0 @@
-<?php
-
-namespace Tests\Unit\Extensions;
-
-use App\Extensions\RemoteUserProvider;
-use Tests\TestCase;
-
-/**
- * @covers \App\Extensions\RemoteUserProvider
- */
-class RemoteUserProviderTest extends TestCase
-{
-    public function test_retreiving_a_user_returns_a_non_persisted_user_instance()
-    {
-        $provider = new RemoteUserProvider;
-
-        $user = $provider->retrieveById([
-            'user'  => 'testUser',
-            'email' => 'test@example.org',
-        ]);
-
-        $this->assertInstanceOf('\App\Models\User', $user);
-        $this->assertEquals(true, $user->exists);
-    }
-}