From a45eae315c7fa47f3724fcf4e1d3783342abbfe7 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Mon, 21 Sep 2026 18:05:54 -0300 Subject: [PATCH] Ask for the password over the page instead of throwing the form away password.confirm redirected every write to the confirm-password screen. A redirect cannot carry a POST body, and Redirector::guest() only remembers the exact URL of a GET, so after confirming, the user landed back on an empty form and the action never ran. On the API token forms that meant typing the name, the scopes and the expiry again. An Inertia request now gets a 423 marked X-Password-Confirmation. A dialog mounted around every page catches it, asks for the password over the current page, and sends the refused request again with the same data and callbacks, so the form finishes as if nothing happened. The check itself is still the framework's. Plain form posts and JSON clients are answered as before, and accounts with no local password are offered a way to set one, as the confirm screen does. --- .../Auth/ConfirmablePasswordController.php | 11 +- .../RequirePasswordConfirmation.php | 61 ++++++ bootstrap/app.php | 5 + lang/es.json | 3 +- resources/js/app.tsx | 18 +- .../password-confirmation-dialog.tsx | 180 ++++++++++++++++++ .../js/pages/settings/connected-accounts.tsx | 8 +- .../Auth/PasswordConfirmationDialogTest.php | 70 +++++++ 8 files changed, 352 insertions(+), 4 deletions(-) create mode 100644 app/Modules/Identity/Http/Middleware/RequirePasswordConfirmation.php create mode 100644 resources/js/components/password-confirmation-dialog.tsx create mode 100644 tests/Feature/Auth/PasswordConfirmationDialogTest.php diff --git a/app/Http/Controllers/Auth/ConfirmablePasswordController.php b/app/Http/Controllers/Auth/ConfirmablePasswordController.php index 356965c5..eb77ef7f 100644 --- a/app/Http/Controllers/Auth/ConfirmablePasswordController.php +++ b/app/Http/Controllers/Auth/ConfirmablePasswordController.php @@ -7,6 +7,7 @@ use App\Modules\Identity\AuthSource; use App\Modules\Identity\PasswordVerification; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; +use Illuminate\Http\Response as HttpResponse; use Illuminate\Validation\ValidationException; use Inertia\Inertia; use Inertia\Response; @@ -41,8 +42,12 @@ class ConfirmablePasswordController extends Controller * their local hash is a Str::password(64) nobody has ever seen -- and * this screen stands in front of enrolling in two-factor, so those * accounts could not enrol at all. + * + * Asked for JSON, it answers with a bare 204: that is the password + * dialog (RequirePasswordConfirmation), which stays on the page and + * sends the refused request again itself, so there is nowhere to go. */ - public function store(Request $request, PasswordVerification $passwords): RedirectResponse + public function store(Request $request, PasswordVerification $passwords): RedirectResponse|HttpResponse { $user = $request->user(); assert($user !== null); @@ -55,6 +60,10 @@ class ConfirmablePasswordController extends Controller $request->session()->put('auth.password_confirmed_at', time()); + if ($request->expectsJson()) { + return response()->noContent(); + } + return redirect()->intended(route('dashboard', absolute: false)); } } diff --git a/app/Modules/Identity/Http/Middleware/RequirePasswordConfirmation.php b/app/Modules/Identity/Http/Middleware/RequirePasswordConfirmation.php new file mode 100644 index 00000000..3b839b50 --- /dev/null +++ b/app/Modules/Identity/Http/Middleware/RequirePasswordConfirmation.php @@ -0,0 +1,61 @@ +header('X-Inertia') && $this->shouldConfirmPassword($request, $passwordTimeoutSeconds)) { + return $this->inertiaRefusal($request); + } + + return parent::handle($request, $next, $redirectToRoute, $passwordTimeoutSeconds); + } + + private function inertiaRefusal(Request $request): Response + { + $user = $request->user(); + + return $this->responseFactory->json([ + 'message' => 'Password confirmation required.', + // The same question the confirm-password screen asks: an account + // provisioned by a provider has no password to type, and the + // dialog has to offer it a way to set one instead. + 'has_local_password' => $user?->auth_source === AuthSource::Local, + ], 423, [self::HEADER => 'required']); + } +} diff --git a/bootstrap/app.php b/bootstrap/app.php index 6c2d2140..5c672179 100644 --- a/bootstrap/app.php +++ b/bootstrap/app.php @@ -12,6 +12,7 @@ use App\Modules\Identity\Http\Middleware\EnforceTwoFactor; use App\Modules\Identity\Http\Middleware\EnsureAccountIsActive; use App\Modules\Identity\Http\Middleware\EnsureSetupIsComplete; use App\Modules\Identity\Http\Middleware\EnsureStaff; +use App\Modules\Identity\Http\Middleware\RequirePasswordConfirmation; use App\Modules\Platform\Http\Middleware\EnsureCapability; use App\Modules\Platform\Http\Middleware\SetLocale; use App\Support\WriteSafeRedirect; @@ -101,6 +102,10 @@ return Application::configure(basePath: dirname(__DIR__)) 'staff-token' => EnsureStaffToken::class, 'token-can' => EnsureTokenCan::class, 'api-active' => EnsureApiAccountIsActive::class, + // Replaces the framework's own: a write that needs the password + // re-proved gets a dialog over the page instead of a redirect + // that throws the submitted form away. See the class. + 'password.confirm' => RequirePasswordConfirmation::class, ]); }) ->withExceptions(function (Exceptions $exceptions) { diff --git a/lang/es.json b/lang/es.json index 94c6d011..0b9c80b9 100644 --- a/lang/es.json +++ b/lang/es.json @@ -2256,5 +2256,6 @@ "You sign in through a connected account. A password of your own lets you sign in without it, and is needed to turn on two-factor authentication.": "Inicias sesión con una cuenta conectada. Tener una contraseña propia te permite entrar sin ella, y hace falta para activar la verificación en dos pasos.", "Your password is managed by your organisation's directory, so it cannot be changed here.": "Tu contraseña la gestiona el directorio de tu organización, así que no se puede cambiar aquí.", "A link anyone can open, without signing in. You can revoke it at any time.": "Un enlace que cualquiera puede abrir, sin iniciar sesión. Puedes revocarlo cuando quieras.", - ":provider only confirms an address when the \"xms_edov\" optional claim is on your app registration (Token configuration → optional claims → ID token). Until it is there, accounts are still created, but every one of them waits in Account requests however this box is set.": ":provider solo confirma una dirección cuando el claim opcional \"xms_edov\" está en el registro de tu aplicación (Token configuration → optional claims → ID token). Hasta que esté, las cuentas se crean igual, pero todas quedan esperando en Solicitudes de cuenta, marques lo que marques aquí." + ":provider only confirms an address when the \"xms_edov\" optional claim is on your app registration (Token configuration → optional claims → ID token). Until it is there, accounts are still created, but every one of them waits in Account requests however this box is set.": ":provider solo confirma una dirección cuando el claim opcional \"xms_edov\" está en el registro de tu aplicación (Token configuration → optional claims → ID token). Hasta que esté, las cuentas se crean igual, pero todas quedan esperando en Solicitudes de cuenta, marques lo que marques aquí.", + "Too many attempts. Wait a minute and try again.": "Demasiados intentos. Espera un minuto e inténtalo de nuevo." } diff --git a/resources/js/app.tsx b/resources/js/app.tsx index a1343438..1a51f6e3 100644 --- a/resources/js/app.tsx +++ b/resources/js/app.tsx @@ -3,8 +3,10 @@ import '../css/app.css'; import { createInertiaApp, router } from '@inertiajs/react'; import axios from 'axios'; import { resolvePageComponent } from 'laravel-vite-plugin/inertia-helpers'; +import { createElement } from 'react'; import { createRoot } from 'react-dom/client'; import { route as routeFn } from 'ziggy-js'; +import { PasswordConfirmationDialog } from './components/password-confirmation-dialog'; import { initializeTheme } from './hooks/use-appearance'; import { xsrfCookieName } from './lib/xsrf'; @@ -75,7 +77,21 @@ createInertiaApp({ const root = createRoot(el); - root.render(); + // The password dialog sits beside every page rather than in any + // one layout: the writes it answers for are spread across the + // staff shell, the settings screens and every portal theme, and + // it needs the page context (translations) the children of + // are given. The page itself renders as Inertia would on its own. + root.render( + + {({ Component, props: pageProps, key }) => ( + <> + {createElement(Component, { key, ...pageProps })} + + + )} + , + ); }, progress: { color: '#4B5563', diff --git a/resources/js/components/password-confirmation-dialog.tsx b/resources/js/components/password-confirmation-dialog.tsx new file mode 100644 index 00000000..dd98ff56 --- /dev/null +++ b/resources/js/components/password-confirmation-dialog.tsx @@ -0,0 +1,180 @@ +import type { PendingVisit } from '@inertiajs/core'; +import { router } from '@inertiajs/react'; +import axios from 'axios'; +import { LoaderCircle } from 'lucide-react'; +import { FormEventHandler, useEffect, useRef, useState } from 'react'; + +import InputError from '@/components/input-error'; +import { Button } from '@/components/ui/button'; +import { Dialog, DialogContent, DialogDescription, DialogFooter, DialogHeader, DialogTitle } from '@/components/ui/dialog'; +import { Input } from '@/components/ui/input'; +import { Label } from '@/components/ui/label'; +import { useTranslation } from '@/hooks/use-translation'; + +interface Refused { + // Typed as pending, but what the start event hands over is the request + // as sent: the visit plus its callbacks (useForm's among them), which + // is what lets a replay finish the form's own submission. + visit: PendingVisit; + hasLocalPassword: boolean; +} + +const visitKey = (method: string, url: string) => `${method.toUpperCase()} ${url}`; + +/** + * The browser half of RequirePasswordConfirmation. + * + * A write that needs the password re-proved comes back as a 423 instead of + * a redirect. Inertia calls that an "invalid" response; this catches it, + * asks for the password over the page the user is on, and then sends the + * refused request again, exactly as it was first sent -- same data, same + * callbacks -- so the form that sent it carries on as if nothing had + * happened. Nothing navigates, so nothing typed into the form is lost. + * + * Mounted once, around every page, in app.tsx. + */ +export function PasswordConfirmationDialog() { + const { t } = useTranslation(); + + // Writes in flight, by method and URL. The invalid event carries only + // the response, so this is how a refusal finds the visit that caused it. + const inFlight = useRef(new Map()); + + const [refused, setRefused] = useState(null); + const [password, setPassword] = useState(''); + const [error, setError] = useState(); + const [processing, setProcessing] = useState(false); + + useEffect(() => { + const removeStart = router.on('start', (event) => { + const visit = event.detail.visit; + if (visit.method !== 'get') { + inFlight.current.set(visitKey(visit.method, visit.url.href), visit); + } + }); + + // After `invalid` for the same request, so the visit is still here + // when a refusal needs it. + const removeFinish = router.on('finish', (event) => { + const visit = event.detail.visit; + inFlight.current.delete(visitKey(visit.method, visit.url.href)); + }); + + const removeInvalid = router.on('invalid', (event) => { + const response = event.detail.response; + if (response.status !== 423 || response.headers['x-password-confirmation'] !== 'required') { + return; + } + + const visit = inFlight.current.get(visitKey(response.config.method ?? '', response.config.url ?? '')); + if (!visit) { + return; + } + + // Stops Inertia's own error modal, which would show the raw JSON. + event.preventDefault(); + + setPassword(''); + setError(undefined); + setRefused({ visit, hasLocalPassword: response.data?.has_local_password !== false }); + }); + + return () => { + removeStart(); + removeFinish(); + removeInvalid(); + }; + }, []); + + const submit: FormEventHandler = async (e) => { + e.preventDefault(); + if (!refused) { + return; + } + + setProcessing(true); + setError(undefined); + + try { + await axios.post(route('password.confirm.store'), { password }); + } catch (failure) { + setPassword(''); + if (axios.isAxiosError(failure) && failure.response?.status === 422) { + setError(failure.response.data?.errors?.password?.[0] ?? t('Something went wrong. Please try again.')); + } else if (axios.isAxiosError(failure) && failure.response?.status === 429) { + setError(t('Too many attempts. Wait a minute and try again.')); + } else { + setError(t('Something went wrong. Please try again.')); + } + setProcessing(false); + + return; + } + + setProcessing(false); + setRefused(null); + + // Sent again as it was. The three state flags describe the first + // attempt, which finished; carried over, they would mark the new + // one finished before it started. + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { url, completed, cancelled, interrupted, ...options } = refused.visit; + router.visit(url, options); + }; + + return ( + !open && setRefused(null)}> + + + {t('Confirm your password')} + + {t('This is a secure area of the application. Please confirm your password before continuing.')} + + + + {refused && !refused.hasLocalPassword ? ( +
+

