Merge ~teward/launchpad:dmarc-for-bugs into launchpad:master

[Thomas Ward]

Use DMARC compliant From addresses in bug notifications

https://bugs.launchpad.net/bugs/1589693 has been filed since 2016. Over the course of four years, DMARC has now become a stable of email protection and more and more companies and email systems are enforcing DMARC rules.

As a result, more and more people need DMARC-compliant From addresses in bug notifications in order to keep receiving them when a DMARC-enforced commenter/sender is being used.

This change alters the get_bugmail_from_address function to only produce addresses for the From field which use the bug's email address for the From field. It also alters the supporting documentation in bugnotification-email.txt to account for the change for DMARC compliant From addresses.

[Andrew Johnson (anj)]

+1

Does this handle emails from merge request comments too? I think they need fixing as well, although they may be handled in a completely different part of the code-base.

[Thomas Ward (teward)]

anj: No, this only handles bug notifications to my knowledge. Thought it best to handle the issues one bit at a time rather than do one massive DMARC conversion for mail. Less can regress that way. (My opinion is phased deployment of DMARC as we make each individual bit work right will be more effective.)

[Colin Watson (cjwatson)]

This looks good, thank you. Just a few minor points.

review: Approve

3a2f6f9... by Thomas Ward on 2020-05-11

Adjust changes per Colin Watson suggestions/requests.

[Thomas Ward (teward)]

> This looks good, thank you. Just a few minor points.

Added changes to my branch per your request. Also have a comment re: your comment.

diff --git a/lib/lp/bugs/doc/bugnotification-email.txt b/lib/lp/bugs/doc/bugnotification-email.txt
index d9c5fbc..38c599b 100644
--- a/lib/lp/bugs/doc/bugnotification-email.txt
+++ b/lib/lp/bugs/doc/bugnotification-email.txt
@@ -454,8 +454,11 @@ The Reply-To address generation is straightforward:
     >>> get_bugmail_replyto_address(bug_four)
     u'Bug 4 <[e-mail]>'
 
-The From address generator handles a few special cases. The trivial case
-is, well, trivial. Stuart has four email addresses:
+In order to send DMARC-compliant bug notifications, the From address generator
+is also quite straightforward and uses the bug's email address for the From
+address, while adjusting the friendly display name field.
+
+This applies for all users.  For example, Stuart has four email addresses:
 
     >>> stub = getUtility(IPersonSet).getByName("stub")
     >>> [(email.email, email.status.name) for email
@@ -465,47 +468,39 @@ is, well, trivial. Stuart has four email addresses:
      (u'[e-mail]', 'NEW'),
      (u'[e-mail]', 'OLD')]
 
-But we use his preferred one:
+However, because of DMARC compliance, we only use the bug's email address in the
+From field, with Stuart's name in the 'display name' portion of the email address:
 
     >>> get_bugmail_from_address(stub, bug_four)
-    'Stuart Bishop <[e-mail]>'
+    'Stuart Bishop <[e-mail]>'
 
-Now, mpo doesn't have a validated email address, but we pick out the
-first address we find:
+This also happens for users with hidden addresses:
+
+    >>> private_person = factory.makePerson(
+    ...     email="[e-mail]", displayname="Ford Prefect")
+    >>> private_person.hide_email_addresses = True
+    >>> get_bugmail_from_address(private_person, bug_four)
+    'Ford Prefect <[e-mail]>'
+
+It also behaves the same for users with no verified email addresses:
 
     >>> mpo = getUtility(IPersonSet).getByName("mpo")
     >>> get_bugmail_from_address(mpo, bug_four)
