Reverted earlier application of on_behalf_of() at mail sending call points, and instead did all on_behalf_of processing in condition_message(). Added insertion of Reply-To header fields in this case. Changed all use of the mail-sending 'extra' parameters to use value lists. Updated a bunch of tests accordingly.

- Legacy-Id: 16020
This commit is contained in:
Henrik Levkowetz 2019-03-11 17:01:33 +00:00
parent fc18cd93d4
commit 682ddee8e4
17 changed files with 107 additions and 82 deletions

View file

@ -9,7 +9,7 @@ from django.urls import reverse as urlreverse
import debug # pyflakes:ignore
from ietf.utils.mail import send_mail, send_mail_text, on_behalf_of
from ietf.utils.mail import send_mail, send_mail_text
from ietf.ipr.utils import iprs_from_docs, related_docs
from ietf.doc.models import WriteupDocEvent, LastCallDocEvent, DocAlias, ConsensusDocEvent
from ietf.doc.utils import needed_ballot_positions
@ -35,7 +35,7 @@ def email_ad_approved_doc(request, doc, text):
to = "iesg@iesg.org"
bcc = "iesg-secretary@ietf.org"
frm = request.user.person.formatted_email()
send_mail(request, to, on_behalf_of(frm),
send_mail(request, to, frm,
"Approved: %s" % doc.filename_with_rev(),
"doc/mail/ad_approval_email.txt",
dict(text=text,
@ -68,7 +68,7 @@ def email_stream_changed(request, doc, old_stream, new_stream, text=""):
def email_pulled_from_rfc_queue(request, doc, comment, prev_state, next_state):
extra=extra_automation_headers(doc)
addrs = gather_address_lists('doc_pulled_from_rfc_queue',doc=doc)
extra['Cc'] = addrs.as_strings().cc
extra['Cc'] = addrs.cc
send_mail(request, addrs.to , None,
"%s changed state from %s to %s" % (doc.name, prev_state.name, next_state.name),
"doc/mail/pulled_from_rfc_queue_email.txt",
@ -318,7 +318,7 @@ def email_resurrect_requested(request, doc, by):
e = by.role_email("ad")
frm = e.address
send_mail(request, to, on_behalf_of(e.formatted_email()),
send_mail(request, to, e.formatted_email(),
"I-D Resurrection Request",
"doc/mail/resurrect_request_email.txt",
dict(doc=doc,
@ -385,10 +385,8 @@ def email_iana(request, doc, to, msg, cc=None):
parsed_msg = email.message_from_string(msg.encode("utf-8"))
parsed_msg.set_charset('UTF-8')
extra = {}
extra["Reply-To"] = "noreply@ietf.org"
extra["X-IETF-Draft-string"] = doc.name
extra["X-IETF-Draft-revision"] = doc.rev
extra = extra_automation_headers(doc)
extra["Reply-To"] = ["noreply@ietf.org", ]
send_mail_text(request, to,
parsed_msg["From"], parsed_msg["Subject"],
@ -398,8 +396,8 @@ def email_iana(request, doc, to, msg, cc=None):
def extra_automation_headers(doc):
extra = {}
extra["X-IETF-Draft-string"] = doc.name
extra["X-IETF-Draft-revision"] = doc.rev
extra["X-IETF-Draft-string"] = [ doc.name, ]
extra["X-IETF-Draft-revision"] = [ doc.rev, ]
return extra
@ -536,5 +534,5 @@ def email_charter_internal_review(request, charter):
milestones=charter.group.groupmilestone_set.filter(state="charter"),
),
cc=addrs.cc,
extra={'Reply-To':"iesg@ietf.org"},
extra={'Reply-To': ["iesg@ietf.org", ]},
)

View file

@ -32,7 +32,7 @@ from ietf.review.utils import reviewer_rotation_list, possibly_advance_next_revi
from ietf.utils.test_utils import TestCase
from ietf.utils.test_data import create_person
from ietf.utils.test_utils import login_testing_unauthorized, unicontent, reload_db_objects
from ietf.utils.mail import outbox, empty_outbox, parseaddr
from ietf.utils.mail import outbox, empty_outbox, parseaddr, on_behalf_of
from ietf.person.factories import PersonFactory
class ReviewTests(TestCase):
@ -645,7 +645,8 @@ class ReviewTests(TestCase):
msgid = outbox[0]["Message-ID"]
message = Message.objects.get(msgid=msgid)
self.assertEqual(parseaddr(outbox[0]["To"]), parseaddr(message.to))
self.assertEqual(parseaddr(outbox[0]["From"]), parseaddr(message.frm))
self.assertEqual(parseaddr(outbox[0]["From"]), parseaddr(on_behalf_of(message.frm)))
self.assertEqual(parseaddr(outbox[0]["Reply-To"]), parseaddr(message.frm))
self.assertEqual(outbox[0].get_payload(decode=True).decode(str(outbox[0].get_charset())), message.body)
# check the review document page

View file

@ -32,7 +32,7 @@ from ietf.message.utils import infer_message
from ietf.name.models import BallotPositionName
from ietf.person.models import Person
from ietf.utils import log
from ietf.utils.mail import send_mail_text, send_mail_preformatted, on_behalf_of
from ietf.utils.mail import send_mail_text, send_mail_preformatted
from ietf.utils.decorators import require_api_key
BALLOT_CHOICES = (("yes", "Yes"),
@ -285,7 +285,6 @@ def api_set_position(request):
# send position email
addrs, frm, subject, body = build_position_email(ad, doc, pos)
frm = on_behalf_of(frm)
send_mail_text(request, addrs.to, frm, subject, body, cc=addrs.cc)
return HttpResponse("Done", status=200, content_type='text/plain')
@ -363,7 +362,7 @@ def send_ballot_comment(request, name, ballot_id):
if extra_cc:
cc.extend(extra_cc)
send_mail_text(request, addrs.to, on_behalf_of(frm), subject, body, cc=u", ".join(cc))
send_mail_text(request, addrs.to, frm, subject, body, cc=u", ".join(cc))
return HttpResponseRedirect(return_to_url)
@ -621,11 +620,10 @@ def ballot_writeupnotes(request, name):
send_mail_preformatted(request, msg, override=override)
addrs = gather_address_lists('ballot_issued_iana',doc=doc).as_strings()
override={ "To": "IANA <%s>"%settings.IANA_EVAL_EMAIL, "Bcc": None , "Reply-To": None}
override={ "To": "IANA <%s>"%settings.IANA_EVAL_EMAIL, "Bcc": None , "Reply-To": []}
if addrs.cc:
override['CC'] = addrs.cc
send_mail_preformatted(request, msg, extra=extra_automation_headers(doc),
override={ "To": "IANA <%s>"%settings.IANA_EVAL_EMAIL, "CC": None, "Bcc": None , "Reply-To": None})
send_mail_preformatted(request, msg, extra=extra_automation_headers(doc), override=override)
e = DocEvent(doc=doc, rev=doc.rev, by=login)
e.by = login
@ -866,7 +864,7 @@ def approve_ballot(request, name):
if action == "to_announcement_list":
addrs = gather_address_lists('ballot_approved_ietf_stream_iana').as_strings(compact=False)
send_mail_preformatted(request, announcement, extra=extra_automation_headers(doc),
override={ "To": addrs.to, "CC": addrs.cc, "Bcc": None, "Reply-To": None})
override={ "To": addrs.to, "CC": addrs.cc, "Bcc": None, "Reply-To": []})
msg = infer_message(announcement)
msg.by = login
@ -911,7 +909,7 @@ def make_last_call(request, name):
if doc.type.slug == 'draft':
addrs = gather_address_lists('last_call_issued_iana',doc=doc).as_strings(compact=False)
send_mail_preformatted(request, announcement, extra=extra_automation_headers(doc),
override={ "To": addrs.to, "CC": addrs.cc, "Bcc": None, "Reply-To": None})
override={ "To": addrs.to, "CC": addrs.cc, "Bcc": None, "Reply-To": []})
msg = infer_message(announcement)
msg.by = login

View file

@ -42,7 +42,7 @@ from ietf.message.models import Message
from ietf.name.models import IntendedStdLevelName, DocTagName, StreamName, DocUrlTagName
from ietf.person.fields import SearchableEmailField
from ietf.person.models import Person, Email
from ietf.utils.mail import send_mail, send_mail_message, on_behalf_of
from ietf.utils.mail import send_mail, send_mail_message
from ietf.utils.textupload import get_cleaned_text_file_content
from ietf.mailtrigger.utils import gather_address_lists
@ -594,10 +594,10 @@ def to_iesg(request,name):
addrs= gather_address_lists('pubreq_iesg',doc=doc)
extra = {}
extra['Cc'] = addrs.as_strings().cc
extra['Cc'] = addrs.cc
send_mail(request=request,
to = addrs.to,
frm = on_behalf_of(by.formatted_email()),
frm = by.formatted_email(),
subject = "Publication has been requested for %s-%s" % (doc.name,doc.rev),
template = "doc/submit_to_iesg_email.txt",
context = dict(doc=doc,by=by,url="%s%s"%(settings.IDTRACKER_BASE_URL,doc.get_absolute_url()),),
@ -1219,7 +1219,7 @@ def request_publication(request, name):
consensus_event = doc.latest_event(ConsensusDocEvent, type="changed_consensus")
m = Message()
m.frm = on_behalf_of(request.user.person.formatted_email())
m.frm = request.user.person.formatted_email()
(m.to, m.cc) = gather_address_lists('pubreq_rfced',doc=doc)
m.by = request.user.person

View file

@ -37,7 +37,7 @@ from ietf.review import mailarch
from ietf.utils.fields import DatepickerDateField
from ietf.utils.text import strip_prefix, xslugify
from ietf.utils.textupload import get_cleaned_text_file_content
from ietf.utils.mail import send_mail_message, on_behalf_of
from ietf.utils.mail import send_mail_message
from ietf.mailtrigger.utils import gather_address_lists
from ietf.utils.fields import MultiEmailField
@ -628,7 +628,7 @@ def complete_review(request, name, request_id):
msg = Message.objects.create(
by=request.user.person,
subject=subject,
frm=on_behalf_of(frm),
frm=frm,
to=", ".join(to),
cc=form.cleaned_data["cc"],
body = render_to_string("review/completed_review.txt", {

View file

@ -352,7 +352,7 @@ def approve(request, name):
for form in formset.forms:
send_mail_preformatted(request,form.cleaned_data['announcement_text'])
send_mail_preformatted(request, form.cleaned_data['announcement_text'], extra={})
c = DocEvent(type="added_comment", doc=status_change, rev=status_change.rev, by=login)
c.desc = "The following approval message was sent\n"+form.cleaned_data['announcement_text']

View file

@ -367,12 +367,12 @@ class ReviewTests(TestCase):
})
self.assertEqual(r.status_code, 302)
self.assertEqual(len(outbox), 1)
self.assertTrue('toaddr' in outbox[0]["To"])
self.assertTrue('ccaddr' in outbox[0]["Cc"])
self.assertTrue('replytoaddr' in outbox[0]["Reply-To"])
self.assertTrue('fromaddr' in outbox[0]["From"])
self.assertIn('toaddr', outbox[0]["To"])
self.assertIn('ccaddr', outbox[0]["Cc"])
self.assertIn('replytoaddr', outbox[0]["Reply-To"])
self.assertIn('fromaddr', outbox[0]["From"])
self.assertEqual(outbox[0]["subject"], "Test subject")
self.assertTrue("Test body" in outbox[0].get_payload(decode=True).decode("utf-8"))
self.assertIn("Test body", outbox[0].get_payload(decode=True).decode("utf-8"))
def test_change_reviewer_settings(self):
reviewer = ReviewerSettingsFactory(person__user__username='reviewer',expertise='Some expertise').person

View file

@ -109,7 +109,7 @@ from ietf.doc.models import LastCallDocEvent
from ietf.name.models import ReviewRequestStateName
from ietf.utils.mail import send_mail_text, parse_preformatted, on_behalf_of
from ietf.utils.mail import send_mail_text, parse_preformatted
from ietf.ietfauth.utils import user_is_person
from ietf.dbtemplate.models import DBTemplate
@ -1600,7 +1600,14 @@ def email_open_review_assignments(request, acronym, group_type=None):
if request.method == "POST" and request.POST.get("action") == "email":
form = EmailOpenAssignmentsForm(request.POST)
if form.is_valid():
send_mail_text(request, form.cleaned_data["to"], on_behalf_of(form.cleaned_data["frm"]), form.cleaned_data["subject"], form.cleaned_data["body"],cc=form.cleaned_data["cc"],extra={"Reply-To":", ".join(form.cleaned_data["reply_to"])})
send_mail_text(request,
to=form.cleaned_data["to"],
frm=form.cleaned_data["frm"],
subject=form.cleaned_data["subject"],
txt=form.cleaned_data["body"],
cc=form.cleaned_data["cc"],
extra={"Reply-To": form.cleaned_data["reply_to"]}
)
return HttpResponseRedirect(back_url)
else:
(to,cc) = gather_address_lists('review_assignments_summarized',group=group)

View file

@ -1,4 +1,7 @@
from collections import namedtuple
import debug # pyflakes:ignore
from ietf.mailtrigger.models import MailTrigger, Recipient
from ietf.submit.models import Submission

View file

@ -64,7 +64,7 @@ from ietf.secr.proceedings.proc_utils import (get_progress_stats, post_process,
create_recording)
from ietf.utils.decorators import require_api_key
from ietf.utils.log import assertion
from ietf.utils.mail import send_mail_message, send_mail_text, on_behalf_of
from ietf.utils.mail import send_mail_message, send_mail_text
from ietf.utils.pipe import pipe
from ietf.utils.pdf import pdf_pages
from ietf.utils.text import xslugify
@ -2307,7 +2307,7 @@ def request_minutes(request, num=None):
if form.is_valid():
send_mail_text(request,
to=form.cleaned_data.get('to'),
frm=on_behalf_of(request.user.person.email_address()),
frm=request.user.person.email_address(),
subject=form.cleaned_data.get('subject'),
txt=form.cleaned_data.get('body'),
cc=form.cleaned_data.get('cc'),

View file

@ -10,6 +10,7 @@ from ietf.group.models import Group
from ietf.doc.models import Document
from ietf.name.models import RoleName
from ietf.utils.models import ForeignKey
from ietf.utils.mail import get_email_addresses_from_text
class Message(models.Model):
time = models.DateTimeField(default=datetime.datetime.now)
@ -34,6 +35,10 @@ class Message(models.Model):
def __unicode__(self):
return "'%s' %s -> %s" % (self.subject, self.frm, self.to)
def get(self, field):
r = getattr(self, field)
return r if isinstance(r, list) else get_email_addresses_from_text(r)
class MessageAttachment(models.Model):
message = ForeignKey(Message)

View file

@ -1,6 +1,6 @@
import re, datetime, email
from ietf.utils.mail import send_mail_text, send_mail_mime, maybe_on_behalf_of
from ietf.utils.mail import send_mail_text, send_mail_mime
from ietf.message.models import Message
first_dot_on_line_re = re.compile(r'^\.', re.MULTILINE)
@ -21,15 +21,13 @@ def infer_message(s):
def send_scheduled_message_from_send_queue(send_queue):
message = send_queue.message
message.frm = maybe_on_behalf_of(message.frm)
message.save()
# for some reason, the old Perl code base substituted away . on line starts
body = first_dot_on_line_re.sub("", message.body)
extra = {}
if message.reply_to:
extra['Reply-To'] = message.reply_to
extra['Reply-To'] = message.get('reply_to')
# announcement.content_type can contain a case-sensitive parts separator,
# so we need to keep it as is, not lowercased, but we want a lowercased

View file

@ -22,7 +22,7 @@ from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream
from ietf.review.models import (ReviewRequest, ReviewRequestStateName, ReviewTypeName,
ReviewerSettings, UnavailablePeriod, ReviewWish, NextReviewerInTeam,
ReviewTeamSettings, ReviewSecretarySettings)
from ietf.utils.mail import send_mail, on_behalf_of, get_email_addresses_from_text
from ietf.utils.mail import send_mail, get_email_addresses_from_text
from ietf.doc.utils import extract_complete_replaces_ancestor_mapping_for_docs
def active_review_teams():
@ -373,11 +373,12 @@ def email_review_request_change(request, review_req, subject, msg, by, notify_se
url = urlreverse("ietf.doc.views_review.review_request_forced_login", kwargs={ "name": review_req.doc.name, "request_id": review_req.pk })
url = request.build_absolute_uri(url)
send_mail(request, to, on_behalf_of(request.user.person.formatted_email()), subject, "review/review_request_changed.txt", {
"review_req_url": url,
"review_req": review_req,
"msg": msg,
})
send_mail(request, to, request.user.person.formatted_email(), subject, "review/review_request_changed.txt", {
"review_req_url": url,
"review_req": review_req,
"msg": msg,
},
)
def email_reviewer_availability_change(request, team, reviewer_role, msg, by):
"""Notify possibly both secretary and reviewer about change, skipping

View file

@ -8,7 +8,7 @@ from ietf.message.models import AnnouncementFrom
from ietf.ietfauth.utils import has_role
from ietf.secr.announcement.forms import AnnounceForm
from ietf.secr.utils.decorators import check_for_cancel
from ietf.utils.mail import send_mail_text, maybe_on_behalf_of
from ietf.utils.mail import send_mail_text
# -------------------------------------------------
# Helper Functions
@ -80,15 +80,16 @@ def confirm(request):
form = AnnounceForm(request.POST, user=request.user)
if request.method == 'POST':
message = form.save(user=request.user,commit=True)
extra = {'Reply-To':message.reply_to}
extra = {'Reply-To': message.get('reply_to') }
send_mail_text(None,
message.to,
maybe_on_behalf_of(message.frm),
message.frm,
message.subject,
message.body,
cc=message.cc,
bcc=message.bcc,
extra=extra)
extra=extra,
)
messages.success(request, 'The announcement was sent.')
return redirect('ietf.secr.announcement.views.main')

View file

@ -77,7 +77,7 @@ TOOLS_ID_HTML_URL = TOOLS_SERVER_URL + '/html/'
SERVER_EMAIL = 'Django Server <django-project@' + TOOLS_SERVER + '>'
DEFAULT_FROM_EMAIL = 'IETF Secretariat <ietf-secretariat-reply@' + IETF_DOMAIN + '>'
UTILS_ON_BEHALF_EMAIL = 'ietf-secretariat-reply@' + IETF_DOMAIN
UTILS_ON_BEHALF_EMAIL = 'noreply@' + IETF_DOMAIN
UTILS_FROM_EMAIL_DOMAINS = [ 'ietf.org', 'iab.org', 'tools.ietf.org', ]
MANAGERS = ADMINS

View file

@ -9,6 +9,7 @@ import time
import traceback
from email.utils import make_msgid, formatdate, formataddr as simple_formataddr, parseaddr as simple_parseaddr, getaddresses
from email.message import Message # pyflakes:ignore
from email.mime.text import MIMEText
from email.mime.message import MIMEMessage
from email.mime.multipart import MIMEMultipart
@ -26,7 +27,7 @@ from django.template import Context,RequestContext
import debug # pyflakes:ignore
import ietf
from ietf.utils.log import log, unreachable
from ietf.utils.log import log, assertion
from ietf.utils.text import isascii
# Testing mode:
@ -191,7 +192,6 @@ def on_behalf_of(frm):
name, addr = parseaddr(frm)
domain = addr.rsplit('@', 1)[-1]
if domain in settings.UTILS_FROM_EMAIL_DOMAINS:
unreachable('2019-03-06')
return frm
if not name:
name = addr
@ -234,6 +234,10 @@ def parseaddr(addr):
return name, addr
def condition_message(to, frm, subject, msg, cc, extra):
if extra:
assertion("isinstance(extra, (dict, Message))")
if 'Reply-To' in extra:
assertion("isinstance(extra['Reply-To'], list)")
if isinstance(frm, tuple):
frm = formataddr(frm)
@ -245,14 +249,13 @@ def condition_message(to, frm, subject, msg, cc, extra):
n, a = parseaddr(frm)
domain = a.rsplit('@', 1)[-1]
if not domain in settings.UTILS_FROM_EMAIL_DOMAINS:
unreachable('2019-03-04')
extra = extra or {}
if 'Reply-To' in extra:
extra['Reply-To'].append(frm)
else:
extra['Reply-To'] = [frm, ]
frm = on_behalf_of(frm)
msg['From'] = frm
if extra and 'Reply-To' in extra:
reply_to = extra['Reply-To']
if isinstance(reply_to, list) or isinstance(reply_to, tuple):
reply_to = ", ".join([isinstance(addr, tuple) and formataddr(addr) or addr for addr in reply_to if addr])
extra['Reply-To'] = reply_to
# The following is a hack to avoid an issue with how the email module (as of version 4.0.3)
# breaks lines when encoding header fields with anything other than the us-ascii codec.
@ -282,7 +285,11 @@ def condition_message(to, frm, subject, msg, cc, extra):
if extra:
for k, v in extra.items():
if v:
msg[k] = v
assertion('len(list(set(v))) == len(v)')
try:
msg[k] = ", ".join(v)
except Exception:
raise
def show_that_mail_was_sent(request,leadline,msg,bcc):
if request and request.user:
@ -348,37 +355,43 @@ def parse_preformatted(preformatted, extra={}, override={}):
msg.set_charset('UTF-8')
for k, v in override.iteritems():
if k in msg:
del msg[k]
if isinstance(v, list):
msg[k] = ', '.join(v)
else:
msg[k] = v
if k in msg:
del msg[k]
if v:
if isinstance(v, list):
msg[k] = ', '.join(v)
else:
msg[k] = v
headers = copy.copy(msg)
extra = copy.deepcopy(extra) # don't modify the caller's extra obj
headers = copy.copy(msg) # don't modify the message
for key in ['To', 'From', 'Subject', 'Bcc']:
del headers[key]
for k, v in extra.iteritems():
if k in headers:
del headers[k]
if isinstance(v, list):
headers[k] = ', '.join(v)
else:
headers[k] = v
for k in headers.keys():
v = headers.get_all(k, [])
if k in extra:
ev = extra[k]
if not isinstance(ev, list):
ev = [ ev, ]
extra[k] = list(set(v + ev))
else:
extra[k] = v
bcc = msg['Bcc']
del msg['Bcc']
return (msg, headers, bcc)
for v in extra.values():
assertion('len(list(set(v))) == len(v)')
return (msg, extra, bcc)
def send_mail_preformatted(request, preformatted, extra={}, override={}):
"""Parse preformatted string containing mail with From:, To:, ...,
and send it through the standard IETF mail interface (inserting
extra headers as needed)."""
(msg,headers,bcc) = parse_preformatted(preformatted, extra, override)
(msg, extra, bcc) = parse_preformatted(preformatted, extra, override)
txt = msg.get_payload().decode(str(msg.get_charset()))
send_mail_text(request, msg['To'], msg["From"], msg["Subject"], txt, extra=headers, bcc=bcc)
send_mail_text(request, msg['To'], msg["From"], msg["Subject"], txt, extra=extra, bcc=bcc)
return msg
def send_mail_message(request, message, extra={}):
@ -387,9 +400,9 @@ def send_mail_message(request, message, extra={}):
e = extra.copy()
if message.reply_to:
e['Reply-To'] = message.reply_to
e['Reply-To'] = message.get('reply_to')
if message.msgid:
e['Message-ID'] = message.msgid
e['Message-ID'] = [ message.msgid, ]
return send_mail_text(request, message.to, message.frm, message.subject,
message.body, cc=message.cc, bcc=message.bcc, extra=e)

View file

@ -110,7 +110,7 @@ body
self.assertEqual(recv['Subject'], 'osubject')
self.assertEqual(recv.get_payload(), 'body\n')
extra = {'Fuzz': 'bucket'}
extra = {'Fuzz': [ 'bucket' ]}
send_mail_preformatted(request=None, preformatted=msg, extra=extra, override={})
recv = outbox[-1]
self.assertEqual(recv['Fuzz'], 'bucket')