diff --git a/.env.example b/.env.example index 27a1103..cbce12a 100644 --- a/.env.example +++ b/.env.example @@ -1,5 +1,11 @@ # Tinker Tickets Environment Configuration # Copy this file to .env and fill in your values +# +# NOTE: This file is parsed with parse_ini_file(). Any value containing special +# characters (#, ;, =, quotes, spaces, etc.) MUST be wrapped in double quotes, +# e.g. DB_PASS="p@ss;word#1". The application now fails loudly (dies with a clear +# error) if the .env file cannot be parsed, so an unquoted special character will +# take the whole app down rather than silently using a wrong value. # Database Configuration DB_HOST=10.10.10.50 @@ -25,11 +31,18 @@ APP_DOMAIN= ALLOWED_HOSTS=localhost,127.0.0.1 # Trusted reverse proxy IP(s), comma-separated (e.g. the Authelia/nginx proxy). -# STRONGLY RECOMMENDED in production: Authelia forward-auth (Remote-User / -# Remote-Groups) and forwarded client IPs are only trusted when REMOTE_ADDR is -# in this list. Leaving it empty disables that protection (relies solely on -# network topology) and lets anything reaching PHP directly spoof admin login. -# Exact IP match only (no CIDR). Example: TRUSTED_PROXIES=10.10.10.27 +# Set this to the IP address(es) of your reverse proxy. Authelia forward-auth +# headers (Remote-User / Remote-Groups) and forwarded client IPs are only +# trusted when REMOTE_ADDR is in this list. +# +# Leaving this EMPTY disables reverse-proxy verification entirely: the app then +# trusts Remote-User / Remote-Groups headers from ANY source. That is unsafe if +# the PHP backend is reachable directly (bypassing the proxy), because a client +# can then spoof those headers and log in as an admin. Only leave it empty when +# network topology guarantees PHP is reachable solely via the trusted proxy. +# +# Exact IP match only (no CIDR). Example (single proxy): TRUSTED_PROXIES=10.10.10.27 +# Example (multiple): TRUSTED_PROXIES=10.10.10.27,10.10.10.28 TRUSTED_PROXIES= # Timezone (default: America/New_York) diff --git a/README.md b/README.md index 08ca70f..b322062 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ The following features are intentionally **not planned** for this system: - **Duplicate Detection**: Similarity check on ticket title surfaces potential duplicates with one-click linking - **Activity Timeline**: Full `lt-timeline` audit trail — color-coded by event type (status, comment, assign, attach) - **Watcher Avatars**: Avatar group shows who is watching a ticket; tooltip lists all names -- **SLA Timer**: P1/P2 tickets display a live elapsed-time banner with progress bar (P1 = 8 h, P2 = 24 h, P3 = 72 h) +- **SLA Timer**: P1/P2 tickets display a live elapsed-time banner with progress bar (P1 = 8 h, P2 = 24 h). Lower priorities (P3–P5) have no SLA banner. - **Priority Alert Banner**: P1 shows a sticky error banner; P2 shows a warning banner — dismissible per session ### Ticket Templates @@ -121,7 +121,7 @@ The following features are intentionally **not planned** for this system: - **Powered by audit_log**: No extra table — notifications are derived from existing audit trail ### Matrix Notifications (hookshot) -- **Ticket Created**: Fires when any ticket is created (manual or via API) +- **Ticket Created**: Fires when a ticket is created via the manual form, the external API (hwmonDaemon), or the recurring-ticket cron. (Cloned tickets do not fire this event.) - **Status Changed**: Fires on every status transition - **@Mentions**: Mentioned users receive a direct Matrix notification - **Assignment**: Optional — set `MATRIX_NOTIFY_ASSIGNMENTS=1` to enable @@ -150,7 +150,7 @@ The following features are intentionally **not planned** for this system: | `?` | Show keyboard shortcuts help | ### Security Features -- **CSRF Protection**: Token-based protection with constant-time comparison; token rotated after each write +- **CSRF Protection**: Token-based protection with constant-time comparison. `bootstrap.php` rotates the token on a successful write and returns the current token in every response (including on rejection); the client (`lt.api`) resyncs from that value. Rejected requests do not rotate the token. - **Rate Limiting**: Session-based AND IP-based rate limiting to prevent abuse - **Security Headers**: CSP with nonces (no unsafe-inline), X-Frame-Options, X-Content-Type-Options - **SQL Injection Prevention**: All queries use prepared statements with parameter binding @@ -179,9 +179,9 @@ Content-Type: application/json **Key behaviours:** - Authenticated via `Authorization: Bearer` header — API key stored in `/etc/hwmonDaemon/.env` -- **Deduplication**: Generates a SHA-256 hash from the issue category, hostname, and device; rejects duplicate tickets within 24 hours +- **Deduplication**: Generates a SHA-256 hash from the issue category, hostname, and device (no time window). A repeat alert matching an existing **open** ticket updates its title/description and escalates the priority if the condition worsened; if the matching ticket was already **closed**, it is reopened instead of creating a new one - Cluster-wide issues (Ceph health, etc.) deduplicate across all nodes (hostname excluded from hash) -- Matrix notification sent automatically after ticket creation +- Matrix notification sent automatically on ticket creation, priority escalation, and reopen - API key must be generated at `/admin/api-keys`; the key goes in hwmonDaemon's `/etc/hwmonDaemon/.env` as `TICKET_API_KEY` ## Technical Architecture @@ -240,6 +240,11 @@ Content-Type: application/json - `tickets`: `ticket_id` (unique), `status`, `priority`, `created_at`, `created_by`, `assigned_to`, `visibility` - `audit_log`: `user_id`, `action_type`, `entity_type`, `created_at` +### Database Schema / Migrations + +- `migrations/000_baseline.sql` is the full schema baseline for the whole database. It is written to be safe to re-run (idempotent) and is the source of truth for a fresh install. +- `php migrations/migrate.php` applies any pending migration files in `migrations/` in order, tracking applied files in the `migrations` table. Use `--status` to list state and `--dry-run` to preview without executing. + ### API Endpoints | Endpoint | Method | Description | @@ -248,6 +253,7 @@ Content-Type: application/json | `/api/update_ticket.php` | POST | Update ticket with workflow validation | | `/api/assign_ticket.php` | POST | Assign ticket to user | | `/api/add_comment.php` | POST | Add comment to ticket | +| `/api/get_comments.php` | GET | Fetch paginated comments for a ticket | | `/api/clone_ticket.php` | POST | Clone an existing ticket | | `/api/get_template.php` | GET | Fetch ticket template | | `/api/get_users.php` | GET | Get user list for assignments | @@ -292,6 +298,7 @@ tinker_tickets/ │ ├── download_attachment.php # GET: Download with visibility check │ ├── export_tickets.php # GET: Export tickets to CSV/JSON │ ├── generate_api_key.php # POST: Generate API key (admin) +│ ├── get_comments.php # GET: Fetch paginated ticket comments │ ├── get_template.php # GET: Fetch ticket template │ ├── get_users.php # GET: Get user list │ ├── health.php # GET: Health check endpoint @@ -329,14 +336,20 @@ tinker_tickets/ ├── config/ │ └── config.php # Config + .env loading ├── controllers/ +│ ├── CommentController.php # Comment create/edit/delete + notifications │ ├── DashboardController.php # Dashboard with stats + filters │ └── TicketController.php # Ticket CRUD + timeline + visibility ├── cron/ +│ ├── cleanup_audit_log.php # Delete audit_log rows past retention (daily) +│ ├── cleanup_ratelimit.php # Purge expired rate-limit files (every few min) │ └── create_recurring_tickets.php # Process recurring ticket schedules ├── helpers/ │ ├── CacheHelper.php # File-based cache (stats, avatars) │ ├── Database.php # Centralized mysqli connection +│ ├── ErrorHandler.php # Global error/exception handler │ ├── NotificationHelper.php # Matrix hookshot webhook events +│ ├── OutputHelper.php # Safe HTML output helpers +│ ├── ResponseHelper.php # JSON API response helpers │ ├── SynapseHelper.php # Resolves usernames → Matrix IDs via Synapse admin API │ └── UrlHelper.php # Canonical ticket URLs using APP_DOMAIN ├── middleware/ @@ -347,6 +360,7 @@ tinker_tickets/ │ └── SecurityHeadersMiddleware.php # CSP headers with per-request nonce generation ├── models/ │ ├── ApiKeyModel.php # API key generation/validation +│ ├── AttachmentModel.php # Ticket file attachment metadata │ ├── AuditLogModel.php # Audit logging + timeline │ ├── BulkOperationsModel.php # Bulk operations tracking │ ├── CommentModel.php # Comment data access @@ -360,11 +374,12 @@ tinker_tickets/ │ ├── UserModel.php # User management + groups │ ├── UserPreferencesModel.php # User preferences │ └── WorkflowModel.php # Status transition workflows +├── migrations/ +│ ├── 000_baseline.sql # Full schema baseline (safe to re-run) +│ └── migrate.php # CLI migration runner (tracks applied migrations) ├── scripts/ -│ ├── add_closed_at_column.php # Migration: add closed_at column to tickets -│ ├── add_comment_updated_at.php # Migration: add updated_at column to ticket_comments -│ ├── cleanup_orphan_uploads.php # Clean orphaned uploads (run manually or via cron) -│ └── create_dependencies_table.php # Create ticket_dependencies table +│ ├── check_requirements.php # Verify PHP extensions/config prerequisites +│ └── cleanup_orphan_uploads.php # Delete orphaned upload files past grace period (cron) ├── uploads/ # File attachment storage │ └── avatars/ # lldap avatar disk cache ├── views/ @@ -455,13 +470,20 @@ AVATAR_CACHE_TTL=3600 ### 2. Cron Jobs -Add to crontab for recurring tickets and optional cleanup: +Add to crontab for recurring tickets and maintenance cleanup: ```bash # Run every hour to create scheduled recurring tickets 0 * * * * php /path/to/tinkertickets/cron/create_recurring_tickets.php -# Optional: clean up orphaned uploads weekly -0 3 * * 0 php /path/to/tinkertickets/scripts/cleanup_orphan_uploads.php +# Purge expired rate-limit files (every 5 minutes) +*/5 * * * * php /path/to/tinkertickets/cron/cleanup_ratelimit.php + +# Delete audit_log rows older than AUDIT_LOG_RETENTION_DAYS (daily) +30 3 * * * php /path/to/tinkertickets/cron/cleanup_audit_log.php + +# Delete orphaned upload files with no attachment row, past a 24h grace period (daily). +# Add --dry-run to preview without deleting. +0 4 * * * php /path/to/tinkertickets/scripts/cleanup_orphan_uploads.php ``` ### 3. File Uploads @@ -502,7 +524,7 @@ Key conventions and gotchas for working with this codebase: 3. **Admin check**: `$_SESSION['user']['is_admin'] ?? false` 4. **Config path**: `config/config.php` (not `config/db.php`) 5. **Comments table**: `ticket_comments` (not `comments`) -6. **CSRF**: Required for all POST/DELETE requests via `X-CSRF-Token` header; bootstrap.php rotates token and returns it in `csrf_token` field of all `apiRespond()` responses +6. **CSRF**: Required for all POST/DELETE requests via `X-CSRF-Token` header. `bootstrap.php` rotates the token only on a successful write and returns the current token in the `csrf_token` field of every `apiRespond()` response (including rejections), so the client can resync. A rejected request keeps the existing token. 7. **Cache busting**: `ASSET_VERSION` is auto-computed from asset file mtimes; override with `ASSET_VERSION=` in `.env` 8. **Ticket linking**: Use `#123456789` in markdown-enabled comments 9. **User groups**: Stored in `users.groups` as comma-separated values @@ -520,8 +542,8 @@ Key conventions and gotchas for working with this codebase: 21. **Confirm dialogs**: Never use browser `confirm()`. Use `showConfirmModal(title, message, type, onConfirm)` (defined in `utils.js`, available on all pages). Types: `'warning'` | `'error'` | `'info'`. 22. **`utils.js` on all pages**: `utils.js` is loaded by all views (including admin). It provides `escapeHtml()`, `getTicketIdFromUrl()`, and `showConfirmModal()`. 23. **No `toast.js`**: `toast.js` is deprecated and no longer loaded by any view. Use `lt.toast.success/error/warning/info()` directly from `base.js`. -24. **Stats cache**: `StatsModel` caches stats for 60 s. Any API that modifies ticket state must call `(new StatsModel($conn))->invalidateCache()` after changes (bulk_operation, assign_ticket, update_ticket, clone_ticket all do this). -25. **External API (`create_ticket_api.php`)**: Uses `ApiKeyAuth` (Bearer token), not session auth. Served directly by the web server from the document root — not through the index.php router. Includes deduplication logic to prevent duplicate hw-alert tickets within 24 h. +24. **Stats cache**: `StatsModel` caches stats for 60 s. Any path that modifies ticket state must call `(new StatsModel($conn))->invalidateCache()` after the change. Callers: `TicketController::create` (manual create), `create_ticket_api.php` (external API create/escalate/reopen), `cron/create_recurring_tickets.php`, `bulk_operation`, `assign_ticket`, `update_ticket`, and `clone_ticket`. +25. **External API (`create_ticket_api.php`)**: Uses `ApiKeyAuth` (Bearer token), not session auth. Served directly by the web server from the document root — not through the index.php router. Includes deduplication logic (SHA-256 hash, no time window) that updates/escalates an existing open duplicate or reopens a closed one rather than creating a new ticket. ## File Reference @@ -557,7 +579,7 @@ Key conventions and gotchas for working with this codebase: |---------|---------------| | SQL Injection | All queries use prepared statements with parameter binding | | XSS Prevention | HTML escaped in markdown parser; CSP with per-request nonces | -| CSRF Protection | Token-based with constant-time comparison (`hash_equals`); rotated on each write | +| CSRF Protection | Token-based with constant-time comparison (`hash_equals`); rotated on successful writes, current token returned in every response (including rejections) for the client to resync — rejected requests do not rotate | | Session Security | Fixation prevention, secure cookies, session timeout | | Rate Limiting | Session-based + IP-based (file storage) | | File Security | Path traversal prevention, MIME type validation, uploads `.htaccess` blocks execution | diff --git a/api/add_comment.php b/api/add_comment.php index 0b13d40..d78be47 100644 --- a/api/add_comment.php +++ b/api/add_comment.php @@ -38,12 +38,16 @@ try { session_start(); } if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { - throw new Exception("Authentication required"); + ob_end_clean(); + http_response_code(401); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Authentication required']); + exit; } - // CSRF Protection + // CSRF Protection for all state-changing methods (any non-GET/HEAD request) require_once dirname(__DIR__) . '/middleware/CsrfMiddleware.php'; - if ($_SERVER['REQUEST_METHOD'] === 'POST') { + if (!in_array($_SERVER['REQUEST_METHOD'], ['GET', 'HEAD'], true)) { $csrfToken = $_SERVER['HTTP_X_CSRF_TOKEN'] ?? ''; if (!CsrfMiddleware::validateToken($csrfToken)) { http_response_code(403); @@ -63,7 +67,11 @@ try { $data = json_decode(file_get_contents('php://input'), true); if (!$data) { - throw new Exception("Invalid JSON data received"); + http_response_code(400); + ob_end_clean(); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Invalid JSON data received']); + exit; } $ticketId = isset($data['ticket_id']) ? trim((string)$data['ticket_id']) : ''; @@ -75,6 +83,20 @@ try { exit; } + // Reject empty/whitespace-only comments + $commentTextRaw = isset($data['comment_text']) ? trim((string)$data['comment_text']) : ''; + if ($commentTextRaw === '') { + http_response_code(400); + ob_end_clean(); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Comment text cannot be empty']); + exit; + } + + // Never trust a client-supplied display name — always attribute the comment to + // the authenticated session user. + $data['user_name'] = $currentUser['display_name'] ?? $currentUser['username'] ?? 'User'; + // Verify user can access the ticket before allowing a comment $ticketModel = new TicketModel($conn); $ticket = $ticketModel->getTicketById($ticketId); @@ -97,6 +119,18 @@ try { $commentModel = new CommentModel($conn); $auditLog = new AuditLogModel($conn); + // If replying, the parent comment must belong to this same (accessible) ticket. + if (isset($data['parent_comment_id']) && $data['parent_comment_id'] !== null && $data['parent_comment_id'] !== '') { + $parentComment = $commentModel->getCommentById((int)$data['parent_comment_id']); + if (!$parentComment || (string)$parentComment['ticket_id'] !== (string)$ticketId) { + http_response_code(400); + ob_end_clean(); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Invalid parent comment']); + exit; + } + } + // Extract @mentions from comment text $mentions = $commentModel->extractMentions($data['comment_text'] ?? ''); $mentionedUsers = []; @@ -130,6 +164,7 @@ try { $authorDisplay = $currentUser['display_name'] ?? $currentUser['username'] ?? null; $commentText = $data['comment_text'] ?? ''; $ticketTitle = $ticket['title'] ?? "Ticket #{$ticketId}"; + $ticketVisibility = $ticket['visibility'] ?? 'public'; // @mention notifications — resolve usernames → Matrix IDs via Synapse Admin API if (!empty($mentionedUsers)) { @@ -142,7 +177,14 @@ try { // General comment notification (opt-in via MATRIX_NOTIFY_COMMENTS) if (!empty($GLOBALS['config']['MATRIX_NOTIFY_COMMENTS'])) { - NotificationHelper::sendCommentNotification($ticketId, $ticketTitle, $commentText, $authorDisplay); + NotificationHelper::sendCommentNotification( + $ticketId, + $ticketTitle, + $commentText, + $authorDisplay, + $ticketVisibility !== 'public', + $ticketVisibility + ); } // Notify watchers of the new comment @@ -152,7 +194,8 @@ try { $ticketTitle, 'comment_added', ['author' => $authorDisplay, 'preview' => mb_strimwidth($commentText, 0, 200, '…')], - (int)$userId + (int)$userId, + $ticketVisibility ); // Add mentioned users to result for frontend diff --git a/api/audit_log.php b/api/audit_log.php index dc0248d..59a71dd 100644 --- a/api/audit_log.php +++ b/api/audit_log.php @@ -9,6 +9,22 @@ require_once __DIR__ . '/bootstrap.php'; require_once dirname(__DIR__) . '/models/AuditLogModel.php'; +/** + * Neutralize CSV/formula injection: prefix a leading apostrophe to any cell that + * a spreadsheet (Excel/Sheets) would otherwise evaluate as a formula. + * + * @param mixed $value + * @return string + */ +function auditCsvSafeCell($value): string +{ + $value = (string)$value; + if ($value !== '' && in_array($value[0], ['=', '+', '-', '@', "\t", "\r"], true)) { + return "'" . $value; + } + return $value; +} + // Check admin status - audit log viewing is admin-only if (!$isAdmin) { http_response_code(403); @@ -69,7 +85,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { $details = json_encode($log['details']); } - fputcsv($output, [ + fputcsv($output, array_map('auditCsvSafeCell', [ $log['audit_id'] ?? ($log['log_id'] ?? ''), $log['created_at'], $log['display_name'] ?? $log['username'] ?? 'N/A', @@ -78,7 +94,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { $log['entity_id'] ?? 'N/A', $log['ip_address'] ?? 'N/A', $details - ]); + ])); } fclose($output); diff --git a/api/bootstrap.php b/api/bootstrap.php index 6256ffa..dd2e70c 100644 --- a/api/bootstrap.php +++ b/api/bootstrap.php @@ -34,9 +34,16 @@ if (in_array($_SERVER['REQUEST_METHOD'], ['POST', 'PUT', 'DELETE'])) { require_once dirname(__DIR__) . '/middleware/CsrfMiddleware.php'; $csrfToken = $_SERVER['HTTP_X_CSRF_TOKEN'] ?? ''; if (!CsrfMiddleware::validateToken($csrfToken)) { + // Do NOT rotate on a rejected request. Return the current valid token so a + // client whose token drifted out of sync can recover on its next request + // (the response body is same-origin only, so this can't aid a CSRF attacker). http_response_code(403); header('Content-Type: application/json'); - echo json_encode(['success' => false, 'error' => 'Invalid CSRF token']); + echo json_encode([ + 'success' => false, + 'error' => 'Invalid CSRF token', + 'csrf_token' => CsrfMiddleware::getToken() + ]); exit; } // Rotate token after successful validation; endpoints include it in their JSON response diff --git a/api/bulk_operation.php b/api/bulk_operation.php index 6d0d93a..f25f73c 100644 --- a/api/bulk_operation.php +++ b/api/bulk_operation.php @@ -19,9 +19,9 @@ if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { exit; } -// CSRF Protection +// CSRF Protection for all state-changing methods (any non-GET/HEAD request) require_once dirname(__DIR__) . '/middleware/CsrfMiddleware.php'; -if ($_SERVER['REQUEST_METHOD'] === 'POST') { +if (!in_array($_SERVER['REQUEST_METHOD'], ['GET', 'HEAD'], true)) { $csrfToken = $_SERVER['HTTP_X_CSRF_TOKEN'] ?? ''; if (!CsrfMiddleware::validateToken($csrfToken)) { http_response_code(403); @@ -47,6 +47,7 @@ $parameters = $data['parameters'] ?? null; // Validate input $validOperationTypes = ['bulk_close', 'bulk_assign', 'bulk_priority', 'bulk_status', 'bulk_delete']; if (!$operationType || !in_array($operationType, $validOperationTypes, true) || empty($ticketIds)) { + http_response_code(400); echo json_encode(['success' => false, 'error' => 'Operation type and ticket IDs required']); exit; } @@ -57,6 +58,7 @@ $ticketIds = array_values(array_filter(array_map(function ($id) { return (ctype_digit($s) && (int)$s > 0) ? $s : null; }, $ticketIds))); if (empty($ticketIds)) { + http_response_code(400); echo json_encode(['success' => false, 'error' => 'No valid ticket IDs provided']); exit; } diff --git a/api/custom_fields.php b/api/custom_fields.php index 50ca414..2ca16a9 100644 --- a/api/custom_fields.php +++ b/api/custom_fields.php @@ -15,6 +15,7 @@ try { require_once dirname(__DIR__) . '/config/config.php'; require_once dirname(__DIR__) . '/helpers/Database.php'; require_once dirname(__DIR__) . '/models/CustomFieldModel.php'; + require_once dirname(__DIR__) . '/models/AuditLogModel.php'; // Check authentication if (session_status() === PHP_SESSION_NONE) { @@ -50,6 +51,8 @@ try { header('Content-Type: application/json'); $model = new CustomFieldModel($conn); + $auditLog = new AuditLogModel($conn); + $currentUserId = $_SESSION['user']['user_id']; $method = $_SERVER['REQUEST_METHOD']; $id = isset($_GET['id']) ? (int)$_GET['id'] : null; $category = isset($_GET['category']) ? $_GET['category'] : null; @@ -75,6 +78,13 @@ try { exit; } $result = $model->createDefinition($data); + if (!empty($result['success'])) { + $auditLog->log($currentUserId, 'create', 'custom_field', (string)($result['field_id'] ?? ''), [ + 'field_name' => $data['field_name'] ?? null, + 'field_label' => $data['field_label'] ?? null, + 'field_type' => $data['field_type'] ?? null + ]); + } echo json_encode($result); break; @@ -92,6 +102,14 @@ try { exit; } $result = $model->updateDefinition($id, $data); + if (!empty($result['success'])) { + $auditLog->log($currentUserId, 'update', 'custom_field', (string)$id, [ + 'entity' => 'custom_field', + 'field_name' => $data['field_name'] ?? null, + 'field_label' => $data['field_label'] ?? null, + 'field_type' => $data['field_type'] ?? null + ]); + } echo json_encode($result); break; @@ -102,7 +120,14 @@ try { exit; } + $toDelete = $model->getDefinition($id); $result = $model->deleteDefinition($id); + if (!empty($result['success'])) { + $auditLog->log($currentUserId, 'delete', 'custom_field', (string)$id, [ + 'entity' => 'custom_field', + 'field_name' => $toDelete['field_name'] ?? 'unknown' + ]); + } echo json_encode($result); break; diff --git a/api/delete_comment.php b/api/delete_comment.php index 85273cf..9b11935 100644 --- a/api/delete_comment.php +++ b/api/delete_comment.php @@ -36,7 +36,11 @@ try { session_start(); } if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { - throw new Exception("Authentication required"); + ob_end_clean(); + http_response_code(401); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Authentication required']); + exit; } // CSRF Protection @@ -64,7 +68,11 @@ try { if (isset($_POST['comment_id'])) { $data = ['comment_id' => $_POST['comment_id']]; } else { - throw new Exception("Missing required field: comment_id"); + ob_end_clean(); + http_response_code(400); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Missing required field: comment_id']); + exit; } } diff --git a/api/export_tickets.php b/api/export_tickets.php index 549dd42..5972694 100644 --- a/api/export_tickets.php +++ b/api/export_tickets.php @@ -15,6 +15,22 @@ error_reporting(E_ALL); require_once dirname(__DIR__) . '/middleware/RateLimitMiddleware.php'; RateLimitMiddleware::apply('api'); +/** + * Neutralize CSV/formula injection: prefix a leading apostrophe to any cell that + * a spreadsheet (Excel/Sheets) would otherwise evaluate as a formula. + * + * @param mixed $value + * @return string + */ +function exportCsvSafeCell($value): string +{ + $value = (string)$value; + if ($value !== '' && in_array($value[0], ['=', '+', '-', '@', "\t", "\r"], true)) { + return "'" . $value; + } + return $value; +} + try { // Include required files require_once dirname(__DIR__) . '/config/config.php'; @@ -124,7 +140,7 @@ try { $ticket['updated_at'], $ticket['description'] ]; - fputcsv($output, $row); + fputcsv($output, array_map('exportCsvSafeCell', $row)); } fclose($output); diff --git a/api/generate_api_key.php b/api/generate_api_key.php index faca757..77f6dec 100644 --- a/api/generate_api_key.php +++ b/api/generate_api_key.php @@ -24,11 +24,13 @@ try { session_start(); } if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { + http_response_code(401); throw new Exception("Authentication required"); } // Check admin privileges if (!isset($_SESSION['user']['is_admin']) || !$_SESSION['user']['is_admin']) { + http_response_code(403); throw new Exception("Admin privileges required"); } @@ -51,6 +53,7 @@ try { // Get request data $input = json_decode(file_get_contents('php://input'), true); if (!$input) { + http_response_code(400); throw new Exception("Invalid request data"); } @@ -58,10 +61,12 @@ try { $expiresInDays = $input['expires_in_days'] ?? null; if (empty($keyName)) { + http_response_code(400); throw new Exception("Key name is required"); } if (strlen($keyName) > 100) { + http_response_code(400); throw new Exception("Key name must be 100 characters or less"); } @@ -69,6 +74,7 @@ try { if ($expiresInDays !== null && $expiresInDays !== '') { $expiresInDays = (int)$expiresInDays; if ($expiresInDays < 1 || $expiresInDays > 3650) { + http_response_code(400); throw new Exception("Expiration must be between 1 and 3650 days"); } } else { @@ -110,11 +116,26 @@ try { ]); } catch (Exception $e) { ob_end_clean(); - error_log("Generate API key error: " . $e->getMessage()); header('Content-Type: application/json'); - http_response_code(isset($conn) ? 400 : 500); - echo json_encode([ - 'success' => false, - 'error' => 'An internal error occurred' - ]); + + // Preserve any specific status set before the throw (401/403/400/...); + // only fall back to 500 when nothing more specific was set. + $code = http_response_code(); + if (!is_int($code) || $code < 400) { + $code = 500; + } + http_response_code($code); + + if ($code >= 500) { + error_log("Generate API key error: " . $e->getMessage()); + echo json_encode([ + 'success' => false, + 'error' => 'An internal error occurred' + ]); + } else { + echo json_encode([ + 'success' => false, + 'error' => $e->getMessage() + ]); + } } diff --git a/api/health.php b/api/health.php index 6f712f5..908eb31 100644 --- a/api/health.php +++ b/api/health.php @@ -135,11 +135,19 @@ $responseTime = round((microtime(true) - $startTime) * 1000, 2); // Set status code http_response_code($healthy ? 200 : 503); +// This endpoint is unauthenticated, so expose only a coarse per-component status +// and never the diagnostic messages (they leak PHP_VERSION, exact missing +// extension names, and filesystem paths to anonymous callers). +$publicChecks = []; +foreach ($checks as $name => $check) { + $publicChecks[$name] = ['status' => $check['status']]; +} + // Return response echo json_encode([ 'status' => $healthy ? 'healthy' : 'unhealthy', 'timestamp' => date('c'), 'response_time_ms' => $responseTime, - 'checks' => $checks, + 'checks' => $publicChecks, 'version' => '1.0.0' ], JSON_PRETTY_PRINT); diff --git a/api/manage_recurring.php b/api/manage_recurring.php index 96c91c9..79c196e 100644 --- a/api/manage_recurring.php +++ b/api/manage_recurring.php @@ -15,6 +15,7 @@ try { require_once dirname(__DIR__) . '/config/config.php'; require_once dirname(__DIR__) . '/helpers/Database.php'; require_once dirname(__DIR__) . '/models/RecurringTicketModel.php'; + require_once dirname(__DIR__) . '/models/AuditLogModel.php'; // Check authentication if (session_status() === PHP_SESSION_NONE) { @@ -52,6 +53,7 @@ try { header('Content-Type: application/json'); $model = new RecurringTicketModel($conn); + $auditLog = new AuditLogModel($conn); $method = $_SERVER['REQUEST_METHOD']; $id = isset($_GET['id']) ? (int)$_GET['id'] : null; $action = isset($_GET['action']) ? $_GET['action'] : null; @@ -70,6 +72,12 @@ try { case 'POST': if ($action === 'toggle' && $id) { $result = $model->toggleActive($id); + if (!empty($result['success'])) { + $auditLog->log($currentUserId, 'update', 'recurring_ticket', (string)$id, [ + 'entity' => 'recurring_ticket', + 'action' => 'toggle_active' + ]); + } echo json_encode($result); } else { $data = json_decode(file_get_contents('php://input'), true); @@ -90,6 +98,14 @@ try { $data['created_by'] = $currentUserId; $result = $model->create($data); + if (!empty($result['success'])) { + $auditLog->log($currentUserId, 'create', 'recurring_ticket', (string)($result['recurring_id'] ?? ''), [ + 'title_template' => $data['title_template'], + 'schedule_type' => $data['schedule_type'], + 'schedule_day' => $data['schedule_day'] ?? null, + 'schedule_time' => $data['schedule_time'] ?? '09:00' + ]); + } echo json_encode($result); } break; @@ -106,16 +122,49 @@ try { exit; } - // Recalculate next run time if schedule changed - $nextRun = calculateNextRun( - $data['schedule_type'], - $data['schedule_day'] ?? null, - $data['schedule_time'] ?? '09:00' - ); - $data['next_run_at'] = $nextRun; + $existing = $model->getById($id); + if (!$existing) { + echo json_encode(['success' => false, 'error' => 'Recurring ticket not found']); + exit; + } + + $newDay = $data['schedule_day'] ?? null; + $newTime = $data['schedule_time'] ?? '09:00'; + + // Only the schedule fields affect when the next occurrence fires. + $scheduleChanged = + (string)$existing['schedule_type'] !== (string)$data['schedule_type'] + || (string)($existing['schedule_day'] ?? '') !== (string)($newDay ?? '') + || substr((string)$existing['schedule_time'], 0, 5) !== substr((string)$newTime, 0, 5); + + $existingNextFuture = !empty($existing['next_run_at']) + && strtotime($existing['next_run_at']) > time(); + + // Recompute only when the schedule actually changed (or the stored + // next_run is already in the past). Editing an unrelated field (e.g. + // title) must NOT move next_run_at backwards past an occurrence that + // may already have fired, which would double-create a ticket. + if ($scheduleChanged || !$existingNextFuture) { + $data['next_run_at'] = calculateNextRun( + $data['schedule_type'], + $newDay, + $newTime + ); + } else { + $data['next_run_at'] = $existing['next_run_at']; + } $data['is_active'] = isset($data['is_active']) ? (int)$data['is_active'] : 1; $result = $model->update($id, $data); + if (!empty($result['success'])) { + $auditLog->log($currentUserId, 'update', 'recurring_ticket', (string)$id, [ + 'entity' => 'recurring_ticket', + 'title_template' => $data['title_template'] ?? null, + 'schedule_type' => $data['schedule_type'], + 'schedule_day' => $newDay, + 'schedule_time' => $newTime + ]); + } echo json_encode($result); break; @@ -125,7 +174,14 @@ try { exit; } + $toDelete = $model->getById($id); $result = $model->delete($id); + if (!empty($result['success'])) { + $auditLog->log($currentUserId, 'delete', 'recurring_ticket', (string)$id, [ + 'entity' => 'recurring_ticket', + 'title_template' => $toDelete['title_template'] ?? 'unknown' + ]); + } echo json_encode($result); break; @@ -139,36 +195,77 @@ try { echo json_encode(['success' => false, 'error' => 'An internal error occurred']); } -function calculateNextRun($scheduleType, $scheduleDay, $scheduleTime) +/** + * Compute the SOONEST FUTURE occurrence matching the schedule. + * + * Returns 'Y-m-d H:i:s' in the app-configured timezone. The current period is + * NOT skipped: a schedule whose time today/this-month is still in the future + * fires then, not one period later. + * + * @param string $scheduleType daily|weekly|monthly + * @param int|null $scheduleDay 1-7 (ISO, 1=Mon..7=Sun) weekly; 1-31 monthly + * @param string $scheduleTime HH:MM or HH:MM:SS + * @param DateTime|null $now Injected "now" for testing + */ +function calculateNextRun($scheduleType, $scheduleDay, $scheduleTime, ?DateTime $now = null) { - $now = new DateTime(); - $time = $scheduleTime ?: '09:00'; + $tz = new DateTimeZone($GLOBALS['config']['TIMEZONE'] ?? date_default_timezone_get()); + $now = $now ? $now : new DateTime('now', $tz); + + $parts = explode(':', $scheduleTime ?: '09:00'); + $hour = (int)($parts[0] ?? 9); + $minute = (int)($parts[1] ?? 0); + $second = (int)($parts[2] ?? 0); + + $next = clone $now; switch ($scheduleType) { - case 'daily': - $next = new DateTime('tomorrow ' . $time); - break; - case 'weekly': - $days = [1 => 'Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday', 'Sunday']; - $dayName = $days[(int)$scheduleDay] ?? 'Monday'; - $next = new DateTime("next {$dayName} " . $time); + $targetDow = (int)$scheduleDay; + if ($targetDow < 1 || $targetDow > 7) { + $targetDow = 1; + } + $next->setTime($hour, $minute, $second); + $currentDow = (int)$next->format('N'); // 1=Mon .. 7=Sun + $daysAhead = ($targetDow - $currentDow + 7) % 7; + // Same weekday but the time already passed today -> next week. + if ($daysAhead === 0 && $next <= $now) { + $daysAhead = 7; + } + if ($daysAhead > 0) { + $next->modify("+{$daysAhead} day"); + $next->setTime($hour, $minute, $second); + } break; case 'monthly': $day = max(1, min(31, (int)$scheduleDay)); - $next = new DateTime(); - $next->modify('first day of next month'); - // Clamp to last day of target month (handles Feb, 30-day months) - $daysInMonth = (int)$next->format('t'); - $day = min($day, $daysInMonth); - $next->setDate((int)$next->format('Y'), (int)$next->format('m'), $day); - $parts = explode(':', $time . ':00'); // ensure at least H:M - $next->setTime((int)$parts[0], (int)$parts[1], 0); + // This month first, clamped to the month's length (e.g. day 31 -> Feb 28/29). + $daysInMonth = (int)$now->format('t'); + $next->setDate((int)$now->format('Y'), (int)$now->format('n'), min($day, $daysInMonth)); + $next->setTime($hour, $minute, $second); + if ($next <= $now) { + // Already passed this month -> first day of next month, then clamp. + $firstNext = clone $now; + $firstNext->modify('first day of next month'); + $daysInMonth = (int)$firstNext->format('t'); + $next->setDate( + (int)$firstNext->format('Y'), + (int)$firstNext->format('n'), + min($day, $daysInMonth) + ); + $next->setTime($hour, $minute, $second); + } break; + case 'daily': default: - $next = new DateTime('tomorrow ' . $time); + $next->setTime($hour, $minute, $second); + if ($next <= $now) { + $next->modify('+1 day'); + $next->setTime($hour, $minute, $second); + } + break; } return $next->format('Y-m-d H:i:s'); diff --git a/api/manage_templates.php b/api/manage_templates.php index abd9f00..e89067d 100644 --- a/api/manage_templates.php +++ b/api/manage_templates.php @@ -14,6 +14,7 @@ RateLimitMiddleware::apply('api'); try { require_once dirname(__DIR__) . '/config/config.php'; require_once dirname(__DIR__) . '/helpers/Database.php'; + require_once dirname(__DIR__) . '/models/AuditLogModel.php'; // Check authentication if (session_status() === PHP_SESSION_NONE) { @@ -48,6 +49,8 @@ try { header('Content-Type: application/json'); + $auditLog = new AuditLogModel($conn); + $currentUserId = $_SESSION['user']['user_id']; $method = $_SERVER['REQUEST_METHOD']; $id = isset($_GET['id']) ? (int)$_GET['id'] : null; @@ -110,7 +113,13 @@ try { ); if ($stmt->execute()) { - echo json_encode(['success' => true, 'template_id' => $conn->insert_id]); + $newTemplateId = $conn->insert_id; + $auditLog->log($currentUserId, 'create', 'template', (string)$newTemplateId, [ + 'template_name' => $templateName, + 'category' => $category, + 'type' => $type + ]); + echo json_encode(['success' => true, 'template_id' => $newTemplateId]); } else { error_log("Template creation failed: " . $stmt->error); echo json_encode(['success' => false, 'error' => 'Failed to create template']); @@ -161,7 +170,15 @@ try { $id ); - echo json_encode(['success' => $stmt->execute()]); + $updated = $stmt->execute(); + if ($updated) { + $auditLog->log($currentUserId, 'update', 'template', (string)$id, [ + 'template_name' => $templateName, + 'category' => $category, + 'type' => $type + ]); + } + echo json_encode(['success' => $updated]); $stmt->close(); break; @@ -171,9 +188,22 @@ try { exit; } + // Capture the name before deletion for the audit record. + $nameStmt = $conn->prepare("SELECT template_name FROM ticket_templates WHERE template_id = ?"); + $nameStmt->bind_param('i', $id); + $nameStmt->execute(); + $delRow = $nameStmt->get_result()->fetch_assoc(); + $nameStmt->close(); + $stmt = $conn->prepare("DELETE FROM ticket_templates WHERE template_id = ?"); $stmt->bind_param('i', $id); - echo json_encode(['success' => $stmt->execute()]); + $deleted = $stmt->execute(); + if ($deleted) { + $auditLog->log($currentUserId, 'delete', 'template', (string)$id, [ + 'template_name' => $delRow['template_name'] ?? 'unknown' + ]); + } + echo json_encode(['success' => $deleted]); $stmt->close(); break; diff --git a/api/revoke_api_key.php b/api/revoke_api_key.php index e7f8d40..fe2bb7f 100644 --- a/api/revoke_api_key.php +++ b/api/revoke_api_key.php @@ -24,11 +24,13 @@ try { session_start(); } if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { + http_response_code(401); throw new Exception("Authentication required"); } // Check admin privileges if (!isset($_SESSION['user']['is_admin']) || !$_SESSION['user']['is_admin']) { + http_response_code(403); throw new Exception("Admin privileges required"); } @@ -51,12 +53,14 @@ try { // Get request data $input = json_decode(file_get_contents('php://input'), true); if (!$input) { + http_response_code(400); throw new Exception("Invalid request data"); } $keyId = (int)($input['key_id'] ?? 0); if ($keyId <= 0) { + http_response_code(400); throw new Exception("Valid key ID is required"); } @@ -68,10 +72,12 @@ try { $keyInfo = $apiKeyModel->getKeyById($keyId); if (!$keyInfo) { + http_response_code(404); throw new Exception("API key not found"); } if (!$keyInfo['is_active']) { + http_response_code(409); throw new Exception("API key is already revoked"); } @@ -79,6 +85,7 @@ try { $success = $apiKeyModel->revokeKey($keyId); if (!$success) { + http_response_code(500); throw new Exception("Failed to revoke API key"); } @@ -103,11 +110,26 @@ try { ]); } catch (Exception $e) { ob_end_clean(); - error_log("Revoke API key error: " . $e->getMessage()); header('Content-Type: application/json'); - http_response_code(isset($conn) ? 400 : 500); - echo json_encode([ - 'success' => false, - 'error' => 'An internal error occurred' - ]); + + // Preserve any specific status set before the throw (401/403/404/409/...); + // only fall back to 500 when nothing more specific was set. + $code = http_response_code(); + if (!is_int($code) || $code < 400) { + $code = 500; + } + http_response_code($code); + + if ($code >= 500) { + error_log("Revoke API key error: " . $e->getMessage()); + echo json_encode([ + 'success' => false, + 'error' => 'An internal error occurred' + ]); + } else { + echo json_encode([ + 'success' => false, + 'error' => $e->getMessage() + ]); + } } diff --git a/api/ticket_dependencies.php b/api/ticket_dependencies.php index 945cd3d..f75b3d5 100644 --- a/api/ticket_dependencies.php +++ b/api/ticket_dependencies.php @@ -27,10 +27,19 @@ register_shutdown_function(function () { ini_set('display_errors', 0); error_reporting(E_ALL); -// Custom error handler +// Custom error handler. Only genuine errors abort the request; notices, +// warnings and deprecations (e.g. new deprecations on a PHP upgrade) are +// logged but must not take the endpoint down with a 500. set_error_handler(function ($errno, $errstr, $errfile, $errline) { - // Log detailed error server-side + // Respect the @-operator / error_reporting. + if (!(error_reporting() & $errno)) { + return false; + } error_log("PHP Error in ticket_dependencies.php: $errstr in $errfile:$errline"); + if (!in_array($errno, [E_ERROR, E_USER_ERROR, E_RECOVERABLE_ERROR, E_PARSE], true)) { + // Non-fatal: log and continue. + return true; + } ob_end_clean(); http_response_code(500); header('Content-Type: application/json'); @@ -80,6 +89,9 @@ if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { $userId = $_SESSION['user']['user_id']; $currentUser = $_SESSION['user']; +$isAdmin = $currentUser['is_admin'] ?? false; +// users.groups is a comma-separated string; the dependency model expects an array. +$userGroups = array_values(array_filter(array_map('trim', explode(',', $currentUser['groups'] ?? '')))); // CSRF Protection for POST/DELETE if ($_SERVER['REQUEST_METHOD'] === 'POST' || $_SERVER['REQUEST_METHOD'] === 'DELETE') { @@ -121,14 +133,14 @@ try { } // Verify user can access this ticket - $ticket = $ticketModel->getTicketById((int)$ticketId); + $ticket = $ticketModel->getTicketById($ticketId); if (!$ticket || !$ticketModel->canUserAccessTicket($ticket, $currentUser)) { ResponseHelper::notFound('Ticket not found'); } try { - $dependencies = $dependencyModel->getDependencies($ticketId); - $dependents = $dependencyModel->getDependentTickets($ticketId); + $dependencies = $dependencyModel->getDependencies($ticketId, $userId, $userGroups, $isAdmin); + $dependents = $dependencyModel->getDependentTickets($ticketId, $userId, $userGroups, $isAdmin); } catch (Exception $e) { error_log('Query error in ticket_dependencies.php GET: ' . $e->getMessage()); ResponseHelper::serverError('Failed to retrieve dependencies'); @@ -157,11 +169,11 @@ try { } // Verify user can access both tickets before creating dependency - $srcTicket = $ticketModel->getTicketById((int)$ticketId); + $srcTicket = $ticketModel->getTicketById($ticketId); if (!$srcTicket || !$ticketModel->canUserAccessTicket($srcTicket, $currentUser)) { ResponseHelper::notFound('Ticket not found'); } - $tgtTicket = $ticketModel->getTicketById((int)$dependsOnId); + $tgtTicket = $ticketModel->getTicketById($dependsOnId); if (!$tgtTicket || !$ticketModel->canUserAccessTicket($tgtTicket, $currentUser)) { ResponseHelper::notFound('Target ticket not found'); } @@ -205,7 +217,7 @@ try { } // Verify user can access the source ticket - $srcTicket = $ticketModel->getTicketById((int)$ticketId); + $srcTicket = $ticketModel->getTicketById($ticketId); if (!$srcTicket || !$ticketModel->canUserAccessTicket($srcTicket, $currentUser)) { ResponseHelper::notFound('Ticket not found'); } @@ -235,7 +247,7 @@ try { ResponseHelper::notFound('Dependency not found'); } - $depTicket = $ticketModel->getTicketById((int)$depRow['ticket_id']); + $depTicket = $ticketModel->getTicketById($depRow['ticket_id']); if (!$depTicket || !$ticketModel->canUserAccessTicket($depTicket, $currentUser)) { ResponseHelper::forbidden('Access denied'); } diff --git a/api/update_comment.php b/api/update_comment.php index 6dc1081..0961053 100644 --- a/api/update_comment.php +++ b/api/update_comment.php @@ -27,12 +27,16 @@ try { session_start(); } if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { - throw new Exception("Authentication required"); + ob_end_clean(); + http_response_code(401); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Authentication required']); + exit; } - // CSRF Protection + // CSRF Protection for all state-changing methods (any non-GET/HEAD request) require_once dirname(__DIR__) . '/middleware/CsrfMiddleware.php'; - if ($_SERVER['REQUEST_METHOD'] === 'POST' || $_SERVER['REQUEST_METHOD'] === 'PUT') { + if (!in_array($_SERVER['REQUEST_METHOD'], ['GET', 'HEAD'], true)) { $csrfToken = $_SERVER['HTTP_X_CSRF_TOKEN'] ?? ''; if (!CsrfMiddleware::validateToken($csrfToken)) { http_response_code(403); @@ -53,7 +57,11 @@ try { $data = json_decode(file_get_contents('php://input'), true); if (!$data || !isset($data['comment_id']) || !isset($data['comment_text'])) { - throw new Exception("Missing required fields: comment_id, comment_text"); + ob_end_clean(); + http_response_code(400); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Missing required fields: comment_id, comment_text']); + exit; } $commentId = (int)$data['comment_id']; @@ -61,7 +69,11 @@ try { $markdownEnabled = isset($data['markdown_enabled']) && $data['markdown_enabled']; if (empty($commentText)) { - throw new Exception("Comment text cannot be empty"); + ob_end_clean(); + http_response_code(400); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Comment text cannot be empty']); + exit; } // Initialize models diff --git a/api/update_ticket.php b/api/update_ticket.php index 6514ea1..c172ec3 100644 --- a/api/update_ticket.php +++ b/api/update_ticket.php @@ -34,7 +34,11 @@ try { session_start(); } if (!isset($_SESSION['user']) || !isset($_SESSION['user']['user_id'])) { - throw new Exception("Authentication required"); + ob_end_clean(); + http_response_code(401); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Authentication required']); + exit; } // CSRF Protection @@ -115,7 +119,8 @@ try { if (empty($updateData['title'])) { return [ 'success' => false, - 'error' => 'Title cannot be empty' + 'error' => 'Title cannot be empty', + 'http_status' => 400 ]; } @@ -123,7 +128,8 @@ try { if ($updateData['priority'] < 1 || $updateData['priority'] > 5) { return [ 'success' => false, - 'error' => 'Priority must be between 1 and 5' + 'error' => 'Priority must be between 1 and 5', + 'http_status' => 400 ]; } @@ -137,11 +143,32 @@ try { $visibilityGroups = implode(',', array_map('trim', $visibilityGroups)); } + // Authorization: only an admin or the ticket's creator may change + // visibility. Enforce only when the requested visibility actually + // differs so ordinary edits that re-send the same value aren't blocked. + $currentVisibility = $currentTicket['visibility'] ?? 'public'; + $currentGroups = $currentTicket['visibility_groups'] ?? null; + $groupsProvided = array_key_exists('visibility_groups', $data); + $visibilityChanged = ($data['visibility'] !== $currentVisibility) + || ($groupsProvided && (string)$visibilityGroups !== (string)$currentGroups); + if ($visibilityChanged) { + $isCreator = $this->userId !== null + && (int)($currentTicket['created_by'] ?? 0) === (int)$this->userId; + if (!$this->isAdmin && !$isCreator) { + return [ + 'success' => false, + 'error' => 'You do not have permission to change ticket visibility', + 'http_status' => 403 + ]; + } + } + // Internal visibility requires at least one group if ($data['visibility'] === 'internal' && (empty($visibilityGroups) || trim($visibilityGroups) === '')) { return [ 'success' => false, - 'error' => 'Internal visibility requires at least one group to be specified' + 'error' => 'Internal visibility requires at least one group to be specified', + 'http_status' => 400 ]; } } @@ -160,6 +187,19 @@ try { 'error' => 'Status transition not allowed: ' . $currentTicket['status'] . ' → ' . $updateData['status'] ]; } + + // Enforce requires_comment transitions server-side. + if ($this->workflowModel->transitionRequiresComment($currentTicket['status'], $updateData['status'])) { + $comment = trim((string)($data['comment'] ?? $data['comment_text'] ?? '')); + if ($comment === '') { + return [ + 'success' => false, + 'error' => 'A comment is required for this status change', + 'requires_comment' => true, + 'http_status' => 400 + ]; + } + } } // Update ticket with user tracking and optional optimistic locking @@ -257,11 +297,19 @@ try { $data = json_decode($input, true); if (!$data) { - throw new Exception("Invalid JSON data received: " . $input); + ob_end_clean(); + http_response_code(400); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Invalid JSON data received']); + exit; } if (!isset($data['ticket_id'])) { - throw new Exception("Missing ticket_id parameter"); + ob_end_clean(); + http_response_code(400); + header('Content-Type: application/json'); + echo json_encode(['success' => false, 'error' => 'Missing ticket_id parameter']); + exit; } $ticketId = trim((string)$data['ticket_id']); diff --git a/api/watch_ticket.php b/api/watch_ticket.php index 375dc69..10c7f3e 100644 --- a/api/watch_ticket.php +++ b/api/watch_ticket.php @@ -10,12 +10,13 @@ require_once __DIR__ . '/bootstrap.php'; require_once dirname(__DIR__) . '/models/TicketModel.php'; +$data = json_decode(file_get_contents('php://input'), true) ?? []; + $ticketId = isset($_GET['ticket_id']) ? (int)$_GET['ticket_id'] - : (isset($data['ticket_id']) ? (int)$data['ticket_id'] : 0); + : (int)($data['ticket_id'] ?? 0); if ($_SERVER['REQUEST_METHOD'] === 'POST') { - $data = json_decode(file_get_contents('php://input'), true) ?? []; $ticketId = (int)($data['ticket_id'] ?? 0); $action = $data['action'] ?? ''; diff --git a/assets/js/base.js b/assets/js/base.js index 91a8b00..5a4ce3b 100644 --- a/assets/js/base.js +++ b/assets/js/base.js @@ -468,7 +468,15 @@ try { resp = await fetch(url, opts); } catch (err) { throw new Error('Network error: ' + err.message); } let data; try { data = await resp.json(); } catch (_) { data = { success: resp.ok }; } - if (!resp.ok) throw new Error(data.error || data.message || 'HTTP ' + resp.status); + // Resync CSRF token from any response body that carries a fresh one + // (bootstrap rotates on success and returns the current token on rejection). + if (data && data.csrf_token) global.CSRF_TOKEN = data.csrf_token; + if (!resp.ok) { + const err = new Error(data.error || data.message || 'HTTP ' + resp.status); + err.data = data; + err.status = resp.status; + throw err; + } return data; } @@ -2004,6 +2012,7 @@ let _focusedIdx = -1; let _items = []; let _debTimer = null; + let _searchSeq = 0; function _render(items, query) { _items = items.slice(0, maxResults); @@ -2028,16 +2037,21 @@ } async function _search(query) { + // Sequence guard: only the latest query is allowed to render, so a slow + // earlier async source() cannot overwrite a newer query's results. + const seq = ++_searchSeq; dropdown.innerHTML = '
Searching…
'; dropdown.classList.add('is-open'); inputEl.setAttribute('aria-busy', 'true'); try { const results = typeof source === 'function' ? await source(query) : source.filter(i => i.label.toLowerCase().includes(query.toLowerCase())); + if (seq !== _searchSeq) return; _render(results, query); } catch(e) { + if (seq !== _searchSeq) return; dropdown.innerHTML = '
Error loading results
'; } finally { - inputEl.setAttribute('aria-busy', 'false'); + if (seq === _searchSeq) inputEl.setAttribute('aria-busy', 'false'); } } @@ -2704,7 +2718,15 @@ } let data; try { data = await resp.json(); } catch (_) { data = { success: resp.ok }; } - if (!resp.ok) throw new Error(data.error || data.message || 'HTTP ' + resp.status); + // Resync CSRF token from any response body that carries a fresh one + // (bootstrap rotates on success and returns the current token on rejection). + if (data && data.csrf_token) global.CSRF_TOKEN = data.csrf_token; + if (!resp.ok) { + const err = new Error(data.error || data.message || 'HTTP ' + resp.status); + err.data = data; + err.status = resp.status; + throw err; + } return data; } api.get = url => _apiFetchAuth('GET', url); @@ -2713,6 +2735,79 @@ api.patch = (u, b) => _apiFetchAuth('PATCH', u, b); api.delete = (u, b) => _apiFetchAuth('DELETE', u, b); + /* ================================================================ + TICKET STATUS CHANGE (comment-aware) + lt.ticketStatus.submit(ticketId, newStatus, { comment? }) → Promise + Posts /api/update_ticket.php. If the server rejects with + requires_comment, opens a comment modal, persists the comment via + /api/add_comment.php, then retries the update once WITH the comment. + Rejects with err.cancelled === true if the user cancels the modal. + ================================================================ */ + function _statusCommentModal(newStatus) { + return new Promise(resolve => { + const modalId = 'ltStatusCommentModal' + Date.now(); + const safeStatus = escHtml(newStatus); + document.body.insertAdjacentHTML('beforeend', + ''); + const modalEl = document.getElementById(modalId); + openModal(modalId); + let done = false; + const finish = (value) => { + if (done) return; + done = true; + closeModal(modalId); + setTimeout(() => { if (modalEl && modalEl.parentNode) modalEl.remove(); }, 300); + resolve(value); + }; + modalEl.querySelector('[data-modal-close]').addEventListener('click', () => finish(null)); + document.getElementById(modalId + '_cancel').addEventListener('click', () => finish(null)); + document.getElementById(modalId + '_confirm').addEventListener('click', () => { + const ta = document.getElementById(modalId + '_comment'); + const comment = ta ? ta.value.trim() : ''; + if (!comment) { if (ta) ta.focus(); toast.warning('Please enter a reason for this status change.'); return; } + finish(comment); + }); + setTimeout(() => { const ta = document.getElementById(modalId + '_comment'); if (ta) ta.focus(); }, 100); + }); + } + + const ticketStatus = { + submit(ticketId, newStatus, opts) { + opts = opts || {}; + const id = String(ticketId); + const payload = { ticket_id: id, status: newStatus }; + if (opts.comment) payload.comment = opts.comment; + return api.post('/api/update_ticket.php', payload).catch(err => { + if (!(err && err.data && err.data.requires_comment)) throw err; + return _statusCommentModal(newStatus).then(comment => { + if (!comment) { + const cancelErr = new Error('Status change cancelled'); + cancelErr.cancelled = true; + throw cancelErr; + } + // Persist the comment, then retry the status change with it included. + return api.post('/api/add_comment.php', { ticket_id: id, comment_text: comment }) + .then(() => api.post('/api/update_ticket.php', { ticket_id: id, status: newStatus, comment: comment })); + }); + }); + }, + }; + /* ================================================================ MODULE 54 — MARKDOWN RENDERER lt.markdown.render(mdString) → HTML string (sanitized) @@ -2722,9 +2817,9 @@ ================================================================ */ const markdown = { render(md) { - // Delegate to window.marked if available - if (global.marked) return global.marked.parse(md); - if (global.markdownit) return global.markdownit().render(md); + // Always use the built-in XSS-safe micro-renderer. Do NOT delegate to + // window.marked / window.markdownit: their raw HTML output is not sanitized + // here, so delegating would enable stored XSS if such a lib were ever loaded. // Micro-renderer: covers headings, bold, italic, code, links, lists, blockquote, hr let html = escHtml(md) // Fenced code blocks @@ -2943,6 +3038,7 @@ lightbox, auth, markdown, + ticketStatus, pagination, sidebarSubmenus: { init: initSidebarSubmenus }, }; diff --git a/assets/js/dashboard.js b/assets/js/dashboard.js index 03628a2..526ef54 100644 --- a/assets/js/dashboard.js +++ b/assets/js/dashboard.js @@ -1000,18 +1000,20 @@ function performQuickStatusChange(ticketId) { if (!quickStatusEl) return; const newStatus = quickStatusEl.value; - lt.api.post('/api/update_ticket.php', { ticket_id: ticketId, status: newStatus }) + // Close this modal first so the comment modal (if requires_comment) stacks cleanly. + closeQuickStatusModal(); + + lt.ticketStatus.submit(ticketId, newStatus) .then(data => { - closeQuickStatusModal(); - if (data.success) { + if (data && data.success) { lt.toast.success(`Status updated to ${newStatus}`, 3000); showTableSkeleton(5); setTimeout(() => window.location.reload(), 1000); } else { - lt.toast.error('Error: ' + (data.error || 'Unknown error'), 4000); + lt.toast.error('Error: ' + ((data && data.error) || 'Unknown error'), 4000); } }) .catch(error => { - closeQuickStatusModal(); + if (error && error.cancelled) return; lt.toast.error('Error updating status', 4000); }); } @@ -1168,8 +1170,9 @@ function populateKanbanCards() { card.dataset.ticketId = ticketId; card.dataset.status = status; card.addEventListener('click', (e) => { - // Don't navigate if drag just ended (drag adds/removes is-dragging briefly) - if (card.dataset.dragged) { delete card.dataset.dragged; return; } + // Don't navigate if a drag just ended. The flag is cleared on a timer + // (see handleKanbanSort), so a genuine later click is not swallowed. + if (card.dataset.dragged) return; window.location.href = '/ticket/' + encodeURIComponent(ticketId); }); card.onkeydown = (e) => { if (e.key === 'Enter' || e.key === ' ') card.click(); }; @@ -1214,6 +1217,9 @@ function populateKanbanCards() { movedCard.dataset.status = newStatus; movedCard.dataset.dragged = '1'; + // Clear the drag flag shortly after the drop so it suppresses only the + // synthetic click fired on drop, not the user's next genuine click. + setTimeout(function () { delete movedCard.dataset.dragged; }, 400); // Optimistically update column counts const dec = document.querySelector(`.column-count[data-status="${oldStatus}"]`); @@ -1230,8 +1236,9 @@ function populateKanbanCards() { if (inc) inc.textContent = '(' + Math.max(0, (parseInt(inc.textContent.replace(/\D/g, ''), 10) || 1) - 1) + ')'; }; - // POST status update via the shared wrapper (adds CSRF + JSON, throws on non-2xx) - lt.api.post('/api/update_ticket.php', { ticket_id: String(ticketId), status: newStatus }) + // Submit via the shared comment-aware helper. Dropping to Closed (or + // reopening) prompts for a required comment and retries; cancel reverts. + lt.ticketStatus.submit(String(ticketId), newStatus) .then(function (data) { if (data && data.success) { lt.toast.success('Ticket #' + ticketId + ' → ' + newStatus, 2500); @@ -1241,8 +1248,8 @@ function populateKanbanCards() { revert(); } }) - .catch(function () { - lt.toast.error('Status update failed — reverting'); + .catch(function (error) { + if (!(error && error.cancelled)) lt.toast.error('Status update failed — reverting'); revert(); }); } diff --git a/assets/js/keyboard-shortcuts.js b/assets/js/keyboard-shortcuts.js index d09cea6..f7da6bf 100644 --- a/assets/js/keyboard-shortcuts.js +++ b/assets/js/keyboard-shortcuts.js @@ -6,11 +6,27 @@ // Track currently selected row for J/K navigation let currentSelectedRowIndex = -1; +let lastNavRowCount = -1; + +// Only navigate real, visible rows — skip skeleton placeholders and rows hidden +// by filters/column toggles (offsetParent is null when display:none). +function getNavigableRows() { + return Array.from(document.querySelectorAll('tbody tr')).filter(function(row) { + return !row.classList.contains('lt-skeleton-row') && row.offsetParent !== null; + }); +} function navigateTableRow(direction) { - const rows = document.querySelectorAll('tbody tr'); + const rows = getNavigableRows(); if (rows.length === 0) return; + // Reset the index when the row set changes (e.g. filter/reload) so navigation + // never lands on a stale/hidden index. + if (rows.length !== lastNavRowCount) { + currentSelectedRowIndex = -1; + lastNavRowCount = rows.length; + } + rows.forEach(row => row.classList.remove('keyboard-selected')); if (direction === 'next') { @@ -47,10 +63,8 @@ document.addEventListener('DOMContentLoaded', function() { } }); - // ?: Show keyboard shortcuts help — use the static #lt-keys-help modal in the footer - lt.keys.on('?', function() { - if (window.lt) lt.modal.open('lt-keys-help'); - }); + // Note: the '?' help shortcut is registered by lt.keys.initDefaults(); do not + // re-bind it here or the help modal opens twice. // J: Next row lt.keys.on('j', () => navigateTableRow('next')); diff --git a/assets/js/markdown.js b/assets/js/markdown.js index 77c83f7..27bc63a 100644 --- a/assets/js/markdown.js +++ b/assets/js/markdown.js @@ -41,9 +41,6 @@ function parseMarkdown(markdown) { .replace(/"/g, '"') .replace(/'/g, '''); - // Ticket references (#123456789) - convert to clickable links - html = html.replace(/#(\d{9})\b/g, '#$1'); - // Code blocks (```code```) - preserve content and don't process further const codeBlocks = []; html = html.replace(/```([\s\S]*?)```/g, function(match, code) { @@ -58,6 +55,11 @@ function parseMarkdown(markdown) { return '%%INLINECODE' + (inlineCodes.length - 1) + '%%'; }); + // Ticket references (#123456789) - convert to clickable links. + // Runs AFTER code extraction so a literal #123456789 inside inline/fenced code + // (now replaced by a placeholder) is not turned into a link. + html = html.replace(/#(\d{9})\b/g, '#$1'); + // Tables (must be processed before other block elements) html = parseMarkdownTables(html); @@ -287,25 +289,33 @@ function buildTable(rows) { if (rows.length === 0) return ''; let html = ''; + let inThead = false; + let inTbody = false; - rows.forEach((row, index) => { + rows.forEach((row) => { const cells = row.content.split('|').filter(cell => cell.trim() !== ''); - const tag = row.type === 'header' ? 'th' : 'td'; - const wrapper = row.type === 'header' ? 'thead' : (index === 1 ? 'tbody' : ''); + const isHeader = row.type === 'header'; + const tag = isHeader ? 'th' : 'td'; - if (wrapper === 'thead') html += ''; - if (wrapper === 'tbody') html += ''; + if (isHeader && !inThead) { html += ''; inThead = true; } + if (!isHeader && !inTbody) { + if (inThead) { html += ''; inThead = false; } + html += ''; + inTbody = true; + } html += ''; cells.forEach(cell => { html += `<${tag}>${cell.trim()}`; }); html += ''; - - if (row.type === 'header') html += ''; }); - html += '
'; + // Close whichever section is still open so tags are balanced for header-only, + // body-only, and header+body tables alike. + if (inThead) html += ''; + if (inTbody) html += ''; + html += ''; return html; } diff --git a/assets/js/ticket.js b/assets/js/ticket.js index 747ac1a..09155a0 100644 --- a/assets/js/ticket.js +++ b/assets/js/ticket.js @@ -291,14 +291,8 @@ function addComment() { // For markdown, use parseMarkdown (sanitizes HTML) displayText = parseMarkdown(commentText); } else { - // For non-markdown, convert line breaks to
and escape HTML - displayText = commentText - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"') - .replace(/'/g, ''') - .replace(/\n/g, '
'); + // For non-markdown, escape HTML then convert line breaks to
+ displayText = lt.escHtml(commentText).replace(/\n/g, '
'); } // Add new comment to the list @@ -538,11 +532,12 @@ function updateTicketStatus() { return; } cleanup(true); - // Post comment first, then change status + // Post comment first (persists it), then change status with the same + // comment included so the server's requires_comment check passes. const ticketId = getTicketIdFromUrl(); lt.api.post('/api/add_comment.php', { ticket_id: ticketId, comment_text: comment }) - .then(() => performStatusChange(statusSelect, selectedOption, newStatus)) - .catch(() => performStatusChange(statusSelect, selectedOption, newStatus)); + .then(() => performStatusChange(statusSelect, selectedOption, newStatus, comment)) + .catch(() => performStatusChange(statusSelect, selectedOption, newStatus, comment)); }); // Focus textarea on open setTimeout(() => { const ta = document.getElementById(`${modalId}_comment`); if (ta) ta.focus(); }, 100); @@ -552,8 +547,11 @@ function updateTicketStatus() { performStatusChange(statusSelect, selectedOption, newStatus); } -// Extract status change logic into reusable function -function performStatusChange(statusSelect, selectedOption, newStatus) { +// Extract status change logic into reusable function. +// `comment` (optional) is included in the update_ticket payload so requires_comment +// transitions pass server validation. lt.ticketStatus.submit handles the +// comment-aware retry if a comment is required but was not pre-collected. +function performStatusChange(statusSelect, selectedOption, newStatus, comment) { const ticketId = getTicketIdFromUrl(); if (!ticketId) { @@ -561,10 +559,10 @@ function performStatusChange(statusSelect, selectedOption, newStatus) { return; } - // Update status via API - lt.api.post('/api/update_ticket.php', { ticket_id: ticketId, status: newStatus }) + // Update status via the shared comment-aware helper + lt.ticketStatus.submit(ticketId, newStatus, { comment: comment }) .then(data => { - if (data.success) { + if (data && data.success) { // Update the dropdown to show new status as current (preserve TDS v1.2 classes) const newClass = 'lt-status-' + newStatus.toLowerCase().replace(/ /g, '-'); statusSelect.className = 'lt-select lt-select-sm lt-status-select ' + newClass; @@ -582,12 +580,14 @@ function performStatusChange(statusSelect, selectedOption, newStatus) { window.location.reload(); }, 500); } else { - lt.toast.error('Error updating status: ' + (data.error || 'Unknown error')); + lt.toast.error('Error updating status: ' + ((data && data.error) || 'Unknown error')); // Reset to current status statusSelect.selectedIndex = 0; } }) .catch(error => { + // User cancelled the required-comment modal — silently revert the dropdown + if (error && error.cancelled) { statusSelect.selectedIndex = 0; return; } lt.toast.error('Error updating status: ' + error.message); // Reset to current status statusSelect.selectedIndex = 0; @@ -938,6 +938,8 @@ function handleFileUpload(files) { if (xhr.status === 200 || xhr.status === 201) { try { const response = JSON.parse(xhr.responseText); + // Keep the CSRF token in sync if the server rotated it + if (response.csrf_token) window.CSRF_TOKEN = response.csrf_token; if (response.success) { if (uploadedCount === totalFiles) { lt.toast.success(`${totalFiles} file(s) uploaded successfully`, 3000); @@ -968,6 +970,9 @@ function handleFileUpload(files) { }); xhr.open('POST', '/api/upload_attachment.php'); + // Send CSRF via header to match the rest of the app (endpoint accepts both + // the X-CSRF-Token header and the csrf_token form field). + if (window.CSRF_TOKEN) xhr.setRequestHeader('X-CSRF-Token', window.CSRF_TOKEN); xhr.send(formData); }); } @@ -1142,12 +1147,17 @@ function handleMentionInput(e) { const text = textarea.value; const cursorPos = textarea.selectionStart; - // Find @ symbol before cursor + // Find @ symbol before cursor. Only trigger when the @ is at a word boundary + // (start of input or preceded by whitespace) so it does not fire inside email + // addresses like foo@bar. let atPos = -1; for (let i = cursorPos - 1; i >= 0; i--) { const char = text[i]; if (char === '@') { - atPos = i; + const prev = i > 0 ? text[i - 1] : ''; + if (i === 0 || /\s/.test(prev)) { + atPos = i; + } break; } if (char === ' ' || char === '\n') { @@ -1277,20 +1287,27 @@ function selectMention(username) { } /** - * Highlight mentions in comment text + * Highlight mentions in comment text. + * Skips content inside existing anchor tags so URLs/emails that contain '@' + * (e.g. auto-linked links or mailto:) are not corrupted or nested. */ function highlightMentions(text) { - return text.replace(/@([a-zA-Z0-9_-]+)/g, '$1'); + return text.replace(/]*>[\s\S]*?<\/a>|@[a-zA-Z0-9_-]+/gi, function (m) { + if (m.charAt(0) === '<') return m; // leave anchor tags untouched + return '' + m.slice(1) + ''; + }); } // Initialize mention autocomplete when DOM is ready document.addEventListener('DOMContentLoaded', function() { initMentionAutocomplete(); - // Highlight @mentions in plain-text comments (markdown.js handles [data-markdown] elements) + // Highlight @mentions in plain-text comments (markdown.js handles [data-markdown] elements). + // Idempotency guard: only process each element once so re-runs don't nest spans. document.querySelectorAll('.comment-text').forEach(el => { - if (!el.hasAttribute('data-markdown')) { + if (!el.hasAttribute('data-markdown') && !el.dataset.mentionsProcessed) { el.innerHTML = highlightMentions(el.innerHTML); + el.dataset.mentionsProcessed = '1'; } }); diff --git a/config/config.php b/config/config.php index e25b5a9..3ffb242 100644 --- a/config/config.php +++ b/config/config.php @@ -6,6 +6,9 @@ if (!file_exists($envFile)) { die('Configuration error: .env file not found. Copy .env.example to .env and configure your database settings.'); } $envVars = parse_ini_file($envFile, false, INI_SCANNER_TYPED); +if (!is_array($envVars)) { + die('Configuration error: .env file could not be parsed. Check for unquoted special characters (e.g. #, ;, =, or quotes) in values and wrap affected values in double quotes.'); +} // Strip quotes from values if present (parse_ini_file may include them) if ($envVars) { diff --git a/controllers/TicketController.php b/controllers/TicketController.php index 2e2c0d4..a03bd76 100644 --- a/controllers/TicketController.php +++ b/controllers/TicketController.php @@ -93,19 +93,27 @@ class TicketController $visibilityGroups = implode(',', array_map('trim', $_POST['visibility_groups'])); } + // Honor the posted status, validated against the app's canonical list + $validStatuses = $GLOBALS['config']['TICKET_STATUSES'] ?? ['Open', 'Pending', 'In Progress', 'Closed']; + $status = $_POST['status'] ?? 'Open'; + if (!in_array($status, $validStatuses, true)) { + $status = 'Open'; + } + $ticketData = [ - 'title' => $_POST['title'] ?? '', + 'title' => trim($_POST['title'] ?? ''), 'description' => $_POST['description'] ?? '', 'priority' => $_POST['priority'] ?? '4', 'category' => $_POST['category'] ?? 'General', 'type' => $_POST['type'] ?? 'Issue', + 'status' => $status, 'visibility' => $_POST['visibility'] ?? 'public', 'visibility_groups' => $visibilityGroups, 'assigned_to' => !empty($_POST['assigned_to']) ? $_POST['assigned_to'] : null ]; - // Validate input - if (empty($ticketData['title'])) { + // Validate input (server-side; form is novalidate) + if ($ticketData['title'] === '') { $error = "Title is required"; $templates = $this->templateModel->getAllTemplates(); $allUsers = $this->userModel->getAllUsers(); @@ -114,6 +122,15 @@ class TicketController return; } + if (trim($ticketData['description']) === '') { + $error = "Description is required"; + $templates = $this->templateModel->getAllTemplates(); + $allUsers = $this->userModel->getAllUsers(); + $conn = $this->conn; // Make $conn available to view + include dirname(__DIR__) . '/views/CreateTicketView.php'; + return; + } + // Create ticket with user tracking $result = $this->ticketModel->createTicket($ticketData, $userId); @@ -123,6 +140,10 @@ class TicketController $GLOBALS['auditLog']->logTicketCreate($userId, $result['ticket_id'], $ticketData); } + // Ticket counts changed — invalidate the cached dashboard stats + require_once dirname(__DIR__) . '/models/StatsModel.php'; + (new StatsModel($this->conn))->invalidateCache(); + // Auto-link as duplicate if requested from create form $linkDupOfRaw = trim($_POST['link_duplicate_of'] ?? ''); if ($linkDupOfRaw !== '' && ctype_digit($linkDupOfRaw)) { diff --git a/create_ticket_api.php b/create_ticket_api.php index 99eab78..3d61579 100644 --- a/create_ticket_api.php +++ b/create_ticket_api.php @@ -60,6 +60,7 @@ require_once __DIR__ . '/config/config.php'; // Authenticate via API key require_once __DIR__ . '/middleware/ApiKeyAuth.php'; require_once __DIR__ . '/models/AuditLogModel.php'; +require_once __DIR__ . '/models/StatsModel.php'; require_once __DIR__ . '/helpers/UrlHelper.php'; $apiKeyAuth = new ApiKeyAuth($conn); @@ -73,18 +74,6 @@ try { $userId = $systemUser['user_id']; -// Create tickets table with hash column if not exists -$createTableSQL = "CREATE TABLE IF NOT EXISTS tickets ( - id INT AUTO_INCREMENT PRIMARY KEY, - ticket_id VARCHAR(9) NOT NULL, - title VARCHAR(255) NOT NULL, - hash VARCHAR(64) NOT NULL, - created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, - UNIQUE KEY unique_hash (hash) -)"; - -$conn->query($createTableSQL); - // Parse input regardless of content-type header $rawInput = file_get_contents('php://input'); $data = json_decode($rawInput, true); @@ -349,6 +338,9 @@ if ($existing) { 'status' => $existingStatus, ], 'automated'); } + + // Ticket state (priority/title/description) changed — refresh dashboard stats. + (new StatsModel($conn))->invalidateCache(); } $conn->close(); @@ -371,7 +363,8 @@ if ($existing) { $reopenStmt->close(); $commentText = "**Issue recurred — ticket reopened automatically.**\n\n" . - "hwmonDaemon detected this condition again. Current sensor data is in the ticket description above."; + "hwmonDaemon detected this condition again. The ticket description reflects the " + . "original report; see this comment's timestamp for when the issue recurred."; $commentStmt = $conn->prepare( "INSERT INTO ticket_comments (ticket_id, user_id, user_name, comment_text, markdown_enabled) VALUES (?, ?, 'hwmonDaemon', ?, 1)" ); @@ -384,6 +377,9 @@ if ($existing) { 'reason' => 'auto-reopened by hwmonDaemon (issue recurred)', ]); + // Ticket reopened (Closed → Open) — refresh dashboard stats. + (new StatsModel($conn))->invalidateCache(); + $conn->close(); require_once __DIR__ . '/helpers/NotificationHelper.php'; @@ -404,13 +400,40 @@ if ($existing) { exit; } -// No existing ticket — create a new one -// Use random_int range 100000000-999999999 to avoid leading-zero IDs -try { - $ticket_id = (string)random_int(100000000, 999999999); -} catch (Exception $e) { - $ticket_id = (string)mt_rand(100000000, 999999999); +// No existing ticket — create a new one. +// Generate a collision-safe unique ticket_id with a pre-check + retry loop (same +// approach as TicketModel::createTicket) so a ticket_id clash cannot happen. That +// way a 1062 on INSERT below can only be the unique_hash (dedup) key racing, and +// is correctly reported as a duplicate rather than a dropped hardware alert. +$ticket_id = null; +$maxAttempts = 50; +$attempts = 0; +do { + try { + $candidateId = sprintf('%09d', random_int(100000000, 999999999)); + } catch (Exception $e) { + $candidateId = sprintf('%09d', mt_rand(100000000, 999999999)); + } + + $idCheckStmt = $conn->prepare("SELECT ticket_id FROM tickets WHERE ticket_id = ? LIMIT 1"); + $idCheckStmt->bind_param("s", $candidateId); + $idCheckStmt->execute(); + $idExists = $idCheckStmt->get_result()->num_rows > 0; + $idCheckStmt->close(); + + if (!$idExists) { + $ticket_id = $candidateId; + } + $attempts++; +} while ($ticket_id === null && $attempts < $maxAttempts); + +if ($ticket_id === null) { + error_log('create_ticket_api: failed to generate a unique ticket_id after ' . $maxAttempts . ' attempts'); + http_response_code(500); + echo json_encode(['success' => false, 'error' => 'Internal server error']); + exit; } + $insertStmt = $conn->prepare( "INSERT INTO tickets (ticket_id, title, description, status, priority, category, type, hash, created_by) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)" @@ -452,6 +475,9 @@ if ($inserted) { 'type' => $type, ]); + // New ticket created — refresh dashboard stats. + (new StatsModel($conn))->invalidateCache(); + $conn->close(); require_once __DIR__ . '/helpers/NotificationHelper.php'; @@ -469,5 +495,7 @@ if ($inserted) { 'message' => 'Ticket created successfully', ]); } else { - echo json_encode(['success' => false, 'error' => $conn->error]); + error_log('create_ticket_api: ticket insert reported failure: ' . $conn->error); + http_response_code(500); + echo json_encode(['success' => false, 'error' => 'Internal server error']); } diff --git a/cron/cleanup_audit_log.php b/cron/cleanup_audit_log.php new file mode 100644 index 0000000..3d09a42 --- /dev/null +++ b/cron/cleanup_audit_log.php @@ -0,0 +1,50 @@ +#!/usr/bin/env php +> /var/log/audit_log_cleanup.log 2>&1 + */ + +// Prevent web access +if (php_sapi_name() !== 'cli') { + http_response_code(403); + exit('CLI access only'); +} + +// Change to project root directory +chdir(dirname(__DIR__)); + +// Include required files +require_once 'config/config.php'; +require_once 'helpers/Database.php'; +require_once 'models/AuditLogModel.php'; + +// Log function +function logMessage($message) +{ + echo '[' . date('Y-m-d H:i:s') . '] ' . $message . "\n"; +} + +$retentionDays = (int)($GLOBALS['config']['AUDIT_LOG_RETENTION_DAYS'] ?? 90); + +logMessage("Starting audit log cleanup (retention: {$retentionDays} days)"); + +try { + $conn = Database::getConnection(); + + $auditLog = new AuditLogModel($conn); + $deleted = $auditLog->deleteOldLogs($retentionDays); + + logMessage("Removed {$deleted} audit log row(s) older than {$retentionDays} days"); + + Database::close(); +} catch (Exception $e) { + logMessage('FATAL ERROR: ' . $e->getMessage()); + exit(1); +} diff --git a/cron/create_recurring_tickets.php b/cron/create_recurring_tickets.php index b5109e7..f2b851d 100644 --- a/cron/create_recurring_tickets.php +++ b/cron/create_recurring_tickets.php @@ -17,9 +17,11 @@ chdir(dirname(__DIR__)); // Include required files require_once 'config/config.php'; require_once 'helpers/Database.php'; +require_once 'helpers/NotificationHelper.php'; require_once 'models/RecurringTicketModel.php'; require_once 'models/TicketModel.php'; require_once 'models/AuditLogModel.php'; +require_once 'models/StatsModel.php'; // Log function function logMessage($message) @@ -92,6 +94,10 @@ try { ['source' => 'recurring', 'recurring_id' => $recurring['recurring_id']] ); + // Fire the same Matrix "ticket created" notification the manual and + // external-API create paths send, so recurring tickets aren't silent. + NotificationHelper::sendTicketNotification($ticketId, $ticketData, 'automated'); + $created++; } else { logMessage("ERROR: Failed to create ticket - " . ($result['error'] ?? 'Unknown error')); @@ -103,6 +109,12 @@ try { } } + // Ticket counts changed — invalidate the cached dashboard stats once for the + // whole run (mirrors the manual/API create paths, which invalidate per create). + if ($created > 0) { + (new StatsModel($conn))->invalidateCache(); + } + logMessage("Completed: Created $created tickets, $errors errors"); Database::close(); diff --git a/generate_api_key.php b/generate_api_key.php deleted file mode 100644 index d72ca4b..0000000 --- a/generate_api_key.php +++ /dev/null @@ -1,107 +0,0 @@ -connect_error) { - die("❌ Database connection failed: " . $conn->connect_error . "\n"); -} - -echo "✅ Connected to database\n\n"; - -// Initialize models -$userModel = new UserModel($conn); -$apiKeyModel = new ApiKeyModel($conn); - -// Get system user (should exist from migration) -echo "Checking for system user...\n"; -$systemUser = $userModel->getSystemUser(); - -if (!$systemUser) { - die("❌ Error: System user not found. Please run migrations first.\n"); -} - -echo "✅ System user found: ID " . $systemUser['user_id'] . " (" . $systemUser['username'] . ")\n\n"; - -// Check if API key already exists -$existingKeys = $apiKeyModel->getKeysByUser($systemUser['user_id']); -if (!empty($existingKeys)) { - echo "⚠️ Warning: API keys already exist for system user:\n\n"; - foreach ($existingKeys as $key) { - echo " - " . $key['key_name'] . " (Prefix: " . $key['key_prefix'] . ")\n"; - echo " Created: " . $key['created_at'] . "\n"; - echo " Active: " . ($key['is_active'] ? 'Yes' : 'No') . "\n\n"; - } - - echo "Do you want to generate a new API key? (yes/no): "; - $handle = fopen("php://stdin", "r"); - $response = trim(fgets($handle)); - fclose($handle); - - if (strtolower($response) !== 'yes') { - echo "\nAborted.\n"; - exit(0); - } - echo "\n"; -} - -// Generate API key -echo "Generating API key for hwmonDaemon...\n"; -$result = $apiKeyModel->createKey( - 'hwmonDaemon', - $systemUser['user_id'], - null // No expiration -); - -if ($result['success']) { - echo "\n"; - echo "==============================================\n"; - echo " ✅ API Key Generated Successfully!\n"; - echo "==============================================\n\n"; - echo "API Key: " . $result['api_key'] . "\n"; - echo "Key Prefix: " . $result['key_prefix'] . "\n"; - echo "Key ID: " . $result['key_id'] . "\n"; - echo "Expires: Never\n\n"; - echo "⚠️ IMPORTANT: Save this API key now!\n"; - echo " It cannot be retrieved later.\n\n"; - echo "==============================================\n"; - echo " Add to hwmonDaemon .env file:\n"; - echo "==============================================\n\n"; - echo "TICKET_API_KEY=" . $result['api_key'] . "\n\n"; - echo "Then restart hwmonDaemon:\n"; - echo " sudo systemctl restart hwmonDaemon\n\n"; -} else { - echo "❌ Error generating API key: " . $result['error'] . "\n"; - exit(1); -} - -$conn->close(); - -echo "Done! Delete this script after use:\n"; -echo " rm " . __FILE__ . "\n\n"; diff --git a/helpers/CacheHelper.php b/helpers/CacheHelper.php index 0e5f2e5..40c1c83 100644 --- a/helpers/CacheHelper.php +++ b/helpers/CacheHelper.php @@ -21,7 +21,13 @@ class CacheHelper if (self::$cacheDir === null) { self::$cacheDir = sys_get_temp_dir() . '/tinker_tickets_cache'; if (!is_dir(self::$cacheDir)) { - mkdir(self::$cacheDir, 0755, true); + // 0700: only the app user may read cached data or create files. + // mkdir mode is masked by umask, so chmod to enforce it. + mkdir(self::$cacheDir, 0700, true); + @chmod(self::$cacheDir, 0700); + } elseif (!function_exists('posix_geteuid') || fileowner(self::$cacheDir) === posix_geteuid()) { + // Existing dir we own: harden a previously world-readable dir. + @chmod(self::$cacheDir, 0700); } } return self::$cacheDir; @@ -106,7 +112,13 @@ class CacheHelper // Store in file cache $filePath = self::getCacheDir() . '/' . $key . '.json'; - return @file_put_contents($filePath, json_encode($cached), LOCK_EX) !== false; + $written = @file_put_contents($filePath, json_encode($cached), LOCK_EX) !== false; + if ($written) { + // 0600: cache may feed security-relevant reads; keep it non-readable + // to other local users and non-poisonable by pre-created files. + @chmod($filePath, 0600); + } + return $written; } /** diff --git a/helpers/Database.php b/helpers/Database.php index 7b75de9..1f8c5d9 100644 --- a/helpers/Database.php +++ b/helpers/Database.php @@ -22,11 +22,9 @@ class Database self::$connection = self::createConnection(); } - // Check if connection is still alive - if (!self::$connection->ping()) { - self::$connection = self::createConnection(); - } - + // Note: no ping()/reconnect check — mysqli auto-reconnect was removed in + // PHP 8.2 and mysqli::ping() is deprecated in 8.4. The connection is + // request-scoped and short-lived, so a liveness check is unnecessary. return self::$connection; } @@ -57,6 +55,32 @@ class Database // Set charset to utf8mb4 for proper Unicode support $conn->set_charset('utf8mb4'); + // Pin the MySQL session time zone to the app's configured zone so that + // NOW()/CURRENT_TIMESTAMP and PHP agree on wall-clock time regardless of + // the DB server's SYSTEM tz. Prefer the named zone (requires the + // mysql.time_zone_* tables); if that isn't available, fall back to the + // fixed numeric offset PHP computes for the same zone. Best-effort: a + // failure here must never fatal the connection. + $tz = $GLOBALS['config']['TIMEZONE'] ?? 'UTC'; + try { + $escaped = $conn->real_escape_string($tz); + try { + // mysqli throws (does not return false) on failure under the + // default PHP 8.1+ report mode, so catch it rather than testing + // the return value. + $conn->query("SET time_zone = '{$escaped}'"); + } catch (\Throwable $inner) { + // Named zone unavailable (mysql.time_zone_* not populated) — fall + // back to a fixed numeric offset so PHP and MySQL still agree on + // wall-clock time regardless of the DB server's SYSTEM tz. + $offset = (new DateTime('now', new DateTimeZone($tz)))->format('P'); + $escapedOffset = $conn->real_escape_string($offset); + $conn->query("SET time_zone = '{$escapedOffset}'"); + } + } catch (\Throwable $e) { + error_log('Database: failed to set session time_zone: ' . $e->getMessage()); + } + return $conn; } diff --git a/helpers/NotificationHelper.php b/helpers/NotificationHelper.php index 7bb1304..1afceac 100644 --- a/helpers/NotificationHelper.php +++ b/helpers/NotificationHelper.php @@ -96,21 +96,31 @@ class NotificationHelper * @param string $commentText Plain text (first 200 chars will be sent) * @param string|null $authorDisplay Display name of commenter * @param bool $isInternal True if the comment is internal-only + * @param string $visibility Ticket visibility: 'public', 'internal', or + * 'confidential'. For non-public tickets the + * comment text preview is redacted so it is + * never leaked to the shared notify list. */ - public static function sendCommentNotification($ticketId, string $ticketTitle, string $commentText, ?string $authorDisplay = null, bool $isInternal = false): void + public static function sendCommentNotification($ticketId, string $ticketTitle, string $commentText, ?string $authorDisplay = null, bool $isInternal = false, string $visibility = 'public'): void { - // Skip if this is an internal-only comment — only the assignee/admin need to know $notifyUsers = self::notifyUsers(); if (empty($notifyUsers)) { return; } + // The shared notify list may include users without access to non-public + // tickets, so never post the comment body for internal/confidential + // tickets — only that activity occurred. + $preview = $visibility === 'public' + ? mb_strimwidth($commentText, 0, 200, '…') + : null; + self::fire([ 'event' => 'comment_added', 'ticket_id' => $ticketId, 'title' => $ticketTitle, 'author' => $authorDisplay, - 'preview' => mb_strimwidth($commentText, 0, 200, '…'), + 'preview' => $preview, 'is_internal' => $isInternal, 'url' => UrlHelper::ticketUrl($ticketId), 'notify_users' => $notifyUsers, @@ -155,8 +165,14 @@ class NotificationHelper * @param string $event One of: status_changed, comment_added, assigned * @param array $extraData Merged into the payload (old_status/new_status, author, etc.) * @param int|null $excludeUserId Don't notify the actor themselves + * @param string $visibility Ticket visibility: 'public', 'internal', or + * 'confidential'. notify_users includes the + * shared list, which may contain users without + * access to non-public tickets, so any comment + * body preview in $extraData is redacted for + * non-public tickets. */ - public static function notifyWatchers(\mysqli $conn, $ticketId, string $ticketTitle, string $event, array $extraData = [], ?int $excludeUserId = null): void + public static function notifyWatchers(\mysqli $conn, $ticketId, string $ticketTitle, string $event, array $extraData = [], ?int $excludeUserId = null, string $visibility = 'public'): void { $webhookUrl = $GLOBALS['config']['MATRIX_WEBHOOK_URL'] ?? null; $domain = $GLOBALS['config']['MATRIX_DOMAIN'] ?? null; @@ -164,6 +180,12 @@ class NotificationHelper return; } + // Don't leak comment/body content to the shared notify list for + // non-public tickets — keep only the fact that activity occurred. + if ($visibility !== 'public' && isset($extraData['preview'])) { + $extraData['preview'] = null; + } + // Fetch watcher usernames, excluding the actor so they don't notify // themselves. Notifications are best-effort: if the watchers table is // absent or the query fails, skip silently rather than fataling the diff --git a/index.php b/index.php index 602f671..f9732ed 100644 --- a/index.php +++ b/index.php @@ -249,8 +249,11 @@ switch (true) { $params = []; $types = ''; - $allowedActionTypes = ['create','update','delete','comment','assign','status_change','login','security', - 'ticket_create','ticket_update','ticket_delete','attachment_delete','attachment_upload']; + // Mirrors AuditLogModel::VALID_ACTION_TYPES so every option offered by the + // audit-log filter dropdown is actually accepted here. + $allowedActionTypes = ['create','update','delete','view','security_event', + 'login','logout','assign','unassign','comment','mention', + 'revoke','attachment_upload','attachment_delete','bulk_update']; if (!empty($_GET['action_type']) && in_array($_GET['action_type'], $allowedActionTypes, true)) { $whereConditions[] = "al.action_type = ?"; $params[] = $_GET['action_type']; @@ -335,9 +338,12 @@ switch (true) { case $requestPath == '/admin/user-activity': requireAdmin($currentUser); + // Validate date params (YYYY-MM-DD) like the audit-log route; fall back to defaults on garbage + $uaFrom = $_GET['date_from'] ?? ''; + $uaTo = $_GET['date_to'] ?? ''; $dateRange = [ - 'from' => $_GET['date_from'] ?? date('Y-m-d', strtotime('-30 days')), - 'to' => $_GET['date_to'] ?? date('Y-m-d') + 'from' => preg_match('/^\d{4}-\d{2}-\d{2}$/', $uaFrom) ? $uaFrom : date('Y-m-d', strtotime('-30 days')), + 'to' => preg_match('/^\d{4}-\d{2}-\d{2}$/', $uaTo) ? $uaTo : date('Y-m-d') ]; // Optimized query using LEFT JOINs with aggregated subqueries instead of correlated subqueries @@ -410,7 +416,7 @@ switch (true) { header("Location: /"); exit; - case preg_match('/^\/ticket\.php/', $requestPath) && isset($_GET['id']): + case preg_match('/^\/ticket\.php$/', $requestPath) && isset($_GET['id']): $legacyId = (string)$_GET['id']; if (ctype_digit($legacyId) && (int)$legacyId > 0) { header("Location: /ticket/" . $legacyId); diff --git a/migrations/000_baseline.sql b/migrations/000_baseline.sql new file mode 100644 index 0000000..dab0008 --- /dev/null +++ b/migrations/000_baseline.sql @@ -0,0 +1,326 @@ +-- ===================================================================== +-- 000_baseline.sql — full schema baseline for tinker_tickets +-- +-- Captured from the live production database so the schema is +-- reproducible from source (a fresh install or disaster recovery). +-- Every table uses CREATE TABLE IF NOT EXISTS, so running this against +-- an existing database is a safe no-op. FK checks are disabled during +-- creation so table order does not matter. +-- ===================================================================== + +SET FOREIGN_KEY_CHECKS = 0; + +-- ============ api_keys ============ +CREATE TABLE IF NOT EXISTS `api_keys` ( + `api_key_id` int(11) NOT NULL AUTO_INCREMENT, + `key_name` varchar(100) NOT NULL, + `key_hash` varchar(255) NOT NULL, + `key_prefix` varchar(20) NOT NULL, + `is_active` tinyint(1) DEFAULT 1, + `created_by` int(11) DEFAULT NULL, + `last_used` timestamp NULL DEFAULT NULL, + `expires_at` timestamp NULL DEFAULT NULL, + `created_at` timestamp NULL DEFAULT current_timestamp(), + PRIMARY KEY (`api_key_id`), + UNIQUE KEY `key_hash` (`key_hash`), + KEY `created_by` (`created_by`), + KEY `idx_key_hash` (`key_hash`), + KEY `idx_is_active` (`is_active`), + CONSTRAINT `api_keys_ibfk_1` FOREIGN KEY (`created_by`) REFERENCES `users` (`user_id`) ON DELETE SET NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ audit_log ============ +CREATE TABLE IF NOT EXISTS `audit_log` ( + `audit_id` bigint(20) NOT NULL AUTO_INCREMENT, + `user_id` int(11) DEFAULT NULL, + `action_type` varchar(50) NOT NULL, + `entity_type` varchar(50) NOT NULL, + `entity_id` varchar(50) DEFAULT NULL, + `details` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL CHECK (json_valid(`details`)), + `ip_address` varchar(45) DEFAULT NULL, + `created_at` timestamp NULL DEFAULT current_timestamp(), + PRIMARY KEY (`audit_id`), + KEY `idx_user_id` (`user_id`), + KEY `idx_created_at` (`created_at`), + KEY `idx_entity` (`entity_type`,`entity_id`), + KEY `idx_action_type` (`action_type`), + KEY `idx_audit_log_user_created` (`user_id`,`created_at` DESC), + KEY `idx_audit_log_action_type` (`action_type`,`created_at` DESC), + KEY `idx_audit_entity` (`entity_type`,`entity_id`), + KEY `idx_audit_user` (`user_id`,`created_at`), + CONSTRAINT `audit_log_ibfk_1` FOREIGN KEY (`user_id`) REFERENCES `users` (`user_id`) ON DELETE SET NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ bulk_operations ============ +CREATE TABLE IF NOT EXISTS `bulk_operations` ( + `operation_id` int(11) NOT NULL AUTO_INCREMENT, + `operation_type` varchar(50) NOT NULL, + `ticket_ids` text NOT NULL, + `performed_by` int(11) NOT NULL, + `parameters` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL CHECK (json_valid(`parameters`)), + `status` varchar(20) DEFAULT 'pending', + `total_tickets` int(11) DEFAULT NULL, + `processed_tickets` int(11) DEFAULT 0, + `failed_tickets` int(11) DEFAULT 0, + `created_at` timestamp NULL DEFAULT current_timestamp(), + `completed_at` timestamp NULL DEFAULT NULL, + PRIMARY KEY (`operation_id`), + KEY `idx_performed_by` (`performed_by`), + KEY `idx_created_at` (`created_at`), + CONSTRAINT `bulk_operations_ibfk_1` FOREIGN KEY (`performed_by`) REFERENCES `users` (`user_id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ custom_field_definitions ============ +CREATE TABLE IF NOT EXISTS `custom_field_definitions` ( + `field_id` int(11) NOT NULL AUTO_INCREMENT, + `field_name` varchar(100) NOT NULL, + `field_label` varchar(255) NOT NULL, + `field_type` enum('text','textarea','select','checkbox','date','number') NOT NULL, + `field_options` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL COMMENT 'Options for select fields: {"options": ["Option 1", "Option 2"]}' CHECK (json_valid(`field_options`)), + `category` varchar(50) DEFAULT NULL COMMENT 'NULL = applies to all categories', + `is_required` tinyint(1) DEFAULT 0, + `display_order` int(11) DEFAULT 0, + `is_active` tinyint(1) DEFAULT 1, + `created_at` timestamp NULL DEFAULT current_timestamp(), + `updated_at` timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(), + PRIMARY KEY (`field_id`), + KEY `idx_custom_fields_category` (`category`,`is_active`), + KEY `idx_custom_fields_order` (`display_order`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ custom_field_values ============ +CREATE TABLE IF NOT EXISTS `custom_field_values` ( + `value_id` int(11) NOT NULL AUTO_INCREMENT, + `ticket_id` varchar(9) NOT NULL, + `field_id` int(11) NOT NULL, + `field_value` text DEFAULT NULL, + `created_at` timestamp NULL DEFAULT current_timestamp(), + `updated_at` timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(), + PRIMARY KEY (`value_id`), + UNIQUE KEY `unique_ticket_field` (`ticket_id`,`field_id`), + KEY `field_id` (`field_id`), + KEY `idx_custom_values_ticket` (`ticket_id`), + CONSTRAINT `custom_field_values_ibfk_1` FOREIGN KEY (`field_id`) REFERENCES `custom_field_definitions` (`field_id`) ON DELETE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ migrations ============ +CREATE TABLE IF NOT EXISTS `migrations` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `filename` varchar(255) NOT NULL, + `applied_at` timestamp NULL DEFAULT current_timestamp(), + PRIMARY KEY (`id`), + UNIQUE KEY `filename` (`filename`), + KEY `idx_filename` (`filename`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ recurring_tickets ============ +CREATE TABLE IF NOT EXISTS `recurring_tickets` ( + `recurring_id` int(11) NOT NULL AUTO_INCREMENT, + `title_template` varchar(255) NOT NULL, + `description_template` text DEFAULT NULL, + `category` varchar(50) DEFAULT 'General', + `type` varchar(50) DEFAULT 'Task', + `priority` int(11) DEFAULT 4, + `assigned_to` int(11) DEFAULT NULL, + `schedule_type` enum('daily','weekly','monthly') NOT NULL, + `schedule_day` int(11) DEFAULT NULL COMMENT 'Day of week (1-7) for weekly, day of month (1-31) for monthly', + `schedule_time` time DEFAULT '09:00:00', + `next_run_at` timestamp NOT NULL, + `last_run_at` timestamp NULL DEFAULT NULL, + `is_active` tinyint(1) DEFAULT 1, + `created_by` int(11) DEFAULT NULL, + `created_at` timestamp NULL DEFAULT current_timestamp(), + `updated_at` timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(), + PRIMARY KEY (`recurring_id`), + KEY `assigned_to` (`assigned_to`), + KEY `created_by` (`created_by`), + KEY `idx_recurring_next_run` (`next_run_at`,`is_active`), + KEY `idx_recurring_active` (`is_active`), + CONSTRAINT `recurring_tickets_ibfk_1` FOREIGN KEY (`assigned_to`) REFERENCES `users` (`user_id`) ON DELETE SET NULL, + CONSTRAINT `recurring_tickets_ibfk_2` FOREIGN KEY (`created_by`) REFERENCES `users` (`user_id`) ON DELETE SET NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ saved_filters ============ +CREATE TABLE IF NOT EXISTS `saved_filters` ( + `filter_id` int(11) NOT NULL AUTO_INCREMENT, + `user_id` int(11) NOT NULL, + `filter_name` varchar(100) NOT NULL, + `filter_criteria` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL CHECK (json_valid(`filter_criteria`)), + `is_default` tinyint(1) DEFAULT 0, + `created_at` timestamp NULL DEFAULT current_timestamp(), + `updated_at` timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(), + PRIMARY KEY (`filter_id`), + UNIQUE KEY `unique_user_filter_name` (`user_id`,`filter_name`), + KEY `idx_user_filters` (`user_id`,`is_default`), + CONSTRAINT `saved_filters_ibfk_1` FOREIGN KEY (`user_id`) REFERENCES `users` (`user_id`) ON DELETE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; + +-- ============ status_transitions ============ +CREATE TABLE IF NOT EXISTS `status_transitions` ( + `transition_id` int(11) NOT NULL AUTO_INCREMENT, + `from_status` varchar(50) NOT NULL, + `to_status` varchar(50) NOT NULL, + `requires_comment` tinyint(1) DEFAULT 0, + `requires_admin` tinyint(1) DEFAULT 0, + `is_active` tinyint(1) DEFAULT 1, + `created_at` timestamp NULL DEFAULT current_timestamp(), + PRIMARY KEY (`transition_id`), + UNIQUE KEY `unique_transition` (`from_status`,`to_status`), + KEY `idx_from_status` (`from_status`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ ticket_attachments ============ +CREATE TABLE IF NOT EXISTS `ticket_attachments` ( + `attachment_id` int(11) NOT NULL AUTO_INCREMENT, + `ticket_id` varchar(9) NOT NULL, + `filename` varchar(255) NOT NULL, + `original_filename` varchar(255) NOT NULL, + `file_size` int(11) NOT NULL, + `mime_type` varchar(100) NOT NULL, + `uploaded_by` int(11) DEFAULT NULL, + `uploaded_at` timestamp NULL DEFAULT current_timestamp(), + PRIMARY KEY (`attachment_id`), + KEY `idx_attachments_ticket` (`ticket_id`), + KEY `idx_attachments_uploaded_by` (`uploaded_by`), + CONSTRAINT `ticket_attachments_ibfk_1` FOREIGN KEY (`uploaded_by`) REFERENCES `users` (`user_id`) ON DELETE SET NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; + +-- ============ ticket_comments ============ +CREATE TABLE IF NOT EXISTS `ticket_comments` ( + `comment_id` int(11) NOT NULL AUTO_INCREMENT, + `parent_comment_id` int(11) DEFAULT NULL, + `thread_depth` tinyint(3) unsigned NOT NULL DEFAULT 0, + `ticket_id` varchar(10) DEFAULT NULL, + `user_name` varchar(50) DEFAULT NULL, + `comment_text` text DEFAULT NULL, + `created_at` timestamp NULL DEFAULT current_timestamp(), + `markdown_enabled` tinyint(1) DEFAULT 0, + `user_id` int(11) DEFAULT NULL, + PRIMARY KEY (`comment_id`), + KEY `fk_comments_user_id` (`user_id`), + KEY `idx_comments_ticket_created` (`ticket_id`,`created_at` DESC), + KEY `idx_parent_comment` (`parent_comment_id`), + CONSTRAINT `fk_comments_user_id` FOREIGN KEY (`user_id`) REFERENCES `users` (`user_id`) ON DELETE SET NULL, + CONSTRAINT `fk_parent_comment` FOREIGN KEY (`parent_comment_id`) REFERENCES `ticket_comments` (`comment_id`) ON DELETE CASCADE, + CONSTRAINT `ticket_comments_ibfk_1` FOREIGN KEY (`ticket_id`) REFERENCES `tickets` (`ticket_id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ ticket_dependencies ============ +CREATE TABLE IF NOT EXISTS `ticket_dependencies` ( + `dependency_id` int(11) NOT NULL AUTO_INCREMENT, + `ticket_id` varchar(9) NOT NULL, + `depends_on_id` varchar(9) NOT NULL, + `dependency_type` enum('blocks','blocked_by','relates_to','duplicates') DEFAULT 'blocks', + `created_by` int(11) DEFAULT NULL, + `created_at` timestamp NULL DEFAULT current_timestamp(), + PRIMARY KEY (`dependency_id`), + UNIQUE KEY `unique_dependency` (`ticket_id`,`depends_on_id`,`dependency_type`), + KEY `idx_ticket_id` (`ticket_id`), + KEY `idx_depends_on_id` (`depends_on_id`), + KEY `created_by` (`created_by`), + CONSTRAINT `ticket_dependencies_ibfk_1` FOREIGN KEY (`created_by`) REFERENCES `users` (`user_id`) ON DELETE SET NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ ticket_templates ============ +CREATE TABLE IF NOT EXISTS `ticket_templates` ( + `template_id` int(11) NOT NULL AUTO_INCREMENT, + `template_name` varchar(100) NOT NULL, + `title_template` varchar(255) NOT NULL, + `description_template` text NOT NULL, + `category` varchar(50) DEFAULT NULL, + `type` varchar(50) DEFAULT NULL, + `default_priority` int(11) DEFAULT 4, + `created_by` int(11) DEFAULT NULL, + `is_active` tinyint(1) DEFAULT 1, + `created_at` timestamp NULL DEFAULT current_timestamp(), + PRIMARY KEY (`template_id`), + KEY `created_by` (`created_by`), + KEY `idx_template_name` (`template_name`), + CONSTRAINT `ticket_templates_ibfk_1` FOREIGN KEY (`created_by`) REFERENCES `users` (`user_id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ ticket_watchers ============ +CREATE TABLE IF NOT EXISTS `ticket_watchers` ( + `ticket_id` int(11) NOT NULL, + `user_id` int(11) NOT NULL, + `created_at` timestamp NOT NULL DEFAULT current_timestamp(), + PRIMARY KEY (`ticket_id`,`user_id`), + KEY `idx_watcher_user` (`user_id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ tickets ============ +CREATE TABLE IF NOT EXISTS `tickets` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `ticket_id` varchar(9) NOT NULL, + `title` varchar(255) NOT NULL, + `category` varchar(100) DEFAULT NULL, + `type` varchar(100) DEFAULT NULL, + `visibility` enum('public','internal','confidential') DEFAULT 'public', + `visibility_groups` varchar(500) DEFAULT NULL, + `status` varchar(20) NOT NULL DEFAULT 'Open', + `description` text DEFAULT NULL, + `created_at` timestamp NULL DEFAULT current_timestamp(), + `updated_at` timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(), + `closed_at` timestamp NULL DEFAULT NULL, + `priority` int(11) NOT NULL DEFAULT 1 CHECK (`priority` between 1 and 6), + `hash` varchar(64) DEFAULT NULL, + `created_by` int(11) DEFAULT NULL, + `updated_by` int(11) DEFAULT NULL, + `assigned_to` int(11) DEFAULT NULL, + PRIMARY KEY (`id`), + UNIQUE KEY `ticket_id` (`ticket_id`), + UNIQUE KEY `unique_hash` (`hash`), + KEY `fk_tickets_updated_by` (`updated_by`), + KEY `idx_status` (`status`), + KEY `idx_priority` (`priority`), + KEY `idx_tickets_created_at` (`created_at`), + KEY `idx_assigned_to` (`assigned_to`), + KEY `idx_tickets_status` (`status`), + KEY `idx_tickets_status_priority_created` (`status`,`priority`,`created_at` DESC), + KEY `idx_tickets_visibility` (`visibility`), + KEY `idx_tickets_category` (`category`), + KEY `idx_tickets_type` (`type`), + KEY `idx_tickets_priority` (`priority`), + KEY `idx_tickets_updated_at` (`updated_at`), + KEY `idx_tickets_created_by` (`created_by`), + KEY `idx_tickets_assigned_to` (`assigned_to`), + KEY `idx_tickets_status_created` (`status`,`created_at`), + KEY `idx_tickets_assigned_status` (`assigned_to`,`status`), + KEY `idx_tickets_visibility_status` (`visibility`,`status`), + KEY `idx_tickets_closed_at` (`closed_at`), + FULLTEXT KEY `ft_title_description` (`title`,`description`), + CONSTRAINT `fk_tickets_assigned_to` FOREIGN KEY (`assigned_to`) REFERENCES `users` (`user_id`) ON DELETE SET NULL, + CONSTRAINT `fk_tickets_created_by` FOREIGN KEY (`created_by`) REFERENCES `users` (`user_id`) ON DELETE SET NULL, + CONSTRAINT `fk_tickets_updated_by` FOREIGN KEY (`updated_by`) REFERENCES `users` (`user_id`) ON DELETE SET NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ user_preferences ============ +CREATE TABLE IF NOT EXISTS `user_preferences` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `user_id` int(11) NOT NULL, + `preference_key` varchar(100) NOT NULL, + `preference_value` text DEFAULT NULL, + `updated_at` timestamp NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(), + PRIMARY KEY (`id`), + UNIQUE KEY `unique_user_pref` (`user_id`,`preference_key`), + KEY `idx_user_preferences_user_key` (`user_id`,`preference_key`), + CONSTRAINT `user_preferences_ibfk_1` FOREIGN KEY (`user_id`) REFERENCES `users` (`user_id`) ON DELETE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + +-- ============ users ============ +CREATE TABLE IF NOT EXISTS `users` ( + `user_id` int(11) NOT NULL AUTO_INCREMENT, + `username` varchar(100) NOT NULL, + `display_name` varchar(255) DEFAULT NULL, + `email` varchar(255) DEFAULT NULL, + `groups` text DEFAULT NULL, + `is_admin` tinyint(1) DEFAULT 0, + `last_login` timestamp NULL DEFAULT NULL, + `created_at` timestamp NULL DEFAULT current_timestamp(), + PRIMARY KEY (`user_id`), + UNIQUE KEY `username` (`username`), + KEY `idx_username` (`username`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; + + +SET FOREIGN_KEY_CHECKS = 1; diff --git a/models/AuditLogModel.php b/models/AuditLogModel.php index 243e32d..f0eea92 100644 --- a/models/AuditLogModel.php +++ b/models/AuditLogModel.php @@ -19,13 +19,15 @@ class AuditLogModel /** @var array Allowed action types for filtering */ private const VALID_ACTION_TYPES = [ 'create', 'update', 'delete', 'view', 'security_event', - 'login', 'logout', 'assign', 'comment', 'bulk_update' + 'login', 'logout', 'assign', 'unassign', 'comment', 'mention', + 'revoke', 'attachment_upload', 'attachment_delete', 'bulk_update' ]; /** @var array Allowed entity types for filtering */ private const VALID_ENTITY_TYPES = [ 'ticket', 'comment', 'user', 'api_key', 'security', - 'template', 'attachment', 'group' + 'template', 'attachment', 'ticket_attachments', 'group', + 'dependency', 'workflow_transition', 'recurring_ticket', 'custom_field' ]; public function __construct($conn) @@ -327,24 +329,44 @@ class AuditLogModel */ private function getClientIP() { - $ipAddress = ''; + $remoteAddr = $_SERVER['REMOTE_ADDR'] ?? ''; - // Check for proxy headers - if (!empty($_SERVER['HTTP_CF_CONNECTING_IP'])) { - // Cloudflare - $ipAddress = $_SERVER['HTTP_CF_CONNECTING_IP']; - } elseif (!empty($_SERVER['HTTP_X_REAL_IP'])) { - // Nginx proxy - $ipAddress = $_SERVER['HTTP_X_REAL_IP']; - } elseif (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) { - // Standard proxy header - $ipAddress = explode(',', $_SERVER['HTTP_X_FORWARDED_FOR'])[0]; - } elseif (!empty($_SERVER['REMOTE_ADDR'])) { - // Direct connection - $ipAddress = $_SERVER['REMOTE_ADDR']; + // Forwarded/proxy headers are client-controlled, so only believe them when + // the request actually came from a trusted reverse proxy (same rule as + // RateLimitMiddleware). Otherwise a client could forge its audit-log IP. + $trusted = $GLOBALS['config']['TRUSTED_PROXIES'] ?? []; + if (empty($trusted) || !in_array($remoteAddr, $trusted, true)) { + return trim($remoteAddr); } - return trim($ipAddress); + // Cloudflare sets CF-Connecting-IP to the real client. + if ( + !empty($_SERVER['HTTP_CF_CONNECTING_IP']) + && filter_var($_SERVER['HTTP_CF_CONNECTING_IP'], FILTER_VALIDATE_IP) + ) { + return trim($_SERVER['HTTP_CF_CONNECTING_IP']); + } + + // The trusted proxy appends the connecting client to X-Forwarded-For, so + // the RIGHTMOST entry is the IP it observed (any client-supplied prefix is + // not trustworthy). + if (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) { + $ips = explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']); + $ip = trim(end($ips)); + if (filter_var($ip, FILTER_VALIDATE_IP)) { + return $ip; + } + } + + // X-Real-IP is set by the proxy itself. + if ( + !empty($_SERVER['HTTP_X_REAL_IP']) + && filter_var($_SERVER['HTTP_X_REAL_IP'], FILTER_VALIDATE_IP) + ) { + return trim($_SERVER['HTTP_X_REAL_IP']); + } + + return trim($remoteAddr); } /** diff --git a/models/BulkOperationsModel.php b/models/BulkOperationsModel.php index 2305670..713e28f 100644 --- a/models/BulkOperationsModel.php +++ b/models/BulkOperationsModel.php @@ -77,6 +77,15 @@ class BulkOperationsModel $ticketIds = explode(',', $operation['ticket_ids']); $parameters = $operation['parameters'] ? json_decode($operation['parameters'], true) : []; + + // Validate operation parameters up front so invalid values (out-of-range + // priority, unknown status, nonexistent assignee) are rejected cleanly + // instead of corrupting tickets or throwing mid-transaction. + $paramError = $this->validateOperationParameters($operation['operation_type'], is_array($parameters) ? $parameters : []); + if ($paramError !== null) { + return ['processed' => 0, 'failed' => count($ticketIds), 'error' => $paramError]; + } + $processed = 0; $failed = 0; $errors = []; @@ -297,6 +306,63 @@ class BulkOperationsModel return $result; } + /** + * Validate the parameters for a bulk operation before any ticket is mutated. + * + * @return string|null Error message, or null if the parameters are valid + */ + private function validateOperationParameters(string $type, array $parameters): ?string + { + switch ($type) { + case 'bulk_priority': + if (!isset($parameters['priority'])) { + return 'Missing priority parameter'; + } + $priority = $parameters['priority']; + // tickets.priority has a CHECK constraint (between 1 and 6). + if (!is_numeric($priority) || (int)$priority < 1 || (int)$priority > 6) { + return 'Invalid priority: must be between 1 and 6'; + } + break; + + case 'bulk_status': + if (!isset($parameters['status'])) { + return 'Missing status parameter'; + } + $validStatuses = $GLOBALS['config']['TICKET_STATUSES'] + ?? ['Open', 'Pending', 'In Progress', 'Closed']; + if (!in_array($parameters['status'], $validStatuses, true)) { + return 'Invalid status value'; + } + break; + + case 'bulk_assign': + if (!isset($parameters['assigned_to'])) { + return 'Missing assigned_to parameter'; + } + $assignedTo = $parameters['assigned_to']; + if (!is_numeric($assignedTo) || (int)$assignedTo <= 0 || !$this->userExists((int)$assignedTo)) { + return 'Invalid assigned_to: user does not exist'; + } + break; + } + + return null; + } + + /** + * Check whether a user ID exists. + */ + private function userExists(int $userId): bool + { + $stmt = $this->conn->prepare("SELECT 1 FROM users WHERE user_id = ? LIMIT 1"); + $stmt->bind_param("i", $userId); + $stmt->execute(); + $exists = $stmt->get_result()->num_rows > 0; + $stmt->close(); + return $exists; + } + /** * Get bulk operation by ID * diff --git a/models/CommentModel.php b/models/CommentModel.php index 50f609a..419f26b 100644 --- a/models/CommentModel.php +++ b/models/CommentModel.php @@ -58,12 +58,12 @@ class CommentModel /** * Get total comment count for a ticket */ - public function getCommentCount(int $ticketId): int + public function getCommentCount(string $ticketId): int { $stmt = $this->conn->prepare( "SELECT COUNT(*) as total FROM ticket_comments WHERE ticket_id = ?" ); - $stmt->bind_param("i", $ticketId); + $stmt->bind_param("s", $ticketId); $stmt->execute(); $row = $stmt->get_result()->fetch_assoc(); $stmt->close(); @@ -108,9 +108,9 @@ class CommentModel $stmt = $this->conn->prepare($sql); if ($limit > 0) { - $stmt->bind_param("iii", $ticketId, $limit, $offset); + $stmt->bind_param("sii", $ticketId, $limit, $offset); } else { - $stmt->bind_param("i", $ticketId); + $stmt->bind_param("s", $ticketId); } $stmt->execute(); $result = $stmt->get_result(); @@ -146,7 +146,7 @@ class CommentModel /** * Paginated threaded comments: fetch one page of root comments + all their replies. */ - private function getThreadedCommentsPaged(int $ticketId, int $limit, int $offset): array + private function getThreadedCommentsPaged(string $ticketId, int $limit, int $offset): array { // Page of root comments $rootSql = "SELECT tc.*, u.display_name, u.username @@ -156,7 +156,7 @@ class CommentModel ORDER BY tc.created_at DESC LIMIT ? OFFSET ?"; $stmt = $this->conn->prepare($rootSql); - $stmt->bind_param("iii", $ticketId, $limit, $offset); + $stmt->bind_param("sii", $ticketId, $limit, $offset); $stmt->execute(); $rootResult = $stmt->get_result(); $stmt->close(); @@ -192,7 +192,7 @@ class CommentModel AND tc.parent_comment_id IN ($placeholders) ORDER BY tc.created_at ASC"; $replyStmt = $this->conn->prepare($replySql); - $types = 'i' . str_repeat('i', count($parentIds)); + $types = 's' . str_repeat('i', count($parentIds)); $replyStmt->bind_param($types, $ticketId, ...$parentIds); $replyStmt->execute(); $replyResult = $replyStmt->get_result(); @@ -394,7 +394,8 @@ class CommentModel 'updated_at' => $hasUpdatedAt ? date('M d, Y H:i') : null ]; } else { - return ['success' => false, 'error' => $this->conn->error]; + error_log('CommentModel::updateComment failed: ' . $this->conn->error); + return ['success' => false, 'error' => 'Failed to update comment']; } } @@ -428,7 +429,8 @@ class CommentModel 'ticket_id' => $ticketId ]; } else { - return ['success' => false, 'error' => $this->conn->error]; + error_log('CommentModel::deleteComment failed: ' . $this->conn->error); + return ['success' => false, 'error' => 'Failed to delete comment']; } } } diff --git a/models/CustomFieldModel.php b/models/CustomFieldModel.php index d4af64d..4ebb3c0 100644 --- a/models/CustomFieldModel.php +++ b/models/CustomFieldModel.php @@ -96,6 +96,10 @@ class CustomFieldModel (field_name, field_label, field_type, field_options, category, is_required, display_order, is_active) VALUES (?, ?, ?, ?, ?, ?, ?, ?)"; + $isRequired = $data['is_required'] ?? 0; + $displayOrder = $data['display_order'] ?? 0; + $isActive = $data['is_active'] ?? 1; + $stmt = $this->conn->prepare($sql); $stmt->bind_param( 'sssssiii', @@ -104,9 +108,9 @@ class CustomFieldModel $data['field_type'], $options, $data['category'], - $data['is_required'] ?? 0, - $data['display_order'] ?? 0, - $data['is_active'] ?? 1 + $isRequired, + $displayOrder, + $isActive ); if ($stmt->execute()) { @@ -135,6 +139,10 @@ class CustomFieldModel category = ?, is_required = ?, display_order = ?, is_active = ? WHERE field_id = ?"; + $isRequired = $data['is_required'] ?? 0; + $displayOrder = $data['display_order'] ?? 0; + $isActive = $data['is_active'] ?? 1; + $stmt = $this->conn->prepare($sql); $stmt->bind_param( 'sssssiiii', @@ -143,9 +151,9 @@ class CustomFieldModel $data['field_type'], $options, $data['category'], - $data['is_required'] ?? 0, - $data['display_order'] ?? 0, - $data['is_active'] ?? 1, + $isRequired, + $displayOrder, + $isActive, $fieldId ); diff --git a/models/DependencyModel.php b/models/DependencyModel.php index ad5e85f..a7caf31 100644 --- a/models/DependencyModel.php +++ b/models/DependencyModel.php @@ -12,25 +12,67 @@ class DependencyModel $this->conn = $conn; } + /** + * Build the extra WHERE fragment (and bound params) that restricts the joined + * ticket alias `t` to tickets the requesting user may see. Reuses + * TicketModel::getVisibilityFilter so the rules stay in one place. + * + * @return array{sql:string,types:string,params:array} + */ + private function buildVisibilityClause($userId, array $userGroups, $isAdmin): array + { + if ($isAdmin) { + return ['sql' => '', 'types' => '', 'params' => []]; + } + + require_once dirname(__DIR__) . '/models/TicketModel.php'; + $ticketModel = new TicketModel($this->conn); + $filter = $ticketModel->getVisibilityFilter([ + 'user_id' => (int)$userId, + 'groups' => implode(',', $userGroups), + 'is_admin' => false, + ]); + + if ($filter['sql'] === '1=1' || $filter['sql'] === '') { + return ['sql' => '', 'types' => '', 'params' => []]; + } + + return [ + 'sql' => ' AND ' . $filter['sql'], + 'types' => $filter['types'], + 'params' => $filter['params'], + ]; + } + /** * Get all dependencies for a ticket * + * The linked ticket's title/status/priority are only returned for tickets the + * requesting user is allowed to see (same rules as TicketModel::getVisibilityFilter). + * With the default (null user, non-admin) only public tickets are exposed. + * * @param string $ticketId Ticket ID + * @param int|null $userId Requesting user's ID (null = anonymous) + * @param array $userGroups Requesting user's group names + * @param bool $isAdmin Whether the requesting user is an admin (bypasses filtering) * @return array Dependencies grouped by type */ - public function getDependencies($ticketId) + public function getDependencies($ticketId, $userId = null, array $userGroups = [], $isAdmin = false) { + $visibility = $this->buildVisibilityClause($userId, $userGroups, $isAdmin); + $sql = "SELECT d.*, t.title, t.status, t.priority FROM ticket_dependencies d LEFT JOIN tickets t ON d.depends_on_id = t.ticket_id - WHERE d.ticket_id = ? + WHERE d.ticket_id = ?" . $visibility['sql'] . " ORDER BY d.dependency_type, d.created_at DESC"; $stmt = $this->conn->prepare($sql); if (!$stmt) { throw new Exception('Prepare failed: ' . $this->conn->error); } - $stmt->bind_param("s", $ticketId); + $types = 's' . $visibility['types']; + $stmt->bind_param($types, $ticketId, ...$visibility['params']); if (!$stmt->execute()) { throw new Exception('Execute failed: ' . $stmt->error); } @@ -54,22 +96,32 @@ class DependencyModel /** * Get tickets that depend on this ticket * + * The linked ticket's title/status/priority are only returned for tickets the + * requesting user is allowed to see (same rules as TicketModel::getVisibilityFilter). + * With the default (null user, non-admin) only public tickets are exposed. + * * @param string $ticketId Ticket ID + * @param int|null $userId Requesting user's ID (null = anonymous) + * @param array $userGroups Requesting user's group names + * @param bool $isAdmin Whether the requesting user is an admin (bypasses filtering) * @return array Dependent tickets */ - public function getDependentTickets($ticketId) + public function getDependentTickets($ticketId, $userId = null, array $userGroups = [], $isAdmin = false) { + $visibility = $this->buildVisibilityClause($userId, $userGroups, $isAdmin); + $sql = "SELECT d.*, t.title, t.status, t.priority FROM ticket_dependencies d LEFT JOIN tickets t ON d.ticket_id = t.ticket_id - WHERE d.depends_on_id = ? + WHERE d.depends_on_id = ?" . $visibility['sql'] . " ORDER BY d.dependency_type, d.created_at DESC"; $stmt = $this->conn->prepare($sql); if (!$stmt) { throw new Exception('Prepare failed: ' . $this->conn->error); } - $stmt->bind_param("s", $ticketId); + $types = 's' . $visibility['types']; + $stmt->bind_param($types, $ticketId, ...$visibility['params']); if (!$stmt->execute()) { throw new Exception('Execute failed: ' . $stmt->error); } diff --git a/models/RecurringTicketModel.php b/models/RecurringTicketModel.php index 2e54b38..500a14f 100644 --- a/models/RecurringTicketModel.php +++ b/models/RecurringTicketModel.php @@ -65,7 +65,7 @@ class RecurringTicketModel $stmt = $this->conn->prepare($sql); $stmt->bind_param( - 'ssssiiisssii', + 'ssssiissssii', $data['title_template'], $data['description_template'], $data['category'], diff --git a/models/TicketModel.php b/models/TicketModel.php index ae77308..b64c837 100644 --- a/models/TicketModel.php +++ b/models/TicketModel.php @@ -9,7 +9,7 @@ class TicketModel $this->conn = $conn; } - public function getTicketById(int $id): ?array + public function getTicketById(string $id): ?array { $sql = "SELECT t.*, u_created.username as creator_username, @@ -24,7 +24,7 @@ class TicketModel LEFT JOIN users u_assigned ON t.assigned_to = u_assigned.user_id WHERE t.ticket_id = ?"; $stmt = $this->conn->prepare($sql); - $stmt->bind_param("i", $id); + $stmt->bind_param("s", $id); $stmt->execute(); $result = $stmt->get_result(); @@ -82,18 +82,22 @@ class TicketModel $paramTypes .= str_repeat('s', count($types)); } - // Search Functionality — use FULLTEXT when available, fall back to LIKE - if ($search && !empty($search)) { - if ($this->hasFulltextIndex()) { + // Search Functionality — use FULLTEXT when available, fall back to LIKE. + // Use a strict emptiness check so a literal "0" search is honored. + if ($search !== null && $search !== '') { + // Strip MySQL boolean mode special chars to prevent parse errors on user input + $ftSearch = trim(preg_replace('/\s+/', ' ', preg_replace('/[+\-><()\~*"@]+/', ' ', $search))); + if ($this->hasFulltextIndex() && $ftSearch !== '') { // MATCH...AGAINST for indexed full-text search (much faster at scale) - // Strip MySQL boolean mode special chars to prevent parse errors on user input - $ftSearch = preg_replace('/[+\-><()\~*"@]+/', ' ', $search); - $ftSearch = trim(preg_replace('/\s+/', ' ', $ftSearch)) . '*'; + $ftSearch .= '*'; $whereConditions[] = "(MATCH(t.title, t.description) AGAINST (? IN BOOLEAN MODE) OR t.ticket_id LIKE ? OR t.category LIKE ? OR t.type LIKE ?)"; $searchTerm = "%$search%"; $params = array_merge($params, [$ftSearch, $searchTerm, $searchTerm, $searchTerm]); $paramTypes .= 'ssss'; } else { + // No FULLTEXT index, or the sanitized boolean query is empty (search was + // only special chars) — fall back to LIKE instead of emitting invalid + // AGAINST('*' ...) syntax. $whereConditions[] = "(t.title LIKE ? OR t.description LIKE ? OR t.ticket_id LIKE ? OR t.category LIKE ? OR t.type LIKE ?)"; $searchTerm = "%$search%"; $params = array_merge($params, [$searchTerm, $searchTerm, $searchTerm, $searchTerm, $searchTerm]); @@ -308,7 +312,7 @@ class TicketModel if ($expectedUpdatedAt !== null) { $stmt->bind_param( - "sissssisis", + "sissssisss", $ticketData['title'], $ticketData['priority'], $ticketData['status'], @@ -322,7 +326,7 @@ class TicketModel ); } else { $stmt->bind_param( - "sissssisi", + "sissssiss", $ticketData['title'], $ticketData['priority'], $ticketData['status'], @@ -343,20 +347,31 @@ class TicketModel return ['success' => false, 'error' => 'Database error: ' . $this->conn->error, 'conflict' => false]; } - // Check for optimistic locking conflict - if ($expectedUpdatedAt !== null && $affectedRows === 0) { - // Either ticket doesn't exist or was modified by someone else + // Zero affected rows is ambiguous: the ticket may not exist, an optimistic + // lock may have failed, or the row simply matched with no column changes + // (identical resubmit). Disambiguate so we neither report a false conflict + // nor silently "succeed" on a non-existent ticket. + if ($affectedRows === 0) { $ticket = $this->getTicketById($ticketData['ticket_id']); - if ($ticket) { - return [ - 'success' => false, - 'error' => 'This ticket was modified by another user. Please refresh and try again.', - 'conflict' => true, - 'current_updated_at' => $ticket['updated_at'] - ]; - } else { + if (!$ticket) { return ['success' => false, 'error' => 'Ticket not found', 'conflict' => false]; } + + if ($expectedUpdatedAt !== null) { + // Only a genuine concurrent modification changes updated_at. If it + // still equals the expected value the WHERE matched but nothing + // changed (e.g. identical data resubmitted within the same second), + // which is not a conflict. + if ($ticket['updated_at'] !== $expectedUpdatedAt) { + return [ + 'success' => false, + 'error' => 'This ticket was modified by another user. Please refresh and try again.', + 'conflict' => true, + 'current_updated_at' => $ticket['updated_at'] + ]; + } + } + // Ticket exists and no conflict: treat no-op update as success. } return ['success' => true, 'error' => null, 'conflict' => false]; @@ -516,9 +531,9 @@ class TicketModel } } - public function addComment(int $ticketId, array $commentData): array + public function addComment(string $ticketId, array $commentData): array { - $sql = "INSERT INTO ticket_comments (ticket_id, user_name, comment_text, markdown_enabled) + $sql = "INSERT INTO ticket_comments (ticket_id, user_name, comment_text, markdown_enabled) VALUES (?, ?, ?, ?)"; $stmt = $this->conn->prepare($sql); @@ -528,7 +543,7 @@ class TicketModel $markdownEnabled = $commentData['markdown_enabled'] ? 1 : 0; $stmt->bind_param( - "issi", + "sssi", $ticketId, $username, $commentData['comment_text'], @@ -557,11 +572,11 @@ class TicketModel * @param int $assignedBy User ID performing the assignment * @return bool Success status */ - public function assignTicket(int $ticketId, int $userId, int $assignedBy): bool + public function assignTicket(string $ticketId, int $userId, int $assignedBy): bool { $sql = "UPDATE tickets SET assigned_to = ?, updated_by = ?, updated_at = NOW() WHERE ticket_id = ?"; $stmt = $this->conn->prepare($sql); - $stmt->bind_param("iii", $userId, $assignedBy, $ticketId); + $stmt->bind_param("iis", $userId, $assignedBy, $ticketId); $result = $stmt->execute(); $stmt->close(); return $result; @@ -574,11 +589,11 @@ class TicketModel * @param int $updatedBy User ID performing the unassignment * @return bool Success status */ - public function unassignTicket(int $ticketId, int $updatedBy): bool + public function unassignTicket(string $ticketId, int $updatedBy): bool { $sql = "UPDATE tickets SET assigned_to = NULL, updated_by = ?, updated_at = NOW() WHERE ticket_id = ?"; $stmt = $this->conn->prepare($sql); - $stmt->bind_param("ii", $updatedBy, $ticketId); + $stmt->bind_param("is", $updatedBy, $ticketId); $result = $stmt->execute(); $stmt->close(); return $result; @@ -733,7 +748,7 @@ class TicketModel * @param int $updatedBy User ID * @return bool */ - public function updateVisibility(int $ticketId, string $visibility, ?string $visibilityGroups, int $updatedBy): bool + public function updateVisibility(string $ticketId, string $visibility, ?string $visibilityGroups, int $updatedBy): bool { $allowedVisibilities = ['public', 'internal', 'confidential']; if (!in_array($visibility, $allowedVisibilities)) { @@ -752,7 +767,7 @@ class TicketModel $sql = "UPDATE tickets SET visibility = ?, visibility_groups = ?, updated_by = ?, updated_at = NOW() WHERE ticket_id = ?"; $stmt = $this->conn->prepare($sql); - $stmt->bind_param("ssii", $visibility, $visibilityGroups, $updatedBy, $ticketId); + $stmt->bind_param("ssis", $visibility, $visibilityGroups, $updatedBy, $ticketId); $result = $stmt->execute(); $stmt->close(); return $result; @@ -790,7 +805,7 @@ class TicketModel "DELETE FROM ticket_watchers WHERE ticket_id = ?", "DELETE FROM ticket_dependencies WHERE ticket_id = ? OR depends_on_id = ?", "DELETE FROM ticket_attachments WHERE ticket_id = ?", - "DELETE FROM ticket_custom_fields WHERE ticket_id = ?", + "DELETE FROM custom_field_values WHERE ticket_id = ?", ]; foreach ($children as $sql) { diff --git a/models/WorkflowModel.php b/models/WorkflowModel.php index 3906eff..473bfed 100644 --- a/models/WorkflowModel.php +++ b/models/WorkflowModel.php @@ -26,31 +26,38 @@ class WorkflowModel */ private function getAllTransitions(): array { - return CacheHelper::remember(self::$CACHE_PREFIX, 'all_transitions', function () { - $sql = "SELECT from_status, to_status, requires_comment, requires_admin - FROM status_transitions - WHERE is_active = TRUE"; - $result = $this->conn->query($sql); + $cached = CacheHelper::get(self::$CACHE_PREFIX, 'all_transitions', self::$CACHE_TTL); + if ($cached !== null) { + return $cached; + } - if (!$result) { - return []; + $sql = "SELECT from_status, to_status, requires_comment, requires_admin + FROM status_transitions + WHERE is_active = TRUE"; + $result = $this->conn->query($sql); + + if (!$result) { + // A transient DB failure must NOT be cached as "no transitions" — that + // would block every status change for the whole TTL. Fail safe by + // returning empty without storing it, so the next call retries. + return []; + } + + $transitions = []; + while ($row = $result->fetch_assoc()) { + $from = $row['from_status']; + if (!isset($transitions[$from])) { + $transitions[$from] = []; } + $transitions[$from][$row['to_status']] = [ + 'to_status' => $row['to_status'], + 'requires_comment' => (bool)$row['requires_comment'], + 'requires_admin' => (bool)$row['requires_admin'] + ]; + } - $transitions = []; - while ($row = $result->fetch_assoc()) { - $from = $row['from_status']; - if (!isset($transitions[$from])) { - $transitions[$from] = []; - } - $transitions[$from][$row['to_status']] = [ - 'to_status' => $row['to_status'], - 'requires_comment' => (bool)$row['requires_comment'], - 'requires_admin' => (bool)$row['requires_admin'] - ]; - } - - return $transitions; - }, self::$CACHE_TTL); + CacheHelper::set(self::$CACHE_PREFIX, 'all_transitions', $transitions); + return $transitions; } /** @@ -107,24 +114,29 @@ class WorkflowModel */ public function getAllStatuses(): array { - return CacheHelper::remember(self::$CACHE_PREFIX, 'all_statuses', function () { - $sql = "SELECT DISTINCT from_status as status FROM status_transitions - UNION - SELECT DISTINCT to_status as status FROM status_transitions - ORDER BY status"; - $result = $this->conn->query($sql); + $cached = CacheHelper::get(self::$CACHE_PREFIX, 'all_statuses', self::$CACHE_TTL); + if ($cached !== null) { + return $cached; + } - if (!$result) { - return []; - } + $sql = "SELECT DISTINCT from_status as status FROM status_transitions + UNION + SELECT DISTINCT to_status as status FROM status_transitions + ORDER BY status"; + $result = $this->conn->query($sql); - $statuses = []; - while ($row = $result->fetch_assoc()) { - $statuses[] = $row['status']; - } + if (!$result) { + // Do not cache an empty list on a transient DB failure. + return []; + } - return $statuses; - }, self::$CACHE_TTL); + $statuses = []; + while ($row = $result->fetch_assoc()) { + $statuses[] = $row['status']; + } + + CacheHelper::set(self::$CACHE_PREFIX, 'all_statuses', $statuses); + return $statuses; } /** @@ -149,6 +161,23 @@ class WorkflowModel ]; } + /** + * Whether a given transition requires a comment. + * + * Convenience accessor so callers (e.g. the update-ticket endpoint) can + * enforce requires_comment server-side without inspecting the full row. + * Returns false for an undefined transition or a no-op (same status). + * + * @param string $fromStatus Current status + * @param string $toStatus Desired status + * @return bool True if the transition requires a comment + */ + public function transitionRequiresComment(string $fromStatus, string $toStatus): bool + { + $requirements = $this->getTransitionRequirements($fromStatus, $toStatus); + return $requirements !== null && !empty($requirements['requires_comment']); + } + /** * Clear workflow cache (call when transitions are modified) */ diff --git a/scripts/cleanup_orphan_uploads.php b/scripts/cleanup_orphan_uploads.php new file mode 100644 index 0000000..267b1f0 --- /dev/null +++ b/scripts/cleanup_orphan_uploads.php @@ -0,0 +1,143 @@ +#!/usr/bin/env php +/ that have NO matching row in + * ticket_attachments (e.g. leftovers from a failed DB insert). Intended to be + * run from cron: + * 0 4 * * * /usr/bin/php /path/to/scripts/cleanup_orphan_uploads.php >> /var/log/orphan_uploads.log 2>&1 + * + * SAFETY: + * - Only files older than a grace period (GRACE_SECONDS, default 24h) are + * considered, so a freshly written file whose DB row has not been inserted + * yet (in-flight upload) is never deleted. + * - Only 9-digit ticket directories are scanned. uploads/avatars/ (and any + * other non-ticket directory) is skipped entirely. + * - A file is deleted only when no ticket_attachments row references its + * stored filename (looked up with a prepared statement). + * + * Usage: + * php cleanup_orphan_uploads.php # delete orphaned files past grace period + * php cleanup_orphan_uploads.php --dry-run # report only, delete nothing + */ + +// Prevent web access +if (php_sapi_name() !== 'cli') { + http_response_code(403); + exit('CLI access only'); +} + +require_once dirname(__DIR__) . '/config/config.php'; +require_once dirname(__DIR__) . '/helpers/Database.php'; + +/** Files younger than this (seconds) are never touched — protects in-flight uploads. */ +const GRACE_SECONDS = 86400; + +$dryRun = in_array('--dry-run', $argv, true); + +function logMessage($message) +{ + echo '[' . date('Y-m-d H:i:s') . '] ' . $message . "\n"; +} + +$uploadDir = $GLOBALS['config']['UPLOAD_DIR'] ?? (dirname(__DIR__) . '/uploads'); +$uploadRoot = realpath($uploadDir); + +if ($uploadRoot === false || !is_dir($uploadRoot)) { + logMessage("Upload directory not found: {$uploadDir}"); + exit(0); +} + +logMessage('Starting orphan upload cleanup' . ($dryRun ? ' (DRY RUN)' : '')); + +try { + $conn = Database::getConnection(); +} catch (Exception $e) { + logMessage('FATAL ERROR: could not connect to database: ' . $e->getMessage()); + exit(1); +} + +// Prepared lookup: does any attachment row reference this stored filename? +// Stored filenames are globally unique (uniqid), so filename alone is sufficient +// and safe — a match in any ticket means the file is a real attachment. +$lookup = $conn->prepare('SELECT 1 FROM ticket_attachments WHERE filename = ? LIMIT 1'); +if ($lookup === false) { + logMessage('FATAL ERROR: could not prepare lookup statement: ' . $conn->error); + exit(1); +} + +$now = time(); +$scanned = 0; +$orphaned = 0; +$deleted = 0; +$skippedTooNew = 0; +$errors = 0; + +foreach (new DirectoryIterator($uploadRoot) as $entry) { + if ($entry->isDot() || !$entry->isDir() || $entry->isLink()) { + continue; + } + + // Ticket directories are 9-digit ticket IDs. Skip avatars/ and anything else. + $dirName = $entry->getFilename(); + if (!preg_match('/^\d{9}$/', $dirName)) { + continue; + } + + foreach (new DirectoryIterator($entry->getPathname()) as $file) { + if ($file->isDot() || !$file->isFile() || $file->isLink()) { + continue; + } + + $scanned++; + $filename = $file->getFilename(); + + // Never touch files younger than the grace period (in-flight uploads). + $age = $now - $file->getMTime(); + if ($age < GRACE_SECONDS) { + $skippedTooNew++; + continue; + } + + // Keep the file if any attachment row references it. + $lookup->bind_param('s', $filename); + $lookup->execute(); + $hasRow = $lookup->get_result()->num_rows > 0; + + if ($hasRow) { + continue; + } + + $orphaned++; + $path = $file->getPathname(); + + if ($dryRun) { + logMessage("WOULD DELETE orphan: {$dirName}/{$filename}"); + continue; + } + + if (@unlink($path)) { + $deleted++; + logMessage("Deleted orphan: {$dirName}/{$filename}"); + } else { + $errors++; + logMessage("ERROR: could not delete: {$dirName}/{$filename}"); + } + } +} + +$lookup->close(); +Database::close(); + +logMessage('Cleanup complete' . ($dryRun ? ' (DRY RUN — nothing deleted)' : '') . ':'); +logMessage(" - Scanned: {$scanned} files"); +logMessage(" - Orphaned: {$orphaned} files"); +logMessage(" - Deleted: {$deleted} files"); +logMessage(" - Skipped (too new): {$skippedTooNew} files"); +if ($errors > 0) { + logMessage(" - Errors: {$errors} files"); +} + +exit($errors > 0 ? 1 : 0); diff --git a/views/DashboardView.php b/views/DashboardView.php index 4384d98..af6895d 100644 --- a/views/DashboardView.php +++ b/views/DashboardView.php @@ -1317,7 +1317,7 @@ if (advForm) advForm.addEventListener('submit', function(e) { var pLabels = { '1':'P1 — Critical', '2':'P2 — High', '3':'P3 — Medium', '4':'P4 — Low', '5':'P5 — Minimal' }; var dotClass = { 'Open':'lt-dot-up', 'In Progress':'lt-dot-warn', 'Pending':'lt-dot--orange', 'Closed':'lt-dot-idle' }; - function esc(s) { return String(s||'').replace(/&/g,'&').replace(//g,'>'); } + function esc(s) { return String(s||'').replace(/&/g,'&').replace(//g,'>').replace(/"/g,'"').replace(/'/g,'''); } function fmtAge(dateStr) { var d = new Date(dateStr); diff --git a/views/TicketView.php b/views/TicketView.php index 635c5f9..767ffc0 100644 --- a/views/TicketView.php +++ b/views/TicketView.php @@ -461,8 +461,8 @@ include __DIR__ . '/layout_header.php';