diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 4b4df4971..c3a52ea35 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -4,9 +4,11 @@ use App\Models\InstanceSettings; use App\Models\User; +use App\Providers\RouteServiceProvider; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Auth\AuthenticationException; use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler; +use Illuminate\Session\TokenMismatchException; use Psr\Log\LogLevel; use RuntimeException; use Sentry\Laravel\Integration; @@ -69,6 +71,11 @@ protected function unauthenticated($request, AuthenticationException $exception) */ public function render($request, Throwable $e) { + // A duplicate login or 2FA submission carries a stale token on an already authenticated session, see https://github.com/coollabsio/coolify/issues/10670 + if ($e instanceof TokenMismatchException && $request->routeIs('login.store', 'two-factor.login.store') && $request->user()) { + return redirect()->intended(RouteServiceProvider::HOME); + } + // Handle authorization exceptions for API routes. Exceptions carrying // an explicit status (e.g. denyAsNotFound) keep it via parent::render. if ($e instanceof AuthorizationException && ! $e->hasStatus()) { diff --git a/resources/views/auth/two-factor-challenge.blade.php b/resources/views/auth/two-factor-challenge.blade.php index b15afc24d..10480b920 100644 --- a/resources/views/auth/two-factor-challenge.blade.php +++ b/resources/views/auth/two-factor-challenge.blade.php @@ -2,6 +2,14 @@
Enter one of the recovery codes you saved when setting up two-factor authentication.

-
+ @csrf
@@ -54,7 +63,7 @@ class="mx-auto h-14 w-64 rounded-md border border-neutral-300 bg-white px-4 text
- + Verify and continue diff --git a/tests/Feature/Auth/TwoFactorChallengeAccessTest.php b/tests/Feature/Auth/TwoFactorChallengeAccessTest.php index f0dd97cc3..a46f6b35c 100644 --- a/tests/Feature/Auth/TwoFactorChallengeAccessTest.php +++ b/tests/Feature/Auth/TwoFactorChallengeAccessTest.php @@ -3,7 +3,11 @@ use App\Models\InstanceSettings; use App\Models\Team; use App\Models\User; +use Illuminate\Contracts\Debug\ExceptionHandler; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Http\Request; +use Illuminate\Session\TokenMismatchException; +use Illuminate\Support\Facades\Route; uses(RefreshDatabase::class); @@ -78,3 +82,33 @@ expect($view)->toContain('error-shell'); expect($view)->not->toContain('url()->previous()'); }); + +it('redirects an authenticated stale two-factor submission home instead of showing 419', function () { + $request = Request::create('/two-factor-challenge', 'POST'); + $request->setRouteResolver(fn () => Route::getRoutes()->match($request)); + $request->setUserResolver(fn () => $this->user); + + $response = app(ExceptionHandler::class)->render($request, new TokenMismatchException('CSRF token mismatch.')); + + expect($response->getStatusCode())->toBe(302) + ->and($response->headers->get('Location'))->toBe(url('/')); +}); + +it('still returns 419 for a stale two-factor submission without an authenticated session', function () { + $request = Request::create('/two-factor-challenge', 'POST'); + $request->setRouteResolver(fn () => Route::getRoutes()->match($request)); + + $response = app(ExceptionHandler::class)->render($request, new TokenMismatchException('CSRF token mismatch.')); + + expect($response->getStatusCode())->toBe(419); +}); + +it('keeps the 419 for stale tokens on routes other than login and the two-factor challenge', function () { + $request = Request::create('/two-factor-challenge', 'GET'); + $request->setRouteResolver(fn () => Route::getRoutes()->match($request)); + $request->setUserResolver(fn () => $this->user); + + $response = app(ExceptionHandler::class)->render($request, new TokenMismatchException('CSRF token mismatch.')); + + expect($response->getStatusCode())->toBe(419); +}); diff --git a/tests/v4/Browser/LoginTest.php b/tests/v4/Browser/LoginTest.php index 7a6d6f2b7..df960a71b 100644 --- a/tests/v4/Browser/LoginTest.php +++ b/tests/v4/Browser/LoginTest.php @@ -4,6 +4,7 @@ use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Hash; +use Laravel\Fortify\Fortify; uses(RefreshDatabase::class); @@ -69,6 +70,43 @@ ->screenshot(filename: 'login-invalid-credentials'); }); +it('submits the automatic two factor challenge only once', function () { + config(['app.maintenance.driver' => 'file']); + + $user = createRootUser(); + $user->forceFill([ + 'two_factor_secret' => Fortify::currentEncrypter()->encrypt('JBSWY3DPEHPK3PXP'), + 'two_factor_confirmed_at' => now(), + ])->save(); + + $page = visit('/login') + ->fill('email', 'test@example.com') + ->fill('password', 'password') + ->click('Login') + ->assertPathIs('/two-factor-challenge'); + + $page->script(<<<'JS' + window.acceptedTwoFactorSubmissions = 0; + + document.querySelector('form[action="/two-factor-challenge"]').addEventListener('submit', (event) => { + if (!event.defaultPrevented) { + window.acceptedTwoFactorSubmissions++; + } + + event.preventDefault(); + }); + JS); + + $page->fill('code', '123456') + ->keys('code', 'Enter') + ->assertScript('window.acceptedTwoFactorSubmissions', 1) + ->assertDisabled('Verify and continue') + ->fill('code', '654321') + ->assertScript('window.acceptedTwoFactorSubmissions', 1) + ->assertNoJavaScriptErrors() + ->screenshot(filename: 'login-two-factor-enter-single-submission'); +}); + /** * Create the root user (id 0) with known credentials for browser login tests. */