Refactoring email sending

[pierreozoux]

I investigated a bit for #6192 and #6195 and the way Rocket.Chat is sending email is... I'd say a bit messy, let me detail

• various methods are used in the wild: ( Mailer.sendMail, Email.send, Accounts.sendResetPasswordEmail )
• these methods are used sometimes differently

And here I can see a potential security issue:

Rocket.Chat/packages/rocketchat-mailer/client/views/mailer.coffee

Line 9 in 07efe5a

from = $(t.find('[name=from]')).val()

Looks like you let the user define the from field.
Which means, as an email provider, my reputation can get lowered by a spammer using my rocket.chat instance.

IMHO, we need to refactor these, to make it easier to understand what is going on.

I guess we need:

• make all emails going though: Mailer.sendMail
• make sure this method is working well
• make sure this method is reading everything from MAIL_URL
• Maybe add a FROM_EMAIL env var that this method would understand, this would allow unattended rocket.chat full configuration
• make accounts emails, also use this method, this way, they would get styled also (footer - header)
• log things to allow to run in debug mode to understand better what is going on

What is the normal way to proceed? Should we open an issue for each item? What is your general opinion on that matter?

[graywolf336]

@engelgabriel @rodrigok

[marceloschmidt]

The Mailer.sendMail function is supposed to be used only by admins through the administration panel. I don't think having the From field there is a problem, unless you don't trust your administrators.

@RocketChat/core thoughts?

[pierreozoux]

@marceloschmidt yes sorry about the security issue, you are right, it is not.

I was confused at first because, I'm a meteor user, and I expect MAIL_URL to work. But as explained in #6192 and the documentation, SMTP settings need also to be specified for a proper working Rocket.Chat instance.

I still think that sending emails should be refactored for 2 reasons:

• clarity of the code, one function to perform always the same action: send a mail to the user
• email styling with footer and header defined by the admin (currently meteor-account emails are not styled)

[marceloschmidt]

I believe the problem of using multiple methods has been fixed. @ggazzo can you please confirm and if so close the issue?

[geekgonecrazy]

@ggazzo you are last mentioned, do you know if we can close this?