394 lines
21 KiB
PHP
394 lines
21 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)
|
|
// 17 from the original seed, plus customers.grant_plan.
|
|
->and(Permission::where('guard_name', 'operator')->count())->toBe(18)
|
|
->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('carries across an operator who holds a permission directly and no role at all', function () {
|
|
// operatorUserIds() used to look only at model_has_roles. A pre-existing
|
|
// console user granted console.view straight through model_has_permissions
|
|
// — never through any role — was invisible to that worklist. Meanwhile the
|
|
// permission's own guard is switched to `operator` a few lines above the
|
|
// worklist loop, so an account like this would silently lose console
|
|
// access while staying behind as a portal user, forever.
|
|
$hash = bcrypt('direkt-passwort');
|
|
$userId = DB::table('users')->insertGetId([
|
|
'name' => 'Direkt', 'email' => 'direkt@clupilot.test', 'password' => $hash,
|
|
'is_admin' => true, 'created_at' => now(), 'updated_at' => now(),
|
|
]);
|
|
$permissionId = DB::table('permissions')->where('name', 'console.view')->value('id');
|
|
DB::table('model_has_permissions')->insert([
|
|
'permission_id' => $permissionId, 'model_type' => User::class, 'model_id' => $userId,
|
|
]);
|
|
// Deliberately no model_has_roles row for this user.
|
|
|
|
(require base_path('database/migrations/2026_07_29_100000_move_rbac_to_operator_guard.php'))->up();
|
|
|
|
$operator = Operator::where('email', 'direkt@clupilot.test')->first();
|
|
|
|
expect($operator)->not->toBeNull()
|
|
->and($operator->roles()->count())->toBe(0)
|
|
->and($operator->can('console.view'))->toBeTrue()
|
|
->and(DB::table('model_has_permissions')
|
|
->where('model_type', Operator::class)->where('model_id', $operator->id)
|
|
->where('permission_id', $permissionId)->exists())->toBeTrue()
|
|
// Carried, and gone from `users` like any other operator — not left
|
|
// behind as a still-working portal login.
|
|
->and(DB::table('users')->where('email', 'direkt@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('preflights every customer conflict before mutating anything, listing all of them at once', function () {
|
|
// The customer-conflict check used to live only inside removeUserRow(),
|
|
// reached after permissions, roles, ownership ids and the four
|
|
// OWNED_COLUMNS constraints had already moved — none of which is
|
|
// transactional on MariaDB (see the class docblock), so that abort left a
|
|
// live server's currently-deployed (web-guard) code unable to
|
|
// authenticate anyone until a human resolved the conflict by hand. Two
|
|
// conflicting accounts, not one: proves the preflight collects everyone
|
|
// before refusing, rather than an operator fixing one conflict, running
|
|
// the migration again, finding the next one, and so on.
|
|
$ownerRoleId = DB::table('roles')->where('name', 'Owner')->value('id');
|
|
|
|
$customerA = Customer::factory()->create();
|
|
$userA = User::factory()->create(['email' => 'beides-a@clupilot.test']);
|
|
DB::table('customers')->where('id', $customerA->id)->update(['user_id' => $userA->id]);
|
|
|
|
$customerB = Customer::factory()->create();
|
|
$userB = User::factory()->create(['email' => 'beides-b@clupilot.test']);
|
|
DB::table('customers')->where('id', $customerB->id)->update(['user_id' => $userB->id]);
|
|
|
|
DB::table('model_has_roles')->insert([
|
|
['role_id' => $ownerRoleId, 'model_type' => User::class, 'model_id' => $userA->id],
|
|
['role_id' => $ownerRoleId, 'model_type' => User::class, 'model_id' => $userB->id],
|
|
]);
|
|
|
|
// Reset to the state up() actually starts from on a real server: nothing
|
|
// migrated yet. RefreshDatabase already moved these once, so without this
|
|
// reset "nothing was mutated" would read true no matter what this second
|
|
// run actually did.
|
|
DB::table('permissions')->where('guard_name', 'operator')->update(['guard_name' => 'web']);
|
|
DB::table('roles')->where('guard_name', 'operator')->update(['guard_name' => 'web']);
|
|
|
|
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-a@clupilot.test')
|
|
->and($e->getMessage())->toContain('beides-b@clupilot.test');
|
|
}
|
|
|
|
// Nothing mutated: still exactly the pre-migration shape, not "moved and
|
|
// then rolled back" — there is nothing here to roll back on a real
|
|
// server, which is the whole reason this has to be checked up front.
|
|
expect(Permission::where('guard_name', 'web')->count())->toBe(18)
|
|
->and(Permission::where('guard_name', 'operator')->count())->toBe(0)
|
|
->and(Role::where('guard_name', 'web')->count())->toBe(6)
|
|
->and(Role::where('guard_name', 'operator')->count())->toBe(0)
|
|
->and(DB::table('users')->where('id', $userA->id)->exists())->toBeTrue()
|
|
->and(DB::table('users')->where('id', $userB->id)->exists())->toBeTrue()
|
|
->and(Operator::where('email', 'beides-a@clupilot.test')->exists())->toBeFalse()
|
|
->and(Operator::where('email', 'beides-b@clupilot.test')->exists())->toBeFalse();
|
|
});
|
|
|
|
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();
|
|
});
|