On 07/13/2015 10:45 AM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire <[email protected]>
# Date 1436729808 -7200
# Sun Jul 12 21:36:48 2015 +0200
# Node ID bdca2f365e3feb81ba7329b8f42eada6a342a811
# Parent 95c18c5fdb59f2d985e2cd8ac6db093cb3e5f884
e-mail: allow specifying a different noreply e-mail than app_email_from
The configuration setting app_email_from is currently used both for the SMTP
envelope sender as for the From header inside the e-mail (what mail clients
show). However, some SMTP servers require a valid, existing address for the
envelope sender.
Actually, all SMTP servers kind of require a valid envelope sender. They
can however usually not verify it and nobody will notice until the mail
for some reason can't be delivered (deleted or full account or
spam/virus filters or whatever) and the attempt to bounce it back will fail.
IMO, the bounce and 'no reply' mails _should_ be monitored by someone.
Clueful users will know not to use it, and clueless users will use it
and need help.
If you really don't want reliably delivery, you can perhaps use a null
sender address "<>". (But I will not recommend it any more than I will
recommend using an invalid header from address.)
When no anonymous real address can be created (e.g. due to
corporate IT rules), app_email_from would need to be that of a real (admin)
user, which would thus also be used in the From header of e-mails and is
therefore undesired.
The hidden reasoning here seems to be that you actually do want to use
an invalid email address inside the mail? Please make that design goal
explicit so we have a chance to disagree with it ;-)
/Mads
Instead, add an extra configuration setting app_email_noreply that is used
for the From header. Setting app_email_from remains being used for the SMTP
envelope, as well as for error e-mails to the specified administrator.
diff --git a/kallithea/bin/template.ini.mako b/kallithea/bin/template.ini.mako
--- a/kallithea/bin/template.ini.mako
+++ b/kallithea/bin/template.ini.mako
@@ -14,9 +14,12 @@ pdebug = false
##
##
## email_to: The e-mail address to send error reports to.
##
## error_email_from: The sender of error e-mails.
##
-## app_email_from: The sender of mails originating from Kallithea, for
##
-## example to notify users about new comments, pull requests,
##
-## etc.
##
+## app_email_noreply: The sender of mails originating from Kallithea, for
##
+## example to notify users about new comments, pull
##
+## requests, etc.
##
+## app_email_from: The SMTP sender of mails originating from Kallithea. This
##
+## address will be used when communicating with the SMTP
##
+## server only.
##
I don't think the "for communication with the SMTP server" description
is correct and it is thus not so helpful.
More correctly: The envelope sender is used for the actual mail delivery
(for example anti spam sender validation and bounce processing all the
way to the recipient(s)) and is often not shown to the user.
Reading these two descriptions, I'm still confused about what they
actually are used for and which formats are accepted.
app_email_from is usually also a 'noreply' address, just as much as your
app_email_noreply is?
Also, I think it should be made clear that 'app_email_noreply' is
optional and usually not necessary and only for overriding
'app_email_from' in the mail headers. In general, I don't think people
will or should use it and it should thus also not have too much
mentioning. Most admins should not need to know about or care about this
setting.
## email_prefix: The subject prefix for mails originating from Kallithea.
##
##
##
## Note: your SMTP server may require app_email_from/error_email_from to be
##
@@ -25,6 +28,7 @@ pdebug = false
#email_to = admin@localhost
#error_email_from = kallithea-noreply@localhost
app_email_from = kallithea-noreply@localhost
+app_email_noreply = kallithea-noreply@localhost
email_prefix = [Kallithea]
#smtp_server = mail.server.com
diff --git a/kallithea/config/deployment.ini_tmpl
b/kallithea/config/deployment.ini_tmpl
--- a/kallithea/config/deployment.ini_tmpl
+++ b/kallithea/config/deployment.ini_tmpl
@@ -15,9 +15,12 @@ pdebug = false
##
##
## email_to: The e-mail address to send error reports to.
##
## error_email_from: The sender of error e-mails.
##
-## app_email_from: The sender of mails originating from Kallithea, for
##
-## example to notify users about new comments, pull requests,
##
-## etc.
##
+## app_email_noreply: The sender of mails originating from Kallithea, for
##
+## example to notify users about new comments, pull
##
+## requests, etc.
##
+## app_email_from: The SMTP sender of mails originating from Kallithea. This
##
+## address will be used when communicating with the SMTP
##
+## server only.
##
## email_prefix: The subject prefix for mails originating from Kallithea.
##
##
##
## Note: your SMTP server may require app_email_from/error_email_from to be
##
@@ -26,6 +29,7 @@ pdebug = false
#email_to = admin@localhost
#error_email_from = kallithea-noreply@localhost
app_email_from = kallithea-noreply@localhost
+app_email_noreply = kallithea-noreply@localhost
email_prefix = [Kallithea]
#smtp_server = mail.server.com
diff --git a/kallithea/lib/celerylib/tasks.py b/kallithea/lib/celerylib/tasks.py
--- a/kallithea/lib/celerylib/tasks.py
+++ b/kallithea/lib/celerylib/tasks.py
@@ -279,19 +279,18 @@ def send_email(recipients, subject, body
# SMTP sender
envelope_from = email_config.get('app_email_from', 'Kallithea')
# From header
+ noreply_addr = email_config.get('app_email_noreply', envelope_from)
if author is None:
- headers['From'] = envelope_from
+ headers['From'] = noreply_addr
else:
# set From header based on author but with a generic e-mail address
# In case app_email_from is in "Some Name <e-mail>" format, we first
# extract the e-mail address.
import re
- m = re.match('.*<(.*)>', envelope_from)
+ m = re.match('.*<(.*)>', noreply_addr)
if m is not None:
- envelope_addr = m.group(1)
- else:
- envelope_addr = envelope_from
- headers['From'] = '%s <%s>' % (author.full_name_or_username,
envelope_addr)
+ noreply_addr = m.group(1)
+ headers['From'] = '%s <%s>' % (author.full_name_or_username,
noreply_addr)
user = email_config.get('smtp_username')
passwd = email_config.get('smtp_password')
diff --git a/kallithea/tests/other/test_mail.py
b/kallithea/tests/other/test_mail.py
--- a/kallithea/tests/other/test_mail.py
+++ b/kallithea/tests/other/test_mail.py
@@ -92,3 +92,54 @@ class TestMail(BaseTestCase):
self.assertIn('Subject: %s' % subject, smtplib_mock.lastmsg)
self.assertIn(body, smtplib_mock.lastmsg)
self.assertIn(html_body, smtplib_mock.lastmsg)
+
+ def test_send_mail_with_author_noreply(self):
+ mailserver = 'smtp.mailserver.org'
+ recipients = ['rcpt1', 'rcpt2']
+ envelope_from = '[email protected]'
+ noreply_addr = '[email protected]'
+ subject = 'subject'
+ body = 'body'
+ html_body = 'html_body'
+ author = User.get_by_username(TEST_USER_REGULAR_LOGIN)
+
+ config_mock = {
+ 'smtp_server': mailserver,
+ 'app_email_from': envelope_from,
+ 'app_email_noreply': noreply_addr,
+ }
+ with mock.patch('kallithea.lib.celerylib.tasks.config', config_mock):
+ t.send_email(recipients, subject, body, html_body, author=author)
+
+ self.assertSetEqual(smtplib_mock.lastdest, set(recipients))
+ self.assertEqual(smtplib_mock.lastsender, envelope_from)
+ self.assertIn('From: Kallithea Admin <%s>' % noreply_addr,
smtplib_mock.lastmsg)
+ self.assertIn('Subject: %s' % subject, smtplib_mock.lastmsg)
+ self.assertIn(body, smtplib_mock.lastmsg)
+ self.assertIn(html_body, smtplib_mock.lastmsg)
+
+ def test_send_mail_with_author_full_mail_noreply(self):
+ mailserver = 'smtp.mailserver.org'
+ recipients = ['rcpt1', 'rcpt2']
+ envelope_from = '[email protected]'
+ noreply_addr = '[email protected]'
+ noreply_full = 'Some Name <%s>' % noreply_addr
+ subject = 'subject'
+ body = 'body'
+ html_body = 'html_body'
+ author = User.get_by_username(TEST_USER_REGULAR_LOGIN)
+
+ config_mock = {
+ 'smtp_server': mailserver,
+ 'app_email_from': envelope_from,
+ 'app_email_noreply': noreply_full,
+ }
+ with mock.patch('kallithea.lib.celerylib.tasks.config', config_mock):
+ t.send_email(recipients, subject, body, html_body, author=author)
+
+ self.assertSetEqual(smtplib_mock.lastdest, set(recipients))
+ self.assertEqual(smtplib_mock.lastsender, envelope_from)
+ self.assertIn('From: Kallithea Admin <%s>' % noreply_addr,
smtplib_mock.lastmsg)
+ self.assertIn('Subject: %s' % subject, smtplib_mock.lastmsg)
+ self.assertIn(body, smtplib_mock.lastmsg)
+ self.assertIn(html_body, smtplib_mock.lastmsg)
_______________________________________________
kallithea-general mailing list
[email protected]
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
