From 89a685a5026621c5c30fa6a49022363666039b30 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Tue, 17 Mar 2026 23:22:24 -0400 Subject: [PATCH] Integrate web_template design system and fix security/quality issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security fixes: - Add HTTP method validation to delete_comment.php (block CSRF via GET) - Remove $_GET fallback in comment deletion (was CSRF bypass vector) - Guard session_start() with session_status() check across API files - Escape json_encode() data attributes with htmlspecialchars in views - Escape inline APP_TIMEZONE config values in DashboardView/TicketView - Validate timezone param against DateTimeZone::listIdentifiers() in index.php - Remove Database::escape() (was using real_escape_string, not safe) - Fix AttachmentModel hardcoded connection; inject via constructor Backend fixes: - Fix CommentModel bind_param type for ticket_id (s→i) - Fix buildCommentThread orphan parent guard - Fix StatsModel JOIN→LEFT JOIN so unassigned tickets aren't excluded - Add ticket ID validation in BulkOperationsModel before implode() - Add duplicate key retry in TicketModel::createTicket() for race conditions - Wrap SavedFiltersModel default filter changes in transactions - Add null result guards in WorkflowModel query methods Frontend JS: - Rewrite toast.js as lt.toast shim (base.js dependency) - Delegate escapeHtml() to lt.escHtml() - Rewrite keyboard-shortcuts.js using lt.keys.on() - Migrate settings.js to lt.api.* and lt.modal.open/close() - Migrate advanced-search.js to lt.api.* and lt.modal.open/close() - Migrate dashboard.js fetch calls to lt.api.*; update all dynamic modals (bulk ops, quick actions, confirm/input) to lt-modal structure - Migrate ticket.js fetchMentionUsers to lt.api.get() - Remove console.log/error/warn calls from JS files Views: - Add /web_template/base.css and base.js to all 10 view files - Call lt.keys.initDefaults() in DashboardView, TicketView, admin views - Migrate all modal HTML from settings-modal/settings-content to lt-modal-overlay/lt-modal/lt-modal-header/lt-modal-body/lt-modal-footer - Replace style="display:none" with aria-hidden="true" on all modals - Replace modal open/close style.display with lt.modal.open/close() - Update modal buttons to lt-btn lt-btn-primary/lt-btn-ghost classes - Remove manual ESC keydown handlers (replaced by lt.keys.initDefaults) - Fix unescaped timezone values in TicketView inline script Co-Authored-By: Claude Sonnet 4.6 --- api/add_comment.php | 4 +- api/delete_attachment.php | 2 +- api/delete_comment.php | 19 +- api/download_attachment.php | 2 +- api/update_comment.php | 4 +- api/update_ticket.php | 4 +- api/upload_attachment.php | 2 +- assets/js/advanced-search.js | 94 ++---- assets/js/ascii-banner.js | 1 - assets/js/dashboard.js | 464 ++++++++++----------------- assets/js/keyboard-shortcuts.js | 342 ++++++++------------ assets/js/settings.js | 54 +--- assets/js/ticket.js | 36 +-- assets/js/toast.js | 100 +----- assets/js/utils.js | 6 +- helpers/Database.php | 10 +- index.php | 4 +- models/AttachmentModel.php | 20 +- models/BulkOperationsModel.php | 8 + models/CommentModel.php | 5 +- models/SavedFiltersModel.php | 76 +++-- models/StatsModel.php | 2 +- models/TicketModel.php | 30 +- models/WorkflowModel.php | 8 + views/CreateTicketView.php | 4 +- views/DashboardView.php | 61 ++-- views/TicketView.php | 41 +-- views/admin/ApiKeysView.php | 2 + views/admin/AuditLogView.php | 2 + views/admin/CustomFieldsView.php | 41 +-- views/admin/RecurringTicketsView.php | 41 +-- views/admin/TemplatesView.php | 41 +-- views/admin/UserActivityView.php | 2 + views/admin/WorkflowDesignerView.php | 41 +-- 34 files changed, 576 insertions(+), 997 deletions(-) diff --git a/api/add_comment.php b/api/add_comment.php index 1d9466a..62c95ef 100644 --- a/api/add_comment.php +++ b/api/add_comment.php @@ -30,7 +30,9 @@ try { require_once dirname(__DIR__) . '/helpers/Database.php'; // Check authentication via session - session_start(); + if (session_status() === PHP_SESSION_NONE) { + session_start(); + } if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { throw new Exception("Authentication required"); } diff --git a/api/delete_attachment.php b/api/delete_attachment.php index 4959c43..fdf9d55 100644 --- a/api/delete_attachment.php +++ b/api/delete_attachment.php @@ -58,7 +58,7 @@ if (!$attachmentId || !is_numeric($attachmentId)) { $attachmentId = (int)$attachmentId; try { - $attachmentModel = new AttachmentModel(); + $attachmentModel = new AttachmentModel(Database::getConnection()); // Get attachment details $attachment = $attachmentModel->getAttachment($attachmentId); diff --git a/api/delete_comment.php b/api/delete_comment.php index 4bac547..5468717 100644 --- a/api/delete_comment.php +++ b/api/delete_comment.php @@ -21,8 +21,19 @@ try { require_once dirname(__DIR__) . '/models/TicketModel.php'; require_once dirname(__DIR__) . '/models/AuditLogModel.php'; + // Only allow POST or DELETE — reject GET to prevent CSRF bypass + $method = $_SERVER['REQUEST_METHOD']; + if ($method !== 'POST' && $method !== 'DELETE') { + http_response_code(405); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Method not allowed']); + exit; + } + // Check authentication via session - session_start(); + if (session_status() === PHP_SESSION_NONE) { + session_start(); + } if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { throw new Exception("Authentication required"); } @@ -48,9 +59,9 @@ try { $data = json_decode(file_get_contents('php://input'), true); if (!$data || !isset($data['comment_id'])) { - // Try query params - if (isset($_GET['comment_id'])) { - $data = ['comment_id' => $_GET['comment_id']]; + // Also check POST params (no GET fallback — prevents CSRF bypass via URL) + if (isset($_POST['comment_id'])) { + $data = ['comment_id' => $_POST['comment_id']]; } else { throw new Exception("Missing required field: comment_id"); } diff --git a/api/download_attachment.php b/api/download_attachment.php index 1cab605..fecdc5c 100644 --- a/api/download_attachment.php +++ b/api/download_attachment.php @@ -33,7 +33,7 @@ if (!$attachmentId || !is_numeric($attachmentId)) { $attachmentId = (int)$attachmentId; try { - $attachmentModel = new AttachmentModel(); + $attachmentModel = new AttachmentModel(Database::getConnection()); // Get attachment details $attachment = $attachmentModel->getAttachment($attachmentId); diff --git a/api/update_comment.php b/api/update_comment.php index b092c48..69353d9 100644 --- a/api/update_comment.php +++ b/api/update_comment.php @@ -22,7 +22,9 @@ try { require_once dirname(__DIR__) . '/models/AuditLogModel.php'; // Check authentication via session - session_start(); + if (session_status() === PHP_SESSION_NONE) { + session_start(); + } if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { throw new Exception("Authentication required"); } diff --git a/api/update_ticket.php b/api/update_ticket.php index 37664c8..e53f724 100644 --- a/api/update_ticket.php +++ b/api/update_ticket.php @@ -28,7 +28,9 @@ try { require_once $workflowModelPath; // Check authentication via session - session_start(); + if (session_status() === PHP_SESSION_NONE) { + session_start(); + } if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { throw new Exception("Authentication required"); } diff --git a/api/upload_attachment.php b/api/upload_attachment.php index 2576ede..218d2b2 100644 --- a/api/upload_attachment.php +++ b/api/upload_attachment.php @@ -46,7 +46,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { } try { - $attachmentModel = new AttachmentModel(); + $attachmentModel = new AttachmentModel(Database::getConnection()); $attachments = $attachmentModel->getAttachments($ticketId); // Add formatted file size and icon to each attachment diff --git a/assets/js/advanced-search.js b/assets/js/advanced-search.js index 26fe318..7ff8086 100644 --- a/assets/js/advanced-search.js +++ b/assets/js/advanced-search.js @@ -7,8 +7,7 @@ function openAdvancedSearch() { const modal = document.getElementById('advancedSearchModal'); if (modal) { - modal.style.display = 'flex'; - document.body.classList.add('modal-open'); + lt.modal.open('advancedSearchModal'); loadUsersForSearch(); populateCurrentFilters(); loadSavedFilters(); @@ -17,11 +16,7 @@ function openAdvancedSearch() { // Close advanced search modal function closeAdvancedSearch() { - const modal = document.getElementById('advancedSearchModal'); - if (modal) { - modal.style.display = 'none'; - document.body.classList.remove('modal-open'); - } + lt.modal.close('advancedSearchModal'); } // Close modal when clicking on backdrop @@ -35,10 +30,7 @@ function closeOnAdvancedSearchBackdropClick(event) { // Load users for dropdown async function loadUsersForSearch() { try { - const response = await fetch('/api/get_users.php', { - credentials: 'same-origin' - }); - const data = await response.json(); + const data = await lt.api.get('/api/get_users.php'); if (data.success && data.users) { const createdBySelect = document.getElementById('adv-created-by'); @@ -68,7 +60,7 @@ async function loadUsersForSearch() { }); } } catch (error) { - console.error('Error loading users:', error); + lt.toast.error('Error loading users'); } } @@ -163,30 +155,14 @@ async function saveCurrentFilter() { const filterCriteria = getCurrentFilterCriteria(); try { - const response = await fetch('/api/saved_filters.php', { - method: 'POST', - credentials: 'same-origin', - headers: { - 'Content-Type': 'application/json', - 'X-CSRF-Token': window.CSRF_TOKEN - }, - body: JSON.stringify({ - filter_name: filterName.trim(), - filter_criteria: filterCriteria - }) + await lt.api.post('/api/saved_filters.php', { + filter_name: filterName.trim(), + filter_criteria: filterCriteria }); - - const result = await response.json(); - - if (result.success) { - toast.success(`Filter "${filterName}" saved successfully!`, 3000); - loadSavedFilters(); - } else { - toast.error('Failed to save filter: ' + (result.error || 'Unknown error'), 4000); - } + lt.toast.success(`Filter "${filterName}" saved successfully!`, 3000); + loadSavedFilters(); } catch (error) { - console.error('Error saving filter:', error); - toast.error('Error saving filter', 4000); + lt.toast.error('Error saving filter: ' + (error.message || 'Unknown error'), 4000); } } ); @@ -233,16 +209,12 @@ function getCurrentFilterCriteria() { // Load saved filters async function loadSavedFilters() { try { - const response = await fetch('/api/saved_filters.php', { - credentials: 'same-origin' - }); - const data = await response.json(); - + const data = await lt.api.get('/api/saved_filters.php'); if (data.success && data.filters) { populateSavedFiltersDropdown(data.filters); } } catch (error) { - console.error('Error loading saved filters:', error); + lt.toast.error('Error loading saved filters'); } } @@ -277,7 +249,7 @@ function loadSavedFilter() { const criteria = JSON.parse(selectedOption.dataset.criteria); applySavedFilterCriteria(criteria); } catch (error) { - console.error('Error loading filter:', error); + lt.toast.error('Error loading filter'); } } @@ -314,9 +286,7 @@ async function deleteSavedFilter() { const selectedOption = dropdown.options[dropdown.selectedIndex]; if (!selectedOption || selectedOption.value === '') { - if (typeof toast !== 'undefined') { - toast.error('Please select a filter to delete'); - } + lt.toast.error('Please select a filter to delete'); return; } @@ -329,45 +299,21 @@ async function deleteSavedFilter() { 'error', async () => { try { - const response = await fetch('/api/saved_filters.php', { - method: 'DELETE', - credentials: 'same-origin', - headers: { - 'Content-Type': 'application/json', - 'X-CSRF-Token': window.CSRF_TOKEN - }, - body: JSON.stringify({ filter_id: filterId }) - }); - - const result = await response.json(); - - if (result.success) { - toast.success('Filter deleted successfully', 3000); - loadSavedFilters(); - resetAdvancedSearch(); - } else { - toast.error('Failed to delete filter', 4000); - } + await lt.api.delete('/api/saved_filters.php', { filter_id: filterId }); + lt.toast.success('Filter deleted successfully', 3000); + loadSavedFilters(); + resetAdvancedSearch(); } catch (error) { - console.error('Error deleting filter:', error); - toast.error('Error deleting filter', 4000); + lt.toast.error('Error deleting filter', 4000); } } ); } -// Keyboard shortcut (Ctrl+Shift+F) +// Keyboard shortcut (Ctrl+Shift+F) — ESC is handled globally by lt.keys.initDefaults() document.addEventListener('keydown', (e) => { if (e.ctrlKey && e.shiftKey && e.key === 'F') { e.preventDefault(); openAdvancedSearch(); } - - // ESC to close - if (e.key === 'Escape') { - const modal = document.getElementById('advancedSearchModal'); - if (modal && modal.style.display === 'flex') { - closeAdvancedSearch(); - } - } }); diff --git a/assets/js/ascii-banner.js b/assets/js/ascii-banner.js index 316af59..2144f23 100644 --- a/assets/js/ascii-banner.js +++ b/assets/js/ascii-banner.js @@ -60,7 +60,6 @@ function renderASCIIBanner(bannerId, containerSelector, speed = 5, addGlow = tru const container = document.querySelector(containerSelector); if (!container || !banner) { - console.error('ASCII Banner: Container or banner not found', { bannerId, containerSelector }); return; } diff --git a/assets/js/dashboard.js b/assets/js/dashboard.js index aaca91f..59d1e9a 100644 --- a/assets/js/dashboard.js +++ b/assets/js/dashboard.js @@ -504,7 +504,6 @@ function initStatusFilter() { function quickSave() { if (!window.ticketData) { - console.error('No ticket data available'); return; } @@ -512,7 +511,6 @@ function quickSave() { const prioritySelect = document.getElementById('priority-select'); if (!statusSelect || !prioritySelect) { - console.error('Status or priority select not found'); return; } @@ -568,12 +566,10 @@ function quickSave() { } } else { - console.error('Error updating ticket:', result.error || 'Unknown error'); toast.error('Error updating ticket: ' + (result.error || 'Unknown error'), 5000); } }) .catch(error => { - console.error('Error updating ticket:', error); toast.error('Error updating ticket: ' + error.message, 5000); }); } @@ -611,7 +607,12 @@ function saveTicket() { statusDisplay.className = `status-${data.status}`; statusDisplay.textContent = data.status; } + } else { + lt.toast.error('Error saving ticket: ' + (data.error || 'Unknown error')); } + }) + .catch(error => { + lt.toast.error('Error saving ticket: ' + error.message); }); } /** @@ -670,12 +671,10 @@ function loadTemplate() { document.getElementById('priority').value = template.default_priority; } } else { - console.error('Failed to load template:', data.error); toast.error('Failed to load template: ' + (data.error || 'Unknown error'), 4000); } }) .catch(error => { - console.error('Error loading template:', error); toast.error('Error loading template: ' + error.message, 4000); }); } @@ -793,7 +792,6 @@ function performBulkCloseAction(ticketIds) { } }) .catch(error => { - console.error('Error performing bulk close:', error); toast.error('Bulk close failed: ' + error.message, 5000); }); } @@ -808,44 +806,31 @@ function showBulkAssignModal() { // Create modal HTML const modalHtml = ` -