-    '=?utf-8?b?TWF0dGkgUMO2bGzDpA==?= <[e-mail]>'
-
-(As you can see in the above example, get_bugmail_from_address() takes
-care of encoding the person's displayname correctly.)
+    '=?utf-8?b?TWF0dGkgUMO2bGzDpA==?= <[e-mail]>'
 
-The team janitor doesn't have an email address at all!
+This also happens for the team janitor:
 
     >>> janitor = getUtility(IPersonSet).getByName("team-membership-janitor")
     >>> get_bugmail_from_address(janitor, bug_four)
     'Team Membership Janitor <[e-mail]>'
 
-The Launchpad Janitor celebrity isn't a real user, and shouldn't be
-sending mail. Notifications from the janitor are sent with the address
-of the bug itself.
+And it also applies for the Launchpad Janitor:
 
     >>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
     >>> lp_janitor = getUtility(ILaunchpadCelebrities).janitor
     >>> get_bugmail_from_address(lp_janitor, bug_four)
     'Launchpad Bug Tracker <[e-mail]>'
 
-If a person has specified that their email remain private,
-get_bugmail_from_address() will return their display name with bug's
-email address.
-
-    >>> private_person = factory.makePerson(
-    ...     email="[e-mail]", displayname="Ford Prefect")
-    >>> private_person.hide_email_addresses = True
-    >>> get_bugmail_from_address(private_person, bug_four)
-    'Ford Prefect <[e-mail]>'
-
-
 Construction of bug notification emails
 ---------------------------------------
 
diff --git a/lib/lp/bugs/doc/bugnotification-sending.txt b/lib/lp/bugs/doc/bugnotification-sending.txt
index d8dc14f..19cd3a8 100644
--- a/lib/lp/bugs/doc/bugnotification-sending.txt
+++ b/lib/lp/bugs/doc/bugnotification-sending.txt
@@ -947,7 +947,7 @@ We can see that Concise Person doesn't receive verbose notifications:
 
     >>> print_notification(collated_messages['[e-mail]'][0])
     To: [e-mail]
-    From: Verbose Person <[e-mail]>
+    From: Verbose Person <[e-mail]>
     Subject: [Bug ...] subject
     X-Launchpad-Message-Rationale: Subscriber
     X-Launchpad-Message-For: concise-person
@@ -975,7 +975,7 @@ that gets verbose email.
 
     >>> print_notification(collated_messages['[e-mail]'][0])
     To: [e-mail]
-    From: Verbose Person <[e-mail]>
+    From: Verbose Person <[e-mail]>
     Subject: [Bug ...] subject
     X-Launchpad-Message-Rationale: Subscriber @verboseteam
     X-Launchpad-Message-For: verboseteam
@@ -995,7 +995,7 @@ Whereas Verbose Person does get the description and task status:
 
     >>> print_notification(collated_messages['[e-mail]'][0])
     To: [e-mail]
-    From: Verbose Person <[e-mail]>
+    From: Verbose Person <[e-mail]>
     Subject: [Bug ...] subject
     X-Launchpad-Message-Rationale: Subscriber
     X-Launchpad-Message-For: verbose-person
@@ -1026,7 +1026,7 @@ And Concise Team Person does too, even though their team doesn't want them:
 
     >>> print_notification(collated_messages['[e-mail]'][0])
     To: [e-mail]
-    From: Verbose Person <[e-mail]>
+    From: Verbose Person <[e-mail]>
     Subject: [Bug ...] subject
     X-Launchpad-Message-Rationale: Subscriber @conciseteam
     X-Launchpad-Message-For: conciseteam
diff --git a/lib/lp/bugs/mail/bugnotificationbuilder.py b/lib/lp/bugs/mail/bugnotificationbuilder.py
index f784d30..0846bb7 100644
--- a/lib/lp/bugs/mail/bugnotificationbuilder.py
+++ b/lib/lp/bugs/mail/bugnotificationbuilder.py
@@ -43,38 +43,12 @@ def format_rfc2822_date(date):
 def get_bugmail_from_address(person, bug):
     """Returns the right From: address to use for a bug notification."""
     if person == getUtility(ILaunchpadCelebrities).janitor:
-        return format_address(
-            'Launchpad Bug Tracker',
-            "%s@%s" % (bug.id, config.launchpad.bugs_domain))
-
-    if person.hide_email_addresses:
-        return format_address(
-            person.displayname,
-            "%s@%s" % (bug.id, config.launchpad.bugs_domain))
-
-    if person.preferredemail is not None:
-        return format_address(person.displayname, person.preferredemail.email)
-
-    # XXX: Bjorn Tillenius 2006-04-05:
-    # The person doesn't have a preferred email set, but they
-    # added a comment (either via the email UI, or because they were
-    # imported as a deaf reporter). It shouldn't be possible to use the
-    # email UI if you don't have a preferred email set, but work around
-    # it for now by trying hard to find the right email address to use.
-    email_addresses = shortlist(
-        getUtility(IEmailAddressSet).getByPerson(person))
-    if not email_addresses:
-        # XXX: Bjorn Tillenius 2006-05-21 bug=33427:
-        # A user should always have at least one email address,
-        # but due to bug #33427, this isn't always the case.
-        return format_address(person.displayname,
-            "%s@%s" % (bug.id, config.launchpad.bugs_domain))
-
-    # At this point we have no validated emails to use: if any of the
-    # person's emails had been validated the preferredemail would be
-    # set. Since we have no idea of which email address is best to use,
-    # we choose the first one.
-    return format_address(person.displayname, email_addresses[0].email)
+        displayname = 'Launchpad Bug Tracker'
+    else:
+        displayname = person.display_name
+
+    return format_address(displayname,
+                          "%s@%s" % (bug.id, config.launchpad.bugs_domain))
 
 
 def get_bugmail_replyto_address(bug):