From 84cc023bc47a619414569972f021def712ad16b3 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 20 Mar 2026 21:42:47 -0400 Subject: [PATCH] Enforce ticket visibility on attachment and update endpoints - delete_attachment.php: check canUserAccessTicket() before allowing deletion; return 404 (not 403) for inaccessible tickets to prevent existence leakage - upload_attachment.php: verify ticket access on both GET (list) and POST (upload) before processing - update_ticket.php: pass currentUser to controller; add canUserAccessTicket() check before permission check; return 404 for inaccessible tickets instead of leaking existence via 403 Co-Authored-By: Claude Sonnet 4.6 --- api/delete_attachment.php | 10 +++++++++- api/update_ticket.php | 19 +++++++++++++++++-- api/upload_attachment.php | 18 +++++++++++++++++- 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/api/delete_attachment.php b/api/delete_attachment.php index fdf9d55..789cc50 100644 --- a/api/delete_attachment.php +++ b/api/delete_attachment.php @@ -23,6 +23,7 @@ require_once dirname(__DIR__) . '/helpers/Database.php'; require_once dirname(__DIR__) . '/helpers/ResponseHelper.php'; require_once dirname(__DIR__) . '/models/AttachmentModel.php'; require_once dirname(__DIR__) . '/models/AuditLogModel.php'; +require_once dirname(__DIR__) . '/models/TicketModel.php'; require_once dirname(__DIR__) . '/middleware/CsrfMiddleware.php'; header('Content-Type: application/json'); @@ -66,7 +67,14 @@ try { ResponseHelper::notFound('Attachment not found'); } - // Check permission + // Verify user can access the parent ticket + $ticketModel = new TicketModel(Database::getConnection()); + $ticket = $ticketModel->getTicketById((int)$attachment['ticket_id']); + if (!$ticket || !$ticketModel->canUserAccessTicket($ticket, $_SESSION['user'])) { + ResponseHelper::notFound('Attachment not found'); + } + + // Check permission (must be uploader or admin) $isAdmin = $_SESSION['user']['is_admin'] ?? false; if (!$attachmentModel->canUserDelete($attachmentId, $_SESSION['user']['user_id'], $isAdmin)) { ResponseHelper::forbidden('You do not have permission to delete this attachment'); diff --git a/api/update_ticket.php b/api/update_ticket.php index 0d58e0d..baa9b71 100644 --- a/api/update_ticket.php +++ b/api/update_ticket.php @@ -59,14 +59,16 @@ try { private $workflowModel; private $userId; private $isAdmin; + private $currentUser; - public function __construct($conn, $userId = null, $isAdmin = false) { + public function __construct($conn, $userId = null, $isAdmin = false, $currentUser = []) { $this->ticketModel = new TicketModel($conn); $this->commentModel = new CommentModel($conn); $this->auditLog = new AuditLogModel($conn); $this->workflowModel = new WorkflowModel($conn); $this->userId = $userId; $this->isAdmin = $isAdmin; + $this->currentUser = $currentUser; } public function update($id, $data) { @@ -79,6 +81,15 @@ try { ]; } + // Visibility check: return 404 for tickets the user cannot access + if (!$this->ticketModel->canUserAccessTicket($currentTicket, $this->currentUser)) { + return [ + 'success' => false, + 'error' => 'Ticket not found', + 'http_status' => 404 + ]; + } + // Authorization: admins can edit any ticket; others only their own or assigned if (!$this->isAdmin && $currentTicket['created_by'] != $this->userId @@ -206,7 +217,7 @@ try { $ticketId = (int)$data['ticket_id']; // Initialize controller - $controller = new ApiTicketController($conn, $userId, $isAdmin); + $controller = new ApiTicketController($conn, $userId, $isAdmin, $currentUser); // Update ticket $result = $controller->update($ticketId, $data); @@ -215,6 +226,10 @@ try { ob_end_clean(); // Return response + if (!empty($result['http_status'])) { + http_response_code($result['http_status']); + unset($result['http_status']); + } header('Content-Type: application/json'); echo json_encode($result); diff --git a/api/upload_attachment.php b/api/upload_attachment.php index e613859..33a2452 100644 --- a/api/upload_attachment.php +++ b/api/upload_attachment.php @@ -23,6 +23,7 @@ require_once dirname(__DIR__) . '/helpers/Database.php'; require_once dirname(__DIR__) . '/helpers/ResponseHelper.php'; require_once dirname(__DIR__) . '/models/AttachmentModel.php'; require_once dirname(__DIR__) . '/models/AuditLogModel.php'; +require_once dirname(__DIR__) . '/models/TicketModel.php'; require_once dirname(__DIR__) . '/middleware/CsrfMiddleware.php'; header('Content-Type: application/json'); @@ -46,7 +47,14 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { } try { - $attachmentModel = new AttachmentModel(Database::getConnection()); + $conn = Database::getConnection(); + $ticketModel = new TicketModel($conn); + $ticket = $ticketModel->getTicketById((int)$ticketId); + if (!$ticket || !$ticketModel->canUserAccessTicket($ticket, $_SESSION['user'])) { + ResponseHelper::notFound('Ticket not found'); + } + + $attachmentModel = new AttachmentModel($conn); $attachments = $attachmentModel->getAttachments($ticketId); // Add formatted file size and icon to each attachment @@ -83,6 +91,14 @@ if (!preg_match('/^\d{9}$/', $ticketId)) { ResponseHelper::error('Invalid ticket ID format'); } +// Verify user can access the ticket before accepting upload +$conn = Database::getConnection(); +$ticketModel = new TicketModel($conn); +$ticket = $ticketModel->getTicketById((int)$ticketId); +if (!$ticket || !$ticketModel->canUserAccessTicket($ticket, $_SESSION['user'])) { + ResponseHelper::notFound('Ticket not found'); +} + // Check if file was uploaded if (!isset($_FILES['file']) || $_FILES['file']['error'] === UPLOAD_ERR_NO_FILE) { ResponseHelper::error('No file uploaded');