Feature/authorization#23
Merged
Merged
Conversation
naveensanjula975
approved these changes
May 4, 2026
There was a problem hiding this comment.
Pull request overview
This PR introduces centralized JWT authentication/authorization middleware and begins enforcing role-based access control (RBAC) across issue- and branch-related endpoints, along with service/controller adjustments to shape returned data by role.
Changes:
- Added
authenticateToken/authorizeRolesmiddleware and applied auth globally for v1 API routes. - Introduced RBAC filtering/authorization logic in issue listing/detail endpoints and restricted certain routes by role.
- Updated user-role fetching to include only role-specific profiles and attach branch details for branch managers.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/middleware/auth.js | Adds JWT auth middleware and role authorization helper. |
| server.js | Applies authentication middleware globally before mounting protected routes. |
| src/routes/auth.js | Refactors to use shared authenticateToken middleware for protected auth endpoints. |
| src/routes/issues.js | Adds role gating to issue creation and reformats routes. |
| src/controllers/issueController.js | Adds RBAC filtering for issue listing and authorization checks for issue detail. |
| src/services/issueService.js | Tightens issue creation validation and changes “updated issue” return payloads for some operations. |
| src/routes/branch.js | Restricts all branch routes to maintenance_executive. |
| src/services/userService.js | Changes getUsersByRole to only include the relevant profile and manually attach branch data for branch managers. |
Comments suppressed due to low confidence (2)
src/controllers/issueController.js:45
- The inline comment says the service will auto-assign a branch manager when
manager_idis 0/absent, butIssueService.createIssuenow always validatesmanager_idand no longer auto-assigns. Update/remove this comment (and the surrounding logic) to reflect the current behavior.
// Include manager_id only if it is a valid non-zero integer
// When it's 0 or absent, issueService will auto-assign a branch manager
const parsedManagerId = manager_id ? parseInt(manager_id) : 0;
if (parsedManagerId > 0) {
issueData.manager_id = parsedManagerId;
}
src/routes/issues.js:46
- The status-log endpoints (
GET/POST /:id/statuses) are not protected byauthorizeRoles(...). If these logs are sensitive, restrict access and ensure the caller is authorized to view the referenced issue (consistent with the RBAC inIssueController.getIssueById).
// GET /api/v1/issues/:id/statuses - Get all status update logs for an issue
router.get('/:id/statuses', statusController.getStatusUpdates);
// POST /api/v1/issues/:id/statuses - Add a status update log entry
router.post('/:id/statuses', statusController.createStatusUpdate);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
33
to
+37
| const issueData = { | ||
| branch_id: parseInt(branch_id), | ||
| title, | ||
| description | ||
| manager_id: parseInt(manager_id), | ||
| description, |
Comment on lines
+194
to
+195
| message: | ||
| "Access denied: This issue does not belong to your branch", |
Comment on lines
21
to
+25
| // PUT /api/v1/issues/:id - Update issue | ||
| router.put('/:id', issueController.updateIssue); | ||
| router.put("/:id", issueController.updateIssue); | ||
|
|
||
| // DELETE /api/v1/issues/:id - Delete issue | ||
| router.delete('/:id', issueController.deleteIssue); | ||
| router.delete("/:id", issueController.deleteIssue); |
|
|
||
| // PUT /api/v1/issues/:id/status - Update issue status | ||
| router.put('/:id/status', issueController.updateStatus); | ||
| router.put("/:id/status", issueController.updateStatus); |
| success: true, | ||
| data: updatedIssueResult.data, | ||
| message: 'Issue status updated successfully' | ||
| data: issue, |
Comment on lines
+307
to
+311
| if (branchId) { | ||
| const branchResult = await branchService.getBranchById(branchId); | ||
| if (branchResult.success) { | ||
| userJson.Branch = branchResult.data; // Attaching branch data | ||
| } |
| app.use("/api/v1/auth", authRoutes); | ||
|
|
||
| // Protected routes - require authentication | ||
| app.use(authenticateToken); |
Comment on lines
33
to
+37
| const issueData = { | ||
| branch_id: parseInt(branch_id), | ||
| title, | ||
| description | ||
| manager_id: parseInt(manager_id), | ||
| description, |
Comment on lines
+121
to
+125
| // Strict Role-Based Access Control Filtering | ||
| const { role, roleSpecificId } = req.user; | ||
|
|
||
| if (role === "maintenance_executive") { | ||
| // Maintenance Executives can view all issues by default. |
Comment on lines
27
to
29
| // POST /api/v1/issues/:id/assign-technician - Assign technician to issue | ||
| router.post('/:id/assign-technician', issueController.assignTechnician); | ||
| router.post("/:id/assign-technician", issueController.assignTechnician); | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.