I've rebased the changes and put them on Gerrit. They are ready for review, should they pass the tests.

@palm  could you please rebase your other patches on top of master some of them are not in sync with the last version of your initial patch and some (like gerrit #11828) should not be dedicated patch on their own but amend of the commit were you got your review.

As a rule of thumb when you are developping a new feature with gerrit, you should be having a dedicated local branch and when there are several commits you should be rebasing them with 'git rebase -i gerrit/master' to properly rebase and squash commits.

 

Your first patch, gerrit #11714 just got merge in Tuleap 10.2.99.132 \o/

Welcome to the team !

Thanks, it works now. I must've had bad luck with the runs, I've had that happen a couple of times with the "slugify" commit. It never happened with the other changes. But now it seems to pass Jenkins, so I'll mark my changes ready for review.

Hello,

It looks like the Karma server took too long to start ("Disconnected (1 times), because no message in 10000 ms.").

This is nothing related to your contribution, so you should retrigger the job

Thank you for you gerrit comment! It solved almost all the issues regarding my change. The last problem before I am ready to publish seems to be related to Karma. It can be found at https://ci.tuleap.org/jenkins/blue/organizations/jenkins/tuleap-gerrit-tests/detail/tuleap-gerrit-tests/6740/pipeline/68

I'll continue debugging in the meanwhile.

I added a comment in gerrit #11829 to inform composer about mbstring

I've made the changes on my machine and they work just fine. However, I'm having some trouble with the changes when I upload them to Gerrit. Jenkins does not seem to like my changes at all. They seem to be related to autoloading and I'm guessing it's because of my additional dependency. I get the following warning before getting fatal errors:

"[Build and run CentOS 6] Warning: require_once(/usr/share/tuleap/plugins/tracker/include/../vendor/autoload.php): failed to open stream: No such file or directory in /usr/share/tuleap/plugins/tracker/include/trackerPlugin.class.php on line 77"

I added the dependency in the composer.json of the tracker plugin. The new dependency, cocur/slugify, depends on the mb extension.
I'll continue debugging this tomorrow, but if you know that I forgot a step then any help would be appreciated!

That's OK for us.
 

Removing the special characters should be no problem what so ever. Keeping spaces and capital letters would be ideal, as getting the user's real name is one of the more useful applications of this feature on our end.

I'll slugify the From field so that it'll have the form of "$slugified_string" <emailaddress> and then update my pull request if that's okay.

After further discussion with @tgerbet, collecting the logs would not help there as the problem that will trigger the more headaches in troubleshooting is when the first MTA accept the mail but another one downard refuse it.

From our experience, what seems to cause issues is chars outside [A-z0-9-_] range so what we propose instead is to slug the part of the part of From that is variable (%last_update_by for instance) with cocur/slugify. The rules to be used might be relaxed (maybe allowing spaces and uppercase for instance).

Would that be OK on your side ?

That's probably too far from what we can do right now.

I'm thinking about something more simple: keep track of mail & delivery failures for tracker admins (it's probably a feature on it's own). It's more or less what we already do for webhooks and other background jobs, we keep a log of what happened for further audit/troubleshooting.

We could keep a log of artifact updates (and creations) with mail deliveries status (MTA ack or not). After a given amout of time (1 month?) the log would be purged.

@tgerbet what do you think ?

Hi and sorry for the delay. I've been doing some research on spam filters in the meantime and just as you said, who knows what will be considered spam in the future? The email providers will change their algorithms without telling anyone.

The only way to know if things get marked as spam is to try sending the mail. The simplest troubleshooting would probably be to try switching off the feature and see if the mail still won't arrive. Another would be to urge users to add the sender address to their address book, which seems to reduce the likelihood of email being marked as spam.

However, I think one can set up some automatic testing on this issue. A way of testing this would be to use a script to generate/mock some changes and send them to a pre-configured email address and try fetching them (after a delay). Sending updates with both custom "From" and the default should help in troubleshooting, as it could give some indication of which of the emails are refused.

Since only project admins are allowed to change the formatting on the "From" field, this problem might be mitigated to some degree.

That's not quite sure as MTA might really be bothersome sometimes.

Just to let you know an issue we had at the beginning of the year. We switched to a more recent version of Zend Framework for mail sending. This switch changed the way headers are encoded (UTF8) and all of the sudden we got weird email refusal from relay without clear error message. After tedious trial/error process we pinpointed that '<>' chars set in platform name local.inc since ages, once sent UTF8 got refused by relay (actually it's the best bet we have, we are not even sure it's the source of the issue).

The central problem here is not really the feature on it's own but the potential impact in troubleshooting later on, esp. if we start to play with user names. As another example, gmail marks as spam a bunch of emails coming from our gerrit instance and we suspect this because there are several user names associated to a single email 'gerrit@gerrit.tuleap.net'. But who knows what's kicking "SPAM" flag in google algorithm...

Do you have a proposal to ease troubleshooting if mails starts to be lost/refused ?

Thanks for your feedback @tgerbet!

The main reason Kristofer and I are arguing for the "From" customization is because of how the information is presented to an email client.
The very first thing you'll see of an email message in your inbox is the supposed sender, then the subject line.

Customizing the subject line could provide the same information, but won't convey that as quickly. For your average user, this might not matter too much. For a project admin, however, this could mean a significant reduction in "information overload" to know if an email notification is important just from seeing the bold "From" field.

I understand your concern about the email filtering. That is certainly a problem and I'm not sure how to solve it. Since only project admins are allowed to change the formatting on the "From" field, this problem might be mitigated to some degree.

Please let me know your thoughts on this!

Hi,

So I took at the feature request and played a bit with the code.
For the possible email headers injections we are safe, this handled correctly by the code sending the emails in Tuleap.
I'm however not sure we want to give that much control on the From header to the users, anti spam tends to have multiple static rules about From headers and letting user customizing it feel like a great way to end up with mails filtered for no obvious reasons. These kind of issues are painful to debug and solve and I would like to avoid that.

What do you think about extending the customization of the mail subject instead? Tuleap already lets you add an [Assigned to me] in the subject when there is an update on artifact assigned to you (see story #10299) and story #11649, yet to be done, has a proposal for more complex customization. You could implement a subset of this story by supporting the minimal fields %id, %tracker_name, %title_semantic and your "special field" %_last_updated_by.
How do you feel about that?

For the security part I'd like to have @tgerbet opinion.

 

I've created an initial patch for this feature. It can be found at
https://gerrit.tuleap.net/#/c/tuleap/+/11715/
and limits the allowed inputs to only a few pieces of user information.

Let us know if you think this sounds reasonable

We could be fine with limiting the scope of what can be input, either through validation of the input according to some format or just allowing a few specific display name formats to choose from in dropdown/textbox..
The main thing we want to be able to put in is the name of the person doing the modification and preferrably of the form that I wrote in the example "$s via $normal_reply_name", so if you think that we need to limit it to that kind of feature, I think we can live with that.

Let us know what limits you think we should have

Thanks for the answer, it makes sense.

However I'm a bit concerned about the ability to put anything in the From, regarding possible injections in mail headers or masquerading as sth else.

We want to modify the display name of the mail that is being sent out fo rthe receiver to get more information without having to open the mail.

For example, the notification from your post only says "Tulep" as the display name of the sender. With the option in this request, the project administrator could have set it to say e.g. "Manuel Vacelet via Tuleap" or something similar so you can seethe information about who modified the tracker. Or to be able to set "Request Tracker at Tuleap" to differentiate which tracker it comes from.

I'm not sure to grasp the use case. Is that the "From: " header that you want to change ?

How is it supposed to be used ?

My current idea on how to add this is through adding another checkbox and text field on the "Email subject customization" settings (found on "<Tracker Name>/Administration/Email Notification Settings") to enable and change the format string of the "From" field in emails sent by the tracker.

This information will then be saved in two tables, one for checking if it's enabled and the other for saving the format.

Then the notifier will be changed to load the format strings instead of the system default and then add the proper information (e.g. change $sender to the full name of the sender)