Carry operators into the RBAC move via direct permissions too, not only roles
operatorUserIds() only discovered console users through model_has_roles. A
pre-existing account granted console.view straight through
model_has_permissions, holding no role at all, was invisible to the
worklist — meanwhile the permission's own guard is switched to `operator`
a few lines above, so that account would silently lose console access
while staying behind as a working portal login. Worklist is now the
deduplicated, sorted union of both tables; orderBy('model_id') kept on
each side since carryAcross() remaps in place and the ordering still
matters regardless of which table an id came from.
feat/operator-identity
parent
2b2bb439a5
commit
4ae5281c91
|
|
@ -123,30 +123,46 @@ return new class extends Migration
|
|||
}
|
||||
|
||||
/**
|
||||
* Every `users` row that currently holds a console role and has not yet
|
||||
* been carried across to `operators`.
|
||||
* Every `users` row that currently holds a console role, a directly
|
||||
* granted console permission, or both — and has not yet been carried
|
||||
* across to `operators`.
|
||||
*
|
||||
* Ordered by model_id: carryAcross() below remaps the four OWNED_COLUMNS
|
||||
* in place, keyed on the OLD users.id, while operators.id — auto-
|
||||
* increment — hands out its values in exactly the order this list is
|
||||
* Spatie allows a permission to be granted straight to a model with no
|
||||
* role in between. Reading only model_has_roles missed exactly that
|
||||
* case: an account like that kept working right up until this migration
|
||||
* flips the permission's own guard to `operator` a few lines above, at
|
||||
* which point it silently lost console access while staying behind as a
|
||||
* portal user — nothing downstream ever noticed, because carryAcross()
|
||||
* was never asked to look at it.
|
||||
*
|
||||
* Both queries keep `orderBy('model_id')`, and the merged, deduplicated
|
||||
* result is sorted again in PHP — the ordering matters regardless of
|
||||
* which table a given id came from: carryAcross() below remaps the four
|
||||
* OWNED_COLUMNS in place, keyed on the OLD users.id, while operators.id —
|
||||
* auto-increment — hands out its values in exactly the order this list is
|
||||
* walked. Unordered, a later operator's freshly assigned operators.id
|
||||
* can equal an earlier operator's still-unprocessed users.id, and the
|
||||
* second remap's `WHERE user_id = X` then catches a row the first remap
|
||||
* already rewrote to that same X. Collision-free today only because
|
||||
* MariaDB happens to satisfy this query from an index that already
|
||||
* returns rows in ascending model_id order; forcing it to answer from
|
||||
* PRIMARY (role_id, model_id, model_type) instead returns them out of
|
||||
* order, and the collision hands one operator another operator's VPN
|
||||
* peer — silently, since both updates report success.
|
||||
* MariaDB happens to satisfy the model_has_roles query from an index
|
||||
* that already returns rows in ascending model_id order; forcing it to
|
||||
* answer from PRIMARY (role_id, model_id, model_type) instead returns
|
||||
* them out of order, and the collision hands one operator another
|
||||
* operator's VPN peer — silently, since both updates report success.
|
||||
*/
|
||||
private function operatorUserIds(): array
|
||||
{
|
||||
return DB::table('model_has_roles')
|
||||
$viaRoles = DB::table('model_has_roles')
|
||||
->where('model_type', User::class)
|
||||
->orderBy('model_id')
|
||||
->pluck('model_id')
|
||||
->unique()
|
||||
->all();
|
||||
->pluck('model_id');
|
||||
|
||||
$viaPermissions = DB::table('model_has_permissions')
|
||||
->where('model_type', User::class)
|
||||
->orderBy('model_id')
|
||||
->pluck('model_id');
|
||||
|
||||
return $viaRoles->merge($viaPermissions)->unique()->sort()->values()->all();
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -56,6 +56,39 @@ it('carries an existing operator user across with password and two-factor intact
|
|||
->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
|
||||
|
|
|
|||
Loading…
Reference in New Issue