From 717852ff6a99391dbd816a2bb0bb3f40cad70f72 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Mon, 5 Oct 2026 00:44:26 -0300 Subject: [PATCH] A provider account's first password comes from its inbox, not its session An account that signs in through a provider has no password to prove, so the password screen let the signed-in session choose one with no proof at all. A stolen session could then make itself permanent: set a password, confirm it, enrol its own second factor and remove the owner's last provider, since the account now read as local. The screen now refuses to set a provider account's password and offers to email a link instead: the ordinary reset link, to the account's own address, so whoever sets the password must read that inbox. The reset pages accept a signed-in visitor, since the owner opens the link in the browser they are signed in with; the token, not the session, is the authority. Using the link signs out every session holding the old password, the one that asked for it included. Compulsory two-factor lets the link through, so a provider account still has a way to enrol. Ordinary accounts are unchanged: they prove their current password. GHSA-4r8h-mwfm-f5f4 --- .../Settings/PasswordController.php | 67 +++++++++++++------ .../Http/Middleware/EnforceTwoFactor.php | 7 +- resources/js/pages/settings/password.tsx | 33 ++++++++- routes/auth.php | 19 ++++-- routes/settings.php | 6 ++ 5 files changed, 102 insertions(+), 30 deletions(-) diff --git a/app/Http/Controllers/Settings/PasswordController.php b/app/Http/Controllers/Settings/PasswordController.php index c2adaa21..9bc0ebb1 100644 --- a/app/Http/Controllers/Settings/PasswordController.php +++ b/app/Http/Controllers/Settings/PasswordController.php @@ -11,6 +11,8 @@ use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Hash; +use Illuminate\Support\Facades\Password as PasswordBroker; +use Illuminate\Validation\ValidationException; use Illuminate\Validation\Rules\Password; use Inertia\Inertia; use Inertia\Response; @@ -53,32 +55,24 @@ class PasswordController extends Controller // honest answer is to refuse rather than to appear to work. abort_if($user->auth_source === AuthSource::Ldap, 403); - $setsFirstPassword = $user->auth_source === AuthSource::Social; + // An account that signs in through a provider has no password to + // prove, so this screen cannot ask it for one, and the session + // alone is not enough: a stolen session that could choose the + // password became the account for good (GHSA-4r8h-mwfm-f5f4). Its + // first password comes from a link emailed to its own address, + // which sendLink() asks for and NewPasswordController completes. + if ($user->auth_source === AuthSource::Social) { + throw ValidationException::withMessages([ + 'password' => __('Ask for a link by email to set your first password.'), + ]); + } $validated = $request->validate([ - // Not asked of an account that has never had one: it signs in - // through a provider, and its stored hash is a generated - // string nobody has seen. Asking anyway left those accounts - // with no way to set a password — and so no way to enrol in - // two-factor, which an installation can make compulsory. - 'current_password' => $setsFirstPassword ? ['nullable'] : ['required', 'current_password'], + 'current_password' => ['required', 'current_password'], 'password' => ['required', Password::defaults(), 'confirmed'], ]); - $attributes = ['password' => Hash::make($validated['password'])]; - - // The same line NewPasswordController writes when a provider - // account resets its password, for the same reason: the hash is - // now what signs this account in, and `has_local_password` is read - // off this column all over the settings screens. - if ($setsFirstPassword) { - $attributes['auth_source'] = AuthSource::Local; - } - - // forceFill, not update(): `auth_source` is guarded, so a mass - // assignment drops it silently — which left the account still - // reading as passwordless after it had a password. - $user->forceFill($attributes)->save(); + $user->forceFill(['password' => Hash::make($validated['password'])])->save(); // Changing a password is how someone reacts to a session they think // is stolen, so it has to actually end that session. AuthenticateSession @@ -93,4 +87,35 @@ class PasswordController extends Controller return back(); } + + /** + * Email an account that signs in through a provider a link to set its + * first password. + * + * The ordinary reset link, to the account's own address: whoever sets + * the password has to read that inbox, which a stolen session cannot + * do. Opening it completes through NewPasswordController, which turns + * the account local, and changing the password signs out every session + * holding the old one, the one that asked for the link included. + */ + public function sendLink(Request $request): RedirectResponse + { + $user = $request->user(); + assert($user !== null); + + // An account with a password uses the form above; a directory + // account's password is not this installation's to set. + abort_unless($user->auth_source === AuthSource::Social, 403); + + $status = PasswordBroker::broker()->sendResetLink(['email' => $user->email]); + + // Their own account, so being told to wait reveals nothing. + if ($status === PasswordBroker::ResetThrottled) { + throw ValidationException::withMessages([ + 'link' => __('A link was sent a moment ago. Check your email, or try again in a minute.'), + ]); + } + + return back()->with('status', 'password-link-sent'); + } } diff --git a/app/Modules/Identity/Http/Middleware/EnforceTwoFactor.php b/app/Modules/Identity/Http/Middleware/EnforceTwoFactor.php index dbedc6b7..4e6aaa57 100644 --- a/app/Modules/Identity/Http/Middleware/EnforceTwoFactor.php +++ b/app/Modules/Identity/Http/Middleware/EnforceTwoFactor.php @@ -57,7 +57,12 @@ class EnforceTwoFactor // too. The loop then had no exit at all, which is how an // installation that made two-factor compulsory locked out // everybody who signs in with Microsoft. - if ($request->routeIs('two-factor.*', 'password.confirm*', 'password.edit', 'password.update', 'logout', 'locale.update')) { + // + // password.link, password.reset and password.store complete that + // same exit now that a provider account's first password arrives by + // email: asking for the link, opening it and saving it all happen + // while signed in, before there is a password to enrol with. + if ($request->routeIs('two-factor.*', 'password.confirm*', 'password.edit', 'password.update', 'password.link', 'password.reset', 'password.store', 'logout', 'locale.update')) { return $next($request); } diff --git a/resources/js/pages/settings/password.tsx b/resources/js/pages/settings/password.tsx index acd671b7..386fb81c 100644 --- a/resources/js/pages/settings/password.tsx +++ b/resources/js/pages/settings/password.tsx @@ -10,6 +10,7 @@ import { SaveButton } from '@/components/save-button'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; import { PasswordRequirements } from '@/components/password-requirements'; +import { Button } from '@/components/ui/button'; import { useTranslation } from '@/hooks/use-translation'; interface PasswordProps { @@ -17,9 +18,11 @@ interface PasswordProps { has_local_password: boolean; /** True for an account whose password lives in a directory, which this screen cannot change. */ managed_elsewhere: boolean; + /** 'password-link-sent' once the link for a first password has been emailed. */ + status?: string; } -export default function Password({ has_local_password, managed_elsewhere }: PasswordProps) { +export default function Password({ has_local_password, managed_elsewhere, status }: PasswordProps) { const { t } = useTranslation(); const breadcrumbs: BreadcrumbItem[] = [ @@ -38,6 +41,17 @@ export default function Password({ has_local_password, managed_elsewhere }: Pass password_confirmation: '', }); + // An account that signs in through a provider gets its first password + // from a link emailed to its own address, not from this form: the + // session alone must not be able to choose it. + const needsLink = !has_local_password && !managed_elsewhere; + const linkForm = useForm({}); + + const sendLink: FormEventHandler = (e) => { + e.preventDefault(); + linkForm.post(route('password.link'), { preserveScroll: true }); + }; + const updatePassword: FormEventHandler = (e) => { e.preventDefault(); @@ -79,7 +93,22 @@ export default function Password({ has_local_password, managed_elsewhere }: Pass

)} -