[FIX] Missing setting to control when to send the ReplyTo field in email notifications #20744
Conversation
|
|
||
| // If direct reply enabled, email content with headers | ||
| if (settings.get('Direct_Reply_Enable')) { | ||
| if (settings.get('Direct_Reply_Enable') && settings.get('Direct_Reply_ReplyTo_Enable')) { |
There was a problem hiding this comment.
In case Direct Reply is enabled, the Reply-To header should be always set, since that is the only way we can make sure to receive a reply from the notification email, this new setting should be only applied to regular email notifications (when Direct Reply is turned off 😉 )
app/lib/server/startup/email.js
Outdated
| placeholder: 'email@domain', | ||
| secret: true, | ||
| }); | ||
| this.add('Direct_Reply_ReplyTo_Enable', false, { |
There was a problem hiding this comment.
As I mentioned on previous comment, this setting should control the Reply-To header from regular notifications, so the name of setting is currently misleading..
I'd recommend moving the setting to the Privacy section and change its name:
| this.add('Direct_Reply_ReplyTo_Enable', false, { | |
| this.add('Add_Sender_To_ReplyTo', false, { |
app/lib/server/startup/email.js
Outdated
| this.add('Direct_Reply_ReplyTo_Enable', false, { | ||
| type: 'boolean', | ||
| env: true, | ||
| i18nLabel: 'Add_ReplyTo_Header', |
There was a problem hiding this comment.
Following the changes on the setting's name, would you recommend changing its label too? @sampaiodiego
There was a problem hiding this comment.
oh, I didn't notice the setting name I suggested is actually very close to the i18nLabel 😅
I'd say you can remove the i18nLabel as the _id of the setting will be used if the label is missing.. 😉
also, I just realized it's missing an i18n key.. =)
There was a problem hiding this comment.
Ok, thanks for the suggestion! 🙂
Is i18nDescription the missing key which you think that must be added here?
There was a problem hiding this comment.
no, sry 🙈
what I meant was you should create a new i18n key at least on the English translation file (https://github.com/RocketChat/Rocket.Chat/blob/develop/packages/rocketchat-i18n/i18n/en.i18n.json), if possible, add the same key to the portuguese (pt-BR.i18n.json) file as well.. this translation will be used as the label of setting on the Admin UI.
the key you'll add to the translation files should be the value of the i18nDescription property (if it exists), otherwise it can be the setting _id.. (I'd recommend adding a i18nDescription property only if the setting _id is not similar to the label that setting should have)..
hopefully it makes more sense now, I'm sorry the confusion.
| const [senderEmail] = sender.emails; | ||
| email.headers['Reply-To'] = generateNameEmail(username, senderEmail.address); | ||
|
|
||
| if (settings.get('Direct_Reply_ReplyTo_Enable')) { |
There was a problem hiding this comment.
This if can be merged with the parent if.
matheusbsilva137
force-pushed
the
fix-email-reply-to
branch
from
dc848ea to
ebd7a97
Compare
…ail notifications (#20744)
RC v3.4.0 adds a Reply-To header containing the user’s email address. Previously, there was no Reply-To header at all. The From header was (and still is) the commenter's real name and a site-wide configured email address, “[email protected]”. The new feature was intended to **prevent** a privacy issue – if a user would reply to an email thinking it would go to the commenter, he might send a confidential message to the [email protected] mailbox. So with the Reply-To, email would go to the intended recipient. However, AFAICT, the user's email address is not displayed to any other (non-admin) users anywhere within the RC UI, so we've always considered the email address to be a private field. However, the Reply-To exposes this private email address. Solution: Remove the Reply-To header. In the future, we may also change the From/Reply-To address to "no-reply@" https://seekingalpha.atlassian.net/browse/MP-931 NOTE: I am currently patching this onto 3.11.1, however I notice that 3.12.0-rc.0 is planning a change to this feature which may obviate the need for this patch. RocketChat#20744
Proposed changes (including videos or screenshots)
falsevalue) by default.Issue(s)
Fixes issue #15767
Steps to test or reproduce
The new setting is displayed in the Direct Reply section of the Email settings' page (Administration > Email > Direct Reply).
Further comments