CluPilotCloud/tests/Feature/Admin/RbacMoveTest.php

307 lines
16 KiB
PHP

<?php
use App\Models\Customer;
use App\Models\Operator;
use App\Models\User;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Str;
use Spatie\Permission\Models\Permission;
use Spatie\Permission\Models\Role;
it('moves every permission and role to the operator guard, leaving none behind', function () {
expect(Permission::where('guard_name', 'web')->count())->toBe(0)
->and(Role::where('guard_name', 'web')->count())->toBe(0)
->and(Permission::where('guard_name', 'operator')->count())->toBe(17)
->and(Role::where('guard_name', 'operator')->count())->toBe(6);
});
it('keeps Owner holding every permission after the move', function () {
$owner = Role::findByName('Owner', 'operator');
expect($owner->permissions()->count())->toBe(Permission::count());
});
it('lets an operator use a console capability and a user not', function () {
$operator = Operator::factory()->role('Owner')->create();
expect($operator->can('console.view'))->toBeTrue()
->and(method_exists(User::factory()->create(), 'hasRole'))->toBeFalse();
});
it('carries an existing operator user across with password and two-factor intact', function () {
// Simulates the pre-migration state on a live server.
$hash = bcrypt('altes-passwort');
DB::table('users')->insert([
'name' => 'Alt', 'email' => 'alt@clupilot.test', 'password' => $hash,
'two_factor_secret' => 'geheimnis', 'is_admin' => true,
'created_at' => now(), 'updated_at' => now(),
]);
$userId = DB::table('users')->where('email', 'alt@clupilot.test')->value('id');
DB::table('model_has_roles')->insert([
'role_id' => DB::table('roles')->where('name', 'Owner')->value('id'),
'model_type' => User::class, 'model_id' => $userId,
]);
// Re-run only the move migration against this hand-built state.
(require base_path('database/migrations/2026_07_29_100000_move_rbac_to_operator_guard.php'))->up();
$operator = Operator::where('email', 'alt@clupilot.test')->first();
expect($operator)->not->toBeNull()
->and($operator->password)->toBe($hash)
->and($operator->two_factor_secret)->toBe('geheimnis')
->and($operator->hasRole('Owner'))->toBeTrue()
// Not mixed: the users row is gone.
->and(DB::table('users')->where('email', 'alt@clupilot.test')->exists())->toBeFalse();
});
it('refuses to delete a users row that has a customer on it', function () {
// `users` has no `customer_id` column (checked against the real schema) —
// the link runs the other way, `customers.user_id`, exactly what
// Customer::ensureUser() writes when it attaches a portal login.
$customer = Customer::factory()->create();
$user = User::factory()->create(['email' => 'beides@clupilot.test']);
DB::table('customers')->where('id', $customer->id)->update(['user_id' => $user->id]);
DB::table('model_has_roles')->insert([
'role_id' => DB::table('roles')->where('name', 'Owner')->value('id'),
'model_type' => User::class, 'model_id' => $user->id,
]);
// Aborting is the point: silently deleting would take billing data with it.
try {
(require base_path('database/migrations/2026_07_29_100000_move_rbac_to_operator_guard.php'))->up();
$this->fail('The migration should have refused.');
} catch (RuntimeException $e) {
expect($e->getMessage())->toContain('beides@clupilot.test');
}
});
it('remaps vpn_peers and maintenance_windows ownership to the new operator, not just the role tables', function () {
// Simulates the live server this was written for: an operator with a VPN
// peer per device and at least one scheduled maintenance window. Both
// columns are foreign keys into `users` that have only ever held an
// operator's id — proving they end up on the new operators.id, and not
// null, is the only test that actually exercises the live case; a fresh
// database has no rows in either table for the migration to get wrong.
//
// A throwaway operator goes first, purely to consume operators.id=1: the
// in-memory test database restarts both the users and operators id
// sequences near 1 every test, so without this, netzwerk@clupilot.test's
// users.id and its own new operators.id would coincide — and a remap
// that was skipped entirely would still leave user_id/created_by
// "correct" by coincidence rather than by the migration doing its job.
// (Caught this exact way, by mutation: the first version of this test had
// no such guard and stayed green with the remap commented out.)
Operator::factory()->create(); // consumes operators.id — see the comment above.
$hash = bcrypt('netzwerk-passwort');
DB::table('users')->insert([
'name' => 'Netzwerk-Owner', 'email' => 'netzwerk@clupilot.test', 'password' => $hash,
'is_admin' => true, 'created_at' => now(), 'updated_at' => now(),
]);
$userId = DB::table('users')->where('email', 'netzwerk@clupilot.test')->value('id');
DB::table('model_has_roles')->insert([
'role_id' => DB::table('roles')->where('name', 'Owner')->value('id'),
'model_type' => User::class, 'model_id' => $userId,
]);
// RefreshDatabase already migrated once, so vpn_peers.user_id/created_by
// and maintenance_windows.created_by are already constrained against
// `operators` — the very thing this migration is supposed to arrange.
// Hand-building the LEGACY shape (a users.id sitting in those columns)
// means briefly turning that constraint off, exactly as it would have
// been before this migration ever ran on a real server. Not
// Schema::disableForeignKeyConstraints() — SQLite's `PRAGMA foreign_keys`
// is a documented no-op while a transaction is open, and RefreshDatabase
// always has one open. `defer_foreign_keys` is the pragma meant for
// exactly this: it can be toggled mid-transaction and only checks at
// commit — which this test's wrapping transaction never reaches.
DB::statement('PRAGMA defer_foreign_keys = ON');
DB::table('vpn_peers')->insert([
'uuid' => (string) Str::uuid(), 'name' => 'Notebook', 'kind' => 'staff',
'user_id' => $userId, 'created_by' => $userId,
'public_key' => 'PUBKEYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=',
'allowed_ip' => '10.66.0.199', 'enabled' => true, 'present' => false,
'created_at' => now(), 'updated_at' => now(),
]);
DB::table('maintenance_windows')->insert([
'uuid' => (string) Str::uuid(), 'title' => 'Netz-Wartung',
'starts_at' => now()->addDay(), 'ends_at' => now()->addDay()->addHours(2),
'state' => 'draft', 'created_by' => $userId,
'created_at' => now(), 'updated_at' => now(),
]);
(require base_path('database/migrations/2026_07_29_100000_move_rbac_to_operator_guard.php'))->up();
$operatorId = Operator::where('email', 'netzwerk@clupilot.test')->value('id');
expect($operatorId)->not->toBeNull()
// The decoy operator above is what makes this a real assertion: it
// took operators.id 1, so netzwerk's own operator cannot be 1 too —
// the id genuinely has to have changed, not merely be numerically
// unchanged by coincidence.
->and($operatorId)->not->toBe($userId);
$peer = DB::table('vpn_peers')->where('name', 'Notebook')->first();
expect($peer->user_id)->not->toBeNull()->and($peer->user_id)->toBe($operatorId)
->and($peer->created_by)->not->toBeNull()->and($peer->created_by)->toBe($operatorId);
$window = DB::table('maintenance_windows')->where('title', 'Netz-Wartung')->first();
expect($window->created_by)->not->toBeNull()->and($window->created_by)->toBe($operatorId);
});
it('finishes the move on a second run after a customer conflict is resolved by hand, instead of quietly no-opping', function () {
// Reproduces the exact live-server sequence this migration's own docblock
// promises: run 1 aborts on the customer conflict (by design), the
// operator resolves it by hand exactly as the exception says to, and a
// second run of the SAME migration is expected to actually finish the
// job — not silently succeed while leaving every `users` row it was
// supposed to remove still sitting there.
//
// Before the fix, operatorUserIds() drove BOTH which `users` rows to
// carry across AND which to delete. carryAcross() repoints
// model_has_roles to Operator for EVERY operator in the first loop,
// before a single deletion is attempted — so by the time run 2 starts,
// model_has_roles has nothing left with model_type=User, the removal
// loop iterates zero times, and `migrate` marks the migration applied
// with BOTH users rows still there: a live portal login for an operator,
// on their operator password.
$ownerRoleId = DB::table('roles')->where('name', 'Owner')->value('id');
// Given the smaller users.id so it is the first one removeUserRow()
// reaches once the worklist is ordered (see the ordering test below) —
// matching the live repro, where the conflicted operator's removal
// aborted before ANY other operator's users row was touched.
$conflictedId = DB::table('users')->insertGetId([
'name' => 'Beides', 'email' => 'beides@clupilot.test', 'password' => bcrypt('beides-passwort'),
'is_admin' => true, 'created_at' => now(), 'updated_at' => now(),
]);
$plainId = DB::table('users')->insertGetId([
'name' => 'Nur-Operator', 'email' => 'nur-operator@clupilot.test', 'password' => bcrypt('nur-operator-passwort'),
'is_admin' => true, 'created_at' => now(), 'updated_at' => now(),
]);
DB::table('model_has_roles')->insert([
['role_id' => $ownerRoleId, 'model_type' => User::class, 'model_id' => $conflictedId],
['role_id' => $ownerRoleId, 'model_type' => User::class, 'model_id' => $plainId],
]);
$customer = Customer::factory()->create();
DB::table('customers')->where('id', $customer->id)->update(['user_id' => $conflictedId]);
// Run 1: aborts on the conflict, exactly as designed.
try {
(require base_path('database/migrations/2026_07_29_100000_move_rbac_to_operator_guard.php'))->up();
$this->fail('The migration should have refused.');
} catch (RuntimeException $e) {
expect($e->getMessage())->toContain('beides@clupilot.test');
}
// Both survive run 1 — including the operator who was never in
// conflict, which is the bug this test guards against.
expect(DB::table('users')->where('id', $conflictedId)->exists())->toBeTrue()
->and(DB::table('users')->where('id', $plainId)->exists())->toBeTrue();
// Resolved by hand, exactly as the exception instructs.
DB::table('customers')->where('id', $customer->id)->update(['user_id' => null]);
// Run 2 must not silently no-op: it has to actually finish the move.
(require base_path('database/migrations/2026_07_29_100000_move_rbac_to_operator_guard.php'))->up();
expect(DB::table('users')->where('id', $conflictedId)->exists())->toBeFalse()
->and(DB::table('users')->where('id', $plainId)->exists())->toBeFalse();
$beides = Operator::where('email', 'beides@clupilot.test')->first();
$nurOperator = Operator::where('email', 'nur-operator@clupilot.test')->first();
expect($beides)->not->toBeNull()->and($beides->hasRole('Owner'))->toBeTrue()
->and($nurOperator)->not->toBeNull()->and($nurOperator->hasRole('Owner'))->toBeTrue();
// The exact failure mode this bug allowed: a leftover `users` row that
// Auth::guard('web') would still accept, on the operator's own password.
expect(Auth::guard('web')->attempt(['email' => 'beides@clupilot.test', 'password' => 'beides-passwort']))->toBeFalse();
});
it('orders operatorUserIds so a remapped operators.id cannot collide with an unprocessed users.id', function () {
// The consequence of an unordered worklist (I3): a later operator's
// freshly assigned operators.id can equal an earlier operator's
// still-unprocessed users.id, and the second UPDATE ... WHERE user_id = X
// then catches a row the first one already rewrote — one operator
// silently inherits another operator's VPN peer, and
// VpnPeerPolicy::downloadConfig is owner-only, so it would hand over
// someone else's private key.
//
// Not testable end-to-end on SQLite: `model_has_roles` carries a
// covering index on (model_id, model_type), and the query this migration
// runs — WHERE model_type = ? — is satisfied entirely from that index
// regardless of insertion order or of whether ->orderBy('model_id') is
// present in the code. SQLite always hands back ascending model_id
// "by luck", exactly as the finding says MariaDB does normally; MariaDB
// only reveals the bug when forced onto its PRIMARY index instead. So
// this asserts the one thing that actually distinguishes the fix on this
// engine: the SQL sent to the database carries an explicit ORDER BY.
$migration = require base_path('database/migrations/2026_07_29_100000_move_rbac_to_operator_guard.php');
$queries = [];
DB::listen(function ($query) use (&$queries) {
$queries[] = $query->sql;
});
(new ReflectionMethod($migration, 'operatorUserIds'))->invoke($migration);
$worklistQueries = array_values(array_filter(
$queries,
fn (string $sql) => str_contains($sql, 'model_has_roles') && str_starts_with(strtolower($sql), 'select'),
));
expect($worklistQueries)->not->toBeEmpty();
expect(strtolower(end($worklistQueries)))->toContain('order by');
});
it('restores direct permissions on rollback, not only role-granted ones', function () {
// The forward move carries BOTH model_has_roles and model_has_permissions
// (carryAcross() above) — down() only restored the first, so an operator
// holding a permission directly (never through any role) silently lost it
// on a rollback.
$operator = Operator::factory()->role('Read-only')->create();
// 'hosts.manage' is not part of Read-only's set (see the seed migration's
// matrix) — granted directly, so this is only present because of the
// direct grant, not because of the role.
$operator->givePermissionTo('hosts.manage');
(require base_path('database/migrations/2026_07_29_100000_move_rbac_to_operator_guard.php'))->down();
$userId = DB::table('users')->where('email', $operator->email)->value('id');
$permissionId = DB::table('permissions')->where('name', 'hosts.manage')->value('id');
expect($userId)->not->toBeNull()
->and(DB::table('model_has_permissions')
->where('model_type', User::class)->where('model_id', $userId)
->where('permission_id', $permissionId)
->exists())->toBeTrue();
});
it('survives a deep rollback and re-migrate without fatal-ing on a stale User role reference', function () {
// seed_roles_and_permissions used to bootstrap legacy is_admin users into
// the Owner role via User::OPERATOR_ROLES / hasAnyRole() / assignRole()
// — all three gone from User now (see R21; Operator::OPERATOR_ROLES is
// the current home for the constant). Harmless on an ordinary `migrate`
// (that loop only ever ran historically, back when User still had them)
// and on a fresh install (no is_admin users exist yet at that point in
// the migration order) — but move_rbac_to_operator_guard's own down()
// recreates `users` rows with is_admin => true, so a deep rollback
// through both migrations followed by `migrate` used to fatal on the
// very first is_admin row it found.
$rbacMove = require base_path('database/migrations/2026_07_29_100000_move_rbac_to_operator_guard.php');
$seedRoles = require base_path('database/migrations/2026_07_25_133900_seed_roles_and_permissions.php');
Operator::factory()->role('Owner')->create();
$rbacMove->down(); // recreates a `users` row with is_admin => true
$seedRoles->down(); // deletes the roles/permissions this migration owns
$seedRoles->up(); // must not fatal on the stale reference
expect(DB::table('roles')->where('name', 'Owner')->where('guard_name', 'web')->exists())->toBeTrue();
});