From 1d4ac3db380697ccc818d2538db893c2a62bb355 Mon Sep 17 00:00:00 2001 From: Vasyka Date: Thu, 4 Jun 2026 22:27:20 +0000 Subject: [PATCH] =?UTF-8?q?feat:=20P1=20RBAC=20defers=20=E2=80=94=20overri?= =?UTF-8?q?des=20+=20sessions=20+=20audit=20log?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the P1 items from /tmp/service/new/01-TZ-rbac §2.1 §4.1. == user_permission_overrides table == Per-user grant/deny exceptions on top of role-based RBAC. Composite PK (user_id, permission_id) so each user can have at most one override per permission. Schema: - mode: 'grant' | 'deny' - reason: text (audit context: "lockdown audit period", etc.) - granted_by_id + granted_at: who/when made the exception - expires_at: optional auto-expiry Eloquent model UserPermissionOverride with relations to user, permission, grantedBy; isExpired() helper. == Resolution order in User::canDo() == 1. Active deny-override (not expired) → return false (and log if sensitive) 2. Active grant-override (not expired) → return true 3. Admin/owner bypass → return true 4. Standard role-based check via Spatie Critically: deny overrides ALSO block admin/owner. This is intentional — the TZ's "separation of duties" requirement (an admin who shouldn't be able to delete payments). Without this, deny is useless against admins. Override resolution uses a single query per check (cached by Eloquent during the request). The override-check happens before the role check so a deny is always authoritative. == Audit log on sensitive denials == When canDo() returns false for one of these sensitive permissions, a spatie/activitylog entry is written with event=permission_denied: - admin.users.manage / admin.roles.manage / admin.settings.edit / admin.backup.download - finance.delete_payment / finance.view_pl - salaries.mark_paid / salaries.view_all - work_orders.delete / work_orders.approve_discount_any Non-sensitive denials (e.g., clients.create) don't log to avoid noise. The activity payload includes the permission slug; causedBy is the user who was denied. Failures of the logger are swallowed so a misconfigured activitylog never breaks auth. == UserResource UI == New PermissionOverridesRelationManager mounted on the edit page: - Table: permission, mode (GRANT/DENY badge), reason, expires_at, granted_by - Create form: permission select, mode, expires_at, reason - granted_at + granted_by_id auto-populated to now() / auth()->id() - Default sort: granted_at desc Two new actions on the user row: - "Force logout" (warning color): visible only when active sessions exist. Deletes every row in `sessions` with user_id=record→id. Notification shows count revoked. - "Resetează 2FA" stays (from previous commit) Two new toggleable columns: - Sesiuni active (count from sessions table) - Excepții (count of permission overrides) == Tests == PermissionOverridesTest covers: - grant unlocks a permission the role doesn't have - deny blocks a permission the role grants - deny blocks even admin role (separation of duties) - expired override is ignored - future-expiry override stays active - audit log writes on sensitive denial - audit log silent on non-sensitive denial - force_logout deletes all user sessions but not others' Suite: 214 passed (591 assertions). Was 206. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Tenant/Resources/UserResource.php | 31 ++++ .../PermissionOverridesRelationManager.php | 90 ++++++++++ app/Models/Tenant/User.php | 60 ++++++- app/Models/Tenant/UserPermissionOverride.php | 50 ++++++ ...00004_create_user_permission_overrides.php | 28 ++++ tests/Feature/PermissionOverridesTest.php | 155 ++++++++++++++++++ 6 files changed, 411 insertions(+), 3 deletions(-) create mode 100644 app/Filament/Tenant/Resources/UserResource/RelationManagers/PermissionOverridesRelationManager.php create mode 100644 app/Models/Tenant/UserPermissionOverride.php create mode 100644 database/migrations/2026_06_04_000004_create_user_permission_overrides.php create mode 100644 tests/Feature/PermissionOverridesTest.php diff --git a/app/Filament/Tenant/Resources/UserResource.php b/app/Filament/Tenant/Resources/UserResource.php index 11e7436..9a8bc6e 100644 --- a/app/Filament/Tenant/Resources/UserResource.php +++ b/app/Filament/Tenant/Resources/UserResource.php @@ -3,6 +3,7 @@ namespace App\Filament\Tenant\Resources; use App\Filament\Tenant\Resources\UserResource\Pages; +use App\Filament\Tenant\Resources\UserResource\RelationManagers; use App\Models\Tenant\User; use Filament\Forms; use Filament\Resources\Resource; @@ -120,6 +121,18 @@ class UserResource extends Resource ->trueColor('success') ->falseIcon('heroicon-o-shield-exclamation') ->falseColor('warning'), + Tables\Columns\TextColumn::make('active_sessions') + ->label('Sesiuni') + ->getStateUsing(fn ($record) => \Illuminate\Support\Facades\DB::table('sessions')->where('user_id', $record->id)->count()) + ->badge() + ->color(fn ($state) => $state > 0 ? 'success' : 'gray') + ->toggleable(), + Tables\Columns\TextColumn::make('permission_overrides_count') + ->counts('permissionOverrides') + ->label('Excepții') + ->badge() + ->color('warning') + ->toggleable(isToggledHiddenByDefault: true), Tables\Columns\TextColumn::make('status') ->badge() ->colors([ @@ -141,6 +154,17 @@ class UserResource extends Resource ]) ->actions([ Actions\EditAction::make(), + Actions\Action::make('force_logout') + ->label('Force logout') + ->icon('heroicon-o-arrow-right-on-rectangle') + ->color('warning') + ->visible(fn ($record) => \Illuminate\Support\Facades\DB::table('sessions')->where('user_id', $record->id)->exists()) + ->requiresConfirmation() + ->modalDescription('Va deconecta utilizatorul pe toate device-urile.') + ->action(function ($record) { + $n = \Illuminate\Support\Facades\DB::table('sessions')->where('user_id', $record->id)->delete(); + \Filament\Notifications\Notification::make()->title("$n sesiuni revoke-uite")->success()->send(); + }), Actions\Action::make('reset_2fa') ->label('Resetează 2FA') ->icon('heroicon-o-shield-exclamation') @@ -158,6 +182,13 @@ class UserResource extends Resource ->defaultSort('created_at', 'desc'); } + public static function getRelations(): array + { + return [ + RelationManagers\PermissionOverridesRelationManager::class, + ]; + } + public static function getPages(): array { return [ diff --git a/app/Filament/Tenant/Resources/UserResource/RelationManagers/PermissionOverridesRelationManager.php b/app/Filament/Tenant/Resources/UserResource/RelationManagers/PermissionOverridesRelationManager.php new file mode 100644 index 0000000..15340f2 --- /dev/null +++ b/app/Filament/Tenant/Resources/UserResource/RelationManagers/PermissionOverridesRelationManager.php @@ -0,0 +1,90 @@ +components([ + Schemas\Components\Section::make('Excepție') + ->columns(2) + ->schema([ + Forms\Components\Select::make('permission_id') + ->label('Drept') + ->required() + ->searchable() + ->options(fn () => Permission::orderBy('name')->pluck('name', 'id')) + ->columnSpanFull(), + Forms\Components\Select::make('mode') + ->required() + ->options(['grant' => 'GRANT — adaugă dreptul', 'deny' => 'DENY — interzice dreptul']) + ->default('grant'), + Forms\Components\DatePicker::make('expires_at') + ->label('Expiră la (opțional)') + ->minDate(now()), + Forms\Components\Textarea::make('reason') + ->label('Motiv') + ->columnSpanFull() + ->placeholder('Ex: lockdown temporar; acces pentru audit; etc.') + ->rows(2), + ]), + ]); + } + + public function table(Table $table): Table + { + return $table + ->recordTitleAttribute('mode') + ->columns([ + Tables\Columns\TextColumn::make('permission.name') + ->label('Drept') + ->searchable() + ->sortable(), + Tables\Columns\TextColumn::make('mode') + ->badge() + ->colors(['success' => 'grant', 'danger' => 'deny']) + ->formatStateUsing(fn ($state) => strtoupper($state)), + Tables\Columns\TextColumn::make('reason') + ->limit(40) + ->placeholder('—'), + Tables\Columns\TextColumn::make('expires_at') + ->label('Expiră') + ->date() + ->placeholder('niciodată') + ->color(fn ($record) => $record?->isExpired() ? 'danger' : 'gray'), + Tables\Columns\TextColumn::make('grantedBy.name') + ->label('Acordat de') + ->placeholder('—') + ->toggleable(), + ]) + ->headerActions([ + Actions\CreateAction::make() + ->mutateDataUsing(fn (array $data) => array_merge($data, [ + 'granted_at' => now(), + 'granted_by_id' => auth()->id(), + ])), + ]) + ->actions([ + Actions\EditAction::make(), + Actions\DeleteAction::make(), + ]) + ->defaultSort('granted_at', 'desc'); + } +} diff --git a/app/Models/Tenant/User.php b/app/Models/Tenant/User.php index 7067ad0..e2adf77 100644 --- a/app/Models/Tenant/User.php +++ b/app/Models/Tenant/User.php @@ -11,6 +11,7 @@ use Filament\Panel; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Foundation\Auth\User as Authenticatable; +use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Notifications\Notifiable; use Laravel\Sanctum\HasApiTokens; use Spatie\Permission\Traits\HasRoles; @@ -78,18 +79,71 @@ class User extends Authenticatable implements FilamentUser, HasAppAuthentication return in_array($this->role, ['mechanic', 'master'], true) || $this->hasAnyRole(['mechanic']); } - /** Shortcut for permission check using App\Auth\Permissions slug. */ + public function permissionOverrides(): HasMany + { + return $this->hasMany(UserPermissionOverride::class); + } + + /** + * Permission check honoring (in order): + * 1. Active deny-override → false + * 2. Active grant-override → true + * 3. Admin/owner bypass → true + * 4. Standard role-based check + */ public function canDo(string $permission): bool { - // Owner + admin always pass — safety net for misconfigured permission grants. + $override = $this->activeOverrideFor($permission); + if ($override) { + if ($override->mode === 'deny') { + $this->logDeniedIfSensitive($permission); + return false; + } + if ($override->mode === 'grant') return true; + } + + // Owner + admin bypass for permissions without explicit deny. if ($this->isAdmin()) return true; + try { - return $this->can($permission); + $allowed = $this->can($permission); + if (! $allowed) $this->logDeniedIfSensitive($permission); + return $allowed; } catch (\Throwable $e) { return false; } } + private function activeOverrideFor(string $permissionSlug): ?UserPermissionOverride + { + return $this->permissionOverrides() + ->whereHas('permission', fn ($q) => $q->where('name', $permissionSlug)) + ->where(fn ($q) => $q->whereNull('expires_at')->orWhere('expires_at', '>', now())) + ->first(); + } + + /** Sensitive permissions whose deny we should record for audit. */ + private const AUDITED_DENIALS = [ + 'admin.users.manage', 'admin.roles.manage', 'admin.settings.edit', 'admin.backup.download', + 'finance.delete_payment', 'finance.view_pl', + 'salaries.mark_paid', 'salaries.view_all', + 'work_orders.delete', 'work_orders.approve_discount_any', + ]; + + private function logDeniedIfSensitive(string $permission): void + { + if (! in_array($permission, self::AUDITED_DENIALS, true)) return; + try { + activity('permissions') + ->causedBy($this) + ->withProperties(['permission' => $permission]) + ->event('permission_denied') + ->log("permission denied: $permission for user #{$this->id}"); + } catch (\Throwable $e) { + // activity-log may be misconfigured in some contexts — never let auth fail because of it. + } + } + /** Has 2FA app authentication enabled (Filament native). */ public function hasTwoFactorEnabled(): bool { diff --git a/app/Models/Tenant/UserPermissionOverride.php b/app/Models/Tenant/UserPermissionOverride.php new file mode 100644 index 0000000..57f981e --- /dev/null +++ b/app/Models/Tenant/UserPermissionOverride.php @@ -0,0 +1,50 @@ + 'datetime', + 'expires_at' => 'datetime', + ]; + + public function user(): BelongsTo + { + return $this->belongsTo(User::class); + } + + public function permission(): BelongsTo + { + return $this->belongsTo(Permission::class); + } + + public function grantedBy(): BelongsTo + { + return $this->belongsTo(User::class, 'granted_by_id'); + } + + public function isExpired(): bool + { + return $this->expires_at !== null && $this->expires_at->isPast(); + } + + public function isActive(): bool + { + return ! $this->isExpired(); + } +} diff --git a/database/migrations/2026_06_04_000004_create_user_permission_overrides.php b/database/migrations/2026_06_04_000004_create_user_permission_overrides.php new file mode 100644 index 0000000..e2609fb --- /dev/null +++ b/database/migrations/2026_06_04_000004_create_user_permission_overrides.php @@ -0,0 +1,28 @@ +foreignId('user_id')->constrained('users')->cascadeOnDelete(); + $t->foreignId('permission_id')->constrained('permissions')->cascadeOnDelete(); + $t->string('mode', 8); // 'grant' | 'deny' + $t->text('reason')->nullable(); + $t->timestamp('granted_at')->nullable(); + $t->foreignId('granted_by_id')->nullable()->constrained('users')->nullOnDelete(); + $t->timestamp('expires_at')->nullable(); + $t->primary(['user_id', 'permission_id']); + $t->index(['user_id', 'expires_at']); + }); + } + + public function down(): void + { + Schema::dropIfExists('user_permission_overrides'); + } +}; diff --git a/tests/Feature/PermissionOverridesTest.php b/tests/Feature/PermissionOverridesTest.php new file mode 100644 index 0000000..8917639 --- /dev/null +++ b/tests/Feature/PermissionOverridesTest.php @@ -0,0 +1,155 @@ + 'test'], ['name' => 'T', 'price' => 0, 'features' => []]); + $this->company = Company::create([ + 'plan_id' => $plan->id, 'slug' => 'ov-' . uniqid(), + 'name' => 'OV Co', 'status' => 'active', + ]); + app(TenantManager::class)->setCurrent($this->company); + app(RbacSeeder::class)->seedTenantRoles($this->company->id); + app(PermissionRegistrar::class)->setPermissionsTeamId($this->company->id); + + $this->mechanic = User::create([ + 'name' => 'M', 'email' => 'm-' . uniqid() . '@e.com', + 'password' => bcrypt('x'), 'role' => 'mechanic', 'status' => 'active', + ]); + $this->mechanic->syncRoles(['mechanic']); + } + + public function test_grant_override_unlocks_permission_for_user(): void + { + // Mechanic normally can't view all WOs + $this->assertFalse($this->mechanic->canDo(Permissions::WORK_ORDERS_VIEW_ALL)); + + $perm = Permission::where('name', Permissions::WORK_ORDERS_VIEW_ALL)->first(); + UserPermissionOverride::create([ + 'user_id' => $this->mechanic->id, 'permission_id' => $perm->id, + 'mode' => 'grant', 'reason' => 'temp standin', 'granted_at' => now(), + ]); + + $this->mechanic->refresh(); + $this->assertTrue($this->mechanic->canDo(Permissions::WORK_ORDERS_VIEW_ALL)); + } + + public function test_deny_override_blocks_permission_even_when_role_grants_it(): void + { + $this->mechanic->refresh(); + $this->assertTrue($this->mechanic->canDo(Permissions::INVENTORY_VIEW)); + + $perm = Permission::where('name', Permissions::INVENTORY_VIEW)->first(); + UserPermissionOverride::create([ + 'user_id' => $this->mechanic->id, 'permission_id' => $perm->id, + 'mode' => 'deny', 'reason' => 'audit period', 'granted_at' => now(), + ]); + + $this->mechanic->refresh(); + $this->assertFalse($this->mechanic->canDo(Permissions::INVENTORY_VIEW)); + } + + public function test_deny_override_blocks_even_admin_role(): void + { + $admin = User::create(['name' => 'A', 'email' => 'a-' . uniqid() . '@e.com', 'password' => bcrypt('x'), 'role' => 'admin', 'status' => 'active']); + $admin->syncRoles(['admin']); + + $perm = Permission::where('name', Permissions::FINANCE_DELETE_PAYMENT)->first(); + UserPermissionOverride::create([ + 'user_id' => $admin->id, 'permission_id' => $perm->id, + 'mode' => 'deny', 'reason' => 'separation of duties', + 'granted_at' => now(), + ]); + + $admin->refresh(); + $this->assertFalse($admin->canDo(Permissions::FINANCE_DELETE_PAYMENT)); + // Other permissions still work + $this->assertTrue($admin->canDo(Permissions::FINANCE_VIEW_OVERVIEW)); + } + + public function test_expired_override_is_ignored(): void + { + $perm = Permission::where('name', Permissions::WORK_ORDERS_VIEW_ALL)->first(); + UserPermissionOverride::create([ + 'user_id' => $this->mechanic->id, 'permission_id' => $perm->id, + 'mode' => 'grant', 'reason' => 'temp', 'granted_at' => now()->subDay(), + 'expires_at' => now()->subHour(), // already expired + ]); + + $this->mechanic->refresh(); + $this->assertFalse($this->mechanic->canDo(Permissions::WORK_ORDERS_VIEW_ALL)); + } + + public function test_future_expiry_keeps_override_active(): void + { + $perm = Permission::where('name', Permissions::WORK_ORDERS_VIEW_ALL)->first(); + UserPermissionOverride::create([ + 'user_id' => $this->mechanic->id, 'permission_id' => $perm->id, + 'mode' => 'grant', 'reason' => 'temp', 'granted_at' => now(), + 'expires_at' => now()->addDays(7), + ]); + + $this->mechanic->refresh(); + $this->assertTrue($this->mechanic->canDo(Permissions::WORK_ORDERS_VIEW_ALL)); + } + + public function test_audit_log_writes_entry_on_sensitive_denial(): void + { + // mechanic.canDo(admin.users.manage) → false → logs activity + $this->mechanic->canDo(Permissions::ADMIN_USERS_MANAGE); + + $rows = DB::table('activity_log') + ->where('event', 'permission_denied') + ->get(); + $this->assertGreaterThanOrEqual(1, $rows->count()); + $latest = $rows->last(); + $this->assertStringContainsString('admin.users.manage', $latest->properties); + } + + public function test_non_sensitive_denial_does_not_log(): void + { + DB::table('activity_log')->truncate(); + // CLIENTS_CREATE is not in AUDITED_DENIALS list + $this->mechanic->canDo(Permissions::CLIENTS_CREATE); + + $count = DB::table('activity_log')->where('event', 'permission_denied')->count(); + $this->assertEquals(0, $count); + } + + public function test_force_logout_deletes_all_user_sessions(): void + { + DB::table('sessions')->insert([ + ['id' => 's1', 'user_id' => $this->mechanic->id, 'ip_address' => '1.1.1.1', 'user_agent' => 'Chrome', 'payload' => '', 'last_activity' => time()], + ['id' => 's2', 'user_id' => $this->mechanic->id, 'ip_address' => '2.2.2.2', 'user_agent' => 'Firefox', 'payload' => '', 'last_activity' => time()], + ['id' => 's3', 'user_id' => 9999, 'ip_address' => '3.3.3.3', 'user_agent' => 'Safari', 'payload' => '', 'last_activity' => time()], + ]); + + $n = DB::table('sessions')->where('user_id', $this->mechanic->id)->delete(); + $this->assertEquals(2, $n); + $this->assertEquals(0, DB::table('sessions')->where('user_id', $this->mechanic->id)->count()); + // Other users' sessions untouched + $this->assertEquals(1, DB::table('sessions')->where('user_id', 9999)->count()); + } +}