Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
82 changes: 82 additions & 0 deletions app/Console/Commands/ExpireRoles.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php

/*
* SPDX-FileCopyrightText: 2025 SecPal Contributors
*
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

declare(strict_types=1);

Comment on lines +9 to +10
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The declare(strict_types=1); directive is inconsistent with other console commands in the codebase. Other commands like GenerateTenantCommand, RotateDekCommand, and RotateKekCommand don't include this directive. For consistency, consider removing it to match the existing command pattern.

Suggested change
declare(strict_types=1);

Copilot uses AI. Check for mistakes.
namespace App\Console\Commands;

use App\Models\RoleAssignmentLog;
use App\Models\TemporalRoleUser;
use Illuminate\Console\Command;
use Illuminate\Support\Facades\DB;

/**
* Expire and revoke temporal role assignments that have passed their valid_until date.
*
* This command runs every minute via Laravel scheduler to automatically remove
* expired role assignments with auto_revoke=true, ensuring principle of least privilege.
*/
class ExpireRoles extends Command
{
/**
* The name and signature of the console command.
*/
protected $signature = 'roles:expire';

/**
* The console command description.
*/
protected $description = 'Expire and revoke temporal role assignments that have passed their valid_until date';

/**
* Execute the console command.
*/
public function handle(): int
{
// Find expired roles using the expired() scope
$expired = TemporalRoleUser::expired()->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++;
Comment on lines +55 to +74
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a potential race condition if multiple scheduler instances run simultaneously (though Laravel's scheduler should prevent this). If two processes fetch the same expired roles, both will create audit log entries before deletion, resulting in duplicate audit logs.

Consider using a database lock or checking the delete result:

DB::transaction(function () use ($assignment, &$count) {
    $deleted = 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();
    
    // Only log if we actually deleted something
    if ($deleted > 0) {
        RoleAssignmentLog::create([...]);
        $count++;
    }
});

Or reverse the order and use lockForUpdate() on the query.

Suggested change
// 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++;
// 1. Delete expired assignment using query (pivot has no primary key)
$deleted = 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();
// 2. Only log to immutable audit trail if we actually deleted something
if ($deleted > 0) {
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,
]);
$count++;
}

Copilot uses AI. Check for mistakes.
});
Comment on lines +41 to +75
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loading all expired roles into memory with get() could cause memory issues if there are many expired roles to process. Consider using chunk() or cursor() to process expired roles in batches:

TemporalRoleUser::expired()->chunk(100, function ($expired) use (&$count) {
    foreach ($expired as $assignment) {
        DB::transaction(function () use ($assignment, &$count) {
            // ... existing logic
        });
    }
});
Suggested change
// Find expired roles using the expired() scope
$expired = TemporalRoleUser::expired()->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++;
});
// Process expired roles in batches to avoid memory issues
$count = 0;
$found = false;
TemporalRoleUser::expired()->chunk(100, function ($expired) use (&$count, &$found) {
$found = true;
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++;
});
}
});
if (! $found) {
$this->info('No expired roles found.');
return self::SUCCESS;

Copilot uses AI. Check for mistakes.
}
Comment on lines +53 to +76
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a separate transaction for each expired role could be inefficient when processing many expired roles. Consider wrapping the entire foreach loop in a single transaction, or batch process the deletions. This would improve performance and reduce database overhead when multiple roles expire simultaneously.

DB::transaction(function () use ($expired, &$count) {
    foreach ($expired as $assignment) {
        // Log and delete operations here
        $count++;
    }
});

Copilot uses AI. Check for mistakes.

$this->info(sprintf('%d role(s) expired and revoked.', $count));

return self::SUCCESS;
}
}
6 changes: 5 additions & 1 deletion app/Models/TemporalRoleUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions routes/console.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
247 changes: 247 additions & 0 deletions tests/Feature/Console/ExpireRolesCommandTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
<?php

/*
* SPDX-FileCopyrightText: 2025 SecPal Contributors
*
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

use App\Models\RoleAssignmentLog;
use App\Models\TemporalRoleUser;
use App\Models\TenantKey;
use App\Models\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Artisan;
use Spatie\Permission\Models\Role;
use Spatie\Permission\PermissionRegistrar;

uses(RefreshDatabase::class);

describe('roles:expire Command', function () {
beforeEach(function () {
// Use process-specific KEK file for parallel test isolation
TenantKey::setKekPath(getTestKekPath());
TenantKey::generateKek();
$keys = TenantKey::generateEnvelopeKeys();
$this->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');
});
});