diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index eda5dcc9d..68429c402 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/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'); diff --git a/app/Models/PasswordSecurity.php b/app/Models/PasswordSecurity.php index f3cca4aa6..0c46b02a9 100644 --- a/app/Models/PasswordSecurity.php +++ b/app/Models/PasswordSecurity.php @@ -17,6 +17,16 @@ class PasswordSecurity extends Model */ protected $guarded = []; + /** + * @return array + */ + protected function casts(): array + { + return [ + 'google2fa_enable' => 'boolean', + ]; + } + /** * @return BelongsTo */ diff --git a/app/Support/Google2FAAuthenticator.php b/app/Support/Google2FAAuthenticator.php index 2cf3664d9..60038e14d 100644 --- a/app/Support/Google2FAAuthenticator.php +++ b/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; + } } diff --git a/tests/Feature/Auth/RememberMeAuthenticationTest.php b/tests/Feature/Auth/RememberMeAuthenticationTest.php index a847ba674..eb3e63d57 100644 --- a/tests/Feature/Auth/RememberMeAuthenticationTest.php +++ b/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 + */ + 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; + } }