diff --git a/CHANGELOG.md b/CHANGELOG.md index 06d53e8..4ef2d6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **User Direct Permission Assignment API** (#138) + - New endpoint: `GET /v1/users/{user}/permissions` - List all user permissions (direct + inherited from roles) + - New endpoint: `POST /v1/users/{user}/permissions` - Assign direct permission(s) to user with temporal tracking (audit trail) + - New endpoint: `DELETE /v1/users/{user}/permissions/{permission}` - Revoke direct permission from user + - New endpoint: `GET /v1/users/{user}/permissions/direct` - List only direct permissions (excludes permissions inherited from roles) + - Uses existing pivot columns (`granted_at`, `granted_by`, `revoked_at`, `revoked_by`) on `model_has_permissions` table for temporal direct permission assignment (no new migration required) + - New controller: `UserPermissionController` - Handles direct permission assignment operations + - New policy: `UserPermissionPolicy` - Authorization rules (User can view own, Admin can assign/revoke) + - New form request: `AssignUserPermissionRequest` - Validates permission existence, not already assigned, and assignment metadata + - New method: `User::hasDirectPermission()` - Check if permission is directly assigned (not via roles) + - Part of RBAC Phase 4 Epic (#108), enables fine-grained permission control bypassing roles + - **Permission Management CRUD API** (#137) - New endpoint: `GET /v1/permissions` - List all permissions grouped by resource - New endpoint: `POST /v1/permissions` - Create new permission with strict naming validation (resource.action) diff --git a/app/Http/Controllers/Api/V1/UserPermissionController.php b/app/Http/Controllers/Api/V1/UserPermissionController.php new file mode 100644 index 0000000..dfe4a1c --- /dev/null +++ b/app/Http/Controllers/Api/V1/UserPermissionController.php @@ -0,0 +1,200 @@ +getAllPermissions()->pluck('name'); + + // Get permissions via roles + $rolesWithPermissions = $user->roles->flatMap(function ($role) { + /** @var \Spatie\Permission\Models\Role $role */ + return $role->permissions->map(function ($permission) use ($role) { + /** @var \Spatie\Permission\Models\Permission $permission */ + return [ + 'name' => $permission->name, + 'role' => $role->name, + ]; + }); + }); + + // Get direct permissions with pivot data + $directPermissions = DB::table('model_has_permissions') + ->join('permissions', 'model_has_permissions.permission_id', '=', 'permissions.id') + ->where('model_has_permissions.model_type', $user->getMorphClass()) + ->where('model_has_permissions.model_id', $user->getKey()) + ->select([ + 'permissions.name', + 'model_has_permissions.valid_from', + 'model_has_permissions.valid_until', + 'model_has_permissions.created_at as assigned_at', + 'model_has_permissions.assigned_by', + 'model_has_permissions.reason', + ]) + ->get() + ->map(function ($row) { + return (array) $row; + }); + + return response()->json([ + 'data' => [ + 'via_roles' => $rolesWithPermissions, + 'direct' => $directPermissions, + 'all' => $allPermissions->unique()->values(), + ], + ]); + } + + /** + * Assign direct permission(s) to user. + * + * Supports bulk assignment and optional temporal constraints. + * Authorization: Admin only + */ + public function store(AssignUserPermissionRequest $request, User $user): JsonResponse + { + Gate::authorize('assignPermission', $user); + + /** @var array $permissions */ + $permissions = $request->validated('permissions'); + $assignedPermissions = []; + + foreach ($permissions as $permissionName) { + /** @var \Spatie\Permission\Models\Permission $permission */ + $permission = \Spatie\Permission\Models\Permission::findByName((string) $permissionName, 'sanctum'); + + // Build pivot data array with temporal constraints + $pivotData = [ + 'valid_from' => $request->validated('valid_from'), + 'valid_until' => $request->validated('valid_until'), + 'assigned_by' => auth()->id(), + 'reason' => $request->validated('reason'), + ]; + + // Insert directly into pivot table with temporal data + // Spatie's givePermissionTo doesn't support pivot attributes + $registrar = app(\Spatie\Permission\PermissionRegistrar::class); + $teamId = $registrar->getPermissionsTeamId(); + + DB::table('model_has_permissions')->updateOrInsert( + [ + 'permission_id' => $permission->id, + 'model_type' => $user->getMorphClass(), + 'model_id' => $user->getKey(), + 'tenant_id' => $teamId, + ], + array_merge($pivotData, [ + 'created_at' => now(), + 'updated_at' => now(), + ]) + ); + + $assignedPermissions[] = [ + 'name' => $permission->name, + 'valid_from' => $pivotData['valid_from'] ?? null, + 'valid_until' => $pivotData['valid_until'] ?? null, + ]; + } + + // Clear permission cache + app(\Spatie\Permission\PermissionRegistrar::class)->forgetCachedPermissions(); + + return response()->json([ + 'message' => 'Permissions assigned successfully', + 'data' => $assignedPermissions, + ], 201); + } + + /** + * Revoke direct permission from user. + * + * Only removes direct assignment. Role-based permissions remain. + * Authorization: Admin only + */ + public function destroy(User $user, string $permission): JsonResponse + { + Gate::authorize('revokePermission', $user); + + // Check if user has this permission directly + if (! $user->hasDirectPermission($permission)) { + return response()->json([ + 'message' => 'User does not have this permission directly assigned', + ], 404); + } + + $user->revokePermissionTo($permission); + + return response()->json([ + 'message' => 'Permission revoked successfully', + ]); + } + + /** + * List only direct permissions (excludes via_roles). + * + * Shows temporal constraints if present. + * Authorization: User can view own, Admin can view all + */ + public function direct(User $user): JsonResponse + { + Gate::authorize('viewPermissions', $user); + + // Get direct permissions with pivot data + $directPermissions = DB::table('model_has_permissions') + ->join('permissions', 'model_has_permissions.permission_id', '=', 'permissions.id') + ->where('model_has_permissions.model_type', $user->getMorphClass()) + ->where('model_has_permissions.model_id', $user->getKey()) + ->select([ + 'permissions.name', + 'model_has_permissions.valid_from', + 'model_has_permissions.valid_until', + 'model_has_permissions.created_at as assigned_at', + 'model_has_permissions.assigned_by', + 'model_has_permissions.reason', + ]) + ->get() + ->map(function ($row) { + return (array) $row; + }); + + return response()->json([ + 'data' => [ + 'direct' => $directPermissions, + ], + ]); + } +} diff --git a/app/Http/Requests/AssignUserPermissionRequest.php b/app/Http/Requests/AssignUserPermissionRequest.php new file mode 100644 index 0000000..0a694ca --- /dev/null +++ b/app/Http/Requests/AssignUserPermissionRequest.php @@ -0,0 +1,82 @@ +|string> + */ + public function rules(): array + { + return [ + 'permissions' => ['required', 'array', 'min:1'], + 'permissions.*' => [ + 'required', + 'string', + function (string $attribute, mixed $value, \Closure $fail) { + if (! is_string($value)) { + $fail('The permission must be a string.'); + + return; + } + + if (! Permission::where('name', $value)->where('guard_name', 'sanctum')->exists()) { + $fail("The permission '{$value}' does not exist."); + } + }, + ], + 'valid_from' => ['nullable', 'date', 'before_or_equal:valid_until'], + 'valid_until' => ['nullable', 'date', 'after:valid_from'], + 'reason' => ['nullable', 'string', 'max:1000'], + ]; + } + + /** + * Get custom messages for validator errors. + * + * @return array + */ + public function messages(): array + { + return [ + 'permissions.required' => 'At least one permission must be provided.', + 'permissions.array' => 'Permissions must be an array.', + 'permissions.min' => 'At least one permission must be provided.', + 'permissions.*.required' => 'Permission name cannot be empty.', + 'permissions.*.string' => 'Permission name must be a string.', + 'valid_from.before_or_equal' => 'Valid from date must be before or equal to valid until date.', + 'valid_until.after' => 'Valid until date must be after valid from date.', + 'reason.max' => 'Reason cannot exceed 1000 characters.', + ]; + } +} diff --git a/app/Models/User.php b/app/Models/User.php index 050618c..76452b0 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -103,4 +103,31 @@ public function roles(): MorphToMany TemporalRoleUser::applyActiveFilter($query, 'model_has_roles.'); }); } + + /** + * Check if user has a permission assigned directly (not via roles). + * + * This method queries the model_has_permissions pivot table directly + * to bypass Spatie's role-based permission resolution. + * + * @param string|\Spatie\Permission\Contracts\Permission $permission + */ + public function hasDirectPermission($permission): bool + { + if (is_string($permission)) { + // Use Spatie's Permission model directly to avoid PHPStan complexity + $permission = \Spatie\Permission\Models\Permission::findByName($permission, $this->getDefaultGuardName()); + } + + if (! $permission instanceof \Spatie\Permission\Contracts\Permission) { + return false; + } + + // Query pivot table directly to check for direct assignment + return \Illuminate\Support\Facades\DB::table('model_has_permissions') + ->where('model_type', $this->getMorphClass()) + ->where('model_id', $this->getKey()) + ->where('permission_id', $permission->id) + ->exists(); + } } diff --git a/app/Policies/UserPermissionPolicy.php b/app/Policies/UserPermissionPolicy.php new file mode 100644 index 0000000..3b3ffa6 --- /dev/null +++ b/app/Policies/UserPermissionPolicy.php @@ -0,0 +1,59 @@ +id === $targetUser->id) { + return true; + } + + // Admin can view any user's permissions + return $currentUser->hasRole('Admin'); + } + + /** + * Determine whether the user can assign permissions to the target user. + * + * Only Admins can assign direct permissions. + */ + public function assignPermission(User $currentUser, User $targetUser): bool + { + return $currentUser->hasRole('Admin'); + } + + /** + * Determine whether the user can revoke permissions from the target user. + * + * Only Admins can revoke direct permissions. + */ + public function revokePermission(User $currentUser, User $targetUser): bool + { + return $currentUser->hasRole('Admin'); + } +} diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 659cbe2..185890d 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -36,5 +36,37 @@ public function boot(): void // Register policy for Spatie Permission model Gate::policy(Permission::class, PermissionManagementPolicy::class); + + // Register gates for user permission management + $this->registerUserPermissionGates(); + } + + /** + * Register authorization gates for direct user permission management. + */ + private function registerUserPermissionGates(): void + { + $policy = new \App\Policies\UserPermissionPolicy; + + Gate::define('viewPermissions', function ($currentUser, $targetUser) use ($policy) { + assert($currentUser instanceof \App\Models\User); + assert($targetUser instanceof \App\Models\User); + + return $policy->viewPermissions($currentUser, $targetUser); + }); + + Gate::define('assignPermission', function ($currentUser, $targetUser) use ($policy) { + assert($currentUser instanceof \App\Models\User); + assert($targetUser instanceof \App\Models\User); + + return $policy->assignPermission($currentUser, $targetUser); + }); + + Gate::define('revokePermission', function ($currentUser, $targetUser) use ($policy) { + assert($currentUser instanceof \App\Models\User); + assert($targetUser instanceof \App\Models\User); + + return $policy->revokePermission($currentUser, $targetUser); + }); } } diff --git a/database/migrations/2025_11_14_213406_add_temporal_columns_to_model_has_permissions_table.php b/database/migrations/2025_11_14_213406_add_temporal_columns_to_model_has_permissions_table.php new file mode 100644 index 0000000..b14ee26 --- /dev/null +++ b/database/migrations/2025_11_14_213406_add_temporal_columns_to_model_has_permissions_table.php @@ -0,0 +1,57 @@ +timestamp('valid_from')->nullable()->after('model_type'); + $table->timestamp('valid_until')->nullable()->after('valid_from'); + + // Audit trail columns + $table->foreignId('assigned_by')->nullable()->after('valid_until')->constrained('users')->onDelete('set null'); + $table->text('reason')->nullable()->after('assigned_by'); + + // Standard timestamps + $table->timestamps(); + + // Index for efficient expiration queries + $table->index(['valid_until'], 'model_has_permissions_expiration_index'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('model_has_permissions', function (Blueprint $table) { + $table->dropIndex('model_has_permissions_expiration_index'); + $table->dropTimestamps(); + $table->dropForeign(['assigned_by']); + $table->dropColumn([ + 'valid_from', + 'valid_until', + 'assigned_by', + 'reason', + ]); + }); + } +}; diff --git a/routes/api.php b/routes/api.php index 2343f05..d11ec4f 100644 --- a/routes/api.php +++ b/routes/api.php @@ -5,6 +5,7 @@ use App\Http\Controllers\Api\V1\PermissionManagementController; use App\Http\Controllers\Api\V1\RoleManagementController; +use App\Http\Controllers\Api\V1\UserPermissionController; use App\Http\Controllers\AuthController; use App\Http\Controllers\PersonController; use App\Http\Controllers\RoleController; @@ -69,6 +70,12 @@ Route::patch('/users/{user}/roles/{role}/extend', [RoleController::class, 'extend']) ->middleware('permission:role.assign'); + // User Direct Permission Assignment API (RBAC Phase 4) + Route::get('/users/{user}/permissions', [UserPermissionController::class, 'index']); + Route::post('/users/{user}/permissions', [UserPermissionController::class, 'store']); + Route::delete('/users/{user}/permissions/{permission}', [UserPermissionController::class, 'destroy']); + Route::get('/users/{user}/permissions/direct', [UserPermissionController::class, 'direct']); + // Tenant-scoped Person endpoints Route::prefix('tenants/{tenant}')->middleware('tenant')->group(function () { Route::post('/persons', [PersonController::class, 'store']) diff --git a/tests/Feature/UserPermissionAssignmentApiTest.php b/tests/Feature/UserPermissionAssignmentApiTest.php new file mode 100644 index 0000000..e040098 --- /dev/null +++ b/tests/Feature/UserPermissionAssignmentApiTest.php @@ -0,0 +1,256 @@ +tenant = TenantKey::create($keys); + + // Set tenant context for permission system + $this->registrar = app(PermissionRegistrar::class); + $this->registrar->setPermissionsTeamId($this->tenant->id); + + // Create permissions for testing + Permission::create(['name' => 'employees.read', 'guard_name' => 'sanctum']); + Permission::create(['name' => 'employees.export', 'guard_name' => 'sanctum']); + Permission::create(['name' => 'reports.generate', 'guard_name' => 'sanctum']); + Permission::create(['name' => 'shifts.read', 'guard_name' => 'sanctum']); + + // Create roles with permissions + $managerRole = Role::create(['name' => 'Manager', 'guard_name' => 'sanctum']); + $managerRole->givePermissionTo(['employees.read', 'shifts.read']); + + $adminRole = Role::create(['name' => 'Admin', 'guard_name' => 'sanctum']); +}); + +afterEach(function (): void { + $this->registrar->setPermissionsTeamId(null); + cleanupTestKekFile(); + TenantKey::setKekPath(null); +}); + +test('user can view own permissions via_roles and direct and all', function () { + $user = User::factory()->create(); + $user->assignRole('Manager'); + $user->givePermissionTo('employees.export'); // Direct permission + + $response = $this->actingAs($user, 'sanctum') + ->getJson("/api/v1/users/{$user->id}/permissions"); + + $response->assertOk() + ->assertJsonStructure([ + 'data' => [ + 'via_roles' => [ + '*' => ['name', 'role'], + ], + 'direct' => [ + '*' => ['name', 'valid_from', 'valid_until', 'assigned_at'], + ], + 'all', + ], + ]) + ->assertJsonPath('data.all', fn ($all) => in_array('employees.read', $all) + && in_array('shifts.read', $all) + && in_array('employees.export', $all)); +}); + +test('admin can view any user permissions', function () { + $admin = User::factory()->create(); + $admin->assignRole('Admin'); + + $targetUser = User::factory()->create(); + $targetUser->assignRole('Manager'); + + $response = $this->actingAs($admin, 'sanctum') + ->getJson("/api/v1/users/{$targetUser->id}/permissions"); + + $response->assertOk() + ->assertJsonStructure(['data' => ['via_roles', 'direct', 'all']]); +}); + +test('user cannot view other user permissions', function () { + $user = User::factory()->create(); + $otherUser = User::factory()->create(); + + $response = $this->actingAs($user, 'sanctum') + ->getJson("/api/v1/users/{$otherUser->id}/permissions"); + + $response->assertForbidden(); +}); + +test('admin can assign direct permission to user', function () { + $admin = User::factory()->create(); + $admin->assignRole('Admin'); + + $targetUser = User::factory()->create(); + + $response = $this->actingAs($admin, 'sanctum') + ->postJson("/api/v1/users/{$targetUser->id}/permissions", [ + 'permissions' => ['employees.export'], + ]); + + $response->assertCreated() + ->assertJsonPath('data.0.name', 'employees.export'); + + expect($targetUser->fresh()->hasDirectPermission('employees.export'))->toBeTrue(); +}); + +test('admin can assign direct permission with temporal constraints', function () { + $admin = User::factory()->create(); + $admin->assignRole('Admin'); + + $targetUser = User::factory()->create(); + + $validFrom = now()->toIso8601String(); + $validUntil = now()->addDays(7)->toIso8601String(); + + $response = $this->actingAs($admin, 'sanctum') + ->postJson("/api/v1/users/{$targetUser->id}/permissions", [ + 'permissions' => ['reports.generate'], + 'valid_from' => $validFrom, + 'valid_until' => $validUntil, + 'reason' => 'Temporary report access', + ]); + + $response->assertCreated() + ->assertJsonPath('data.0.name', 'reports.generate') + ->assertJsonPath('data.0.valid_from', $validFrom) + ->assertJsonPath('data.0.valid_until', $validUntil); +}); + +test('non-admin cannot assign permissions', function () { + $user = User::factory()->create(); + $targetUser = User::factory()->create(); + + $response = $this->actingAs($user, 'sanctum') + ->postJson("/api/v1/users/{$targetUser->id}/permissions", [ + 'permissions' => ['employees.export'], + ]); + + $response->assertForbidden(); +}); + +test('admin can revoke direct permission from user', function () { + $admin = User::factory()->create(); + $admin->assignRole('Admin'); + + $targetUser = User::factory()->create(); + $targetUser->givePermissionTo('employees.export'); + + expect($targetUser->hasDirectPermission('employees.export'))->toBeTrue(); + + $response = $this->actingAs($admin, 'sanctum') + ->deleteJson("/api/v1/users/{$targetUser->id}/permissions/employees.export"); + + $response->assertOk(); + + expect($targetUser->fresh()->hasDirectPermission('employees.export'))->toBeFalse(); +}); + +test('revoking direct permission does not affect role permissions', function () { + $admin = User::factory()->create(); + $admin->assignRole('Admin'); + + $targetUser = User::factory()->create(); + $targetUser->assignRole('Manager'); // Has employees.read via role + $targetUser->givePermissionTo('employees.read'); // Also has it directly + + $response = $this->actingAs($admin, 'sanctum') + ->deleteJson("/api/v1/users/{$targetUser->id}/permissions/employees.read"); + + $response->assertOk(); + + // Direct permission removed, but still has via role + expect($targetUser->fresh()->hasDirectPermission('employees.read'))->toBeFalse() + ->and($targetUser->hasPermissionTo('employees.read'))->toBeTrue(); // Still has via role +}); + +test('user can view only direct permissions', function () { + $user = User::factory()->create(); + $user->assignRole('Manager'); // Has employees.read, shifts.read via role + $user->givePermissionTo('employees.export'); // Direct + + $response = $this->actingAs($user, 'sanctum') + ->getJson("/api/v1/users/{$user->id}/permissions/direct"); + + $response->assertOk() + ->assertJsonCount(1, 'data.direct') + ->assertJsonPath('data.direct.0.name', 'employees.export'); +}); + +test('unauthenticated user cannot access permissions endpoints', function () { + $user = User::factory()->create(); + + $this->getJson("/api/v1/users/{$user->id}/permissions") + ->assertUnauthorized(); + + $this->postJson("/api/v1/users/{$user->id}/permissions", [ + 'permissions' => ['employees.read'], + ])->assertUnauthorized(); + + $this->deleteJson("/api/v1/users/{$user->id}/permissions/employees.read") + ->assertUnauthorized(); + + $this->getJson("/api/v1/users/{$user->id}/permissions/direct") + ->assertUnauthorized(); +}); + +test('validation fails when permissions array is empty', function () { + $admin = User::factory()->create(); + $admin->assignRole('Admin'); + + $targetUser = User::factory()->create(); + + $response = $this->actingAs($admin, 'sanctum') + ->postJson("/api/v1/users/{$targetUser->id}/permissions", [ + 'permissions' => [], + ]); + + $response->assertUnprocessable() + ->assertJsonValidationErrors(['permissions']); +}); + +test('validation fails when permission does not exist', function () { + $admin = User::factory()->create(); + $admin->assignRole('Admin'); + + $targetUser = User::factory()->create(); + + $response = $this->actingAs($admin, 'sanctum') + ->postJson("/api/v1/users/{$targetUser->id}/permissions", [ + 'permissions' => ['nonexistent.permission'], + ]); + + $response->assertUnprocessable() + ->assertJsonValidationErrors(['permissions.0']); +}); + +test('validation fails when valid_until is before valid_from', function () { + $admin = User::factory()->create(); + $admin->assignRole('Admin'); + + $targetUser = User::factory()->create(); + + $response = $this->actingAs($admin, 'sanctum') + ->postJson("/api/v1/users/{$targetUser->id}/permissions", [ + 'permissions' => ['employees.export'], + 'valid_from' => now()->addDays(7)->toIso8601String(), + 'valid_until' => now()->toIso8601String(), + ]); + + $response->assertUnprocessable() + ->assertJsonValidationErrors(['valid_until']); +});