Harden 2FA

app/Http/Controllers/Auth/LoginController.php

@@ -120,7 +120,7 @@ class LoginController extends Controller
 
             $passwordBreached = $this->isPasswordBreached((string) $request->input('password'));
 
-            if ($user->passwordSecurity && $user->passwordSecurity->google2fa_enable) {
+            if ($this->userRequiresTwoFactor($user)) {
                 if ($this->trustedDeviceCookieIsValid($request, $user)) {
                     session([config('google2fa.session_var') => true]);
                     session([config('google2fa.session_var').'.auth.passed_at' => time()]);
@@ -135,13 +135,7 @@ class LoginController extends Controller
                     );
                 }
 
-                Auth::logoutOtherDevices((string) $request->input('password'));
-                $request->session()->put('url.intended', $this->redirectPath());
-                $request->session()->put('2fa:remember', $rememberMe);
-                $request->session()->put('2fa:password_breached', $passwordBreached);
-
-                Auth::logout();
-                $request->session()->put('2fa:user:id', $user->id);
+                $this->prepareTwoFactorChallenge($request, $user, $rememberMe, $passwordBreached);
 
                 return redirect()->route('2fa.verify');
             }
@@ -262,6 +256,32 @@ class LoginController extends Controller
         return $redirect;
     }
 
