fix: prevent duplicate 2FA challenge submissions causing HTTP 419 (#11223)
This commit is contained in:
commit
ff24e82fd2
4 changed files with 90 additions and 2 deletions
|
|
@ -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()) {
|
||||
|
|
|
|||
|
|
@ -2,6 +2,14 @@
|
|||
<x-auth.shell title="Coolify" description="Verify your identity to finish signing in.">
|
||||
<div class="flex flex-col gap-4" x-data="{
|
||||
showRecovery: false,
|
||||
submitting: false,
|
||||
handleSubmit(event) {
|
||||
if (this.submitting) {
|
||||
event.preventDefault();
|
||||
}
|
||||
|
||||
this.submitting = true;
|
||||
},
|
||||
submitAuthenticatorCode(event) {
|
||||
event.target.value = event.target.value.replace(/\D/g, '').slice(0, 6);
|
||||
|
||||
|
|
@ -30,7 +38,8 @@
|
|||
<p x-show="showRecovery" x-cloak>Enter one of the recovery codes you saved when setting up two-factor authentication.</p>
|
||||
</div>
|
||||
|
||||
<form x-ref="challengeForm" action="/two-factor-challenge" method="POST" class="flex flex-col gap-4">
|
||||
<form x-ref="challengeForm" action="/two-factor-challenge" method="POST" class="flex flex-col gap-4"
|
||||
@submit="handleSubmit($event)">
|
||||
@csrf
|
||||
|
||||
<div x-show="!showRecovery" class="flex flex-col gap-3">
|
||||
|
|
@ -54,7 +63,7 @@ class="mx-auto h-14 w-64 rounded-md border border-neutral-300 bg-white px-4 text
|
|||
</button>
|
||||
</div>
|
||||
|
||||
<x-forms.button class="w-full justify-center" type="submit" isHighlighted>
|
||||
<x-forms.button class="w-full justify-center" type="submit" x-bind:disabled="submitting" isHighlighted>
|
||||
Verify and continue
|
||||
</x-forms.button>
|
||||
</form>
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
*/
|
||||
|
|
|
|||
Loading…
Reference in a new issue