+ {t('You sign in through a connected account, so there is no password here to confirm. Set one to continue.')} +

+ + + + +
+ ) : ( +
+
+ + setPassword(e.target.value)} + /> + +
+ + + + + +
+ )} + + + ); +} diff --git a/resources/js/pages/settings/connected-accounts.tsx b/resources/js/pages/settings/connected-accounts.tsx index f7627579..ee1f5988 100644 --- a/resources/js/pages/settings/connected-accounts.tsx +++ b/resources/js/pages/settings/connected-accounts.tsx @@ -91,7 +91,13 @@ export default function ConnectedAccounts({ providers, has_local_password }: Con router.post( route('connected-accounts.connect', { provider: provider.provider }), {}, - { onStart: () => setProcessing(true) }, + // onFinish too: a password confirmation the + // user cancels ends the request here, and + // the button must not stay dead. + { + onStart: () => setProcessing(true), + onFinish: () => setProcessing(false), + }, ) } > diff --git a/tests/Feature/Auth/PasswordConfirmationDialogTest.php b/tests/Feature/Auth/PasswordConfirmationDialogTest.php new file mode 100644 index 00000000..c2d22749 --- /dev/null +++ b/tests/Feature/Auth/PasswordConfirmationDialogTest.php @@ -0,0 +1,70 @@ +user = User::factory()->create(); +}); + +test('an inertia write without a fresh confirmation is refused in place, not redirected', function () { + $this->actingAs($this->user) + ->withHeaders(['X-Inertia' => 'true']) + ->post('/settings/two-factor') + ->assertStatus(423) + ->assertHeader('X-Password-Confirmation', 'required') + ->assertJson(['has_local_password' => true]); + + // Refused, not half-done: the action did not run. + expect($this->user->refresh()->two_factor_secret)->toBeNull(); +}); + +test('the refusal tells the dialog when there is no password to type', function () { + $user = User::factory()->create(['auth_source' => AuthSource::Social]); + + $this->actingAs($user) + ->withHeaders(['X-Inertia' => 'true']) + ->post('/settings/two-factor') + ->assertStatus(423) + ->assertJson(['has_local_password' => false]); +}); + +test('a plain form post is still redirected to the confirm-password screen', function () { + $this->actingAs($this->user) + ->post('/settings/two-factor') + ->assertRedirect(route('password.confirm')); +}); + +test('the dialog confirms over json, and the replayed request goes through', function () { + $this->actingAs($this->user) + ->postJson('/confirm-password', ['password' => 'password']) + ->assertNoContent(); + + $this->actingAs($this->user) + ->withHeaders(['X-Inertia' => 'true']) + ->post('/settings/two-factor') + ->assertRedirect(); + + expect($this->user->refresh()->two_factor_secret)->not->toBeNull(); +}); + +test('a wrong password in the dialog is a validation error, and confirms nothing', function () { + $this->actingAs($this->user) + ->postJson('/confirm-password', ['password' => 'wrong-password']) + ->assertStatus(422) + ->assertJsonValidationErrors('password'); + + $this->actingAs($this->user) + ->withHeaders(['X-Inertia' => 'true']) + ->post('/settings/two-factor') + ->assertStatus(423); +});