fix(admin): harden impersonation + datacenter code per Codex review

- normalize datacenter code before uniqueness validation
- ensureUser: race-safe create + refuse linking admin accounts
- impersonate start/leave are POST (CSRF-protected)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat/portal-design
nexxo 2026-07-25 13:50:16 +02:00
parent 879697c6ea
commit 4e86e135a6
6 changed files with 72 additions and 21 deletions

View File

@ -21,10 +21,14 @@ class Datacenters extends Component
public function save(): void public function save(): void
{ {
// Normalize before validating so the unique check matches how the row is
// stored — otherwise `FSN` passes the rule but the lowercased insert collides.
$this->code = strtolower(trim($this->code));
$data = $this->validate(); $data = $this->validate();
Datacenter::create([ Datacenter::create([
'code' => strtolower($data['code']), 'code' => $data['code'],
'name' => $data['name'], 'name' => $data['name'],
'location' => $data['location'] ?: null, 'location' => $data['location'] ?: null,
'active' => true, 'active' => true,

View File

@ -7,8 +7,10 @@ use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Database\UniqueConstraintViolationException;
use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Hash;
use Illuminate\Support\Str; use Illuminate\Support\Str;
use RuntimeException;
class Customer extends Model class Customer extends Model
{ {
@ -32,20 +34,49 @@ class Customer extends Model
return $this->hasMany(Instance::class); return $this->hasMany(Instance::class);
} }
/** Find or create the portal login account for this customer and link it. */ /**
* Find or create the portal login account for this customer and link it.
* Race-safe against the unique email index, and never links an operator
* account that would carry admin authorization into an impersonated
* "customer" session.
*/
public function ensureUser(): User public function ensureUser(): User
{ {
if ($this->user) { if ($this->user) {
return $this->user; return $this->user;
} }
$user = User::query()->firstOrCreate( if (($existing = User::query()->where('email', $this->email)->first()) !== null) {
['email' => $this->email], $this->assertNotAdmin($existing);
['name' => $this->name, 'password' => Hash::make(Str::random(40)), 'is_admin' => false], $this->update(['user_id' => $existing->id]);
);
return $existing;
}
try {
$user = User::query()->create([
'email' => $this->email,
'name' => $this->name,
'password' => Hash::make(Str::random(40)),
'is_admin' => false,
]);
} catch (UniqueConstraintViolationException) {
// Concurrent first-time creation — re-fetch and link the winner.
$user = User::query()->where('email', $this->email)->firstOrFail();
$this->assertNotAdmin($user);
}
$this->update(['user_id' => $user->id]); $this->update(['user_id' => $user->id]);
return $user->refresh(); return $user->refresh();
} }
private function assertNotAdmin(User $user): void
{
if ($user->is_admin) {
throw new RuntimeException(
"Refusing to link admin account {$user->email} as a portal login for customer {$this->id}.",
);
}
}
} }

View File

@ -13,10 +13,13 @@
{{-- Admin impersonation banner visible on every portal page until the admin returns. --}} {{-- Admin impersonation banner visible on every portal page until the admin returns. --}}
<div class="flex flex-wrap items-center justify-center gap-x-3 gap-y-1 border-b border-warning-border bg-warning-bg px-4 py-2 text-center text-sm font-medium text-ink"> <div class="flex flex-wrap items-center justify-center gap-x-3 gap-y-1 border-b border-warning-border bg-warning-bg px-4 py-2 text-center text-sm font-medium text-ink">
<span>{{ __('impersonate.banner', ['name' => auth()->user()->name]) }}</span> <span>{{ __('impersonate.banner', ['name' => auth()->user()->name]) }}</span>
<a href="{{ route('impersonate.leave') }}" class="inline-flex items-center gap-1 rounded-pill border border-warning-border px-3 py-0.5 font-semibold text-warning underline-offset-2 hover:underline"> <form method="POST" action="{{ route('impersonate.leave') }}" class="inline">
<x-ui.icon name="log-out" class="size-4" /> @csrf
{{ __('impersonate.leave') }} <button type="submit" class="inline-flex items-center gap-1 rounded-pill border border-warning-border px-3 py-0.5 font-semibold text-warning underline-offset-2 hover:underline">
</a> <x-ui.icon name="log-out" class="size-4" />
{{ __('impersonate.leave') }}
</button>
</form>
</div> </div>
@endif @endif
<div class="flex min-h-screen lg:h-screen lg:overflow-hidden"> <div class="flex min-h-screen lg:h-screen lg:overflow-hidden">

View File

@ -28,11 +28,14 @@
<td class="px-4 py-3 font-mono text-body">{{ $r['mrr'] }}</td> <td class="px-4 py-3 font-mono text-body">{{ $r['mrr'] }}</td>
<td class="px-4 py-3"><x-ui.badge :status="$r['status']">{{ __('admin.status.'.$r['status']) }}</x-ui.badge></td> <td class="px-4 py-3"><x-ui.badge :status="$r['status']">{{ __('admin.status.'.$r['status']) }}</x-ui.badge></td>
<td class="px-4 py-3 text-right"> <td class="px-4 py-3 text-right">
<a href="{{ route('admin.impersonate', $r['uuid']) }}" <form method="POST" action="{{ route('admin.impersonate', $r['uuid']) }}" class="inline">
class="inline-flex items-center gap-1.5 rounded-md border border-line px-3 py-1.5 text-xs font-semibold text-body hover:border-accent-border hover:text-accent-text"> @csrf
<x-ui.icon name="log-out" class="size-3.5" /> <button type="submit"
{{ __('admin.impersonate') }} class="inline-flex items-center gap-1.5 rounded-md border border-line px-3 py-1.5 text-xs font-semibold text-body hover:border-accent-border hover:text-accent-text">
</a> <x-ui.icon name="log-out" class="size-3.5" />
{{ __('admin.impersonate') }}
</button>
</form>
</td> </td>
</tr> </tr>
@empty @empty

View File

@ -46,7 +46,8 @@ Route::middleware('auth')->group(function () {
Route::get('/support', Support::class)->name('support'); Route::get('/support', Support::class)->name('support');
// Return from an admin impersonation session (accessible as the customer user). // Return from an admin impersonation session (accessible as the customer user).
Route::get('/impersonate/leave', [ImpersonationController::class, 'leave'])->name('impersonate.leave'); // POST so Laravel's CSRF middleware protects the identity change.
Route::post('/impersonate/leave', [ImpersonationController::class, 'leave'])->name('impersonate.leave');
}); });
// Admin / operator console — dark theme, gated to is_admin users (R1/R2, R13). // Admin / operator console — dark theme, gated to is_admin users (R1/R2, R13).
@ -58,7 +59,8 @@ Route::middleware(['auth', 'admin'])->prefix('admin')->name('admin.')->group(fun
Route::get('/hosts/create', Admin\HostCreate::class)->name('hosts.create'); Route::get('/hosts/create', Admin\HostCreate::class)->name('hosts.create');
Route::get('/hosts/{host}', Admin\HostDetail::class)->name('hosts.show'); Route::get('/hosts/{host}', Admin\HostDetail::class)->name('hosts.show');
Route::get('/datacenters', Admin\Datacenters::class)->name('datacenters'); Route::get('/datacenters', Admin\Datacenters::class)->name('datacenters');
Route::get('/impersonate/{customer}', [ImpersonationController::class, 'start'])->name('impersonate'); // POST so the identity change is CSRF-protected (a GET could be forced cross-site).
Route::post('/impersonate/{customer}', [ImpersonationController::class, 'start'])->name('impersonate');
Route::get('/provisioning', Admin\Provisioning::class)->name('provisioning'); Route::get('/provisioning', Admin\Provisioning::class)->name('provisioning');
Route::get('/revenue', Admin\Revenue::class)->name('revenue'); Route::get('/revenue', Admin\Revenue::class)->name('revenue');
}); });

