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 <noreply@anthropic.com>
This commit is contained in:
@@ -23,6 +23,7 @@ require_once dirname(__DIR__) . '/helpers/Database.php';
|
|||||||
require_once dirname(__DIR__) . '/helpers/ResponseHelper.php';
|
require_once dirname(__DIR__) . '/helpers/ResponseHelper.php';
|
||||||
require_once dirname(__DIR__) . '/models/AttachmentModel.php';
|
require_once dirname(__DIR__) . '/models/AttachmentModel.php';
|
||||||
require_once dirname(__DIR__) . '/models/AuditLogModel.php';
|
require_once dirname(__DIR__) . '/models/AuditLogModel.php';
|
||||||
|
require_once dirname(__DIR__) . '/models/TicketModel.php';
|
||||||
require_once dirname(__DIR__) . '/middleware/CsrfMiddleware.php';
|
require_once dirname(__DIR__) . '/middleware/CsrfMiddleware.php';
|
||||||
|
|
||||||
header('Content-Type: application/json');
|
header('Content-Type: application/json');
|
||||||
@@ -66,7 +67,14 @@ try {
|
|||||||
ResponseHelper::notFound('Attachment not found');
|
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;
|
$isAdmin = $_SESSION['user']['is_admin'] ?? false;
|
||||||
if (!$attachmentModel->canUserDelete($attachmentId, $_SESSION['user']['user_id'], $isAdmin)) {
|
if (!$attachmentModel->canUserDelete($attachmentId, $_SESSION['user']['user_id'], $isAdmin)) {
|
||||||
ResponseHelper::forbidden('You do not have permission to delete this attachment');
|
ResponseHelper::forbidden('You do not have permission to delete this attachment');
|
||||||
|
|||||||
@@ -59,14 +59,16 @@ try {
|
|||||||
private $workflowModel;
|
private $workflowModel;
|
||||||
private $userId;
|
private $userId;
|
||||||
private $isAdmin;
|
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->ticketModel = new TicketModel($conn);
|
||||||
$this->commentModel = new CommentModel($conn);
|
$this->commentModel = new CommentModel($conn);
|
||||||
$this->auditLog = new AuditLogModel($conn);
|
$this->auditLog = new AuditLogModel($conn);
|
||||||
$this->workflowModel = new WorkflowModel($conn);
|
$this->workflowModel = new WorkflowModel($conn);
|
||||||
$this->userId = $userId;
|
$this->userId = $userId;
|
||||||
$this->isAdmin = $isAdmin;
|
$this->isAdmin = $isAdmin;
|
||||||
|
$this->currentUser = $currentUser;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function update($id, $data) {
|
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
|
// Authorization: admins can edit any ticket; others only their own or assigned
|
||||||
if (!$this->isAdmin
|
if (!$this->isAdmin
|
||||||
&& $currentTicket['created_by'] != $this->userId
|
&& $currentTicket['created_by'] != $this->userId
|
||||||
@@ -206,7 +217,7 @@ try {
|
|||||||
$ticketId = (int)$data['ticket_id'];
|
$ticketId = (int)$data['ticket_id'];
|
||||||
|
|
||||||
// Initialize controller
|
// Initialize controller
|
||||||
$controller = new ApiTicketController($conn, $userId, $isAdmin);
|
$controller = new ApiTicketController($conn, $userId, $isAdmin, $currentUser);
|
||||||
|
|
||||||
// Update ticket
|
// Update ticket
|
||||||
$result = $controller->update($ticketId, $data);
|
$result = $controller->update($ticketId, $data);
|
||||||
@@ -215,6 +226,10 @@ try {
|
|||||||
ob_end_clean();
|
ob_end_clean();
|
||||||
|
|
||||||
// Return response
|
// Return response
|
||||||
|
if (!empty($result['http_status'])) {
|
||||||
|
http_response_code($result['http_status']);
|
||||||
|
unset($result['http_status']);
|
||||||
|
}
|
||||||
header('Content-Type: application/json');
|
header('Content-Type: application/json');
|
||||||
echo json_encode($result);
|
echo json_encode($result);
|
||||||
|
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ require_once dirname(__DIR__) . '/helpers/Database.php';
|
|||||||
require_once dirname(__DIR__) . '/helpers/ResponseHelper.php';
|
require_once dirname(__DIR__) . '/helpers/ResponseHelper.php';
|
||||||
require_once dirname(__DIR__) . '/models/AttachmentModel.php';
|
require_once dirname(__DIR__) . '/models/AttachmentModel.php';
|
||||||
require_once dirname(__DIR__) . '/models/AuditLogModel.php';
|
require_once dirname(__DIR__) . '/models/AuditLogModel.php';
|
||||||
|
require_once dirname(__DIR__) . '/models/TicketModel.php';
|
||||||
require_once dirname(__DIR__) . '/middleware/CsrfMiddleware.php';
|
require_once dirname(__DIR__) . '/middleware/CsrfMiddleware.php';
|
||||||
|
|
||||||
header('Content-Type: application/json');
|
header('Content-Type: application/json');
|
||||||
@@ -46,7 +47,14 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
|
|||||||
}
|
}
|
||||||
|
|
||||||
try {
|
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);
|
$attachments = $attachmentModel->getAttachments($ticketId);
|
||||||
|
|
||||||
// Add formatted file size and icon to each attachment
|
// 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');
|
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
|
// Check if file was uploaded
|
||||||
if (!isset($_FILES['file']) || $_FILES['file']['error'] === UPLOAD_ERR_NO_FILE) {
|
if (!isset($_FILES['file']) || $_FILES['file']['error'] === UPLOAD_ERR_NO_FILE) {
|
||||||
ResponseHelper::error('No file uploaded');
|
ResponseHelper::error('No file uploaded');
|
||||||
|
|||||||
Reference in New Issue
Block a user