[PATCH] Use updater's name and email in ticket notifications

[gm.outside+trac@…]

This patch adds a new boolean config option named 'smtp_from_smart'. When this option is set to true notifications use updater's name and email for their 'From:' message header. We find it very useful since this patch helps to easily see who was modifying a ticket in a list of messages. Also, it's now possible to reply to the person directly.

The patch was created by (GalaxyMaster) from ​Openwall Project

This work was sponsored by ​CivicActions LLC

[Thijs Triemstra]

I'm not such a fan of the smtp_from_smart name, but I like the feature, even though I wonder if it's not possible already. If not, let's raise the priority of this ticket.

[Thijs Triemstra]

Here's an updated patch that re-uses the stmp_from_name and applies the updater's name when it's passed the value __updater__.

trac/ticket/notification.py

@@ -85 +85 @@
         changes_body = ''
         self.reporter = ''
         self.owner = ''
+        self.updater = ''
         changes_descr = ''
         change_data = {}
         link = self.env.abs_href.ticket(ticket.id)
@@ -329 +330 @@
             for updater, in cursor:
                 break
 
+        self.updater = updater
         if not notify_updater:
             filter_out = True
             if notify_reporter and (updater == self.reporter):

trac/notification.py

@@ -276 +276 @@
         self._init_pref_encoding()
         domains = self.env.config.get('notification', 'ignore_domains', '')
         self._ignore_domains = [x.strip() for x in domains.lower().split(',')]
-        # Get the email addresses of all known users
+        # Get the names and email addresses of all known users
         self.email_map = {}
+        self.name_map = {}
         for username, name, email in self.env.get_known_users(self.db):
+            if name:
+                self.name_map[username] = name
             if email:
                 self.email_map[username] = email
                 
@@ -404 +407 @@
             reactivate(t)
         projname = self.env.project_name
         public_cc = self.config.getbool('notification', 'use_public_cc')
+
+        if self.from_name == '__updater__' and self.updater:
+            name = self.updater
+            if self.updater in self.name_map:
+                name = self.name_map[self.updater]
+            self.from_name = name
+
+            if self.updater in self.email_map:
+                self.from_email = self.email_map[self.updater]
+
         headers = {}
         headers['X-Mailer'] = 'Trac %s, by Edgewall Software' % __version__
         headers['X-Trac-Version'] =  __version__

I'll add a new patch with documentation if it gets accepted.

[Thijs Triemstra]

The patch on #6512 seems to do almost the same, except it uses the username instead of the name specified in the preferences panel.

[Remy Blank]

Putting on my todo list for review. The patch in #6512 looks simpler at first sight, but I'll review both properly.

[anonymous]

Replying to thijstriemstra:

Here's an updated patch that re-uses the stmp_from_name and applies the updater's name when it's passed the value __updater__.

I think you miss one feature I introduced in the original patch — it also uses {{smtp_from_name}}, but it appends it to updater's name, e.g. if {{smtp_from_name}} was configured to be '[via Trac]' and updater is "Happy Joe", then the resulting From in my patch will be '"Happy Joe [via Trac]" <happy.joe@…'.

The rationale for this is that people are not happy to see an automated messages which they didn't write, but with the addition of, say, '[via Trac]' it's clear that the message was generated on their behalf. Also, it's much easier to filter/search for these messages.

[Remy Blank]

Use the change author as a sender if [notifiation] smtp_from_author is true.

[Remy Blank]

8360-smtp-from-author-r10410.patch​ is a slightly more complete version of the patch in comment:3, with the following changes:

• It uses a separate boolean option [notifiation] smtp_from_author to enable the functionality, so that smtp_from and smtp_from_name can still be set independently. The latter two are used if the author of a change hasn't configured an e-mail address.
• It adds a test case for the new behavior.

I didn't keep the suggested "misuse" of smtp_from_name as a name suffix. Sure, the author of a change didn't write the notification e-mail, but she did make the change, so sending in her name feels acceptable. And filtering is better done with other headers (e.g. X-Trac-Project).

Feedback and testing welcome.

[Remy Blank]

#6512 was closed as a duplicate. The patch above uses the idea from #6512 to use the reporter in the From: header for new tickets, and the change author for changes.

[Remy Blank]

