From ce39f7437b020addbcddb5f8abefb53b27d7953b Mon Sep 17 00:00:00 2001 From: Holger Schmermbeck Date: Sat, 8 Nov 2025 23:42:41 +0100 Subject: [PATCH] feat(rbac): implement Phase 2 - Temporal Logic & Auto-Expiration (#106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements automatic expiration of time-limited role assignments with comprehensive audit trail and scheduled execution. **Core Features:** - roles:expire command runs every minute via Laravel scheduler - Only processes roles with auto_revoke=true flag - Logs all expirations to role_assignments_log before deletion - Transaction-based processing ensures data integrity - Timezone-aware (UTC storage) **Implementation:** - ExpireRoles command in app/Console/Commands/ - Scheduler config in routes/console.php (everyMinute) - Query-based deletion (pivot table has no primary key) - Uses TemporalRoleUser::expired() scope for filtering **Tests:** - 10 comprehensive tests (31 assertions) - Covers: expiration logic, audit logging, auto_revoke flag - Tests: active/future role preservation, timezone handling - Validates: batch processing, mixed scenarios **Bug Fixes:** - Migration: assigned_by changed from uuid() to foreignId() - TemporalRoleUser: Added PHPDoc for pivot properties - PHPStan Level 9 compliance **Quality Gates:** ✅ All 179 tests pass (513 assertions) ✅ PHPStan Level 9: No errors ✅ Laravel Pint: Code style compliant ✅ REUSE 3.3: Compliant **Related:** - Issue: #106 (RBAC Phase 2) - Epic: #5 (RBAC System) - ADR: ADR-004 (RBAC Architecture) - Depends on: Phase 1 (merged) - Blocks: Phase 3 (#107) - API Endpoints **Breaking Changes:** None **Migration Required:** Yes - run: php artisan migrate - adds scheduler requirement: * * * * * cd /path && php artisan schedule:run --- CHANGELOG.md | 9 + app/Console/Commands/ExpireRoles.php | 82 ++++++ app/Models/TemporalRoleUser.php | 6 +- ...poral_columns_to_model_has_roles_table.php | 2 +- routes/console.php | 4 + .../Console/ExpireRolesCommandTest.php | 247 ++++++++++++++++++ 6 files changed, 348 insertions(+), 2 deletions(-) create mode 100644 app/Console/Commands/ExpireRoles.php create mode 100644 tests/Feature/Console/ExpireRolesCommandTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index e19a7a2..9323b9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **RBAC Phase 2: Temporal Logic & Auto-Expiration** - Automatic expiration of time-limited role assignments (#106) + - `roles:expire` scheduled command runs every minute to revoke expired roles + - Only processes roles with `auto_revoke=true` flag set + - Logs all expirations to immutable `role_assignments_log` audit trail before deletion + - Transaction-based processing ensures data integrity (log → delete) + - Comprehensive test suite (10 tests): expiration logic, audit logging, timezone handling, batch processing + - Scheduler configuration in `routes/console.php` for automatic execution + - PHPDoc annotations for pivot model properties (PHPStan compliance) + - Database constraint fix: `assigned_by` column type changed from UUID to bigInteger (matches User table) - **RBAC Phase 2 Part 1: Audit Trail Infrastructure** - Immutable log for role assignment actions (#106) - `role_assignments_log` table migration with UUID primary key and bigInteger foreign keys - `RoleAssignmentLog` model with read-only enforcement (prevents updates and deletes) diff --git a/app/Console/Commands/ExpireRoles.php b/app/Console/Commands/ExpireRoles.php new file mode 100644 index 0000000..e3b8803 --- /dev/null +++ b/app/Console/Commands/ExpireRoles.php @@ -0,0 +1,82 @@ +get(); + + if ($expired->isEmpty()) { + $this->info('No expired roles found.'); + + return self::SUCCESS; + } + + $count = 0; + + // Process each expired assignment in a transaction for safety + foreach ($expired as $assignment) { + DB::transaction(function () use ($assignment, &$count) { + // 1. Log to immutable audit trail + RoleAssignmentLog::create([ + 'user_id' => $assignment->model_id, + 'role_id' => $assignment->role_id, + 'action' => 'expired', + 'valid_from' => $assignment->valid_from, + 'valid_until' => $assignment->valid_until, + 'assigned_by' => $assignment->assigned_by, + 'reason' => $assignment->reason, + ]); + + // 2. Delete expired assignment using query (pivot has no primary key) + DB::table('model_has_roles') + ->where('model_type', $assignment->model_type) + ->where('model_id', $assignment->model_id) + ->where('role_id', $assignment->role_id) + ->where('tenant_id', $assignment->tenant_id) + ->delete(); + + $count++; + }); + } + + $this->info(sprintf('%d role(s) expired and revoked.', $count)); + + return self::SUCCESS; + } +} diff --git a/app/Models/TemporalRoleUser.php b/app/Models/TemporalRoleUser.php index b1c890e..a3f7a69 100644 --- a/app/Models/TemporalRoleUser.php +++ b/app/Models/TemporalRoleUser.php @@ -20,10 +20,14 @@ * Extends Spatie's model_has_roles pivot table with temporal validity periods * and audit trail capabilities for time-limited role assignments. * + * @property int $role_id + * @property string $model_type + * @property int $model_id + * @property int $tenant_id * @property \Carbon\Carbon|null $valid_from * @property \Carbon\Carbon|null $valid_until * @property bool $auto_revoke - * @property string|null $assigned_by + * @property int|null $assigned_by * @property string|null $reason */ class TemporalRoleUser extends MorphPivot diff --git a/database/migrations/2025_11_08_143609_add_temporal_columns_to_model_has_roles_table.php b/database/migrations/2025_11_08_143609_add_temporal_columns_to_model_has_roles_table.php index 6fe3e9d..86549cb 100644 --- a/database/migrations/2025_11_08_143609_add_temporal_columns_to_model_has_roles_table.php +++ b/database/migrations/2025_11_08_143609_add_temporal_columns_to_model_has_roles_table.php @@ -24,7 +24,7 @@ public function up(): void $table->boolean('auto_revoke')->default(true)->after('valid_until'); // Audit trail columns - $table->uuid('assigned_by')->nullable()->after('auto_revoke'); + $table->foreignId('assigned_by')->nullable()->after('auto_revoke')->constrained('users')->onDelete('set null'); $table->text('reason')->nullable()->after('assigned_by'); // Standard timestamps diff --git a/routes/console.php b/routes/console.php index 489db00..47c8ea2 100644 --- a/routes/console.php +++ b/routes/console.php @@ -5,7 +5,11 @@ use Illuminate\Foundation\Inspiring; use Illuminate\Support\Facades\Artisan; +use Illuminate\Support\Facades\Schedule; Artisan::command('inspire', function () { $this->comment(Inspiring::quote()); })->purpose('Display an inspiring quote'); + +// Schedule: Expire temporal role assignments every minute +Schedule::command('roles:expire')->everyMinute(); diff --git a/tests/Feature/Console/ExpireRolesCommandTest.php b/tests/Feature/Console/ExpireRolesCommandTest.php new file mode 100644 index 0000000..5f5f453 --- /dev/null +++ b/tests/Feature/Console/ExpireRolesCommandTest.php @@ -0,0 +1,247 @@ +tenant = TenantKey::create($keys); + + // Get Permission Registrar for tenant context management + $this->registrar = app(PermissionRegistrar::class); + $this->registrar->setPermissionsTeamId($this->tenant->id); + + // Create test data + $this->user = User::factory()->create(); + $this->admin = User::factory()->create(); + $this->role = Role::create(['name' => 'manager']); + }); + + afterEach(function () { + // Reset tenant context after each test + $this->registrar->setPermissionsTeamId(null); + + // Cleanup test KEK file + cleanupTestKekFile(); + TenantKey::setKekPath(null); + }); + + it('identifies expired roles with auto_revoke=true', function () { + // Create expired role assignment with auto_revoke=true + assignTemporalRole($this->user, $this->role, $this->tenant->id, [ + 'valid_from' => now()->subDays(2), + 'valid_until' => now()->subDay(), + 'auto_revoke' => true, + 'assigned_by' => $this->admin->id, + 'reason' => 'Test expiration', + ]); + + // Verify assignment exists before command + expect(TemporalRoleUser::count())->toBe(1); + + // Run command + Artisan::call('roles:expire'); + + // Verify assignment was deleted + expect(TemporalRoleUser::count())->toBe(0); + }); + + it('does not delete expired roles with auto_revoke=false', function () { + // Create expired role assignment with auto_revoke=false + assignTemporalRole($this->user, $this->role, $this->tenant->id, [ + 'valid_from' => now()->subDays(2), + 'valid_until' => now()->subDay(), + 'auto_revoke' => false, + 'assigned_by' => $this->admin->id, + ]); + + // Verify assignment exists before command + expect(TemporalRoleUser::count())->toBe(1); + + // Run command + Artisan::call('roles:expire'); + + // Verify assignment still exists + expect(TemporalRoleUser::count())->toBe(1); + }); + + it('logs expired roles to audit trail before deletion', function () { + $validFrom = now()->subDays(2); + $validUntil = now()->subDay(); + + // Create expired role assignment + assignTemporalRole($this->user, $this->role, $this->tenant->id, [ + 'valid_from' => $validFrom, + 'valid_until' => $validUntil, + 'auto_revoke' => true, + 'assigned_by' => $this->admin->id, + 'reason' => 'Vacation coverage', + ]); + + // No audit logs before command + expect(RoleAssignmentLog::count())->toBe(0); + + // Run command + Artisan::call('roles:expire'); + + // Verify audit log was created + expect(RoleAssignmentLog::count())->toBe(1); + + $log = RoleAssignmentLog::first(); + expect($log->user_id)->toBe($this->user->id); + expect($log->role_id)->toBe($this->role->id); + expect($log->action)->toBe('expired'); + expect($log->assigned_by)->toBe($this->admin->id); + expect($log->reason)->toBe('Vacation coverage'); + expect($log->valid_from->toDateTimeString())->toBe($validFrom->toDateTimeString()); + expect($log->valid_until->toDateTimeString())->toBe($validUntil->toDateTimeString()); + }); + + it('does not affect active roles', function () { + // Create active role assignment + assignTemporalRole($this->user, $this->role, $this->tenant->id, [ + 'valid_from' => now()->subHour(), + 'valid_until' => now()->addHour(), + 'auto_revoke' => true, + ]); + + // Run command + Artisan::call('roles:expire'); + + // Verify assignment still exists + expect(TemporalRoleUser::count())->toBe(1); + expect(RoleAssignmentLog::count())->toBe(0); + }); + + it('does not affect future roles', function () { + // Create future role assignment + assignTemporalRole($this->user, $this->role, $this->tenant->id, [ + 'valid_from' => now()->addDay(), + 'valid_until' => now()->addDays(2), + 'auto_revoke' => true, + ]); + + // Run command + Artisan::call('roles:expire'); + + // Verify assignment still exists + expect(TemporalRoleUser::count())->toBe(1); + expect(RoleAssignmentLog::count())->toBe(0); + }); + + it('handles multiple expired roles in batch', function () { + // Create 3 expired roles + for ($i = 0; $i < 3; $i++) { + $user = User::factory()->create(); + assignTemporalRole($user, $this->role, $this->tenant->id, [ + 'valid_from' => now()->subDays(2), + 'valid_until' => now()->subDay(), + 'auto_revoke' => true, + ]); + } + + expect(TemporalRoleUser::count())->toBe(3); + + // Run command + Artisan::call('roles:expire'); + + // All expired roles should be deleted + expect(TemporalRoleUser::count())->toBe(0); + expect(RoleAssignmentLog::count())->toBe(3); + }); + + it('handles mixed scenarios (expired + active + no auto_revoke)', function () { + $user2 = User::factory()->create(); + $user3 = User::factory()->create(); + + // Expired with auto_revoke=true (should be deleted) + assignTemporalRole($this->user, $this->role, $this->tenant->id, [ + 'valid_from' => now()->subDays(2), + 'valid_until' => now()->subDay(), + 'auto_revoke' => true, + ]); + + // Expired with auto_revoke=false (should NOT be deleted) + assignTemporalRole($user2, $this->role, $this->tenant->id, [ + 'valid_from' => now()->subDays(2), + 'valid_until' => now()->subDay(), + 'auto_revoke' => false, + ]); + + // Active role (should NOT be deleted) + assignTemporalRole($user3, $this->role, $this->tenant->id, [ + 'valid_from' => now()->subHour(), + 'valid_until' => now()->addHour(), + 'auto_revoke' => true, + ]); + + expect(TemporalRoleUser::count())->toBe(3); + + // Run command + Artisan::call('roles:expire'); + + // Only auto_revoke=true expired role should be deleted + expect(TemporalRoleUser::count())->toBe(2); + expect(RoleAssignmentLog::count())->toBe(1); + + // Verify correct role was deleted (user1's expired role) + $remaining = TemporalRoleUser::pluck('model_id')->toArray(); + expect($remaining)->toContain($user2->id); + expect($remaining)->toContain($user3->id); + expect($remaining)->not->toContain($this->user->id); + }); + + it('handles timezone correctly (UTC storage)', function () { + // Assign role that expired 1 hour ago (in UTC) + assignTemporalRole($this->user, $this->role, $this->tenant->id, [ + 'valid_from' => now()->subDays(1), + 'valid_until' => now()->subHour(), + 'auto_revoke' => true, + ]); + + // Run command + Artisan::call('roles:expire'); + + // Should be deleted + expect(TemporalRoleUser::count())->toBe(0); + }); + + it('returns success message when roles are expired', function () { + assignTemporalRole($this->user, $this->role, $this->tenant->id, [ + 'valid_from' => now()->subDays(2), + 'valid_until' => now()->subDay(), + 'auto_revoke' => true, + ]); + + $exitCode = Artisan::call('roles:expire'); + + expect($exitCode)->toBe(0); + expect(Artisan::output())->toContain('expired and revoked'); + }); + + it('returns info message when no roles to expire', function () { + // No expired roles + $exitCode = Artisan::call('roles:expire'); + + expect($exitCode)->toBe(0); + expect(Artisan::output())->toContain('No expired roles found'); + }); +});