Prune watchers when a ticket's visibility is tightened (#73)

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
This commit is contained in:
2026-09-11 13:30:59 -04:00
co-authored by Claude Sonnet 5
parent cabdae35cc
commit 9e83f8903a
+59
View File
@@ -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.