An improved patch that also handles anonymous users having defined a name and an e-mail address was applied in [10421].

[Remy Blank]

Documentation updated in 0.13/TracNotification@2.

[mpotter@…]

Maybe I've missed something in my reading of this ticket, but I think I see a few issues with the current solution:

1. With the "improvement" mentioned in 9, it would be possible for a malefactor to:
   • Attached to a Trac system which allows anonymous users to add or comment to tickets and uses this feature.
   • Use the Preferences to set an email address to someone else's email address and name, the fall-guy.
   • Enter a inappropriate comment and/or new ticket.
   • The Trac system would then send inappropriate emails that would appear to have come from the fall-guy.
   Suggestion: This feature should only use email addresses from authenticated users.

2. Many SMTP servers are configured in such a way that they will refuse any message with a From: field not being the same as the authenticated user (i.e. smtp_from != smtp_user). If one has such an SMTP server, one cannot utilize this new feature.
   
   Suggestion: Additionally allow, which could be used instead:
   • Setting the Reply-To: field to the change author.
   • Adding the author to the from-name. e.g. User Ford makes a change to a ticket in the HHGTrac system from Megadodo Publications. This results in emails with a from line something like:
     • From: Ford via HHGTrac <trac@megadodo.com>

[Mark Potter <mpotter@…>]

Since I commented on this ticket with issues I feel should be addressed or at least acknowledge by a Trac authority, I am reopening this ticket. If the powers that be do not think these issues need further examination, feel free to just re-close. Just wanted to make sure that someone of authority thought about these issues and that they weren't just overlooked in the noise.

[Remy Blank]

Replying to mpotter@…:

1. With the "improvement" mentioned in 9, it would be possible for a malefactor to:

Yes, that is correct. Given that it's trivial to send emails impersonating someone else, I don't think this is actually a problem. And the feature should probably not be enabled for externally-facing Trac instances that allow anonymous posting. It's disabled by default.

Suggestion: Additionally allow, which could be used instead:

• Setting the Reply-To: field to the change author.
• Adding the author to the from-name. e.g. User Ford makes a change to a ticket in the HHGTrac system from Megadodo Publications. This results in emails with a from line something like:
  • From: Ford via HHGTrac <trac@megadodo.com>

Those sound like good ideas. Unfortunately, our notification subsystem is in a relatively sorry state, and adding the features would make the situation even worse. We were hoping for the integration of the ​th:AnnouncerPlugin to give a new breath to that part, allowing to more easily customize notifications and how they are sent, but that effort seems to have stalled.

I'm closing this again, as the initial feature request was implemented. Could you please file a new ticket with your suggestions? Thanks.

[Remy Blank]

Actually closing.

[Mark Potter <mpotter@…>]

Replying to rblank:

Could you please file a new ticket with your suggestions? Thanks.

Filed new ticket #10579 to cover the first issue (do not use "anonymous" email addresses).

Filed new ticket #10578 to cover the first bullet of the second issue (changing the ReplyTo: field). Left off the second bullet (changing the From: name) for negative comments on this had been made before which I can agree with, but some may disagree. Conversations can be take there.

[Steffen Hoffmann]

Replying to rblank:

Replying to mpotter@…:

Suggestion: Additionally allow, which could be used instead:

• Setting the Reply-To: field to the change author.
• Adding the author to the from-name. e.g. User Ford makes a change to a ticket in the HHGTrac system from Megadodo Publications. This results in emails with a from line something like:
  • From: Ford via HHGTrac <trac@megadodo.com>

Those sound like good ideas. Unfortunately, our notification subsystem is in a relatively sorry state, and adding the features would make the situation even worse. We were hoping for the integration of the ​th:AnnouncerPlugin to give a new breath to that part, allowing to more easily customize notifications and how they are sent, but that effort seems to have stalled.

Right, I've been one of the supporters of the proposal. Only I've got distracted by other Trac plugin projects for a while. Please note, that ​th:AnnouncerPlugin development as just continued, and I foresee that we'll follow-up on the original proposal in due time as well.

I'll think about using the preliminary state from TracNotification regarding this feature and preparing a more competing feature for AnnouncerPlugin along the lines of #10578. Please consider to follow-up on ​th:#10613 with comments.