Files
ignacionelson 83a8fe2288 Claim the installation instead of checking whether it is free
Reported by @ry2811 as GHSA-w3w9-prpw-qx77, with a working two-worker
reproducer.

Setup asked the database whether any staff user existed, and created one
some time later, in a separate statement with nothing joining the two. So
two POSTs arriving together both read "no staff" and both inserted a System
Administrator. Different addresses do not collide; `users.email` is the only
unique key and it has nothing to say about there being one first
administrator.

The gap is not narrow. Between the check and the insert sits password
hashing at BCRYPT_ROUNDS=12, which is slow on purpose, so the window is
hundreds of milliseconds wide and observable without trying.

What makes this worth fixing is not that a stranger can set up an
unconfigured installation — first-run setup is open to whoever reaches it
first, and always was. It is that racing the operator is *quiet*. The
operator's own request also succeeds, also redirects to /setup/success, and
the installation they get looks exactly like the one they expected. The
second administrator is discovered later or not at all, and closing setup
afterwards does not revoke it.

FirstAdministrator::claim() makes it one operation. The row it locks is the
System Administrator role, because the obvious candidate cannot work: there
are no staff rows on a fresh install and a lock over an empty result
serialises nothing. That role row is written by the roles migration and
rewritten on every boot, so it is always there to be locked. The second
caller waits on it, and by the time it has the lock the first caller's user
is committed and visible to the re-check it then makes.

Everything the request writes moved inside the claim, including the site
name. A request that loses now writes nothing at all, rather than renaming
the installation on its way to the login screen.

`projectsend:admin --if-none` had the same shape and is fixed the same way
— two containers coming up against one database is the version of this that
needs no attacker. The early check stays where it is so an unattended boot
does not prompt for a password it is about to discard; it is simply asked
again under the lock.

Both tests fail on the unfixed code. They stage the interleaving rather than
attempting real concurrency, creating the winning administrator from a query
listener after the request has made its first check — which is exactly the
window, and the re-check is the only thing that closes it. The lock itself
is invisible to them: the suite runs SQLite, where lockForUpdate() compiles
to nothing. That half was verified against MySQL 8.4 by running the
reporter's race for real, two processes through the full HTTP kernel: two
administrators before, one after, repeatably.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CNFU55Tkq6MuEQ73nbbBRx
2026-09-11 11:34:26 -03:00

146 lines
5.6 KiB
PHP

<?php
declare(strict_types=1);
namespace App\Modules\Identity\Http\Controllers;
use App\Http\Controllers\Controller;
use App\Models\User;
use App\Modules\Audit\Action;
use App\Modules\Audit\ActivityLogger;
use App\Modules\Identity\FirstAdministrator;
use App\Modules\Identity\Models\Role;
use App\Modules\Identity\Permissions\SystemRole;
use App\Modules\Identity\UserType;
use App\Modules\Platform\Onboarding\InstallationWelcome;
use App\Modules\Platform\Settings\Setting;
use App\Modules\Platform\Settings\Settings;
use Illuminate\Http\RedirectResponse;
use Illuminate\Http\Request;
use Illuminate\Validation\Rules\Password;
use Inertia\Inertia;
use Inertia\Response;
/**
* First-run setup: create the initial staff administrator. Only reachable
* while no staff user exists; afterwards the routes bounce home.
*/
class SetupController extends Controller
{
public function __construct(
private readonly Settings $settings,
private readonly ActivityLogger $activity,
private readonly InstallationWelcome $welcome,
) {}
public function show(): Response|RedirectResponse
{
if ($this->setupIsComplete()) {
return redirect()->route('home');
}
return Inertia::render('setup');
}
public function store(Request $request): RedirectResponse
{
if ($this->setupIsComplete()) {
return redirect()->route('home');
}
$validated = $request->validate([
'site_name' => ['required', 'string', 'max:255'],
'name' => ['required', 'string', 'max:255'],
'email' => ['required', 'string', 'lowercase', 'email', 'max:255'],
'password' => ['required', 'confirmed', Password::defaults()],
]);
// The check above is not enough on its own: it is a plain read, and
// between it and the insert a second setup request can do the same
// read and insert an administrator of its own. Everything this
// request writes therefore happens inside the claim, so a request
// that loses the race writes nothing at all — not the site name
// either. See FirstAdministrator.
$admin = FirstAdministrator::claim(
fn (): bool => ! $this->setupIsComplete(),
function () use ($validated): User {
$this->settings->set(Setting::SiteName, $validated['site_name']);
$admin = User::create([
'type' => UserType::Staff,
'active' => true,
'role_id' => Role::query()->where('name', SystemRole::SystemAdministrator->value)->value('id'),
'name' => $validated['name'],
'email' => $validated['email'],
'password' => $validated['password'],
]);
// forceFill, not part of the create() array:
// email_verified_at is deliberately absent from
// User::$fillable, so mass assignment dropped it in silence
// and this account was never marked verified. The first
// administrator typed their own address into the form in
// front of them; there is nobody to confirm it to. (Inert
// today, since MustVerifyEmail is not enabled on the model,
// but the column is what a later switch would read.)
$admin->forceFill(['email_verified_at' => now()])->save();
return $admin;
},
);
// Somebody else finished setup while this request was in flight.
// Theirs is the administrator that exists; this one is sent to the
// login screen like any other visitor to an installed site.
if ($admin === null) {
return redirect()->route('home');
}
// v1 logged installation as action 0; setup is a recorded action.
$this->activity->log(Action::SetupCompleted, $admin);
$this->activity->log(Action::UserCreated, $admin, $admin);
if ($this->settings->get(Setting::AdminNotificationEmails) === []) {
$this->settings->set(Setting::AdminNotificationEmails, [$admin->email]);
}
// They will be shown around the first time they sign in — which is
// the next thing that happens, since setup deliberately does not
// log anybody in.
$this->welcome->raise();
// Deliberately no auto-login: the new administrator proves their
// credentials at the login form, which also confirms they work.
return redirect()->route('setup.success')->with('setup_completed', true);
}
public function success(Request $request): Response|RedirectResponse
{
if (! $this->setupIsComplete()) {
return redirect()->route('setup');
}
if (! $request->session()->get('setup_completed')) {
return redirect()->route('login');
}
return Inertia::render('setup-success');
}
/**
* Trashed staff count, for the reason EnsureSetupIsComplete gives:
* this asks whether the installation was ever set up, and store()
* below is the door a stranger walks through if the answer is wrong.
* The middleware and this must agree — one of them saying "not set
* up" while the other says "set up" is either a redirect loop or an
* open form.
*
* @phpstan-impure asking twice can honestly give two answers, which is
* the entire reason store() asks a second time under a lock
*/
private function setupIsComplete(): bool
{
return User::query()->withTrashed()->where('type', UserType::Staff)->exists();
}
}