+    private function userRequiresTwoFactor(User $user): bool
+    {
+        $user->unsetRelation('passwordSecurity');
+        $passwordSecurity = $user->passwordSecurity()->first();
+
+        return $passwordSecurity !== null
+            && $passwordSecurity->google2fa_enable === true
+            && is_string($passwordSecurity->google2fa_secret)
+            && $passwordSecurity->google2fa_secret !== '';
+    }
+
+    private function prepareTwoFactorChallenge(Request $request, User $user, bool $rememberMe, bool $passwordBreached): void
+    {
+        $sessionVar = config('google2fa.session_var');
+        if (is_string($sessionVar) && $sessionVar !== '') {
+            $request->session()->forget($sessionVar);
+        }
+
+        $request->session()->put('url.intended', $this->redirectPath());
+        $request->session()->put('2fa:remember', $rememberMe);
+        $request->session()->put('2fa:password_breached', $passwordBreached);
+
+        Auth::logout();
+        $request->session()->put('2fa:user:id', $user->id);
+    }
+
     private function trustedDeviceCookieIsValid(Request $request, User $user): bool
     {
         $trustedCookie = $request->cookie('2fa_trusted_device');

app/Models/PasswordSecurity.php

@@ -17,6 +17,16 @@ class PasswordSecurity extends Model
      */
     protected $guarded = [];
 
+    /**
+     * @return array<string, string>
+     */
+    protected function casts(): array
+    {
+        return [
+            'google2fa_enable' => 'boolean',
+        ];
+    }
+
     /**
      * @return BelongsTo<User, $this>
      */

app/Support/Google2FAAuthenticator.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace App\Support;
 
+use App\Models\TrustedDevice;
 use PragmaRX\Google2FALaravel\Exceptions\InvalidSecretKey;
 use PragmaRX\Google2FALaravel\Support\Authenticator;
 
@@ -32,32 +33,10 @@ class Google2FAAuthenticator extends Authenticator
     /**
      * Directly validate the cookie without any output or logging
      */
-    private function checkCookieValidity(mixed $cookie): mixed
+    private function checkCookieValidity(mixed $cookie): bool
     {
         try {
-            $data = @json_decode($cookie, true);
-
-            if (! is_array($data)) {
-                return false;
-            }
-
-            // Validate all required fields
-            if (! isset($data['user_id'], $data['token'], $data['expires_at'])) {
-                return false;
-            }
-
-            // Ensure the user ID matches
-            if ((int) $data['user_id'] !== (int) $this->getUser()->id) {
-                return false;
-            }
-
-            // Ensure the token is not expired
-            if (time() > $data['expires_at']) {
-                return false;
-            }
-
-            // All checks passed - this is a valid cookie
-            return true;
+            return $this->trustedDeviceCookieIsValid($cookie);
         } catch (\Exception $e) {
             return false;
         }
@@ -83,37 +62,7 @@ class Google2FAAuthenticator extends Authenticator
     protected function isDeviceTrusted(): bool
     {
         try {
-            $cookie = request()->cookie('2fa_trusted_device');
-            $user = $this->getUser();
-
-            if (! $cookie) {
-                return false;
-            }
-
-            $data = @json_decode($cookie, true);
-
-            // Check for JSON decode errors silently
-            if (json_last_error() !== JSON_ERROR_NONE) {
-                return false;
-            }
-
-            // Validate the required fields silently
-            if (! isset($data['user_id'], $data['token'], $data['expires_at'])) {
-                return false;
-            }
-
-            // Check if the token belongs to the current user
-            if ((int) $data['user_id'] !== (int) $user->id) {
-                return false;
-            }
-
-            // Check if the token is expired
-            if (time() > $data['expires_at']) {
-                return false;
-            }
-
-            // Device is trusted and token is valid
-            return true;
+            return $this->trustedDeviceCookieIsValid(request()->cookie('2fa_trusted_device'));
         } catch (\Exception $e) {
             // Silently handle any exceptions
             return false;
@@ -146,18 +95,10 @@ class Google2FAAuthenticator extends Authenticator
 
         if ($trustedCookie && auth()->check()) {
             try {
-                $cookieData = json_decode($trustedCookie, true);
-
-                if (json_last_error() === JSON_ERROR_NONE &&
-                    isset($cookieData['user_id'], $cookieData['token'], $cookieData['expires_at']) &&
-                    (int) $cookieData['user_id'] === (int) auth()->id() &&
-                    time() <= $cookieData['expires_at']) {
-
-                    // If we have a valid cookie, force-disable 2FA
+                if ($this->trustedDeviceCookieIsValid($trustedCookie)) {
                     session([config('google2fa.session_var') => true]);
                     session([config('google2fa.session_var').'.auth.passed_at' => time()]);
 
-                    // Completely disable 2FA for this request
                     return false;
                 }
             } catch (\Exception $e) {
@@ -168,4 +109,32 @@ class Google2FAAuthenticator extends Authenticator
         // Otherwise, use the parent implementation
         return parent::isEnabled();
     }
+
+    private function trustedDeviceCookieIsValid(mixed $cookie): bool
+    {
+        if (! is_string($cookie) || $cookie === '') {
+            return false;
+        }
+
+        $data = json_decode($cookie, true);
+
+        if (json_last_error() !== JSON_ERROR_NONE || ! is_array($data)) {
+            return false;
+        }
+
+        if (! isset($data['user_id'], $data['token'], $data['expires_at'])) {
+            return false;
+        }
+
+        $user = $this->getUser();
+        if ((int) $data['user_id'] !== (int) $user->id) {
+            return false;
+        }
+
+        if (time() > (int) $data['expires_at']) {
+            return false;
+        }
+
+        return TrustedDevice::findValidForUser((int) $user->id, (string) $data['token']) !== null;
+    }
 }

tests/Feature/Auth/RememberMeAuthenticationTest.php

@@ -6,14 +6,17 @@ namespace Tests\Feature\Auth;
 
 use App\Events\UserLoggedIn;
 use App\Models\PasswordSecurity;
+use App\Models\TrustedDevice;
 use App\Models\User;
 use App\Services\PasswordBreachService;
+use Illuminate\Contracts\Console\Kernel;
 use Illuminate\Database\Schema\Blueprint;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Event;
 use Illuminate\Support\Facades\Route;
 use Illuminate\Support\Facades\Schema;
+use PDO;
 use PragmaRX\Google2FALaravel\Facade as Google2FA;
 use Spatie\Permission\Models\Role;
 use Spatie\Permission\PermissionRegistrar;
@@ -21,13 +24,54 @@ use Tests\TestCase;
 
 class RememberMeAuthenticationTest extends TestCase
 {
+    private string $databasePath;
+
+    /**
+     * @var array<string, string|false>
+     */
+    private array $originalEnvironment = [];
+
+    public function createApplication()
+    {
+        $this->databasePath = sys_get_temp_dir().'/nntmux-remember-me-auth-test.sqlite';
+
+        $this->originalEnvironment = [
+            'APP_ENV' => getenv('APP_ENV'),
+            'DB_CONNECTION' => getenv('DB_CONNECTION'),
+            'DB_DATABASE' => getenv('DB_DATABASE'),
+        ];
+
+        if (file_exists($this->databasePath)) {
+            unlink($this->databasePath);
+        }
+
+        $pdo = new PDO('sqlite:'.$this->databasePath);
+        $pdo->exec('CREATE TABLE settings (name VARCHAR PRIMARY KEY, value TEXT NULL)');
+        $pdo->exec("INSERT INTO settings (name, value) VALUES
+            ('categorizeforeign', '0'),
+            ('catwebdl', '0'),
+            ('innerfileblacklist', ''),
+            ('title', 'NNTmux Test'),
+            ('home_link', '/')");
+
+        $this->setEnvironmentValue('APP_ENV', 'testing');
+        $this->setEnvironmentValue('DB_CONNECTION', 'sqlite');
+        $this->setEnvironmentValue('DB_DATABASE', $this->databasePath);
+
+        $app = require __DIR__.'/../../../bootstrap/app.php';
+
+        $app->make(Kernel::class)->bootstrap();
+
+        return $app;
+    }
+
     protected function setUp(): void
     {
         parent::setUp();
 
         config([
             'database.default' => 'sqlite',
-            'database.connections.sqlite.database' => ':memory:',
+            'database.connections.sqlite.database' => $this->databasePath,
             'app.key' => 'base64:'.base64_encode(random_bytes(32)),
             'session.driver' => 'array',
             'google2fa.session_var' => 'google2fa',
@@ -50,6 +94,19 @@ class RememberMeAuthenticationTest extends TestCase
         Route::middleware(['web', 'auth'])->get('__remember_me_probe', fn () => response('ok', 200));
     }
 
+    protected function tearDown(): void
+    {
+        if ($this->databasePath !== '' && file_exists($this->databasePath)) {
+            unlink($this->databasePath);
+        }
+
+        parent::tearDown();
+
+        foreach ($this->originalEnvironment as $key => $value) {
+            $this->setEnvironmentValue($key, $value === false ? null : $value);
+        }
+    }
+
     public function test_password_login_with_remember_me_queues_recaller_cookie(): void
     {
         Event::fake([UserLoggedIn::class]);
@@ -149,6 +206,7 @@ class RememberMeAuthenticationTest extends TestCase
         $this->assertGuest();
         $this->assertTrue((bool) session('2fa:remember'));
         $this->assertSame($user->id, session('2fa:user:id'));
+        $this->assertFalse((bool) session(config('google2fa.session_var')));
 
         $verifyResponse = $this->post(route('2fa.post'), [
             'one_time_password' => Google2FA::getCurrentOtp($secret),
@@ -159,6 +217,8 @@ class RememberMeAuthenticationTest extends TestCase
         $this->assertAuthenticatedAs($user);
         $this->assertTrue((bool) session(config('google2fa.session_var')));
         $this->assertNull(session('2fa:remember'));
+        $this->assertNull(session('2fa:user:id'));
+        $this->assertNull(session('2fa:password_breached'));
     }
 
     public function test_two_factor_login_without_remember_me_does_not_queue_recaller_cookie_after_otp_success(): void
@@ -186,6 +246,63 @@ class RememberMeAuthenticationTest extends TestCase
         $this->assertAuthenticatedAs($user);
     }
 
+    public function test_two_factor_login_with_valid_trusted_device_logs_in_without_otp(): void
+    {
+        Event::fake([UserLoggedIn::class]);
+        $user = $this->createUser('trusted-2fa@example.test');
+        PasswordSecurity::query()->create([
+            'user_id' => $user->id,
+            'google2fa_enable' => 1,
+            'google2fa_secret' => Google2FA::generateSecretKey(),
+        ]);
+        $trustedDevice = TrustedDevice::issueForUser($user, '127.0.0.1', 'Feature Test');
+        $cookieValue = json_encode([
+            'user_id' => $user->id,
+            'token' => $trustedDevice['plain'],
+            'expires_at' => $trustedDevice['device']->expires_at->getTimestamp(),
+        ], JSON_THROW_ON_ERROR);
+
+        $response = $this
+            ->withCookie('2fa_trusted_device', $cookieValue)
+            ->post(route('login'), [
+                'username' => $user->email,
+                'password' => 'password',
+            ]);
+
+        $response->assertRedirect('/');
+        $this->assertAuthenticatedAs($user);
+        $this->assertTrue((bool) session(config('google2fa.session_var')));
+        $this->assertNull(session('2fa:user:id'));
+    }
+
+    public function test_two_factor_login_with_forged_trusted_device_cookie_still_requires_otp(): void
+    {
+        Event::fake([UserLoggedIn::class]);
+        $user = $this->createUser('forged-2fa@example.test');
+        PasswordSecurity::query()->create([
+            'user_id' => $user->id,
+            'google2fa_enable' => 1,
+            'google2fa_secret' => Google2FA::generateSecretKey(),
+        ]);
+        $cookieValue = json_encode([
+            'user_id' => $user->id,
+            'token' => 'forged-client-token',
+            'expires_at' => time() + 3600,
+        ], JSON_THROW_ON_ERROR);
+
+        $response = $this
+            ->withCookie('2fa_trusted_device', $cookieValue)
+            ->post(route('login'), [
+                'username' => $user->email,
+                'password' => 'password',
+            ]);
+
+        $response->assertRedirect(route('2fa.verify'));
+        $this->assertGuest();
+        $this->assertSame($user->id, session('2fa:user:id'));
+        $this->assertFalse((bool) session(config('google2fa.session_var')));
+    }
+
     private function recallerCookieName(): string
     {
         return Auth::guard()->getRecallerName();
@@ -193,6 +310,21 @@ class RememberMeAuthenticationTest extends TestCase
 
     protected function createSchema(): void
     {
+        foreach ([
+            'user_activities',
+            'trusted_devices',
+            'role_has_permissions',
+            'model_has_permissions',
+            'model_has_roles',
+            'password_securities',
+            'users',
+            'permissions',
+            'roles',
+            'settings',
+        ] as $table) {
+            Schema::dropIfExists($table);
+        }
+
         Schema::create('settings', function (Blueprint $table): void {
             $table->string('name')->primary();
             $table->text('value')->nullable();
@@ -242,6 +374,19 @@ class RememberMeAuthenticationTest extends TestCase
             $table->timestamps();
         });
 
+        Schema::create('trusted_devices', function (Blueprint $table): void {
+            $table->id();
+            $table->unsignedInteger('user_id');
+            $table->string('token_hash', 64)->unique();
+            $table->timestamp('expires_at')->index();
+            $table->timestamp('last_used_at')->nullable();
+            $table->string('ip_address', 45)->nullable();
+            $table->string('user_agent', 500)->nullable();
+            $table->timestamps();
+
+            $table->index(['user_id', 'expires_at']);
+        });
+
         Schema::create('model_has_roles', function (Blueprint $table): void {
             $table->unsignedInteger('role_id');
             $table->string('model_type');
@@ -280,6 +425,7 @@ class RememberMeAuthenticationTest extends TestCase
             ['name' => 'home_link', 'value' => '/'],
             ['name' => 'categorizeforeign', 'value' => '0'],
             ['name' => 'catwebdl', 'value' => '0'],
+            ['name' => 'innerfileblacklist', 'value' => ''],
         ]);
     }
 
@@ -306,4 +452,18 @@ class RememberMeAuthenticationTest extends TestCase
 
         return $user->fresh();
     }
+
+    private function setEnvironmentValue(string $key, ?string $value): void
+    {
+        if ($value === null) {
+            putenv($key);
+            unset($_ENV[$key], $_SERVER[$key]);
+
+            return;
+        }
+
+        putenv($key.'='.$value);
+        $_ENV[$key] = $value;
+        $_SERVER[$key] = $value;
+    }
 }