Security hardening and performance improvements
- Add visibility check to attachment downloads (prevents unauthorized access) - Fix ticket ID collision with uniqueness verification loop - Harden CSP: replace unsafe-inline with nonce-based script execution - Add IP-based rate limiting (supplements session-based) - Add visibility checks to bulk operations - Validate internal visibility requires groups - Optimize user activity query (JOINs vs subqueries) - Update documentation with design decisions and security info Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -258,8 +258,35 @@ class TicketModel {
|
||||
}
|
||||
|
||||
public function createTicket($ticketData, $createdBy = null) {
|
||||
// Generate ticket ID (9-digit format with leading zeros)
|
||||
$ticket_id = sprintf('%09d', mt_rand(1, 999999999));
|
||||
// Generate unique ticket ID (9-digit format with leading zeros)
|
||||
// Loop until we find an ID that doesn't exist to prevent collisions
|
||||
$maxAttempts = 10;
|
||||
$attempts = 0;
|
||||
$ticket_id = null;
|
||||
|
||||
do {
|
||||
$candidate_id = sprintf('%09d', mt_rand(100000000, 999999999));
|
||||
|
||||
// Check if this ID already exists
|
||||
$checkSql = "SELECT ticket_id FROM tickets WHERE ticket_id = ? LIMIT 1";
|
||||
$checkStmt = $this->conn->prepare($checkSql);
|
||||
$checkStmt->bind_param("s", $candidate_id);
|
||||
$checkStmt->execute();
|
||||
$checkResult = $checkStmt->get_result();
|
||||
|
||||
if ($checkResult->num_rows === 0) {
|
||||
$ticket_id = $candidate_id;
|
||||
}
|
||||
$checkStmt->close();
|
||||
$attempts++;
|
||||
} while ($ticket_id === null && $attempts < $maxAttempts);
|
||||
|
||||
if ($ticket_id === null) {
|
||||
return [
|
||||
'success' => false,
|
||||
'error' => 'Failed to generate unique ticket ID after ' . $maxAttempts . ' attempts'
|
||||
];
|
||||
}
|
||||
|
||||
$sql = "INSERT INTO tickets (ticket_id, title, description, status, priority, category, type, created_by, visibility, visibility_groups)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
|
||||
@@ -280,8 +307,16 @@ class TicketModel {
|
||||
$visibility = 'public';
|
||||
}
|
||||
|
||||
// Clear visibility_groups if not internal
|
||||
if ($visibility !== 'internal') {
|
||||
// Validate internal visibility requires groups
|
||||
if ($visibility === 'internal') {
|
||||
if (empty($visibilityGroups) || trim($visibilityGroups) === '') {
|
||||
return [
|
||||
'success' => false,
|
||||
'error' => 'Internal visibility requires at least one group to be specified'
|
||||
];
|
||||
}
|
||||
} else {
|
||||
// Clear visibility_groups if not internal
|
||||
$visibilityGroups = null;
|
||||
}
|
||||
|
||||
@@ -529,8 +564,13 @@ class TicketModel {
|
||||
$visibility = 'public';
|
||||
}
|
||||
|
||||
// Clear visibility_groups if not internal
|
||||
if ($visibility !== 'internal') {
|
||||
// Validate internal visibility requires groups
|
||||
if ($visibility === 'internal') {
|
||||
if (empty($visibilityGroups) || trim($visibilityGroups) === '') {
|
||||
return false; // Internal visibility requires groups
|
||||
}
|
||||
} else {
|
||||
// Clear visibility_groups if not internal
|
||||
$visibilityGroups = null;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user