launchpad-reviewers team mailing list archive

[Thomas Ward <teward@xxxxxxxxxxxxxxx>]

Thomas Ward has proposed merging ~teward/launchpad:dmarc-for-bugs into launchpad:master.

Commit message:
Use DMARC compliant From addresses in bug notifications

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~teward/launchpad/+git/launchpad/+merge/383643

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.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~teward/launchpad:dmarc-for-bugs into launchpad:master.

diff --git a/lib/lp/bugs/doc/bugnotification-email.txt b/lib/lp/bugs/doc/bugnotification-email.txt
index d9c5fbc..337e778 100644
--- a/lib/lp/bugs/doc/bugnotification-email.txt
+++ b/lib/lp/bugs/doc/bugnotification-email.txt
@@ -454,8 +454,12 @@ The Reply-To address generation is straightforward:
     >>> get_bugmail_replyto_address(bug_four)
     u'Bug 4 <4@xxxxxxxxxxxxxxxxxx>'
 
-The From address generator handles a few special cases. The trivial case
-is, well, trivial. Stuart has four email addresses:
+In order to implement proper DMARC-compliant bug notification email
+messages to be sent, 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 +469,39 @@ is, well, trivial. Stuart has four email addresses:
      (u'stub@xxxxxxxxxxx', 'NEW'),
      (u'zen@xxxxxxxxxxxxxxxxxxxxxxxxx', '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 <stuart.bishop@xxxxxxxxxxxxx>'
+    'Stuart Bishop <4@xxxxxxxxxxxxxxxxxx>'
 
-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="hidden@xxxxxxxxxxx", displayname="Ford Prefect")
+    >>> private_person.hide_email_addresses = True
+    >>> get_bugmail_from_address(private_person, bug_four)
+    'Ford Prefect <4@xxxxxxxxxxxxxxxxxx>'
+
+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==?= <mpo@xxxxxx>'
-
-(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==?= <4@xxxxxxxxxxxxxxxxxx>'
 
-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 <4@xxxxxxxxxxxxxxxxxx>'
 
-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 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 <4@xxxxxxxxxxxxxxxxxx>'
 
-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="hidden@xxxxxxxxxxx", displayname="Ford Prefect")
-    >>> private_person.hide_email_addresses = True
-    >>> get_bugmail_from_address(private_person, bug_four)
-    'Ford Prefect <4@xxxxxxxxxxxxxxxxxx>'
-
-
 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['concise@xxxxxxxxxxx'][0])
     To: concise@xxxxxxxxxxx
-    From: Verbose Person <verbose@xxxxxxxxxxx>
+    From: Verbose Person <...@bugs.launchpad.net>
     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['verboseteam@xxxxxxxxxxx'][0])
     To: verboseteam@xxxxxxxxxxx
-    From: Verbose Person <verbose@xxxxxxxxxxx>
+    From: Verbose Person <...@bugs.launchpad.net>
     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['verbose@xxxxxxxxxxx'][0])
     To: verbose@xxxxxxxxxxx
-    From: Verbose Person <verbose@xxxxxxxxxxx>
+    From: Verbose Person <...@bugs.launchpad.net>
     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['conciseteam@xxxxxxxxxxx'][0])
     To: conciseteam@xxxxxxxxxxx
-    From: Verbose Person <verbose@xxxxxxxxxxx>
+    From: Verbose Person <...@bugs.launchpad.net>
     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..be5dede 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.displayname
+
+    return format_address(displayname,
+                          "%s@%s" % (bug.id, config.launchpad.bugs_domain))
 
 
 def get_bugmail_replyto_address(bug):