From 9e83f8903a02a368770fa8a8d012e6f44a2709f1 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 11 Sep 2026 13:30:59 -0400 Subject: [PATCH] Prune watchers when a ticket's visibility is tightened (#73) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TicketModel::updateVisibility() only updated the tickets row — it never touched ticket_watchers. A user watching a public ticket that's later made confidential/internal, and who isn't creator/assignee/ admin/in the new visibility_groups, kept receiving Matrix notifications (title + redacted activity preview) about a ticket canUserAccessTicket() would now reject them from opening directly. After a successful visibility update, re-evaluates every current watcher against the new visibility rules via the same canUserAccessTicket() check the rest of the app uses, and removes any who no longer qualify. Verified against real MariaDB: tightening to confidential correctly drops watchers with no standing access while keeping an admin watcher; tightening to internal with a specific group correctly keeps a watcher in that group and drops one who isn't. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv --- models/TicketModel.php | 59 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/models/TicketModel.php b/models/TicketModel.php index 7e05e42..5079224 100644 --- a/models/TicketModel.php +++ b/models/TicketModel.php @@ -773,9 +773,68 @@ class TicketModel $stmt->bind_param("ssis", $visibility, $visibilityGroups, $updatedBy, $ticketId); $result = $stmt->execute(); $stmt->close(); + + if ($result) { + $this->pruneWatchersForVisibility($ticketId, $visibility, $visibilityGroups); + } + return $result; } + /** + * Remove any watchers who no longer qualify for a ticket's access rules + * after its visibility was tightened. Without this, a user watching a + * ticket that's later made confidential/internal (and who isn't + * creator/assignee/admin/in the new visibility_groups) keeps receiving + * Matrix notifications about a ticket canUserAccessTicket() would now + * reject them from opening directly. + */ + private function pruneWatchersForVisibility(string $ticketId, string $visibility, ?string $visibilityGroups): void + { + $ticket = $this->getTicketById($ticketId); + if (!$ticket) { + return; + } + // getTicketById() reflects the just-committed UPDATE, but set these + // explicitly so pruning is correct even if a caller reorders things. + $ticket['visibility'] = $visibility; + $ticket['visibility_groups'] = $visibilityGroups; + + $sql = "SELECT tw.user_id, u.is_admin, u.`groups` + FROM ticket_watchers tw + JOIN users u ON tw.user_id = u.user_id + WHERE tw.ticket_id = ?"; + $stmt = $this->conn->prepare($sql); + $stmt->bind_param('s', $ticketId); + $stmt->execute(); + $watchers = $stmt->get_result()->fetch_all(MYSQLI_ASSOC); + $stmt->close(); + + $toRemove = []; + foreach ($watchers as $watcher) { + $watcherUser = [ + 'user_id' => $watcher['user_id'], + 'is_admin' => $watcher['is_admin'], + 'groups' => $watcher['groups'], + ]; + if (!$this->canUserAccessTicket($ticket, $watcherUser)) { + $toRemove[] = $watcher['user_id']; + } + } + + if (empty($toRemove)) { + return; + } + + $placeholders = implode(',', array_fill(0, count($toRemove), '?')); + $delSql = "DELETE FROM ticket_watchers WHERE ticket_id = ? AND user_id IN ($placeholders)"; + $delStmt = $this->conn->prepare($delSql); + $types = 's' . str_repeat('i', count($toRemove)); + $delStmt->bind_param($types, $ticketId, ...$toRemove); + $delStmt->execute(); + $delStmt->close(); + } + /** * Delete a ticket and all its associated records. * Admin-only operation. Removes comments, attachments, watchers, dependencies.