docs: Add comprehensive RBAC API documentation (#140) #160
Conversation
- Add complete API reference (docs/api/rbac-endpoints.md) - 16 endpoints across 4 functional areas - cURL and JavaScript examples for each endpoint - Error responses and troubleshooting - Add role management guide (docs/guides/role-management.md) - Predefined roles (Admin, Manager, Guard, Client, Works Council) - Custom role creation workflow - Temporal role assignments - Best practices and examples - Add permission system guide (docs/guides/permission-system.md) - Permission naming convention (resource.action) - Resource organization by domain - Permission matrix for all predefined roles - Custom permission creation All files: - Include SPDX headers (REUSE compliant) - Markdownlint clean (MD029, MD036 fixed) - Correct API URLs (api.secpal.app/v1) Part of RBAC Phase 4 Epic (#108) Closes #140
💡 Tip: Consider Using Draft PRsBenefits of opening PRs as drafts initially:
How to convert:
This is just a friendly reminder - feel free to continue as is! 😊 |
kevalyq
added
the
large-pr-approved
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive RBAC API documentation consisting of three new documentation files totaling ~2,800 lines. The documentation covers all 16 RBAC endpoints across four functional areas: Role Assignment, Role Management, Permission Management, and Direct Permissions. Each endpoint includes detailed request/response examples, error handling, and both cURL and JavaScript code samples.
Key Changes:
- Complete API endpoint reference with authentication, parameters, and error responses
- Detailed guides for role management including predefined roles and custom role creation
- Permission system documentation explaining naming conventions, organization, and matrices for all predefined roles
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 22 comments.
| File | Description |
|---|---|
| docs/api/rbac-endpoints.md | Complete API reference for 16 RBAC endpoints with examples and error responses |
| docs/guides/role-management.md | Comprehensive guide for creating, assigning, and managing roles including temporal assignments |
| docs/guides/permission-system.md | Permission naming conventions, resource organization, and permission matrices for predefined roles |
Critical Issues Found:
The documentation contains systematic inconsistencies with the actual codebase:
-
Permission Naming Mismatch: Documentation uses plural forms (
roles.assign,roles.read,permissions.create) but the routes file uses singular forms (role.assign,role.read). This affects approximately 19 locations across all three files. -
Missing Authorization Middleware: Documentation claims many endpoints require specific permissions (e.g.,
roles.create,permissions.update), but the actual routes inroutes/api.phplines 49-61 and 74-77 have no permission middleware - onlyauth:sanctum. This affects 12 endpoints. -
Endpoint Path Notation: Inconsistent use of
/api/v1/...in endpoint descriptions versus/v1/...in cURL examples and base URL definition.
These issues suggest either the documentation is aspirational (describing planned authorization), or the routes are missing critical authorization middleware that should be implemented.
…orization descriptions - Fix permission naming: roles.* → role.*, permissions.* → permission.* (consistency with Phase 3) - Update authorization descriptions for Phase 4 endpoints to reflect controller-level Policy authorization - Add reference to Issue #161 for planned route-level middleware implementation - Fix URL notation: /api/v1/ → /v1/ (correct pattern without redundant /api/) - Update GUARD_ARCHITECTURE.md examples to use correct permission names Addresses 22 review comments from Copilot in PR #160 Related: #140, #161
Corrected permission name inconsistency in permission-system.md: - Line 82 (Core Resources table): reports.read → reports.view - Line 167 (Manager Role): reports.read → reports.view - Line 207 (Client Role): reports.read → reports.view - Lines 566-586 (Example 2): reports.read → reports.view These changes align documentation with actual permission names defined in RolesAndPermissionsSeeder.php where reports resource uses 'view' action instead of 'read' action. Addresses 3 new Copilot review comments on commit 5fea381. Related to #140
- Fix permission name in role-management.md (role.* -> roles.*) - Add clarifying comments to routes/api.php about authorization - Document that authorization is handled by Policies, not middleware The RBAC routes correctly use Policy-based authorization via Gate::authorize() in the controllers. Copilot review comments were about missing middleware, but the actual implementation uses Policies which check the correct permissions: - RoleManagementPolicy checks roles.create/read/update/delete - PermissionManagementPolicy checks permissions.create/read/update/delete - UserPermissionPolicy checks Admin role for assignment/revocation Fixes Copilot review comments in PR #160 regarding authorization. Related: #140
Summary
Adds complete RBAC API documentation as required by Issue #140.
Changes
New Documentation Files
docs/api/rbac-endpoints.md(~1000 lines)docs/guides/role-management.md(~900 lines)docs/guides/permission-system.md(~900 lines)resource.action)Quality Gates
api.secpal.app/v1)PR Size
Justification: Complete API documentation requires all 3 files together for coherence. Splitting would break the interconnected references between files.
Part of
Checklist
/api/)Review Notes
Focus areas for review:
routes/api.php)rbac-architecture.md)Total Changes: +2826 lines (3 new files)
No breaking changes