diff --git a/docs/usage/email.rst b/docs/usage/email.rst --- a/docs/usage/email.rst +++ b/docs/usage/email.rst @@ -39,9 +39,14 @@ Recipients will see these emails origina ``app_email_from`` setting in the configuration file. This setting can either contain only an email address, like `kallithea-noreply@example.com`, or both a name and an address in the following format: `Kallithea -`. The subject of these emails can -optionally be prefixed with the value of ``email_prefix`` in the configuration -file. +`. However, if the email is sent due to an +action of a particular user, for example when a comment is given or a pull +request created, the name of that user will be combined with the email address +specified in ``app_email_from`` to form the sender (and any name part in that +configuration setting disregarded). + +The subject of these emails can optionally be prefixed with the value of +``email_prefix`` in the configuration file. Error emails 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 @@ -31,6 +31,7 @@ from celery.decorators import task import os import traceback import logging +import rfc822 from os.path import join as jn from time import mktime @@ -45,6 +46,7 @@ from kallithea.lib.celerylib import run_ from kallithea.lib.helpers import person from kallithea.lib.rcmail.smtp_mailer import SmtpMailer from kallithea.lib.utils import add_cache, action_logger +from kallithea.lib.vcs.utils import author_email from kallithea.lib.compat import json, OrderedDict from kallithea.lib.hooks import log_create_repository @@ -247,7 +249,7 @@ def get_commits_stats(repo_name, ts_min_ @task(ignore_result=True) @dbsession -def send_email(recipients, subject, body='', html_body='', headers=None): +def send_email(recipients, subject, body='', html_body='', headers=None, author=None): """ Sends an email with defined parameters from the .ini files. @@ -256,9 +258,16 @@ def send_email(recipients, subject, body :param subject: subject of the mail :param body: body of the mail :param html_body: html version of body + :param headers: dictionary of prepopulated e-mail headers + :param author: User object of the author of this mail, if known and relevant """ log = get_logger(send_email) assert isinstance(recipients, list), recipients + if headers is None: + headers = {} + else: + # do not modify the original headers object passed by the caller + headers = headers.copy() email_config = config email_prefix = email_config.get('email_prefix', '') @@ -280,7 +289,18 @@ def send_email(recipients, subject, body log.warning("No recipients specified for '%s' - sending to admins %s", subject, ' '.join(recipients)) - mail_from = email_config.get('app_email_from', 'Kallithea') + # SMTP sender + envelope_from = email_config.get('app_email_from', 'Kallithea') + # 'From' header + if author is not None: + # set From header based on author but with a generic e-mail address + # In case app_email_from is in "Some Name " format, we first + # extract the e-mail address. + envelope_addr = author_email(envelope_from) + headers['From'] = '"%s" <%s>' % ( + rfc822.quote('%s (no-reply)' % author.full_name_or_username), + envelope_addr) + user = email_config.get('smtp_username') passwd = email_config.get('smtp_password') mail_server = email_config.get('smtp_server') @@ -306,7 +326,7 @@ def send_email(recipients, subject, body return False try: - m = SmtpMailer(mail_from, user, passwd, mail_server, smtp_auth, + m = SmtpMailer(envelope_from, user, passwd, mail_server, smtp_auth, mail_port, ssl, tls, debug=debug) m.send(recipients, subject, body, html_body, headers=headers) except: diff --git a/kallithea/model/notification.py b/kallithea/model/notification.py --- a/kallithea/model/notification.py +++ b/kallithea/model/notification.py @@ -145,7 +145,7 @@ class NotificationModel(BaseModel): .get_email_tmpl(type_, 'html', **html_kwargs) run_task(tasks.send_email, [rec.email], email_subject, email_txt_body, - email_html_body, headers) + email_html_body, headers, author=created_by_obj) return notif 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 @@ -2,6 +2,7 @@ import mock import kallithea from kallithea.tests import * +from kallithea.model.db import User class smtplib_mock(object): @@ -89,3 +90,78 @@ 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(self): + mailserver = 'smtp.mailserver.org' + recipients = ['rcpt1', 'rcpt2'] + envelope_from = 'noreply@mailserver.org' + 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, + } + with mock.patch('kallithea.lib.celerylib.tasks.config', config_mock): + kallithea.lib.celerylib.tasks.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 (no-reply)" <%s>' % envelope_from, 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_from(self): + mailserver = 'smtp.mailserver.org' + recipients = ['rcpt1', 'rcpt2'] + envelope_addr = 'noreply@mailserver.org' + envelope_from = 'Some Name <%s>' % envelope_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, + } + with mock.patch('kallithea.lib.celerylib.tasks.config', config_mock): + kallithea.lib.celerylib.tasks.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 (no-reply)" <%s>' % envelope_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_extra_headers(self): + mailserver = 'smtp.mailserver.org' + recipients = ['rcpt1', 'rcpt2'] + envelope_from = 'noreply@mailserver.org' + subject = 'subject' + body = 'body' + html_body = 'html_body' + author = User(name='foo', lastname='(fubar) "baz"') + headers = {'extra': 'yes'} + + config_mock = { + 'smtp_server': mailserver, + 'app_email_from': envelope_from, + } + with mock.patch('kallithea.lib.celerylib.tasks.config', config_mock): + kallithea.lib.celerylib.tasks.send_email(recipients, subject, body, html_body, + author=author, headers=headers) + + self.assertSetEqual(smtplib_mock.lastdest, set(recipients)) + self.assertEqual(smtplib_mock.lastsender, envelope_from) + self.assertIn(r'From: "foo (fubar) \"baz\" (no-reply)" <%s>' % envelope_from, smtplib_mock.lastmsg) + self.assertIn('Subject: %s' % subject, smtplib_mock.lastmsg) + self.assertIn(body, smtplib_mock.lastmsg) + self.assertIn(html_body, smtplib_mock.lastmsg) + self.assertIn('Extra: yes', smtplib_mock.lastmsg) + # verify that headers dict hasn't mutated by send_email + self.assertDictEqual(headers, {'extra': 'yes'})