View File

@ -10,7 +10,7 @@ it('lets an admin impersonate a customer and returns to the admin session', func
$customer = Customer::factory()->create(['email' => 'c@imp.test', 'name' => 'Imp Kunde']); $customer = Customer::factory()->create(['email' => 'c@imp.test', 'name' => 'Imp Kunde']);
$this->actingAs($admin) $this->actingAs($admin)
->get(route('admin.impersonate', $customer->uuid)) ->post(route('admin.impersonate', $customer->uuid))
->assertRedirect(route('dashboard')); ->assertRedirect(route('dashboard'));
$customer->refresh(); $customer->refresh();
@ -19,7 +19,7 @@ it('lets an admin impersonate a customer and returns to the admin session', func
->and(session('impersonator_id'))->toBe($admin->id); ->and(session('impersonator_id'))->toBe($admin->id);
// Return to the admin session. // Return to the admin session.
$this->get(route('impersonate.leave'))->assertRedirect(route('admin.overview')); $this->post(route('impersonate.leave'))->assertRedirect(route('admin.overview'));
expect(auth()->id())->toBe($admin->id) expect(auth()->id())->toBe($admin->id)
->and(session()->has('impersonator_id'))->toBeFalse(); ->and(session()->has('impersonator_id'))->toBeFalse();
@ -33,17 +33,25 @@ it('reuses an existing portal user with the same email', function () {
expect($customer->fresh()->user_id)->toBe($existing->id); expect($customer->fresh()->user_id)->toBe($existing->id);
}); });
it('refuses to link an admin account as a portal login', function () {
$adminUser = User::factory()->create(['email' => 'ops@imp.test', 'is_admin' => true]);
$customer = Customer::factory()->create(['email' => 'ops@imp.test']);
expect(fn () => $customer->ensureUser())->toThrow(RuntimeException::class);
expect($customer->fresh()->user_id)->toBeNull();
});
it('forbids a non-admin from impersonating', function () { it('forbids a non-admin from impersonating', function () {
$user = User::factory()->create(['is_admin' => false]); $user = User::factory()->create(['is_admin' => false]);
$customer = Customer::factory()->create(); $customer = Customer::factory()->create();
$this->actingAs($user) $this->actingAs($user)
->get(route('admin.impersonate', $customer->uuid)) ->post(route('admin.impersonate', $customer->uuid))
->assertForbidden(); ->assertForbidden();
}); });
it('gates impersonation start to authenticated users', function () { it('gates impersonation start to authenticated users', function () {
$customer = Customer::factory()->create(); $customer = Customer::factory()->create();
$this->get(route('admin.impersonate', $customer->uuid))->assertRedirect('/login'); $this->post(route('admin.impersonate', $customer->uuid))->assertRedirect('/login');
}); });