From e6b6a2a88c2950fda7984fa5364993c8b5844cf6 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Sun, 29 Mar 2026 18:23:16 -0400 Subject: [PATCH] Security/correctness: visibility filtering, Content-Type headers, group validation - TicketModel::getAllTickets() now accepts optional $user param and applies getVisibilityFilter() so non-admin users cannot see internal/confidential tickets they lack access to from the dashboard listing - DashboardController passes $GLOBALS['currentUser'] to getAllTickets() - clone_ticket.php: move Content-Type header to top so all error paths send correct JSON content type - AuthMiddleware: filter group names from HTTP header to [a-z0-9_-] only, preventing header injection via malformed group names - add_comment.php: return HTTP 201 on success, 500 in catch block - update_comment.php, delete_comment.php: return 500 in catch blocks Co-Authored-By: Claude Sonnet 4.6 --- api/add_comment.php | 6 +++++- api/clone_ticket.php | 3 ++- api/delete_comment.php | 1 + api/update_comment.php | 1 + controllers/DashboardController.php | 2 +- middleware/AuthMiddleware.php | 6 +++++- models/TicketModel.php | 12 +++++++++++- 7 files changed, 26 insertions(+), 5 deletions(-) diff --git a/api/add_comment.php b/api/add_comment.php index d15396d..3db5a3b 100644 --- a/api/add_comment.php +++ b/api/add_comment.php @@ -138,9 +138,12 @@ try { ob_end_clean(); // Return JSON response + if ($result['success']) { + http_response_code(201); + } header('Content-Type: application/json'); echo json_encode($result); - + } catch (Exception $e) { // Discard any unexpected output ob_end_clean(); @@ -149,6 +152,7 @@ try { error_log("Add comment API error: " . $e->getMessage()); // Return error response + http_response_code(500); header('Content-Type: application/json'); echo json_encode([ 'success' => false, diff --git a/api/clone_ticket.php b/api/clone_ticket.php index 8d49c66..94710f0 100644 --- a/api/clone_ticket.php +++ b/api/clone_ticket.php @@ -7,6 +7,8 @@ ini_set('display_errors', 0); error_reporting(E_ALL); +header('Content-Type: application/json'); + require_once dirname(__DIR__) . '/middleware/RateLimitMiddleware.php'; RateLimitMiddleware::apply('api'); @@ -109,7 +111,6 @@ try { $dependencyModel = new DependencyModel($conn); $dependencyModel->addDependency($result['ticket_id'], $sourceTicketId, 'relates_to', $userId); - header('Content-Type: application/json'); echo json_encode([ 'success' => true, 'new_ticket_id' => $result['ticket_id'], diff --git a/api/delete_comment.php b/api/delete_comment.php index 5468717..f6250db 100644 --- a/api/delete_comment.php +++ b/api/delete_comment.php @@ -115,6 +115,7 @@ try { } catch (Exception $e) { ob_end_clean(); error_log("Delete comment API error: " . $e->getMessage()); + http_response_code(500); header('Content-Type: application/json'); echo json_encode([ 'success' => false, diff --git a/api/update_comment.php b/api/update_comment.php index 69353d9..043b960 100644 --- a/api/update_comment.php +++ b/api/update_comment.php @@ -104,6 +104,7 @@ try { } catch (Exception $e) { ob_end_clean(); error_log("Update comment API error: " . $e->getMessage()); + http_response_code(500); header('Content-Type: application/json'); echo json_encode([ 'success' => false, diff --git a/controllers/DashboardController.php b/controllers/DashboardController.php index a0bcfe5..a767c90 100644 --- a/controllers/DashboardController.php +++ b/controllers/DashboardController.php @@ -142,7 +142,7 @@ class DashboardController { if ($assignedTo !== null) $filters['assigned_to'] = $assignedTo; // Get tickets with pagination, sorting, search, and advanced filters - $result = $this->ticketModel->getAllTickets($page, $limit, $status, $sortColumn, $sortDirection, $category, $type, $search, $filters); + $result = $this->ticketModel->getAllTickets($page, $limit, $status, $sortColumn, $sortDirection, $category, $type, $search, $filters, $GLOBALS['currentUser'] ?? []); // Get categories and types for filters (single query) $filterOptions = $this->getCategoriesAndTypes(); diff --git a/middleware/AuthMiddleware.php b/middleware/AuthMiddleware.php index 779dc39..2297c4b 100644 --- a/middleware/AuthMiddleware.php +++ b/middleware/AuthMiddleware.php @@ -155,7 +155,11 @@ class AuthMiddleware { } // Check for admin or employee group membership - $userGroups = array_map('trim', explode(',', strtolower($groups))); + // Filter to safe characters only to prevent header injection attacks + $userGroups = array_filter( + array_map('trim', explode(',', strtolower($groups))), + function($g) { return preg_match('/^[a-z0-9_\-]+$/', $g); } + ); $requiredGroups = ['admin', 'employee']; return !empty(array_intersect($userGroups, $requiredGroups)); diff --git a/models/TicketModel.php b/models/TicketModel.php index 74bffa3..35e1e8c 100644 --- a/models/TicketModel.php +++ b/models/TicketModel.php @@ -31,7 +31,7 @@ class TicketModel { return $result->fetch_assoc(); } - public function getAllTickets(int $page = 1, int $limit = 15, ?string $status = 'Open', string $sortColumn = 'ticket_id', string $sortDirection = 'desc', ?string $category = null, ?string $type = null, ?string $search = null, array $filters = []): array { + public function getAllTickets(int $page = 1, int $limit = 15, ?string $status = 'Open', string $sortColumn = 'ticket_id', string $sortDirection = 'desc', ?string $category = null, ?string $type = null, ?string $search = null, array $filters = [], ?array $user = null): array { // Calculate offset $offset = ($page - 1) * $limit; @@ -40,6 +40,16 @@ class TicketModel { $params = []; $paramTypes = ''; + // Visibility filtering + if ($user !== null) { + $visFilter = $this->getVisibilityFilter($user); + if ($visFilter['sql'] !== '1=1') { + $whereConditions[] = $visFilter['sql']; + $params = array_merge($params, $visFilter['params']); + $paramTypes .= $visFilter['types']; + } + } + // Status filtering if ($status) { $statuses = explode(',', $status);