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 <noreply@anthropic.com>
This commit is contained in:
+5
-1
@@ -138,9 +138,12 @@ try {
|
|||||||
ob_end_clean();
|
ob_end_clean();
|
||||||
|
|
||||||
// Return JSON response
|
// Return JSON response
|
||||||
|
if ($result['success']) {
|
||||||
|
http_response_code(201);
|
||||||
|
}
|
||||||
header('Content-Type: application/json');
|
header('Content-Type: application/json');
|
||||||
echo json_encode($result);
|
echo json_encode($result);
|
||||||
|
|
||||||
} catch (Exception $e) {
|
} catch (Exception $e) {
|
||||||
// Discard any unexpected output
|
// Discard any unexpected output
|
||||||
ob_end_clean();
|
ob_end_clean();
|
||||||
@@ -149,6 +152,7 @@ try {
|
|||||||
error_log("Add comment API error: " . $e->getMessage());
|
error_log("Add comment API error: " . $e->getMessage());
|
||||||
|
|
||||||
// Return error response
|
// Return error response
|
||||||
|
http_response_code(500);
|
||||||
header('Content-Type: application/json');
|
header('Content-Type: application/json');
|
||||||
echo json_encode([
|
echo json_encode([
|
||||||
'success' => false,
|
'success' => false,
|
||||||
|
|||||||
@@ -7,6 +7,8 @@
|
|||||||
ini_set('display_errors', 0);
|
ini_set('display_errors', 0);
|
||||||
error_reporting(E_ALL);
|
error_reporting(E_ALL);
|
||||||
|
|
||||||
|
header('Content-Type: application/json');
|
||||||
|
|
||||||
require_once dirname(__DIR__) . '/middleware/RateLimitMiddleware.php';
|
require_once dirname(__DIR__) . '/middleware/RateLimitMiddleware.php';
|
||||||
RateLimitMiddleware::apply('api');
|
RateLimitMiddleware::apply('api');
|
||||||
|
|
||||||
@@ -109,7 +111,6 @@ try {
|
|||||||
$dependencyModel = new DependencyModel($conn);
|
$dependencyModel = new DependencyModel($conn);
|
||||||
$dependencyModel->addDependency($result['ticket_id'], $sourceTicketId, 'relates_to', $userId);
|
$dependencyModel->addDependency($result['ticket_id'], $sourceTicketId, 'relates_to', $userId);
|
||||||
|
|
||||||
header('Content-Type: application/json');
|
|
||||||
echo json_encode([
|
echo json_encode([
|
||||||
'success' => true,
|
'success' => true,
|
||||||
'new_ticket_id' => $result['ticket_id'],
|
'new_ticket_id' => $result['ticket_id'],
|
||||||
|
|||||||
@@ -115,6 +115,7 @@ try {
|
|||||||
} catch (Exception $e) {
|
} catch (Exception $e) {
|
||||||
ob_end_clean();
|
ob_end_clean();
|
||||||
error_log("Delete comment API error: " . $e->getMessage());
|
error_log("Delete comment API error: " . $e->getMessage());
|
||||||
|
http_response_code(500);
|
||||||
header('Content-Type: application/json');
|
header('Content-Type: application/json');
|
||||||
echo json_encode([
|
echo json_encode([
|
||||||
'success' => false,
|
'success' => false,
|
||||||
|
|||||||
@@ -104,6 +104,7 @@ try {
|
|||||||
} catch (Exception $e) {
|
} catch (Exception $e) {
|
||||||
ob_end_clean();
|
ob_end_clean();
|
||||||
error_log("Update comment API error: " . $e->getMessage());
|
error_log("Update comment API error: " . $e->getMessage());
|
||||||
|
http_response_code(500);
|
||||||
header('Content-Type: application/json');
|
header('Content-Type: application/json');
|
||||||
echo json_encode([
|
echo json_encode([
|
||||||
'success' => false,
|
'success' => false,
|
||||||
|
|||||||
@@ -142,7 +142,7 @@ class DashboardController {
|
|||||||
if ($assignedTo !== null) $filters['assigned_to'] = $assignedTo;
|
if ($assignedTo !== null) $filters['assigned_to'] = $assignedTo;
|
||||||
|
|
||||||
// Get tickets with pagination, sorting, search, and advanced filters
|
// 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)
|
// Get categories and types for filters (single query)
|
||||||
$filterOptions = $this->getCategoriesAndTypes();
|
$filterOptions = $this->getCategoriesAndTypes();
|
||||||
|
|||||||
@@ -155,7 +155,11 @@ class AuthMiddleware {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Check for admin or employee group membership
|
// 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'];
|
$requiredGroups = ['admin', 'employee'];
|
||||||
|
|
||||||
return !empty(array_intersect($userGroups, $requiredGroups));
|
return !empty(array_intersect($userGroups, $requiredGroups));
|
||||||
|
|||||||
+11
-1
@@ -31,7 +31,7 @@ class TicketModel {
|
|||||||
return $result->fetch_assoc();
|
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
|
// Calculate offset
|
||||||
$offset = ($page - 1) * $limit;
|
$offset = ($page - 1) * $limit;
|
||||||
|
|
||||||
@@ -40,6 +40,16 @@ class TicketModel {
|
|||||||
$params = [];
|
$params = [];
|
||||||
$paramTypes = '';
|
$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
|
// Status filtering
|
||||||
if ($status) {
|
if ($status) {
|
||||||
$statuses = explode(',', $status);
|
$statuses = explode(',', $status);
|
||||||
|
|||||||
Reference in New Issue
Block a user