diff --git a/api/add_comment.php b/api/add_comment.php index 06538b7..6fecce9 100644 --- a/api/add_comment.php +++ b/api/add_comment.php @@ -177,9 +177,16 @@ try { $ticketTitle = $ticket['title'] ?? "Ticket #{$ticketId}"; $ticketVisibility = $ticket['visibility'] ?? 'public'; - // @mention notifications — resolve usernames → Matrix IDs via Synapse Admin API - if (!empty($mentionedUsers)) { - $mentionedUsernames = array_column($mentionedUsers, 'username'); + // @mention notifications — resolve usernames → Matrix IDs via Synapse Admin API. + // Only notify mentioned users who actually have access to this ticket; + // otherwise a mention would DM them the ticket's title and comment text + // even though canUserAccessTicket() would deny them the ticket itself. + $accessibleMentionedUsers = array_filter( + $mentionedUsers, + fn($u) => $ticketModel->canUserAccessTicket($ticket, $u) + ); + if (!empty($accessibleMentionedUsers)) { + $mentionedUsernames = array_column($accessibleMentionedUsers, 'username'); $mentionedMatrixIds = SynapseHelper::resolveUsernames($mentionedUsernames); if (!empty($mentionedMatrixIds)) { NotificationHelper::sendMentionNotification($ticketId, $ticketTitle, $commentText, $authorDisplay, $mentionedMatrixIds); diff --git a/api/assign_ticket.php b/api/assign_ticket.php index c616de5..1046909 100644 --- a/api/assign_ticket.php +++ b/api/assign_ticket.php @@ -76,7 +76,8 @@ if ($assignedTo === null || $assignedTo === '') { $ticket['title'] ?? "Ticket #{$ticketId}", $assigneeName, $assigneeMatrix, - $changedByDisplay + $changedByDisplay, + $ticket['visibility'] ?? 'public' ); } } diff --git a/api/ticket_status_api.php b/api/ticket_status_api.php index 144ce68..c7007dd 100644 --- a/api/ticket_status_api.php +++ b/api/ticket_status_api.php @@ -171,7 +171,8 @@ if ($currentStatus !== $newStatus) { $currentStatus, $newStatus, (string)$ticket['title'], - $keyName + $keyName, + $ticket['visibility'] ?? 'public' ); NotificationHelper::notifyWatchers( $conn, diff --git a/api/update_ticket.php b/api/update_ticket.php index de210be..0b51442 100644 --- a/api/update_ticket.php +++ b/api/update_ticket.php @@ -267,7 +267,8 @@ try { $currentTicket['status'], $updateData['status'], $updateData['title'], - $changedBy + $changedBy, + $currentTicket['visibility'] ?? 'public' ); NotificationHelper::notifyWatchers( $this->conn, diff --git a/helpers/NotificationHelper.php b/helpers/NotificationHelper.php index af4cf91..179043f 100644 --- a/helpers/NotificationHelper.php +++ b/helpers/NotificationHelper.php @@ -40,20 +40,40 @@ class NotificationHelper return array_values(array_filter(array_map('trim', explode(',', $raw)))); } + /** + * Redact a ticket title for the shared Matrix notify list when the + * ticket isn't public, matching how sendCommentNotification() and + * notifyWatchers() already redact comment/activity previews for the + * same list. + */ + private static function redactedTitle(string $title, string $visibility): string + { + return $visibility === 'public' ? $title : '(restricted ticket — title hidden)'; + } + // ─── Public event methods ───────────────────────────────────────────────── /** * New ticket created (manual or automated/API). + * + * $ticketData['visibility'] ('public', 'internal', or 'confidential') is + * used to redact the title sent to the shared MATRIX_NOTIFY_USERS list + * for non-public tickets, same as sendCommentNotification()'s preview + * redaction. Defaults to 'public' for callers (e.g. the hwmonDaemon + * Bearer-API paths) that never set a non-default visibility. */ public static function sendTicketNotification($ticketId, array $ticketData, string $trigger = 'manual'): void { - preg_match('/^\[([^\]]+)\]/', $ticketData['title'] ?? '', $m); + $visibility = $ticketData['visibility'] ?? 'public'; + $title = $ticketData['title'] ?? 'Untitled'; + + preg_match('/^\[([^\]]+)\]/', $title, $m); $source = $m[1] ?? ($trigger === 'automated' ? 'Automated' : 'Manual'); self::fire([ 'event' => 'ticket_created', 'ticket_id' => $ticketId, - 'title' => $ticketData['title'] ?? 'Untitled', + 'title' => self::redactedTitle($title, $visibility), 'priority' => (int)($ticketData['priority'] ?? 4), 'category' => $ticketData['category'] ?? 'General', 'type' => $ticketData['type'] ?? 'Issue', @@ -73,13 +93,16 @@ class NotificationHelper * @param string $newStatus * @param string $ticketTitle * @param string|null $changedByDisplay Display name of the user who changed status + * @param string $visibility Ticket visibility; non-public titles are + * redacted before being sent to the shared + * notify list, same as sendTicketNotification(). */ - public static function sendStatusChangeNotification($ticketId, string $oldStatus, string $newStatus, string $ticketTitle, ?string $changedByDisplay = null): void + public static function sendStatusChangeNotification($ticketId, string $oldStatus, string $newStatus, string $ticketTitle, ?string $changedByDisplay = null, string $visibility = 'public'): void { self::fire([ 'event' => 'status_changed', 'ticket_id' => $ticketId, - 'title' => $ticketTitle, + 'title' => self::redactedTitle($ticketTitle, $visibility), 'old_status' => $oldStatus, 'new_status' => $newStatus, 'changed_by' => $changedByDisplay, @@ -166,11 +189,12 @@ class NotificationHelper * @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. + * 'confidential'. The shared notify list may + * contain users without access to non-public + * tickets, so for those tickets it's excluded + * entirely (only actual watchers are notified) + * and both the title and any comment/body + * preview in $extraData are redacted. */ public static function notifyWatchers(\mysqli $conn, $ticketId, string $ticketTitle, string $event, array $extraData = [], ?int $excludeUserId = null, string $visibility = 'public'): void { @@ -230,13 +254,17 @@ class NotificationHelper return; } - // Remove the global notify list duplicates and build payload - $allNotify = array_unique(array_merge($matrixIds, self::notifyUsers())); + // The shared notify list may include users without access to + // non-public tickets, so only mix it in for public tickets — for + // internal/confidential tickets, notify actual watchers only. + $allNotify = $visibility === 'public' + ? array_unique(array_merge($matrixIds, self::notifyUsers())) + : $matrixIds; $payload = array_merge($extraData, [ 'event' => $event, 'ticket_id' => $ticketId, - 'title' => $ticketTitle, + 'title' => self::redactedTitle($ticketTitle, $visibility), 'url' => UrlHelper::ticketUrl($ticketId), 'notify_users' => array_values($allNotify), ]); @@ -252,8 +280,14 @@ class NotificationHelper * @param string|null $assigneeName Display name of new assignee * @param string|null $assigneeMatrix Matrix user ID of new assignee (to DM) * @param string|null $changedByDisplay + * @param string $visibility Ticket visibility; non-public titles are + * redacted before being sent to the shared + * notify list, same as sendTicketNotification(). + * The assignee is DMed directly regardless, + * since they now have standing access to the + * ticket by virtue of being assigned to it. */ - public static function sendAssignmentNotification($ticketId, string $ticketTitle, ?string $assigneeName, ?string $assigneeMatrix, ?string $changedByDisplay = null): void + public static function sendAssignmentNotification($ticketId, string $ticketTitle, ?string $assigneeName, ?string $assigneeMatrix, ?string $changedByDisplay = null, string $visibility = 'public'): void { $notifyUsers = self::notifyUsers(); // Also notify the assignee directly if we know their Matrix ID @@ -267,7 +301,7 @@ class NotificationHelper self::fire([ 'event' => 'assigned', 'ticket_id' => $ticketId, - 'title' => $ticketTitle, + 'title' => self::redactedTitle($ticketTitle, $visibility), 'assignee' => $assigneeName, 'changed_by' => $changedByDisplay, 'url' => UrlHelper::ticketUrl($ticketId), diff --git a/models/CommentModel.php b/models/CommentModel.php index 419f26b..57b816e 100644 --- a/models/CommentModel.php +++ b/models/CommentModel.php @@ -38,7 +38,7 @@ class CommentModel } $placeholders = str_repeat('?,', count($usernames) - 1) . '?'; - $sql = "SELECT user_id, username, display_name FROM users WHERE username IN ($placeholders)"; + $sql = "SELECT user_id, username, display_name, is_admin, `groups` FROM users WHERE username IN ($placeholders)"; $stmt = $this->conn->prepare($sql); $types = str_repeat('s', count($usernames));