-
Notifications
You must be signed in to change notification settings - Fork 0
Closed
Labels
Description
Context
Phase 4 RBAC endpoints (Role Management, Permission Management, Direct Permissions) currently use controller-level authorization via Gate::authorize(), which is functionally secure but not following Laravel best practices.
Current implementation:
// routes/api.php
Route::post('/permissions', [PermissionManagementController::class, 'store']);
// Controller (PermissionManagementController.php)
public function store(CreatePermissionRequest $request): JsonResponse
{
Gate::authorize('create', Permission::class); // Authorization happens here
// ...
}Issue:
- Request validation runs BEFORE authorization check
- Inconsistent with Phase 3 endpoints (which use route-level middleware)
- Request processing overhead for unauthorized users
- Not following Laravel best practices
Phase 3 comparison (correct implementation):
// routes/api.php
Route::post('/users/{user}/roles', [RoleController::class, 'store'])
->middleware('permission:role.assign'); // ✅ Authorization at route levelProposed Solution
Add route-level authorization middleware to all Phase 4 RBAC endpoints:
Affected Routes (routes/api.php)
Role Management (lines 50-54):
Route::get('/roles', [RoleManagementController::class, 'index'])
->middleware('permission:role.read');
Route::post('/roles', [RoleManagementController::class, 'store'])
->middleware('permission:role.create');
Route::get('/roles/{id}', [RoleManagementController::class, 'show'])
->middleware('permission:role.read');
Route::patch('/roles/{id}', [RoleManagementController::class, 'update'])
->middleware('permission:role.update');
Route::delete('/roles/{id}', [RoleManagementController::class, 'destroy'])
->middleware('permission:role.delete');Permission Management (lines 57-61):
Route::get('/permissions', [PermissionManagementController::class, 'index'])
->middleware('permission:permission.read');
Route::post('/permissions', [PermissionManagementController::class, 'store'])
->middleware('permission:permission.create');
Route::get('/permissions/{id}', [PermissionManagementController::class, 'show'])
->middleware('permission:permission.read');
Route::patch('/permissions/{id}', [PermissionManagementController::class, 'update'])
->middleware('permission:permission.update');
Route::delete('/permissions/{id}', [PermissionManagementController::class, 'destroy'])
->middleware('permission:permission.delete');Direct Permissions (lines 71-77):
Route::get('/users/{user}/permissions', [UserPermissionController::class, 'index'])
->middleware('permission:permission.read');
Route::post('/users/{user}/permissions', [UserPermissionController::class, 'store'])
->middleware('permission:permission.assign');
Route::delete('/users/{user}/permissions/{permission}', [UserPermissionController::class, 'destroy'])
->middleware('permission:permission.revoke');
Route::get('/users/{user}/permissions/direct', [UserPermissionController::class, 'direct'])
->middleware('permission:permission.read');Acceptance Criteria
- All Phase 4 RBAC routes have explicit authorization middleware
- Middleware uses singular permission names (
role.*,permission.*) for consistency with Phase 3 - All existing tests pass (270 tests)
- PHPStan level max passes
- Controller-level
Gate::authorize()calls can remain as defense-in-depth (optional)
Benefits
- ✅ Early rejection of unauthorized requests (before validation)
- ✅ Consistency with Phase 3 implementation
- ✅ Better performance (no unnecessary request processing)
- ✅ Follows Laravel best practices
- ✅ Clearer API documentation (matches route definitions)
Related
- Epic RBAC Phase 4: Documentation & Final Testing #108 (RBAC Phase 4)
- Issue RBAC API Documentation (Phase 4) #140 (RBAC API Documentation) - discovered during documentation review
- PR docs: Add comprehensive RBAC API documentation (#140) #160 - Documentation will be updated to reflect controller-level authorization until this issue is resolved
Priority
Medium - Not a security vulnerability (authorization works), but technical debt that should be addressed for maintainability and consistency.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status
✅ Done