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.
This commit is contained in:
ignacionelson
2026-09-21 18:12:52 -03:00
parent a45eae315c
commit 3a3fd5358d
6 changed files with 56 additions and 19 deletions
@@ -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,
]);
}
@@ -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']);
}
}
@@ -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 (
<Dialog open={refused !== null} onOpenChange={(open) => !open && setRefused(null)}>
<Dialog open={refused !== null} onOpenChange={(open) => !open && close()}>
<DialogContent>
<DialogHeader>
<DialogTitle>{t('Confirm your password')}</DialogTitle>
@@ -132,13 +139,13 @@ export function PasswordConfirmationDialog() {
</DialogDescription>
</DialogHeader>
{refused && !refused.hasLocalPassword ? (
{refused && !refused.hasPassword ? (
<div className="space-y-4">
<p className="text-muted-foreground text-sm">
{t('You sign in through a connected account, so there is no password here to confirm. Set one to continue.')}
</p>
<DialogFooter>
<Button variant="ghost" type="button" onClick={() => setRefused(null)}>
<Button variant="ghost" type="button" onClick={close}>
{t('Cancel')}
</Button>
<Button asChild>
@@ -164,7 +171,7 @@ export function PasswordConfirmationDialog() {
</div>
<DialogFooter>
<Button variant="ghost" type="button" onClick={() => setRefused(null)}>
<Button variant="ghost" type="button" onClick={close}>
{t('Cancel')}
</Button>
<Button type="submit" disabled={processing || password === ''}>
+4 -4
View File
@@ -12,10 +12,10 @@ import AuthLayout from '@/layouts/auth-layout';
interface ConfirmPasswordProps {
/** False for an account that signs in through a provider and has no password to confirm with. */
has_local_password: boolean;
has_password: boolean;
}
export default function ConfirmPassword({ has_local_password }: ConfirmPasswordProps) {
export default function ConfirmPassword({ has_password }: ConfirmPasswordProps) {
const { t } = useTranslation();
const { data, setData, post, processing, errors, reset } = useForm({
@@ -37,7 +37,7 @@ export default function ConfirmPassword({ has_local_password }: ConfirmPasswordP
>
<Head title={t('Confirm password')} />
{!has_local_password && (
{!has_password && (
<div className="space-y-4">
<p className="text-muted-foreground text-sm">
{t('You sign in through a connected account, so there is no password here to confirm. Set one to continue.')}
@@ -48,7 +48,7 @@ export default function ConfirmPassword({ has_local_password }: ConfirmPasswordP
</div>
)}
<form onSubmit={submit} hidden={!has_local_password}>
<form onSubmit={submit} hidden={!has_password}>
<div className="space-y-6">
<div className="grid gap-2">
<Label htmlFor="password">{t('Password')}</Label>
@@ -22,7 +22,7 @@ test('an inertia write without a fresh confirmation is refused in place, not red
->post('/settings/two-factor')
->assertStatus(423)
->assertHeader('X-Password-Confirmation', 'required')
->assertJson(['has_local_password' => true]);
->assertJson(['has_password' => true]);
// Refused, not half-done: the action did not run.
expect($this->user->refresh()->two_factor_secret)->toBeNull();
@@ -35,7 +35,27 @@ test('the refusal tells the dialog when there is no password to type', function
->withHeaders(['X-Inertia' => 'true'])
->post('/settings/two-factor')
->assertStatus(423)
->assertJson(['has_local_password' => false]);
->assertJson(['has_password' => false]);
});
// A directory account's password is the directory's, and the confirmation
// accepts it (LdapAuthenticationTest). Asking "is this account Local?"
// told it to set a password instead, which it is not allowed to do, so it
// could not get past the confirmation at all -- on the screen or here.
test('a directory account is asked for its password, not told to set one', function () {
$user = User::factory()->create(['auth_source' => AuthSource::Ldap]);
// The screen first: withHeaders() sticks to every later request in a
// test, and this GET must not be sent as an Inertia visit.
$this->actingAs($user)
->get('/confirm-password')
->assertInertia(fn ($page) => $page->component('auth/confirm-password')->where('has_password', true));
$this->actingAs($user)
->withHeaders(['X-Inertia' => 'true'])
->post('/settings/two-factor')
->assertStatus(423)
->assertJson(['has_password' => true]);
});
test('a plain form post is still redirected to the confirm-password screen', function () {
@@ -86,7 +86,7 @@ test('the screen says which of the two it is', function () {
test('the confirm-password screen offers to set one instead of asking for it', function () {
$this->actingAs(providerAccount())->get('/confirm-password')->assertInertia(
fn (AssertableInertia $page) => $page->component('auth/confirm-password')->where('has_local_password', false),
fn (AssertableInertia $page) => $page->component('auth/confirm-password')->where('has_password', false),
);
});