From 3db3749c466b17b8b64c9138ab7f6dc523f4fdb8 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 11 Sep 2026 13:31:08 -0400 Subject: [PATCH] Re-check ticket visibility before surfacing in-app notifications (#48) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All four notification queries in api/notifications.php (assign, comment, status-change, mention) were scoped purely by created_by/assigned_to/ticket_watchers membership and historical audit_log contents — never by canUserAccessTicket(). If a ticket's visibility was later tightened, or a user's group/watcher access revoked, a notification still surfaced in their bell dropdown, disclosing the ticket's title and that activity occurred even though opening the ticket itself would now be blocked. Batch-fetches the tickets referenced by all candidate notifications (via the existing getTicketsByIds()) and filters out any whose current state canUserAccessTicket() would reject for the requesting user, before formatting the response — so a notification for a ticket the user can no longer see simply disappears rather than lingering as a disclosure. Verified against real MariaDB with a running server: an assignment notification is visible while the user is the assignee of a public ticket, and disappears once the ticket is reassigned away and made confidential. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv --- api/notifications.php | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/api/notifications.php b/api/notifications.php index ff22d70..b3d5dbd 100644 --- a/api/notifications.php +++ b/api/notifications.php @@ -15,8 +15,10 @@ require_once __DIR__ . '/bootstrap.php'; require_once dirname(__DIR__) . '/models/UserPreferencesModel.php'; +require_once dirname(__DIR__) . '/models/TicketModel.php'; $prefsModel = new UserPreferencesModel($conn); +$ticketModel = new TicketModel($conn); // ── POST: mark all read (update last_seen timestamp) ────────────── if ($_SERVER['REQUEST_METHOD'] === 'POST') { @@ -204,7 +206,44 @@ foreach (array_merge($assignRows, $commentRows, $statusRows, $mentionRows) as $r $all[] = $row; } usort($all, fn($a, $b) => strcmp($b['created_at'], $a['created_at'])); -$all = array_slice($all, 0, 30); + +// Re-check current ticket visibility before surfacing anything: a +// notification's audit_log entry reflects historical activity, but the +// ticket's visibility (or the user's group/watcher standing) may have +// tightened since. Without this, a notification still discloses the +// ticket's title and that activity occurred to someone who currently +// shouldn't see it, even though the ticket view's own access check would +// correctly reject them from opening it. +$candidateTicketIds = []; +foreach ($all as $row) { + $details = json_decode($row['details'] ?? '{}', true) ?? []; + $actionType = ($row['action_type'] === 'create' && $row['entity_type'] === 'comment') + ? 'comment' + : $row['action_type']; + $tid = ($actionType === 'comment' || $actionType === 'mention') + ? ($details['ticket_id'] ?? 0) + : $row['entity_id']; + if ($tid) { + $candidateTicketIds[(string)$tid] = true; + } +} +$ticketsById = !empty($candidateTicketIds) + ? $ticketModel->getTicketsByIds(array_keys($candidateTicketIds)) + : []; + +$all = array_filter($all, function ($row) use ($ticketsById, $currentUser, $ticketModel) { + $details = json_decode($row['details'] ?? '{}', true) ?? []; + $actionType = ($row['action_type'] === 'create' && $row['entity_type'] === 'comment') + ? 'comment' + : $row['action_type']; + $tid = (string)(($actionType === 'comment' || $actionType === 'mention') + ? ($details['ticket_id'] ?? 0) + : $row['entity_id']); + $ticket = $ticketsById[$tid] ?? null; + return $ticket && $ticketModel->canUserAccessTicket($ticket, $currentUser); +}); + +$all = array_slice(array_values($all), 0, 30); // Format for response $notifications = [];