From 3a3fd5358d5d90f23fe29cc07e3bddb1a4385cf2 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Mon, 21 Sep 2026 18:12:52 -0300 Subject: [PATCH] Let directory accounts confirm their password The confirm-password screen hid its field from every account that was not Local, and told it to set a password instead. That is right for an account a provider created, which has no password anybody has seen. It is wrong for a directory account: its password is the directory's, PasswordVerification accepts it, and /settings/password refuses to let it set another. So an LDAP account could not get past the confirmation at all, and everything behind it, turning on two-factor included, was out of reach. The new dialog copied the same question. Both now ask whether the account came from a provider, and the prop is called has_password, which is what it means. The dialog also clears the typed password when it closes or once it has been used, instead of keeping it in component state. --- .../Auth/ConfirmablePasswordController.php | 8 ++++++- .../RequirePasswordConfirmation.php | 14 +++++++---- .../password-confirmation-dialog.tsx | 19 ++++++++++----- resources/js/pages/auth/confirm-password.tsx | 8 +++---- .../Auth/PasswordConfirmationDialogTest.php | 24 +++++++++++++++++-- .../Identity/ProviderAccountPasswordTest.php | 2 +- 6 files changed, 56 insertions(+), 19 deletions(-) diff --git a/app/Http/Controllers/Auth/ConfirmablePasswordController.php b/app/Http/Controllers/Auth/ConfirmablePasswordController.php index eb77ef7f..400c264c 100644 --- a/app/Http/Controllers/Auth/ConfirmablePasswordController.php +++ b/app/Http/Controllers/Auth/ConfirmablePasswordController.php @@ -28,7 +28,13 @@ class ConfirmablePasswordController extends Controller // has seen. The screen offers to set one instead of asking for // it, which is the only way past this for those accounts, and // this screen stands in front of two-factor enrolment. - 'has_local_password' => $user->auth_source === AuthSource::Local, + // + // Social, not "anything but Local": a directory account has a + // password -- the directory's -- and store() accepts it. Asking + // whether the account was Local told those accounts to set one + // here instead, which /settings/password refuses them, and left + // them no way past this screen at all. + 'has_password' => $user->auth_source !== AuthSource::Social, ]); } diff --git a/app/Modules/Identity/Http/Middleware/RequirePasswordConfirmation.php b/app/Modules/Identity/Http/Middleware/RequirePasswordConfirmation.php index 3b839b50..66521da9 100644 --- a/app/Modules/Identity/Http/Middleware/RequirePasswordConfirmation.php +++ b/app/Modules/Identity/Http/Middleware/RequirePasswordConfirmation.php @@ -39,7 +39,10 @@ class RequirePasswordConfirmation extends RequirePassword public function handle($request, Closure $next, $redirectToRoute = null, $passwordTimeoutSeconds = null) { - if ($request->header('X-Inertia') && $this->shouldConfirmPassword($request, $passwordTimeoutSeconds)) { + // Middleware parameters arrive as strings ("password.confirm:,300"). + $timeout = $passwordTimeoutSeconds === null || $passwordTimeoutSeconds === '' ? null : (int) $passwordTimeoutSeconds; + + if ($request->header('X-Inertia') && $this->shouldConfirmPassword($request, $timeout)) { return $this->inertiaRefusal($request); } @@ -52,10 +55,11 @@ class RequirePasswordConfirmation extends RequirePassword 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, + // The same question the confirm-password screen asks, and see + // there for why it is Social and not "anything but Local": 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_password' => $user !== null && $user->auth_source !== AuthSource::Social, ], 423, [self::HEADER => 'required']); } } diff --git a/resources/js/components/password-confirmation-dialog.tsx b/resources/js/components/password-confirmation-dialog.tsx index dd98ff56..8c23e10a 100644 --- a/resources/js/components/password-confirmation-dialog.tsx +++ b/resources/js/components/password-confirmation-dialog.tsx @@ -16,7 +16,7 @@ interface Refused { // 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; + hasPassword: boolean; } const visitKey = (method: string, url: string) => `${method.toUpperCase()} ${url}`; @@ -76,7 +76,7 @@ export function PasswordConfirmationDialog() { setPassword(''); setError(undefined); - setRefused({ visit, hasLocalPassword: response.data?.has_local_password !== false }); + setRefused({ visit, hasPassword: response.data?.has_password !== false }); }); return () => { @@ -86,6 +86,12 @@ export function PasswordConfirmationDialog() { }; }, []); + // Cancelling drops the refused request, and the password with it. + const close = () => { + setPassword(''); + setRefused(null); + }; + const submit: FormEventHandler = async (e) => { e.preventDefault(); if (!refused) { @@ -112,6 +118,7 @@ export function PasswordConfirmationDialog() { } setProcessing(false); + setPassword(''); setRefused(null); // Sent again as it was. The three state flags describe the first @@ -123,7 +130,7 @@ export function PasswordConfirmationDialog() { }; return ( - !open && setRefused(null)}> + !open && close()}> {t('Confirm your password')} @@ -132,13 +139,13 @@ export function PasswordConfirmationDialog() { - {refused && !refused.hasLocalPassword ? ( + {refused && !refused.hasPassword ? (

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

-
-