Security: add authorization checks to ticket_dependencies API
- POST /ticket_dependencies: verify user can access both the source ticket and the target ticket before creating a dependency - DELETE by ticket IDs: verify user can access source ticket; also validate dependency_type against the allowed whitelist - DELETE by dependency_id: look up dependency's ticket before deletion and verify user can access it, preventing IDOR - custom_fields.php: validate json_decode returns an array on POST/PUT; add http_response_code(400) to all error responses Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -66,23 +66,35 @@ try {
|
|||||||
|
|
||||||
case 'POST':
|
case 'POST':
|
||||||
$data = json_decode(file_get_contents('php://input'), true);
|
$data = json_decode(file_get_contents('php://input'), true);
|
||||||
|
if (!is_array($data)) {
|
||||||
|
http_response_code(400);
|
||||||
|
echo json_encode(['success' => false, 'error' => 'Invalid JSON']);
|
||||||
|
exit;
|
||||||
|
}
|
||||||
$result = $model->createDefinition($data);
|
$result = $model->createDefinition($data);
|
||||||
echo json_encode($result);
|
echo json_encode($result);
|
||||||
break;
|
break;
|
||||||
|
|
||||||
case 'PUT':
|
case 'PUT':
|
||||||
if (!$id) {
|
if (!$id) {
|
||||||
|
http_response_code(400);
|
||||||
echo json_encode(['success' => false, 'error' => 'ID required']);
|
echo json_encode(['success' => false, 'error' => 'ID required']);
|
||||||
exit;
|
exit;
|
||||||
}
|
}
|
||||||
|
|
||||||
$data = json_decode(file_get_contents('php://input'), true);
|
$data = json_decode(file_get_contents('php://input'), true);
|
||||||
|
if (!is_array($data)) {
|
||||||
|
http_response_code(400);
|
||||||
|
echo json_encode(['success' => false, 'error' => 'Invalid JSON']);
|
||||||
|
exit;
|
||||||
|
}
|
||||||
$result = $model->updateDefinition($id, $data);
|
$result = $model->updateDefinition($id, $data);
|
||||||
echo json_encode($result);
|
echo json_encode($result);
|
||||||
break;
|
break;
|
||||||
|
|
||||||
case 'DELETE':
|
case 'DELETE':
|
||||||
if (!$id) {
|
if (!$id) {
|
||||||
|
http_response_code(400);
|
||||||
echo json_encode(['success' => false, 'error' => 'ID required']);
|
echo json_encode(['success' => false, 'error' => 'ID required']);
|
||||||
exit;
|
exit;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -143,6 +143,10 @@ switch ($method) {
|
|||||||
// Add a new dependency
|
// Add a new dependency
|
||||||
$data = json_decode(file_get_contents('php://input'), true);
|
$data = json_decode(file_get_contents('php://input'), true);
|
||||||
|
|
||||||
|
if (!is_array($data)) {
|
||||||
|
ResponseHelper::error('Invalid JSON');
|
||||||
|
}
|
||||||
|
|
||||||
$ticketId = $data['ticket_id'] ?? null;
|
$ticketId = $data['ticket_id'] ?? null;
|
||||||
$dependsOnId = $data['depends_on_id'] ?? null;
|
$dependsOnId = $data['depends_on_id'] ?? null;
|
||||||
$type = $data['dependency_type'] ?? 'blocks';
|
$type = $data['dependency_type'] ?? 'blocks';
|
||||||
@@ -151,6 +155,16 @@ switch ($method) {
|
|||||||
ResponseHelper::error('Both ticket_id and depends_on_id are required');
|
ResponseHelper::error('Both ticket_id and depends_on_id are required');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Verify user can access both tickets before creating dependency
|
||||||
|
$srcTicket = $ticketModel->getTicketById((int)$ticketId);
|
||||||
|
if (!$srcTicket || !$ticketModel->canUserAccessTicket($srcTicket, $currentUser)) {
|
||||||
|
ResponseHelper::notFound('Ticket not found');
|
||||||
|
}
|
||||||
|
$tgtTicket = $ticketModel->getTicketById((int)$dependsOnId);
|
||||||
|
if (!$tgtTicket || !$ticketModel->canUserAccessTicket($tgtTicket, $currentUser)) {
|
||||||
|
ResponseHelper::notFound('Target ticket not found');
|
||||||
|
}
|
||||||
|
|
||||||
$result = $dependencyModel->addDependency($ticketId, $dependsOnId, $type, $userId);
|
$result = $dependencyModel->addDependency($ticketId, $dependsOnId, $type, $userId);
|
||||||
|
|
||||||
if ($result['success']) {
|
if ($result['success']) {
|
||||||
@@ -171,6 +185,10 @@ switch ($method) {
|
|||||||
// Remove a dependency
|
// Remove a dependency
|
||||||
$data = json_decode(file_get_contents('php://input'), true);
|
$data = json_decode(file_get_contents('php://input'), true);
|
||||||
|
|
||||||
|
if (!is_array($data)) {
|
||||||
|
ResponseHelper::error('Invalid JSON');
|
||||||
|
}
|
||||||
|
|
||||||
$dependencyId = $data['dependency_id'] ?? null;
|
$dependencyId = $data['dependency_id'] ?? null;
|
||||||
|
|
||||||
// Alternative: delete by ticket IDs
|
// Alternative: delete by ticket IDs
|
||||||
@@ -179,6 +197,18 @@ switch ($method) {
|
|||||||
$dependsOnId = $data['depends_on_id'];
|
$dependsOnId = $data['depends_on_id'];
|
||||||
$type = $data['dependency_type'] ?? 'blocks';
|
$type = $data['dependency_type'] ?? 'blocks';
|
||||||
|
|
||||||
|
// Validate dependency type
|
||||||
|
$validTypes = ['blocks', 'blocked_by', 'relates_to', 'duplicates'];
|
||||||
|
if (!in_array($type, $validTypes, true)) {
|
||||||
|
ResponseHelper::error('Invalid dependency type');
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify user can access the source ticket
|
||||||
|
$srcTicket = $ticketModel->getTicketById((int)$ticketId);
|
||||||
|
if (!$srcTicket || !$ticketModel->canUserAccessTicket($srcTicket, $currentUser)) {
|
||||||
|
ResponseHelper::notFound('Ticket not found');
|
||||||
|
}
|
||||||
|
|
||||||
$result = $dependencyModel->removeDependencyByTickets($ticketId, $dependsOnId, $type);
|
$result = $dependencyModel->removeDependencyByTickets($ticketId, $dependsOnId, $type);
|
||||||
|
|
||||||
if ($result) {
|
if ($result) {
|
||||||
@@ -192,6 +222,23 @@ switch ($method) {
|
|||||||
ResponseHelper::error('Failed to remove dependency');
|
ResponseHelper::error('Failed to remove dependency');
|
||||||
}
|
}
|
||||||
} elseif ($dependencyId) {
|
} elseif ($dependencyId) {
|
||||||
|
// Look up dependency to verify ticket access before deletion
|
||||||
|
$depLookupSql = "SELECT ticket_id FROM ticket_dependencies WHERE dependency_id = ?";
|
||||||
|
$depLookupStmt = $conn->prepare($depLookupSql);
|
||||||
|
$depLookupStmt->bind_param("i", $dependencyId);
|
||||||
|
$depLookupStmt->execute();
|
||||||
|
$depRow = $depLookupStmt->get_result()->fetch_assoc();
|
||||||
|
$depLookupStmt->close();
|
||||||
|
|
||||||
|
if (!$depRow) {
|
||||||
|
ResponseHelper::notFound('Dependency not found');
|
||||||
|
}
|
||||||
|
|
||||||
|
$depTicket = $ticketModel->getTicketById((int)$depRow['ticket_id']);
|
||||||
|
if (!$depTicket || !$ticketModel->canUserAccessTicket($depTicket, $currentUser)) {
|
||||||
|
ResponseHelper::forbidden('Access denied');
|
||||||
|
}
|
||||||
|
|
||||||
$result = $dependencyModel->removeDependency($dependencyId);
|
$result = $dependencyModel->removeDependency($dependencyId);
|
||||||
|
|
||||||
if ($result) {
|
if ($result) {
|
||||||
|
|||||||
Reference in New Issue
Block a user